dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Fix group page lists in PR #1548

Open Atmire-Kristof opened this issue 3 years ago • 1 comments

References

  • Fixes bug in #1548
  • Depends on #1741

Description

This PR fixes 3 bugs concerning cached requests of the groups for an eperson and vice versa

Instructions for Reviewers

To reproduce issue 1 before these changes:

  • Sign in as an admin and go to the detail page for an eperson that's not part of any group via the sidebar menu → access control → people
  • Add that person to a group
  • Click on the eperson name in the members list of the group to go back to their detail page
  • The new group isn't in the list

To reproduce issue 2 before these changes:

  • Visit the page of a group not containing any epersons
  • Add an eperson
  • Click the added eperson in the list
  • Click the group again on the eperson's page
  • At this point, the page would break

To reproduce issue 3 before these changes:

  • Visit the page of a group containing exactly one eperson
  • Remove the eperson
  • At this point, the list of epersons wouldn't update and the removed eperson would be visible until the user reloads the page

After the changes of this PR, the page and list of epersons should load correctly again.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • [x] 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.
  • [x] My PR passes TSLint validation using yarn run lint
  • [x] My PR doesn't introduce circular dependencies
  • [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • [x] If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

Atmire-Kristof avatar Sep 02 '22 09:09 Atmire-Kristof

This pull request introduces 1 alert when merging 95e8346228eb70df95674f2846e1304f75852c46 into 342a71251337f69b524a4012afaf075afa8e82d5 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar Sep 02 '22 09:09 lgtm-com[bot]

@Atmire-Kristof : I just merged the PR this depends on (#1741). Could you rebase this PR so that we can more easily review the new code here? Thanks!

tdonohue avatar Sep 07 '22 14:09 tdonohue

@Atmire-Kristof : This has minor merge conflicts now. Could you rebase this when you get the chance? Thanks!

tdonohue avatar Sep 13 '22 14:09 tdonohue

This pull request introduces 1 alert when merging 0f6fdceb45850d60092f47a92b8c8e14cb02eb5d into 6d361beb8812fa6c26e7c957ee2f4990e6612d6b - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar Sep 15 '22 09:09 lgtm-com[bot]

@Atmire-Kristof : apologies, but after merging #1791 (which is a larger, but important refactor), this now has some small merge conflicts (again). I'd appreciate it if you could rebase this again when you get the chance.

tdonohue avatar Sep 15 '22 18:09 tdonohue

@tdonohue Fixed the merge conflicts here so I can move on with the ones in https://github.com/DSpace/dspace-angular/pull/1813 Dit a quick check locally & the fixes in this PR still work, didn't notice any new issues

ybnd avatar Sep 16 '22 08:09 ybnd

This pull request introduces 1 alert when merging 87a7baa2e8d083faa12e7c07dd3b52b7ea42322a into a2f8a5ccfa114fc766b7fa6fd1a6d199cb152b4b - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar Sep 16 '22 08:09 lgtm-com[bot]

@paulo-graca That seems to be a symptom of issue https://github.com/DSpace/dspace-angular/issues/1695. It looks like https://github.com/DSpace/dspace-angular/pull/1846 fixes it

artlowel avatar Sep 22 '22 09:09 artlowel

Merging as this is at +2. Thanks again @Atmire-Kristof ! I've added a comment to #1846 to remind reviewers there to test that @paulo-graca 's issue reported above is fixed.

tdonohue avatar Sep 22 '22 13:09 tdonohue