Show sharee in dropdown to send notification mail again

[danxuliu]

When a file is shared with a user and that user has an e-mail set in her profile a mail notification is sent to that e-mail. However, once a file has been shared there is currently no way to send again that e-mail notification if needed for any reason.

Users that are already sharees are no longer shown in the share dropdown, so one option could be to provide an additional action in the share menu; however, although this would work for a small list of sharees it could be slow if there are a lot of them. Therefore, instead of adding an action in the share menu, the share dropdown was modified so if a user is already a sharee and she has an e-mail set in her profile then the user will appear in the dropdown and clicking on it will send again the e-mail notification.

In the example screenshot below user0 has an e-mail address set in the profile while user1 does not:

Note that this applies only to shares to another user; shares to an e-mail address are not taken into account here, as they use a different workflow.

Although sending the e-mail should work (as I just copied some code from the createShares) it should be explicitly tested, as I could not do that yet. In the same way please review the PHP changes carefully, as I am not familiar with the area that I had to modify to get this working.

[mention-bot]

@danxuliu, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nickvergessen, @schiessle and @rullzer to be potential reviewers.

[jancborchardt]

@danxuliu hm, interesting idea. However only having a tooltip to show it’s already shared is not enough and does not work on mobile. We need to say smth like »Lukas (already shared, resend notification?)« – which then is pretty long.

We should at least also have the button »Resend notification« in the 3-dot menu as specified. :)

[jancborchardt]

A big confusing point here is that the list entry now has a different functionality on tap, and also is different to the list below anyway. We could say that the share input field is a search box even for the people with whom the file is shared already – so they should be shown like in the list below (with the [ ] can edit and the 3-dot menu)

[MorrisJobke]

Let me take care of this.

[MorrisJobke]

Okay - I tested this. My idea here would be to split this a bit up, because it is a lot of code and get it in step by step. The PHP code itself looks quite good and should be easy to incorporate without side effects. Also a backport is here possible. Then we can review the actual behaviour in a separate PR.

@oparoz I would go for the 3 dots menu in our product itself. It is then based on the same backend code, but we are a bit more flexible and don't need to maintain over 1k of patch, but just a few lines of frontend code that may be different. Or just present the way we would do in our product and see if this is fine as well.

@danxuliu I would do a separate PR with only the backend changes (first 6 commits). We could also port them to stable12. Then after this we should look into the JS modifications and check out what we want and what not. Does that sound like a plan? Let me do a PR with the first six commits and call the sharing guys, like @rullzer and @schiessle ;)

[MorrisJobke]

Here is the PHP part: #6322

[nickvergessen]

this method got extended and now has additional arguments

[blizzz]

Needs a rebase, and adaption to /ShareesAPIController.php / refactor of #6328.

If this needs to go back to 12, you likely want to save the current change for backporting.

[MorrisJobke]

@oparoz @danxuliu I guess we don't want this anymore, right?

[danxuliu]

@MorrisJobke Mmmm, I think that we decided to not add this feature to the main Nextcloud code, but @oparoz added this to Nextcloud 13 milestone, so I am not sure...

[oparoz]

OK, so I think @jancborchardt should decide here.
We will have another feature in 13, #7196, which makes it possible to quickly re-send a notification by typing an email address and pressing on the send button. This is somewhat hidden, but intuitive at the same time, so it could be enough. If Jan decides that it would be good to have this PR merged as well, then this can also be included, but it doesn't need to be in 13 imo.

[danxuliu]

We will have another feature in 13, #7196, which makes it possible to quickly re-send a notification by typing an email address and pressing on the send button.

Not exactly; #7196 will make possible to create a share by typing an email address and pressing on the send button, but it will keep the default behaviour, so if the share is already created it will fail.

The changes in this pull request are what cause an e-mail notification to be sent again instead of failing if a share is already created, so both pull requests would be needed for the behaviour you described.

[oparoz]

OK, so this has to be split in 2.

1. Email notification mechanism so that a notification can always be sent -> 13
2. Email icon/button next to an existing share -> 14

[MorrisJobke]

Email notification mechanism so that a notification can always be sent -> 13

Then it blocks the beta 2. Because we will not accept new features anymore.

[danxuliu]

OK, so should a Send e-mail notification again action be added to the three-dots menu of a share, or should it appear in the search results (as shown in the original screenshot)?

Note that in the former case even after merging #7196 it will not be possible to send the e-mail again just typing the sharee name in the search box and hitting enter/clicking on the send button (it will just say that the share already exists; to send the notification again the three-dots menu would have to be used anyway), but the later case has some usability issues as mentioned by @jancborchardt above.

[jancborchardt]

I wouldn’t take this in because it’s confusing. People you already shared with (like user0 in the screenshot) are still suggested in the dropdown. And on top the notice is only visible on hover.

As said, we should do it via an entry in the 3-dot menu if at all.

[codecov]

Codecov Report

Merging #6276 into master will increase coverage by 0.03%.
The diff coverage is 83.14%.

@@             Coverage Diff              @@
##             master    #6276      +/-   ##
============================================
+ Coverage     51.11%   51.14%   +0.03%     
- Complexity    24900    24912      +12     
============================================
  Files          1601     1601              
  Lines         94772    94842      +70     
  Branches       1367     1373       +6     
============================================
+ Hits          48438    48508      +70     
  Misses        46334    46334

