Add expiration date to share email by RageZBla · Pull Request #17379 · nextcloud/server

RageZBla · 2019-10-02T22:00:03Z

Fixes #15680

RageZBla · 2019-10-02T22:01:10Z

I could not run phpunit test. Can someone point me out the documentation?

kesselb · 2019-10-03T10:19:02Z

Can someone point me out the documentation?

https://docs.nextcloud.com/server/17/developer_manual/core/unit-testing.html
(not sure how up to date this page is).

https://drone.nextcloud.com/nextcloud/server/22100/15/5

RageZBla · 2019-10-03T12:16:23Z

@kesselb thanks for the hand. I think that the test is fixed. If you have the chance to review the change 🙇‍♂️

kesselb

Great contribution 🚀 Thank you 👍

apps/sharebymail/lib/ShareByMailProvider.php

kesselb · 2019-10-03T12:52:36Z

apps/sharebymail/lib/ShareByMailProvider.php

 		$text = $this->l->t('%1$s shared »%2$s« with you.', [$initiatorDisplayName, $filename]);
+		if ($expiration) {
+			$relativeDate = (string)$expiration->diff(new \DateTime('now'))->format("%a");
+			$text .= ' ' . $this->l->t('It will expire in %1$s days.', [$relativeDate]);


Hmm. Don't know much about UX but I think we should make this easier to understand.

Example:
A) You are able to access this file until xyz.
B) For security the share is only valid for x days. It's not possible to access the data after: xzy.

Let's ask @jancborchardt and @jospoortvliet for help.

Yes, it would be good to make this expiration info a second paragraph below the other one so that it’s clearly separated. (Also the sentence "Click the button below" can really be cut …)

This share will expire in 5 days, on October 30 at 12:00 UTC.

Otherwise this pull request seems good! Yes we can improve a lot on the design and should do that – @RageZBla your call if you want to do more here. :) (E.g. the triplication of subject, title and first sentence of the content.)

For the record, my comment above still stands. :) Also, it would be nice to handle singular correctly cause it looks weird if "this share will expire in 1 days". ;)

skjnldsv · 2019-10-03T12:55:39Z

Hey! Thanks for this!
Could you post screenshots of the mail?

kesselb · 2019-10-03T13:10:03Z

@skjnldsv is it possible that "set default expiration date" (from the sharing settings) is not used by the sharebymail app?

skjnldsv · 2019-10-03T13:13:58Z

@kesselb the expiration date enforce is only for share links! :)

Signed-off-by: Olivier Lechevalier <olivier.lechevalier@gmail.com>

Co-Authored-By: Daniel Kesselberg <mail@danielkesselberg.de> Signed-off-by: Olivier Lechevalier <olivier.lechevalier@gmail.com>

kesselb · 2019-10-03T13:16:25Z

How can I set an expiration date then? ;)

skjnldsv · 2019-10-03T13:20:35Z

How can I set an expiration date then? ;)

In any sharing? It will just not be enforced :)

kesselb · 2019-10-03T13:26:23Z

Hmm. I'm creating a new share by entering my email. Hit enter. I receive the share notification email. For this feature we need a expiration date before the share notification is sent?

skjnldsv · 2019-10-03T13:29:21Z

Hmm. I'm creating a new share by entering my email. Hit enter. I receive the share notification email. For this feature we need a expiration date before the share notification is sent?

@kesselb sharing with a user with an email should work as well.

skjnldsv · 2019-10-03T13:31:16Z

Hmm. I'm creating a new share by entering my email. Hit enter. I receive the share notification email. For this feature we need a expiration date before the share notification is sent?

ah yes, I guess it doesn't send another email after you set the expiration date.
on the new sharing ui, you will have the same settings for mail share than link shares, so you will be able to enforce it for mail shares, so I guess it will make more sense :)

kesselb · 2019-10-03T13:33:35Z

@skjnldsv thanks 👍

Index: apps/sharebymail/lib/ShareByMailProvider.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- apps/sharebymail/lib/ShareByMailProvider.php	(revision 9bf55c33e15e6c2a576604e35e86b344fa6f534f)
+++ apps/sharebymail/lib/ShareByMailProvider.php	(date 1570109535142)
@@ -343,6 +343,8 @@
 			$share->getSendPasswordByTalk()
 		);
 
