Here is what's left:
@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.
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,
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.