Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed pagination issues on item mapper #2642

Conversation

alexandrevryghem
Copy link
Member

@alexandrevryghem alexandrevryghem commented Nov 15, 2023

References

Description

Fixed the Item Mapper not paginating the items correctly. Because the page config was improperly configured the request to the backend did not include the page size & the current page. This lead to all items being displayed in the Item Mapper & the different pages always displaying the same result.

Additionally this PR also improves the performance of the item mapper page by saving the observables from canSelect & getSelected in a new list in order to prevent those methods from being called every time change detection is triggered.

Instructions for Reviewers

List of changes in this

  • Ensured the pagination options were correctly passed to the backend in CollectionItemMapperComponent.
  • Created DSpaceObjectSelect which will contain the Observables from canSelect & getSelected in order to prevent those Observables from being recreated each time change detection is triggered.
  • Removed the @Input() pageInfoState from PaginationComponent since it's not used and almost all the components who use it, pass incorrect data to it.

Guidance for how to test or review this PR:

  • Add a total of 11 items to a certain collection
  • Create a new collection and go to Edit Collection > Item Mapper
  • Map those 11 items to this new collection and check that the pagination in Browse mapped items is fixed

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.

@alexandrevryghem alexandrevryghem added bug component: Discovery related to discovery search or browse system medium priority claimed: Atmire Atmire team is working on this issue & will contribute back labels Nov 15, 2023
@alexandrevryghem alexandrevryghem added this to the 8.0 milestone Nov 15, 2023
@tdonohue tdonohue added the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label Nov 15, 2023
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge label Feb 1, 2024
@tdonohue tdonohue self-requested a review February 1, 2024 16:00
Copy link

github-actions bot commented Mar 8, 2024

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

…per-pagination_contribute-main

# Conflicts:
#	src/app/collection-page/collection-item-mapper/collection-item-mapper.component.ts
@kanasznagyzoltan
Copy link

My colleague (Sutya Miklós (QULTO)) has tested it yesterday:
For him it did not work properly on his local installation:

  1. Logged in as admin user. Clicked on Edit → Collection. Selected a collection.
  2. Clicked on Item Mapper → Map New Items
  3. Entered “***” as Search query.
  4. The results come up, but the whole page freezes and does not respond.
  5. This happened in locally run DSpace and on sandbox.dspace.org as well

@alexandrevryghem
Copy link
Member Author

@kanasznagyzoltan: This is another issue see #2953 but I already created a fix for this and was planning to contribute it to this PR later today

@kanasznagyzoltan
Copy link

@kanasznagyzoltan: This is another issue see #2953 but I already created a fix for this and was planning to contribute it to this PR later today

Thank you for the info!

…ll the components gave the incorrect type of data to it
…fix-broken-item-mapper-pagination_contribute-main

# Conflicts:
#	src/app/access-control/bulk-access/browse/bulk-access-browse.component.html
#	src/app/access-control/epeople-registry/epeople-registry.component.html
#	src/app/access-control/epeople-registry/eperson-form/eperson-form.component.html
#	src/app/access-control/group-registry/group-form/members-list/members-list.component.html
#	src/app/access-control/group-registry/group-form/subgroup-list/subgroups-list.component.html
#	src/app/process-page/overview/process-overview.component.html
#	src/app/shared/object-select/collection-select/collection-select.component.html
#	src/app/shared/object-select/collection-select/collection-select.component.ts
#	src/app/shared/object-select/item-select/item-select.component.html
#	src/app/shared/object-select/item-select/item-select.component.ts
#	src/app/shared/object-select/object-select/object-select.component.ts
#	src/app/shared/pagination/pagination.component.ts
Copy link

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

…per-pagination_contribute-main

# Conflicts:
#	src/app/shared/object-list/object-list.component.html
#	src/app/shared/object-select/collection-select/collection-select.component.ts
#	src/app/shared/object-select/item-select/item-select.component.ts
#	src/app/shared/pagination/pagination.component.ts
@Leano1998
Copy link
Contributor

I tested it in a local environment and it works perfectly. Thanks @alexandrevryghem !

@alexandrevryghem alexandrevryghem added component: Collection Collection display or editing testathon Reported by a tester during Community Testathon labels Apr 19, 2024
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks @alexandrevryghem ! I've also verified this fixes the issues with the ItemMapper (both the hanging issue & pagination). Tested CollectionMapper and other pagination based components as well to make sure there are no side effects. Didn't find any.

@tdonohue
Copy link
Member

I've realized the original bug ticket (#2613) was for 7.6.x, so we may need to find a way to port this to 7.6.x if possible. Adding the port to dspace-7_x label.

@tdonohue tdonohue merged commit a4387bb into DSpace:main Apr 19, 2024
13 checks passed
@dspace-bot
Copy link
Contributor

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-2642-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-2642-to-dspace-7_x
git switch --create backport-2642-to-dspace-7_x
git cherry-pick -x 45e8977db7d34d7b0684c16f92e9e917f6ac4579 da31c4f253ecc504e65ebc3ebe18b4ce2a0960a2 59197cff2d663240cb771d58d1f01cf97d78c3b4 268d2e54fcb0c35216669e52022564a9c5a651d1

@tdonohue
Copy link
Member

It appears this will require a manual port to 7.x as the automated port failed ☝️

@alexandrevryghem alexandrevryghem removed the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label Apr 19, 2024
@alexandrevryghem alexandrevryghem deleted the fix-broken-item-mapper-pagination_contribute-main branch April 19, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge bug claimed: Atmire Atmire team is working on this issue & will contribute back component: Collection Collection display or editing component: Discovery related to discovery search or browse system medium priority testathon Reported by a tester during Community Testathon
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Item Mapper hangs when a search is run Mapped items do not paginate correctly
5 participants