Share personal cleanup + note #29935

felixheidecke · 2017-12-20T15:07:23Z

Description

Reduce length of date field for external link
Change button text 'Save' to 'Share'
Change mail error text
Add send confirmation after sending notification
Add message field for email

Related Issue

Replaces (because contains): #29721
Fixes: #29694

How Has This Been Tested?

Visual tests in latest Chrome & Firefox

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests pass.

PVince81 · 2017-12-20T15:39:14Z

Code looks good so far.

We talked about injecting the default template in there. I'll try exposing this somehow from the backend.

PVince81 · 2017-12-20T15:41:13Z

~~TODO: expose default template~~ => Expose link share email template through APIs #29937
TODO: send message body to server
TODO: server must use message when sending

PVince81 · 2017-12-20T15:58:34Z

pushed: server now uses the specified message. If empty, it falls back to the default template.

Note that when specified it's a plain text email and it strips away the tags with strip_tags.

Next up: find a way to pass original template back for use in the dialog, in a way that clients could also reuse...

PVince81 · 2017-12-20T16:00:30Z

It looks like this would need a new API endpoint. Considering that we want clients to be able to use this email feature too, we'd likely need to add a new public endpoint, maybe an extension of the existing OCS Share API.

And if we need to do this, we might as well move the current emailing code (the sending part) to said endpoint as well. So the endpoint would have two functions:

get email template for given values (expiration, filename, etc)
and
send email with given body

PVince81 · 2017-12-20T16:01:32Z

I suggest merging this PR as is, maybe tweak the current message and work on the email endpoint separately, as I have the feeling this will take longer due to API design decisions needing to be made.

@pmaier1 thoughts ?

PVince81 · 2017-12-20T16:11:17Z

Raised #29937 for exposing template.

PVince81 · 2017-12-20T16:13:59Z

add CC option

@felixheidecke can you add ? I can adjust the backend then

pmaier1 · 2017-12-21T10:02:01Z

I suggest merging this PR as is, maybe tweak the current message and work on the email endpoint separately, as I have the feeling this will take longer due to API design decisions needing to be made.
@pmaier1 thoughts ?

Yes, when the stuff in here #29694 is covered and this is tested we should merge. I would be also very happy if you could provide a patch for this PR then.

codecov · 2018-01-15T11:07:10Z

Codecov Report

Merging #29935 into master will decrease coverage by 0.01%.
The diff coverage is 34.69%.

@@             Coverage Diff              @@
##             master   #29935      +/-   ##
============================================
- Coverage     60.87%   60.85%   -0.02%     
- Complexity    18568    18577       +9     
============================================
  Files          1093     1093              
  Lines         61325    61361      +36     
============================================
+ Hits          37333    37343      +10     
- Misses        23992    24018      +26

