Add copy link after token been sent for the user doesnt have email set up from design #19707

peterhashair · 2022-09-05T04:01:11Z

Description:

Fixes: #19625

Add copy link after the token has been sent for the user who doesn't have an email set up from design.

As per the design, add a popup to resend invites.
As suggested, add an additional database field for the link token, and make 2 option works.
Add a new API endpoint.
Add 2 new Icon, arrow left and arrow right
As suggested, when over HTTPS using js WriteText otherwise executeCommand('copy'), there is still a security risk in WriteText, when failed, show the whole link.

This problem needs to update docs as well.

Review

add design copy link

# Conflicts: # plugins/CoreHome/vue/dist/CoreHome.umd.js # plugins/CoreHome/vue/dist/CoreHome.umd.min.js

This reverts commit bb081bd.

add link token

update tests

update xml tests

update UI

update ui

update tests

update screenshots

update ui

justinvelluppillai

I left a few screenshots and comments for you to get it conforming to spec in slack 👍🏽

update css

remove password confirm for invite

update css

…19738) * Adding the abililty to exclude specific sites from the Vue site selector component. * Adding more time to make sure that the select is loaded before trying to search. * Changed things to filter the site on the server side. * Added a few new test cases. * Increasing amount of time for test. * Updated the omnifixture. * Fixing logic error in view. * Removed collation from omnifixture since it was causing issues. * Switching omnifixture back to 4.x-dev version. * Making just the bare essential changes to make local test fixtures build correctly. * Updating screenshot of API list. * revert OmniFixture-dump to 4.x-dev revert OmniFixture-dump to 4.x-dev Co-authored-by: Peter <peter@innocraft.com>

# Conflicts: # tests/UI/expected-screenshots/UIIntegrationTest_api_listing.png

merge 4.x and update copied success ui tests

rebase to next release

justinvelluppillai

In safari the copied link shows up in the header when I click "Copy Link" (see screenshot):

Also when I click Copy Link in the modal in chrome, the modal remains open but the Copy Link and Resend Invite buttons now do nothing when clicked.

update safrai and enable copy success and copy again

peterhashair · 2022-09-29T23:35:44Z

@justinvelluppillai should be fixed now.

fix Safrai showing random token

bx80 · 2022-09-30T02:09:28Z

This is my first look at this PR, it does work 👍 and I'm aware there have already been a lot of points raised in the discussion above, I'll try to avoid rehashing previously covered comments.

a) If I want to create a new user and send them their invite link on slack, then I'd probably end up doing this:

Click 'Invite a new user' (goes to a screen labeled 'Add a new user')
Click 'Invite User'.
Enter my password, then 'Invitation is sent' is shown.
Go back to the user list.
Search for the newly invited user (what was their email again? Type it in...)
Click on an envelope row icon ("but I don't want to email, I want to copy...")
Click 'Copy link' on the popup.
Enter my password again (second time in 30 seconds 🙁 )
Finally the invite link is in the clipboard.

I did this a couple of times before I realized that the 'Resend Invite' link shown in step 3) would allow me to jump to step 7). So although I saw 'Resend Invite' link, it didn't register because I want to copy the link. I never wanted to send an invite in the first place, so wasn't intuitive.

Could we either rename this to 'Copy Invite Link' or duplicate the link so we have 'Resend invite' and 'Copy invite link'?

b) Once the 'copy link' button is used the buttons are disabled but it isn't obvious that they no longer work. If I accidentally copied something else into my clipboard and lost the copied link then clicking the 'Copy link' button again has no effect. Wouldn't it be better to hide the buttons altogether and/or replace them with a close button?

c) Minor cosmetic: The resend invite popup box seems to have a different font size from the normal popups? The password popup font is much smaller, it looks like part of a different application.

d) Minor point: It might be slightly clearer to say 'Link copied to the clipboard' rather than just 'Link copied'.

