update invite user #19366

Description:

Here is what's left:

• Activity log should recognize "invite user" and "invite user accepted". (create plugin issue)

Review

• [ ] Functional review done
• [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• [ ] Security review done
• [ ] Wording review done
• [ ] Code review done
• [ ] Tests were added if useful/possible
• [ ] Reviewed for breaking changes
• [ ] Developer changelog updated if needed
• [ ] Documentation added if needed
• [ ] Existing documentation updated if needed

@peterhashair I only had a very rough look through your changes (couple of minutes only). And without looking into details it's already clear, that your changes would not be approve-able.

• The PR still does not fulfill the requirements of the original
  • user should be removed when he declines
  • privacy url should be shown if available (not only terms & conditions)
  • ...
• Your PR updates screenshots that are unrelated (e.g. overlay, visitor map)
• ....

Overall it looks a bit like you've only addressed half of the comments I left on the previous PR.

@bx80 I believe, this time I update all the remaining list,

• the status filter is added
• add coupe more invite user API tests
• config for the default expiration date
• a task for deleting expired invite
• wording change
• permission update, admin and super admin use can invite, only the invited from admin and super admin can see their invites
• remove pending status to expire in n days.

I'm seeing the 'Resend invite' envelope action for all users on the list. Shouldn't this just be showing for users who have a pending invite?

@bx80 yes, js error, updated

If the token expiry days config setting is set to something other than the default 7, then the correct expiry timestamp is used, but the invite email message will still show 7 days. It looks like the UserInviteNotes translation has the 7 days text hard-coded.

@bx80 updated

@peterhashair Looks like it needed another UI screenshot update for UsersManager_resend_popup.png - I've added it.

@sgiehl Hopefully all the issues you raised on the previous PR and all the requirements mentioned on the original issue should now have been addressed :crossed_fingers:. This PR is ready for a final review when you get a chance. :slightly_smiling_face:

@peterhashair to get the UI tests working correctly you still need to update the version here:
https://github.com/matomo-org/matomo/blob/bd5571897f4e8408ba168b34754f4fff523894a7/core/Version.php#L24
Otherwise the update script won't be triggered when setting up the omnifxiture.

@sgiehl try to do the reset password pending user check https://github.com/matomo-org/matomo/pull/19366/commits/2a61251978d1fa36fb2f8e9f98153ba7b4c06e66, but it keeps failing on of the UI test. https://app.travis-ci.com/github/matomo-org/matomo/jobs/575038221

any ideas?

@peterhashair That's quite easy to explain. You changed the update script. It now renames the column invite_status to invite_token. While I'm not sure what that should be good for, that actually causes all existing user having an invite_token value of accept (default value of invite_status) after updating.

@peterhashair I had another look and directly pushed a couple of fixes, so we might be able to get this done by next week.
Will have another look tomorrow and check again the comments on the previous PR to see if everything I mentioned back then is fixed now.

I think I'm done with applying fixes & improvements here. I really hope I didn't miss anything else. There were a couple of bugs and regressions (introduced to existing methods). Also the usability was broken at some places. In addition the test coverage of that new feature was quite bad, so I've added some additional tests.

@bx80 Could you please do another review of that issue? Would be good to look through the code changes as well as doing some local testing to ensure the whole invite user process works as expected.

@peterhashair please have a look at all the changes I did as well and leave comments if it's unclear why something was changed.

@sgiehl had a look through, make sense, thanks for fixing my mess 🤙