Impacted Files	Coverage Δ	Complexity Δ
core/js/config.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
core/ajax/share.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
lib/private/Share/MailNotifications.php	`89.69% <89.47%> (-0.9%)`	`24 <14> (+9)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c86e515...1916d25. Read the comment docs.

PVince81 · 2018-01-16T13:24:59Z

Fixes #29696

Felix: CC must be a checkbox only that appearrs only if OC.getCurrentUser().email is set
Vincent: export email address through OC.getCurrentUser()

PVince81 · 2018-01-16T15:15:05Z

backend must actually set CC to current user's email address

PVince81 · 2018-01-19T11:52:49Z

bcc to self or cc ? @pmaier1

bcc might be a better idea here

DeepDiver1975 · 2018-01-19T11:55:27Z

rebase needed because of conflicts .....

DeepDiver1975 · 2018-01-19T11:57:21Z

core/ajax/share.php

+				$emailBody = trim((string)$_POST['emailBody']);
+			}
+
+			$l10n = \OC::$server->getL10N('lib');


why lib? this file lives in core - did this ever work?

@PVince81 yours?

Maybe this never worked... I just moved up the getL10N call from below.

Considering that emails are usually be taken from the mail template, I don't think the template is translated.

Needs separate testing.

DeepDiver1975 · 2018-01-19T11:57:58Z

core/css/global.css

@@ -24,7 +33,7 @@
 	clear: both;
 }

-.hidden {
+.hidden.hidden {


duplicate hidden?

Yes. This a lesser version of the !important statement;

DeepDiver1975 · 2018-01-19T12:01:11Z

core/js/sharedialogmailview.js

+				action      : 'email',
+				toAddress   : mail.to,
+				emailBody   : mail.body,
+                bccSelf     : mail.bccSelf,


felixheidecke · 2018-01-19T12:15:43Z

@PVince81 pls help with conflicts in share.php

IljaN · 2018-02-07T16:17:06Z

Rebase error?

felixheidecke · 2018-02-07T16:22:41Z

Rebase error?

Never 😇 … 👅

Reduce length of date field for external link Change button text 'Save' to 'Share' Change mail error text Add send confirmation after sending notification defer link creation upon failing email notification Modify UI tests for Public Link dialog Share button Show button text "share" if creating a new share Add message field to share modal Send custom link share mail notification when specified Add email to OC.getCurrentUser() Add bccToSelf checkbox for linkshare Add bcc to self for public link share Add method description Spaces to tabs Rework confusing shorthand code Correctly check bcc post Move email attribute to oc_user attribute Disable autocomplete on email input Bring back OC.currentUser for legacy code... Fix faulty autocomplete Make dialog css more readable Propperly test $options in MailNotification Fix currentUser related tests Fix the fix Add new tests for bcc functionality, remove legacy autocomplete test

PVince81 · 2018-02-08T16:16:57Z

@felixheidecke did it work our with the JS test ?

felixheidecke · 2018-02-08T16:27:22Z

did it work out with the JS test?

Sure did. The test had to fail since it tested a feature that was scaped. I should have guessed that :-P

PVince81 · 2018-02-09T15:51:55Z

@felixheidecke please backport to stable10

PVince81 · 2018-02-20T17:18:10Z

regression: #30557

PVince81 · 2018-04-10T08:43:01Z

stable10: #30486

SergioBertolinSG · 2018-04-13T10:33:29Z

When sending an email it should fail when it is unable to send the email, specially if the email address is not complete:

SergioBertolinSG · 2018-04-18T07:12:07Z

#29935 (comment) is fixed in 10.0.8RC2.

lock · 2019-07-31T10:01:50Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

felixheidecke added 2 - Developing feature:sharing p1-urgent Critical issue, need to consider hotfix with just that issue labels Dec 20, 2017

felixheidecke added this to the development milestone Dec 20, 2017

felixheidecke requested review from PVince81 and pmaier1 December 20, 2017 15:07

ownclouders added the review carefully! label Dec 20, 2017

felixheidecke changed the title ~~Share personal note~~ Share personal cleanup + note Dec 20, 2017

PVince81 mentioned this pull request Dec 20, 2017

Expose link share email template through APIs #29937

Closed

PVince81 mentioned this pull request Dec 20, 2017

Grouped link share #29938

Closed

ownclouders added the status/STALE label Dec 29, 2017

PVince81 modified the milestones: development, planned Jan 12, 2018

ownclouders removed the status/STALE label Jan 15, 2018

felixheidecke force-pushed the share-personal-note branch from c4815b1 to 8371b1a Compare January 16, 2018 13:42

DeepDiver1975 previously requested changes Jan 19, 2018

View reviewed changes

ownclouders added security technical debt labels Feb 7, 2018

ownclouders assigned IljaN Feb 7, 2018

felixheidecke force-pushed the share-personal-note branch 2 times, most recently from d32ecfd to c44a0e2 Compare February 7, 2018 16:20

IljaN assigned felixheidecke and unassigned IljaN and felixheidecke Feb 7, 2018

felixheidecke force-pushed the share-personal-note branch from c44a0e2 to 1916d25 Compare February 8, 2018 15:19

PVince81 merged commit 144b5b1 into master Feb 9, 2018

PVince81 deleted the share-personal-note branch February 9, 2018 15:51

This was referenced Feb 12, 2018

Public links: Add personal note and option to receive a copy #29696

Closed

[stable10] Share personal cleanup + note #30486

Merged

This was referenced Feb 15, 2018

Improve public link share mail address input field #29695

Closed

Email subject for link share is missing values #30557

Closed

PVince81 removed the backport-request label Feb 26, 2018

patrickjahns mentioned this pull request Apr 13, 2018

10.0.8RC1/2/3 - Changelog Testing #31098

Closed

29 tasks

pmaier1 mentioned this pull request Apr 16, 2018

[10.0.8 RC1] Public link share dialog #31125

Closed

9 tasks

lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Share personal cleanup + note #29935

Share personal cleanup + note #29935

felixheidecke commented Dec 20, 2017

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017 •

edited

Loading

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

pmaier1 commented Dec 21, 2017

codecov bot commented Jan 15, 2018 •

edited

Loading

PVince81 commented Jan 16, 2018 •

edited by felixheidecke

Loading

PVince81 commented Jan 16, 2018

PVince81 commented Jan 19, 2018

DeepDiver1975 commented Jan 19, 2018

DeepDiver1975 Jan 19, 2018

felixheidecke Jan 19, 2018

PVince81 Jan 22, 2018

DeepDiver1975 Jan 19, 2018

felixheidecke Jan 19, 2018

DeepDiver1975 Jan 19, 2018

felixheidecke commented Jan 19, 2018

IljaN commented Feb 7, 2018

felixheidecke commented Feb 7, 2018 •

edited

Loading

PVince81 commented Feb 8, 2018

felixheidecke commented Feb 8, 2018

PVince81 commented Feb 9, 2018

PVince81 commented Feb 20, 2018

PVince81 commented Apr 10, 2018

SergioBertolinSG commented Apr 13, 2018

SergioBertolinSG commented Apr 18, 2018

lock bot commented Jul 31, 2019

Share personal cleanup + note #29935

Share personal cleanup + note #29935

Conversation

felixheidecke commented Dec 20, 2017

Description

Related Issue

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017 • edited Loading

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

PVince81 commented Dec 20, 2017

pmaier1 commented Dec 21, 2017

codecov bot commented Jan 15, 2018 • edited Loading

Codecov Report

PVince81 commented Jan 16, 2018 • edited by felixheidecke Loading

PVince81 commented Jan 16, 2018

PVince81 commented Jan 19, 2018

DeepDiver1975 commented Jan 19, 2018

DeepDiver1975 Jan 19, 2018

Choose a reason for hiding this comment

felixheidecke Jan 19, 2018

Choose a reason for hiding this comment

PVince81 Jan 22, 2018

Choose a reason for hiding this comment

DeepDiver1975 Jan 19, 2018

Choose a reason for hiding this comment

felixheidecke Jan 19, 2018

Choose a reason for hiding this comment

DeepDiver1975 Jan 19, 2018

Choose a reason for hiding this comment

felixheidecke commented Jan 19, 2018

IljaN commented Feb 7, 2018

felixheidecke commented Feb 7, 2018 • edited Loading

PVince81 commented Feb 8, 2018

felixheidecke commented Feb 8, 2018

PVince81 commented Feb 9, 2018

PVince81 commented Feb 20, 2018

PVince81 commented Apr 10, 2018

SergioBertolinSG commented Apr 13, 2018

SergioBertolinSG commented Apr 18, 2018

lock bot commented Jul 31, 2019

PVince81 commented Dec 20, 2017 •

edited

Loading

codecov bot commented Jan 15, 2018 •

edited

Loading

PVince81 commented Jan 16, 2018 •

edited by felixheidecke

Loading

felixheidecke commented Feb 7, 2018 •

edited

Loading