fix: linkName from decorator assign value to the correct property

[pcg-kk]

References

Add references/links to any related issues or PRs. These may include:

• Fixes The linkName in the @link decorator doesn't assign the value on the correct property #2819

Description

In this PR I provide a fix to the HALLink mechanism. Before, the link was assigned to the variable from the linkName, not the real model property.

Instructions for Reviewers

List of changes in this PR:

• First, I changed the link.service to retrieve the correct name of the property
• Second, I add a unit test for a new case
• Third, I fixed the test mock to use the Map instead of the array because in other cases some tests were false positive

To test this issue please follow reproduction steps by @alexandrevryghem from this commit bb13004

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes ESLint validation using yarn lint
• My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[tdonohue]

@alexandrevryghem : Would you have time to review the changes in this PR? It appears it was created to solve a minor issue that you logged in #2819, so I'd appreciate your feedback on this solution.

[tdonohue]

Temporarily closing / reopening to try to force automated tests to re-run. This PR has test failures, but I suspect they may have been temporary issues that are now fixed?

[alexandrevryghem]

@pcg-kk: Thx for this PR it does fix the problem in the model class where the data isn't assigned to the correct property, but you still need to modify this code to be compatible with the possibility of having a @link on a property with a different name than the linkName. That code is responsible for automatically invalidating the RemoteData when one of its followLinks is invalidated. So if you use the example from bb13004, currently hasValue(remoteDataObject.payload['thumbnail']) is used here instead of hasValue(remoteDataObject.payload['thumbnailBitstream'])

[pcg-kk]

@alexandrevryghem am I correct that what we need now is to provide a correct followName for payload, something like:

const linkDefinition = getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName);
if (linkDefinition?.propertyName && hasValue(remoteDataObject.payload[linkDefinition.propertyName]) && followLinkName !== 'self') {
  // followLink can be either an individual HALLink or a HALLink[]
  const followLinksList: HALLink[] = [].concat(remoteDataObject.payload._links[followLinkName]);
  for (const individualFollowLink of followLinksList) {
    if (hasValue(individualFollowLink?.href)) {
      this.addDependency(response$, individualFollowLink.href);
    }
  }
}

So we are use the proper name of link to check it in remoteDataObject.payload and then provide all other operations?

[alexandrevryghem]

@pcg-kk: Sorry for the delay. I tested your solution, and it seems to work nicely. I can see that the addDependency function is called again even when you change the name of the property to which the link decorator is attached. Once you add the fix to your branch, I’m a +1.

[pcg-kk]

@alexandrevryghem version is updated

[alexandrevryghem]

@pcg-kk: Could you also update the ‎BaseDataService.findListByHref‎ with the same logic

[pcg-kk]

@alexandrevryghem BaseDataService.findListByHref‎ is fixed too... but now we have an issue with unit tests

[github-actions]

Hi @pcg-kk,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

[alexandrevryghem]

@pcg-kk: Thnx! I tested it again today, and the BaseDataService#findByHref works correctly, but I saw that BaseDataService#findListByHref's addDependency was never called, even for paginated lists that contained embedded data, so I added that fix. I also updated the test case's mock payload with a class containing real @links, this way we can simulate how it would work in a real environment and restored that test assertion. I'm a +1 now

[dspace-bot]

Backport failed for dspace-7_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-7_x
git worktree add -d .worktree/backport-2972-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-2972-to-dspace-7_x
git switch --create backport-2972-to-dspace-7_x
git cherry-pick -x 3351615140e2e2c0023b61fbe3c867efda6c0222 973fd339da24b0d96ef4484e20f4a2a93224bd04 a862d84738a573c559d18bf523b4cc346000dde2 d9d39b0cb5f1aed5ff7ec9c51c60fb354232a01c 752eb4c57bd9b41f65cae0094006c2e887f37147 e4b2ebe7aaaa8afe9ca775bde51679fba20fc09c 7ca4d8f2b13b68cbcedddae5d212caf8f00247f7

[dspace-bot]

Backport failed for dspace-8_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-8_x
git worktree add -d .worktree/backport-2972-to-dspace-8_x origin/dspace-8_x
cd .worktree/backport-2972-to-dspace-8_x
git switch --create backport-2972-to-dspace-8_x
git cherry-pick -x 3351615140e2e2c0023b61fbe3c867efda6c0222 973fd339da24b0d96ef4484e20f4a2a93224bd04 a862d84738a573c559d18bf523b4cc346000dde2 d9d39b0cb5f1aed5ff7ec9c51c60fb354232a01c 752eb4c57bd9b41f65cae0094006c2e887f37147 e4b2ebe7aaaa8afe9ca775bde51679fba20fc09c 7ca4d8f2b13b68cbcedddae5d212caf8f00247f7

[tdonohue]

@pcg-kk and @alexandrevryghem : I'm not sure if this is important to backport, but I tried anyways (since it was an older ticket). The backports all failed. We can keep this as a fix in 9.0 only for now. But, if either of you want to see this in 8.x or 7.6.x, please create backport PRs.