+		$share->setExpirationDate((new \DateTime())->add(new \DateInterval('P7D')));
+
 		try {
 			$link = $this->urlGenerator->linkToRouteAbsolute('files_sharing.sharecontroller.showShare',
 				['token' => $share->getToken()]);

In the meantime I used above to set test this email 😆

skjnldsv · 2019-10-03T13:34:25Z

A bit more easier from the console @kesselb

$.post("/ocs/v2.php/apps/files_sharing/api/v1/shares?format=json", {
    shareType: 4,
    shareWith: 'email@domain.ext',
    permissions: 27,
    path: '/welcome.txt',
	expireDate: '24-10-2019'
})

skjnldsv · 2019-10-03T13:51:32Z

I don't really like the design but not related to this pr, the current state is a bit weird already 😉).
I think we should improve and use similar stuff like calendar does:

rullzer · 2020-04-24T13:46:44Z

So, the issue is that this is not always there as the file is already shared right away.
We should really do this as a bigger picture where the share is properly set before and then also fix all the mails etc.

skjnldsv · 2024-02-27T13:05:11Z

As there is no feedback since a while I will close this ticket.
If you will decide to work on this feature again and if it hasn't been fixed or implemented already, feel free to re-open and solve the various conflicts.

Thanks for the interest in Nextcloud and the effort put into this! 🙇

kesselb added 2. developing Work in progress enhancement labels Oct 3, 2019

RageZBla force-pushed the add-expiration-date-share-email branch 4 times, most recently from 2a6ab0d to 322ca81 Compare October 3, 2019 11:36

kesselb reviewed Oct 3, 2019

View reviewed changes

kesselb requested a review from skjnldsv October 3, 2019 12:53

skjnldsv added feature: emails feature: sharing labels Oct 3, 2019

skjnldsv added this to the Nextcloud 18 milestone Oct 3, 2019

RageZBla and others added 3 commits October 3, 2019 22:16

Add expiration date to share email

2176866

Signed-off-by: Olivier Lechevalier <olivier.lechevalier@gmail.com>

Update apps/sharebymail/lib/ShareByMailProvider.php

ce5bc5b

Co-Authored-By: Daniel Kesselberg <mail@danielkesselberg.de> Signed-off-by: Olivier Lechevalier <olivier.lechevalier@gmail.com>

Update apps/sharebymail/lib/ShareByMailProvider.php

fff5511

Co-Authored-By: Daniel Kesselberg <mail@danielkesselberg.de> Signed-off-by: Olivier Lechevalier <olivier.lechevalier@gmail.com>

RageZBla force-pushed the add-expiration-date-share-email branch from 2fa4042 to fff5511 Compare October 3, 2019 13:16

rullzer mentioned this pull request Dec 11, 2019

18 Beta 2 #18343

Merged

43 tasks

This was referenced Dec 12, 2019

18 beta 3 #18370

Merged

18 beta 4 #18480

Merged

rullzer modified the milestones: Nextcloud 18, Nextcloud 19 Dec 19, 2019

This was referenced Apr 4, 2020

19 beta 2 #20303

Merged

19 beta 3 #20386

Merged

This was referenced Apr 15, 2020

19 beta 4 #20506

Merged

19 Beta 5 #20510

Merged

rullzer mentioned this pull request Apr 23, 2020

19 beta 6 #20611

Merged

11 tasks

rullzer removed this from the Nextcloud 19 milestone Apr 24, 2020

skjnldsv closed this Feb 27, 2024

Uh oh!

Conversation

RageZBla commented Oct 2, 2019 • edited by kesselb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RageZBla commented Oct 2, 2019

Uh oh!

kesselb commented Oct 3, 2019

Uh oh!

RageZBla commented Oct 3, 2019

Uh oh!

kesselb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kesselb Oct 3, 2019

Choose a reason for hiding this comment

Uh oh!

jancborchardt Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

jancborchardt Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

jancborchardt Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

skjnldsv commented Oct 3, 2019

Uh oh!

kesselb commented Oct 3, 2019

Uh oh!

skjnldsv commented Oct 3, 2019

Uh oh!

kesselb commented Oct 3, 2019

Uh oh!

skjnldsv commented Oct 3, 2019

Uh oh!

kesselb commented Oct 3, 2019

Uh oh!

skjnldsv commented Oct 3, 2019

Uh oh!

skjnldsv commented Oct 3, 2019

Uh oh!

kesselb commented Oct 3, 2019

Uh oh!

skjnldsv commented Oct 3, 2019

Uh oh!

skjnldsv commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rullzer commented Apr 24, 2020

Uh oh!

skjnldsv commented Feb 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

RageZBla commented Oct 2, 2019 •

edited by kesselb

Loading

skjnldsv commented Oct 3, 2019 •

edited

Loading