dspace-angular
dspace-angular copied to clipboard
The number of people and groups on access control page are now configurable
References
- Fixes #1764
Description
The size of the groups and epeople list on access control page can be configured.
The new configs are under a new accesscontrol property in config.yml
- accesscontrol > epeople > pageSize
- accesscontrol > groups > pageSize
The default value is 5 for both.
Instructions for Reviewers
List of changes in this PR:
- New configuration property have been created: accesscontrol
- New configuration properties have been created: epeople and groups under "accesscontrol" property
- New configuration property created under both: pageSize
- The hard-coded number 5 have been replaced by the value of pageSize
The value of pageSize must match the pagination pages sizes of the browseby like https://github.com/DSpace/dspace-angular/pull/1771#pullrequestreview-1093428318
Checklist
- [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.
Hi @arvoConsultores
I took the liberty to test this pr, when we set 5 it works fine but when I set 8 for example it shows 10
when i put for example 15 or 20 it shows 10 in the same way. is this expected behavior?

Hi @jtimal
Yes, that is a known behavior, it's explained in the comment.
You must enter a value that exist in the property pageSizeOptions in pagination-component-options.model.ts. If you use another value it will be rounded to the nearest.
https://github.com/DSpace/dspace-angular/blob/a5bc6c1bf7cc0b9edc5da4736bb3694ae99b5edd/src/app/shared/pagination/pagination-component-options.model.ts#L25
It's the same as https://github.com/DSpace/dspace-angular/pull/1771#pullrequestreview-1093428318
@arvoConsultores : I tested this today, and the basics work. However, I'm finding that both pages load very slowly if you set the
pageSize> 5. When I set them both topageSize: 10, each page takes a while to load (nearly 30 seconds for me). When I set both topageSize: 20, I cannot load either page (it's almost like it hits an infinite loop because my Chrome window just starts using more and more memory)I don't think this performance issue is caused by this PR. Rather, I'm suspecting the code on these pages has some sort of bug where larger page sizes result in major performance issues.
Basically, I think your PR is working properly...but the unfortunate result is that these pages do not perform properly when the
pageSizeconfiguration is changed. At least, that's what I'm seeing on my end (and I'd be interested to hear if this same behavior is seen by others or not -- as it's always possible I'm hitting a problem specific to my setup).
Hi, yes I confirm that it is cycling even when loading people to be more than 20 but I thought it was just my computer, I have tried in chrome and firefox developer.

Hi @arvoConsultores, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!
This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.
Unfortunately, while I'd like to be able to move this forward, merging this will likely result in major performance issues for any site that choses to use this feature (as setting pageSize > 5 will result in performance problems on the people/group pages, as described above).
Therefore, this is "on hold" until some solution can be found for the performance issues. Moving this to the 8.0 board, simply so that we are reminded of this issue later on.
Hi @arvoConsultores, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!
Hi @arvoConsultores . If you could find time to update this PR, it's possible this will work much better now that https://github.com/DSpace/DSpace/issues/9052 has been fixed. I suspect the prior issues we encountered with this PR were all related to that performance bug.
Strangely, after testing this again today, I've discovered that there are still performance issues when the pageSize is increased greater than 5 on these pages. It appears the slowness is somehow related to how performance checks are done (in order to display edit/delete buttons). So, it was not fixed by https://github.com/DSpace/DSpace/issues/9052 as it seems to be a different issue.
To reproduce:
- Install this PR
- Set a pageSize of 10 for either the "Access Control" -> "People" or "Groups" page (it's reproducible on either page).
- Open your browser DevTools on the Network tab
- Login as an Admin
- Visit that page where pagesize is set to 10.
- On the Network tab (in DevTools) you'll notice that 10 requests are sent to
/server/api/authz/authorizations/search/objects?feature=canDelete(one for each object displayed). The first 5-6 will return quickly. The last few may take a long time (several seconds) to respond. Until all these requests finish, nothing on the page will load.- This behavior is even worse when the pageSize is set to 20.
- However, it's not noticeable when the pageSize is set to 5.
So, I'm still uncertain how to move this forward. The page loading flaw doesn't appear to have been caused by this PR. However, these pages only load in a reasonable time if the pageSize remains at 5. So, I'm still worried about merging this since it is unusable until the page can be sped up.
Hi @arvoConsultores, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!