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

Jump to bottom

Improve role/capability handling in usermanager #19388

Merged
merged 12 commits into from Jul 20, 2022
Merged

Improve role/capability handling in usermanager #19388

merged 12 commits into from Jul 20, 2022

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jun 22, 2022

Description:

While trying to fix #18750 a couple of other issue within the UserManager popped up. Guess some of them might have regressed during the vue migration.

Note: This PR also adds a proper handling of bulk requests to AjaxHandler. This is e.g. important when setting a role/capability for multiple users at once. The requests are sent as bulk. Currently it would not show any error if some of the requests failed.

fixes #18750

Review

@sgiehl sgiehl added this to the 4.12.0 milestone Jun 22, 2022
@sgiehl sgiehl added the Needs Review PRs that need a code review label Jun 23, 2022
@sgiehl sgiehl marked this pull request as ready for review June 23, 2022 09:20
@justinvelluppillai justinvelluppillai removed the Needs Review PRs that need a code review label Jun 27, 2022
@sgiehl sgiehl force-pushed the m18750 branch 3 times, most recently from 998905a to 4b9c9ec Compare July 12, 2022 08:44
@sgiehl
Copy link
Member Author

sgiehl commented Jul 12, 2022

This one should be ready for review now. I've already tried to explain some of the changes in some comments above. Hope that makes it easier to understand everything.

@sgiehl sgiehl added c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Needs Review PRs that need a code review labels Jul 12, 2022
@sgiehl sgiehl requested a review from bx80 July 12, 2022 08:47
bx80
bx80 approved these changes Jul 20, 2022
View reviewed changes
Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

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

I've done some before and after functional testing, including trying to add capabilities for a no access user and view access user via bulk actions, checking that the capabilities UI is only shown on user editing if the user has site access and checking the paged user list. Also read through the code, including the bulk request handling improvements. The comments were helpful, thanks. 👍 I haven't found any issues so this looks good to merge 🙂

@sgiehl sgiehl merged commit d149ffb into 4.x-dev Jul 20, 2022
@sgiehl sgiehl deleted the m18750 branch July 20, 2022 08:30
@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bx80 bx80 bx80 approved these changes

Assignees
No one assigned
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.12.0
Development

Successfully merging this pull request may close these issues.

Improve role/capability handling in usermanager
3 participants
@sgiehl @bx80 @justinvelluppillai