Impacted Files	Coverage Δ	Complexity Δ
apps/files_sharing/appinfo/routes.php	0% <0%> (ø)	0 <0> (ø)	⬇️
...private/Collaboration/Collaborators/MailPlugin.php	67.07% <0%> (+8.32%)	19 <0> (ø)	⬇️
lib/private/Group/Manager.php	89.28% <100%> (+0.39%)	59 <12> (+2)	⬆️
...private/Collaboration/Collaborators/UserPlugin.php	81.08% <80.95%> (-2%)	21 <0> (+3)
core/js/shareitemmodel.js	87.12% <84.61%> (-0.11%)	0 <0> (ø)
...iles_sharing/lib/Controller/ShareAPIController.php	68.3% <88.88%> (+0.62%)	158 <3> (+3)	⬆️
core/js/sharedialogview.js	77.82% <93.75%> (+2.38%)	0 <0> (ø)	⬇️
lib/private/Share20/Manager.php	86.84% <94.11%> (+0.2%)	240 <4> (+4)	⬆️
apps/files_trashbin/lib/Trashbin.php	72.53% <0%> (+0.24%)	136% <0%> (ø)	⬇️
... and 2 more

[danxuliu]

I have been thinking about this and the best I can come up with that covers both requirements (being able to quickly send again an e-mail notification but without showing people you already shared with in the dropdown) is the following:

• The Send again e-mail notification action is shown in the three-dots menu of each share in which it is applicable
• The list of suggestions is shown above the input field instead of below like now and it only contains sharees with whom the file is not shared yet
• The list of shares is filtered based on the current value of the input field, so only the shares that match that value are shown below the input field

That would still require the user to filter the shares and then show the three-dots menu and trigger the Send again e-mail notification action; it could be further streamlined with the send button/pressing enter:

• If a sharee with whom the file is not shared yet is an exact match for the input field value clicking on the send button/pressing enter creates a new share
• If a sharee with whom the file is already shared is an exact match for the input field value and the sharee has an e-mail address set clicking on the send button/pressing enter sends again the e-mail notification

If the sharee does not have an e-mail address set or if there is not an exact match then the send button would be disabled; pressing enter when the send button is disabled would show a notification like No e-mail address set for $user or No exact match for $inputFieldValue, depending on the case.

@oparoz @jancborchardt What do you think?

[jancborchardt]

The Send again e-mail notification action is shown in the three-dots menu of each share in which it is applicable

Yup, as I said. :)

The list of suggestions is shown above the input field instead of below like now and it only contains sharees with whom the file is not shared yet

I’ve never seen that anywhere, it’s very very confusing.

The list of shares is filtered based on the current value of the input field, so only the shares that match that value are shown below the input field

That completely changes the meaning of the field from a »Share with new person« to »Filter the people already shared«.

It might be good to have a means to filter the current sharees, but I feel this is a separate discussion and this proposal would completely break the normal base flow.

What is really the problem in only having the option in the 3-dot-menu?

[jancborchardt]

But yes, what we could do is what you mention above:

If a sharee with whom the file is already shared is an exact match for the input field value and the sharee has an e-mail address set clicking on the send button/pressing enter sends again the e-mail notification

In additon though it then should show the person in the dropdown – but only when there is an exact match! With the second line being something like »Already shared, resend notification?«

[danxuliu]

The list of suggestions is shown above the input field instead of below like now and it only contains sharees with whom the file is not shared yet

I’ve never seen that anywhere, it’s very very confusing.

Well, it is exactly the same as now, but changing the position where the suggestions are shown to above the input field instead of below; I would call it inconsistent but I do not think that it is very very confusing :-)

The list of shares is filtered based on the current value of the input field, so only the shares that match that value are shown below the input field

That completely changes the meaning of the field from a »Share with new person« to »Filter the people already shared«.

I would say that it changes the meaning of the field to both of them, as the new persons would be shown above the field and the people already shared would be shown below it.

It might be good to have a means to filter the current sharees, but I feel this is a separate discussion and this proposal would completely break the normal base flow.

OK, it was just an idea ;-)

What is really the problem in only having the option in the 3-dot-menu?

Ask the customer, not me; I am just trying to satisfy both of you :-P

In additon though it then should show the person in the dropdown – but only when there is an exact match! With the second line being something like »Already shared, resend notification?«

Sorry, what do you mean by the second line?

To sum up, would this behaviour be acceptable?:

• The Send again e-mail notification action is shown in the three-dots menu of each share in which it is applicable.
• The list of suggestions is shown below the input field and it only contains sharees with whom the file is not shared yet (exactly like done now).

Once the send button is implemented the following behaviour will be added:

• If a sharee with whom the file is not shared yet is an exact match for the input field value clicking on the send button/pressing enter creates a new share.
• If a sharee with whom the file is already shared is an exact match for the input field value and the sharee has an e-mail address set clicking on the send button/pressing enter sends again the e-mail notification. In this case the sharee would appear too in the list of suggestions.
• If the sharee does not have an e-mail address set or if there is not an exact match then the send button would be disabled; pressing enter when the send button is disabled would show a notification like No e-mail address set for $user or No exact match for $inputFieldValue, depending on the case.

[jancborchardt]

Yep, that sounds good!

Remember I'm here to represent more regular, less technical users. Sharing is pretty core so it has to be dead-simple. :)

[MorrisJobke]

As this does not seem to be the way we want to go for 100% we should rather focus on the other paper cuts like #5336