e) Having to enter the password twice in the space of 10 seconds isn't great, probably beyond the scope of this PR, but maybe we note this somewhere else for improvement, since we're adding password prompts to more places in the UI and risk increasing user frustration.

peterhashair · 2022-09-30T02:31:39Z

@bx80 I agree.

e) Having to enter the password twice in the space of 10 seconds isn't great, probably beyond the scope of this PR, but maybe we note this somewhere else for improvement, since we're adding password prompts to more places in the UI and risk increasing user frustration.

I guess this has come to the design question. @Javi-Ormaechea

Minor point: It might be slightly clearer to say 'Link copied to the clipboard' rather than just 'Link copied'.

…elector (#19738)" This reverts commit a53a628.

bx80 · 2022-09-30T02:48:47Z

e) Having to enter the password twice in the space of 10 seconds isn't great, probably beyond the scope of this PR, but maybe we note this somewhere else for improvement, since we're adding password prompts to more places in the UI and risk increasing user frustration.

I guess this has come to the design question.

We could possibly look at a system level solution - same as sudo won't prompt for your password over and over if you entered it in the last 5 minutes. At the moment being tasked with adding 10 new users and copying their invites would make the admin type their password 20 times in under 5 minutes, it might be secure but it's the sort of UX that causes the admin to change their password to 12345 out of frustration 🙂 I can log a RFC issue on this if you like?

justinvelluppillai · 2022-09-30T02:50:30Z

@bx80 yes it'd be good to maybe create a separate issue for that. I agree it wasn't fun getting links or sending invites during testing 👍🏽

update fonts and text

update wording

bx80

I've spent some time doing functional testing, apart from the multiple password entry annoyance which we can address elsewhere, it works as expected and allows an admin to get an invite link copied to the clipboard fairly quickly after creating the initial invite. I can't see any outstanding issues with the code.

I've pushed a couple of UI test screenshot updates, once the test builds complete without issues then I think this is ready to merge 👍

sgiehl · 2022-10-05T13:44:03Z

@peterhashair There is no need to recreate pull requests when you want to target another branch. You can simply change the target branch of a PR on GitHub. Also it is important to rebase your changes to the new target branch.
This PR actually included some other PRs that had been merged to 4.x-dev. It doesn't hurt that those changes are now included in the release. But it's important to keep any eye on the changes your PRs include.
In general you should avoid closing PRs in order to create a new one afterwards. Unless the changes included in the PR are completely different it would be good to always use the same PR till it is merged. This ensures that all comments that were made can be directly seen. By closing and recreating a PR you actually kind of loose all comments done before, as no one will search if there might have been previous PRs.

peterhashair · 2022-10-05T19:58:17Z

@sgiehl yes, sorry. I confused myself there, after I recreate the PR, and release I can rebase the PR. Will avoid that in the future.

Peter added 5 commits August 26, 2022 12:50

add copy link button and style

0b3ca56

add design copy link

bb081bd

add design copy link

Merge branch '4.x-dev' into m19625

ef5867d

# Conflicts: # plugins/CoreHome/vue/dist/CoreHome.umd.js # plugins/CoreHome/vue/dist/CoreHome.umd.min.js

Revert "add design copy link"

9b99156

This reverts commit bb081bd.

add link token

66b4089

add link token

peterhashair added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Sep 6, 2022

peterhashair added this to the 4.12.0 milestone Sep 6, 2022

Peter and others added 13 commits September 6, 2022 14:27

update tests

4e31f4a

update tests

built vue files

18a4d04

update xml tests

7133ffa

update xml tests

Merge branch 'm19625' of github.com:matomo-org/matomo into m19625

6e6d11d

update UI

9309f2c

update UI

built vue files

ff72d6b

update ui

cf484bf

update ui

Merge branch 'm19625' of github.com:matomo-org/matomo into m19625

4274ed5

Update UsersManagerTest.php

ec0149f

update tests

update tests

3fb941c

update tests

update tests

1e838b3

