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

Params name not matching, caused first website granted not working #19575

Merged
merged 17 commits into from Jul 28, 2022

Conversation

peterhashair
Copy link
Contributor

Description:

Fixes: #19574

This was caused by the param name, update it works now.

Review

update Param name
@peterhashair peterhashair added the Needs Review PRs that need a code review label Jul 27, 2022
@peterhashair peterhashair requested a review from bx80 July 27, 2022 22:09
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 recreated the issue prior to this fix, applied the fix and the functional issue is resolved.

I think it would be good to add a unit test where we call inviteUser with specific userLogin, email, siteId and expiryInDays parameters value then read back the created user to assert that all the supplied parameters were applied as expected. This will protect against future regressions.

Peter added 3 commits July 28, 2022 13:09
add invite User initialIdSite tests
add 2 new tests
plugins/UsersManager/API.php Outdated Show resolved Hide resolved
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 taken a look through the other API methods added by this PR and they all seem to have appropriate unit tests. It's now not possible to invite a user without explicitly specifying a site, either from the UI or via the API 👍

If the build tests all pass then this should be good to go.

Peter added 3 commits July 28, 2022 15:22
update tests
remove another function
update tests
@peterhashair
Copy link
Contributor Author

Our Team finally agree to throw an exception on InviteUser API when initSiteID is empty, but there is no change to addUser API.

@sgiehl sgiehl added this to the 4.11.0 milestone Jul 28, 2022
@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. and removed Needs Review PRs that need a code review labels Jul 28, 2022
@sgiehl sgiehl merged commit 4505820 into next_release Jul 28, 2022
@sgiehl sgiehl deleted the 19574 branch July 28, 2022 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Invite user" feature grants access to the default website, ignoring the selected website
3 participants