update tests

Update UsersManager_spec.js

b34728e

update screenshots

update ui

d8dad8f

update ui

peterhashair added the Needs Review PRs that need a code review label Sep 6, 2022

peterhashair requested review from bx80 and justinvelluppillai September 6, 2022 22:38

justinvelluppillai suggested changes Sep 7, 2022

View reviewed changes

justinvelluppillai removed the Needs Review PRs that need a code review label Sep 7, 2022

Peter and others added 5 commits September 7, 2022 14:18

update css

0669f5a

update css

built vue files

524ac7b

remove password confirm for invite

936a06b

remove password confirm for invite

built vue files

dc9288a

update css

27dee5b

update css

peterhashair and others added 2 commits September 26, 2022 23:46

built vue files

4583cae

peterhashair mentioned this pull request Sep 27, 2022

Bring back an option for Add Users / New user the old way #19772

Open

Peter added 5 commits September 27, 2022 14:26

Merge branch '4.x-dev' into m19625

e0617f9

# Conflicts: # tests/UI/expected-screenshots/UIIntegrationTest_api_listing.png

merge 4.x and update copied success ui tests

5764c4e

merge 4.x and update copied success ui tests

Merge branch 'next_release' into m19625

b913cfb

rebase to next release

5730ba6

rebase to next release

rebase to next release

a6579c0

rebase to next release

peterhashair changed the base branch from 4.x-dev to next_release September 27, 2022 01:39

justinvelluppillai suggested changes Sep 29, 2022

View reviewed changes

justinvelluppillai removed the Needs Review PRs that need a code review label Sep 29, 2022

update safrai and enable copy success and copy again

9d1daf7

update safrai and enable copy success and copy again

Peter and others added 2 commits September 30, 2022 12:37

fix Safrai showing random token

d385921

fix Safrai showing random token

built vue files

97dd025

Revert "Adding the abililty to exclude specific sites from the site s…

65ca855

…elector (#19738)" This reverts commit a53a628.

Peter and others added 5 commits September 30, 2022 15:51

update fonts and text

39bbc74

update fonts and text

built vue files

b5c97a0

update wording

ccb4574

update wording

built vue files

19bab37

Updated UI test screenshots

b8505df

bx80 approved these changes Sep 30, 2022

View reviewed changes

bx80 merged commit 2f7c352 into next_release Sep 30, 2022

bx80 deleted the m19625 branch September 30, 2022 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add copy link after token been sent for the user doesnt have email set up from design #19707

Add copy link after token been sent for the user doesnt have email set up from design #19707

peterhashair commented Sep 5, 2022 •

edited

justinvelluppillai left a comment

justinvelluppillai left a comment

peterhashair commented Sep 29, 2022

bx80 commented Sep 30, 2022 •

edited

peterhashair commented Sep 30, 2022

bx80 commented Sep 30, 2022

justinvelluppillai commented Sep 30, 2022

bx80 left a comment

sgiehl commented Oct 5, 2022

peterhashair commented Oct 5, 2022 •

edited

Add copy link after token been sent for the user doesnt have email set up from design #19707

Add copy link after token been sent for the user doesnt have email set up from design #19707

Conversation

peterhashair commented Sep 5, 2022 • edited

Description:

Review

justinvelluppillai left a comment

Choose a reason for hiding this comment

justinvelluppillai left a comment

Choose a reason for hiding this comment

peterhashair commented Sep 29, 2022

bx80 commented Sep 30, 2022 • edited

peterhashair commented Sep 30, 2022

bx80 commented Sep 30, 2022

justinvelluppillai commented Sep 30, 2022

bx80 left a comment

Choose a reason for hiding this comment

sgiehl commented Oct 5, 2022

peterhashair commented Oct 5, 2022 • edited

peterhashair commented Sep 5, 2022 •

edited

bx80 commented Sep 30, 2022 •

edited

peterhashair commented Oct 5, 2022 •

edited