Enhance calendar invitation emails #6402

leonklingele-work · 2017-09-07T14:36:03Z

This PR adds support to customize DAV-related emails on a per-attendee basis.

codecov · 2017-09-08T09:47:01Z

Codecov Report

Merging #6402 into master will increase coverage by 0.02%.
The diff coverage is 72.41%.

@@             Coverage Diff              @@
##             master    #6402      +/-   ##
============================================
+ Coverage     50.61%   50.64%   +0.02%     
- Complexity    24297    24329      +32     
============================================
  Files          1577     1577              
  Lines         92922    93047     +125     
  Branches       1359     1359              
============================================
+ Hits          47036    47121      +85     
- Misses        45886    45926      +40

Impacted Files	Coverage Δ	Complexity Δ
apps/dav/appinfo/v1/caldav.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
lib/private/L10N/L10N.php	`87.93% <100%> (+0.43%)`	`5 <0> (ø)`	⬇️
apps/dav/lib/Server.php	`46.62% <100%> (+0.36%)`	`17 <0> (ø)`	⬇️
apps/dav/lib/CalDAV/Schedule/IMipPlugin.php	`71.72% <72.14%> (-3.64%)`	`54 <46> (+32)`
lib/private/Mail/Message.php	`77.55% <0%> (-6.13%)`	`20% <0%> (ø)`
lib/private/Security/CertificateManager.php	`91.08% <0%> (-1%)`	`39% <0%> (ø)`
lib/private/Server.php	`83.41% <0%> (+0.12%)`	`125% <0%> (ø)`	⬇️

leonklingele-work · 2017-09-08T09:50:29Z

apps/dav/appinfo/v1/caldav.php

@@ -84,7 +84,15 @@
 $server->addPlugin(new \Sabre\DAV\Sync\Plugin());
 $server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());
 $server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin());
-$server->addPlugin(new \OCA\DAV\CalDAV\Schedule\IMipPlugin( \OC::$server->getMailer(), \OC::$server->getLogger(), new \OC\AppFramework\Utility\TimeFactory()));
+$server->addPlugin(new \OCA\DAV\CalDAV\Schedule\IMipPlugin(
+	'dav', // TODO(leon): Retrieve dynamically, but where to find it? :(


@nickvergessen any idea on how to retrieve the current app's name from here?

"current" app name is dav, because this code is part of the dav app and the dav api is used by the calendar app

I know, I'm the one who added 'dav'.. But how to retrieve this value dynamically? I don't want to hardcode it.

In this scope here you cannot retrieve this dynamically with public API and need to hard-code it. In a controller it will get injected…

nickvergessen · 2017-09-08T09:53:16Z

apps/dav/lib/CalDAV/Schedule/IMipPlugin.php

+
+	private function renderMailTemplates($name, $params) {
+		$tmplBase = 'mail/' . $name;
+		$plainTemplate = new TemplateResponse($this->appName, $tmplBase . '-plain', $params, 'blank');


Please use the new $mailer->createMail()->createEMailTemplate() for this...

Yay, new stuff is cool.

Hmpf, do you really prefer the new style? I don't.

Proper link: a4deea5

nickvergessen · 2017-09-08T09:53:55Z

apps/dav/lib/CalDAV/Schedule/IMipPlugin.php

+		return null;
+	}
+
+	private function stringOrDefault($str, $def) {


use $str ?: $def instead

Is this already in all PHP versions currently supported by NC?

EDIT:
Fix typo, sorry ;)

Yeah its available since 5.3, we require 5.6

you misunderstood. Drop this complete method and use ?: directly.
No need to have a custom method which just wraps an operator?!

Makes sense, updated.

leonklingele-work · 2017-10-09T11:42:08Z

apps/dav/lib/Server.php

-		$this->server->addPlugin(new IMipPlugin($mailer, $logger, $timezone));
+		$this->server->addPlugin(new IMipPlugin(
+			'dav', // TODO(leon): Retrieve dynamically, but where to find it? :(
+			\OC::$server->getUserSession()->getUser()->getUID(),


This fails on anonymous users. How to properly retrieve the user id?

You want the owner of the event? Or which user should be returned?

Otherwise try:

$user = \OC::$server->getUserSession()->getUser(); $userId = $user instanceof IUser ? $user->getUID() : '';

leonklingele-work · 2017-10-09T11:42:33Z

apps/dav/appinfo/v1/caldav.php

-$server->addPlugin(new \OCA\DAV\CalDAV\Schedule\IMipPlugin( \OC::$server->getMailer(), \OC::$server->getLogger(), new \OC\AppFramework\Utility\TimeFactory()));
+$server->addPlugin(new \OCA\DAV\CalDAV\Schedule\IMipPlugin(
+	'dav', // TODO(leon): Retrieve dynamically, but where to find it? :(
+	\OC::$server->getUserSession()->getUser()->getUID(),


See https://github.com/nextcloud/server/pull/6402/files#r143444470

fancycode · 2017-10-13T13:33:08Z

It would probably be a lot less intrusive to just dispatch an event before sending the mail in

server/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php

Line 136 in a1df91d

$failed = $this->mailer->send($message);

and pass the $iTipMessage and $message as parameters. That way any app could listen for the event and update the mail contents as needed (for the changes here, that would be the calendar app with the Spreed integration).

georgehrke · 2017-10-15T13:42:52Z

Any reason why u use Swift directly? I would really like to see this use the Nextcloud 12 Email Template class.

leonklingele-work · 2017-10-16T08:49:32Z

@fancycode

It would probably be a lot less intrusive to just dispatch an event

How would this be less intrusive? This PR is not spreed-specific, though I agree I should probably use a different wording here, event instead of meeting.

@georgehrke

Nextcloud 12 Email Template class

i.e.? Isn't this what Joas suggested here: #6402 (comment)
How to not use swiftmail for attachments?

leonklingele-work · 2017-10-16T08:53:21Z

@fancycode wouldn't this require some kind of way to load the Calendar app on requests to remote.php?
I'd prefer to trigger an event to allow other apps to extend the mail template from this PR with their own section.

georgehrke · 2017-10-16T09:14:36Z

How to not use swiftmail for attachments?

I think we should extend EMailTemplate to be able to add extensions in that case.

fancycode · 2017-10-16T10:22:35Z

@fancycode wouldn't this require some kind of way to load the Calendar app on requests to remote.php?

Sure, but wouldn't that also be the case for apps that listen for events to extend the mail template?

I'd prefer to trigger an event to allow other apps to extend the mail template from this PR with their own section.

georgehrke · 2017-10-31T16:07:30Z

please revert 3rdparty changes

georgehrke · 2017-11-02T10:00:27Z

hijacking this PR to fix various issues in the email

Signed-Off-By: Leon Klingele <[email protected]> Signed-off-by: Georg Ehrke <[email protected]>

Signed-off-by: Joas Schilling <[email protected]>

Signed-off-by: Georg Ehrke <[email protected]>

georgehrke · 2017-11-03T13:00:04Z

please review @LukasReschke @MorrisJobke

georgehrke · 2017-11-03T13:00:30Z

Let's get this in and do further tiny enhancements after feature freeze.

leonklingele mentioned this pull request Sep 7, 2017

Spreed integration nextcloud/calendar#563

Closed

10 tasks

leonklingele-work commented Sep 8, 2017

View reviewed changes

nickvergessen reviewed Sep 8, 2017

View reviewed changes

MorrisJobke changed the title ~~WIP:DAV: Initial email customization support~~ DAV: Initial email customization support Sep 8, 2017

MorrisJobke added the 2. developing Work in progress label Sep 8, 2017

leonklingele-work commented Oct 9, 2017

View reviewed changes

nickvergessen force-pushed the dav-email-customization branch from 49614be to 71bd8bf Compare October 31, 2017 15:24

nickvergessen changed the title ~~DAV: Initial email customization support~~ Enhance calendar invitation emails Oct 31, 2017

nickvergessen added enhancement feature: dav labels Oct 31, 2017

nickvergessen force-pushed the dav-email-customization branch from 71bd8bf to e16f670 Compare October 31, 2017 19:59

nickvergessen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 1, 2017

georgehrke force-pushed the dav-email-customization branch from 0c62d18 to e16f670 Compare November 1, 2017 22:05

georgehrke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 2, 2017

georgehrke self-assigned this Nov 2, 2017

Leon Klingele and others added 2 commits November 3, 2017 11:19

DAV: Initial email customization support

c899f35

Signed-Off-By: Leon Klingele <[email protected]> Signed-off-by: Georg Ehrke <[email protected]>

Fix phpStorm complaints

cf04093

Signed-off-by: Joas Schilling <[email protected]>

nickvergessen and others added 4 commits November 3, 2017 11:20

Language depending dates

b14b933

Signed-off-by: Joas Schilling <[email protected]>

Correctly use the email templating

1bb048d

Signed-off-by: Joas Schilling <[email protected]>

fancify invitation emails

43147ae

Signed-off-by: Georg Ehrke <[email protected]>

Make L10N->l provide weekdayName

e111da7

Signed-off-by: Georg Ehrke <[email protected]>

georgehrke force-pushed the dav-email-customization branch from 4214fbc to e111da7 Compare November 3, 2017 10:20

georgehrke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 3, 2017

georgehrke requested a review from MorrisJobke November 3, 2017 13:00

LukasReschke approved these changes Nov 3, 2017

View reviewed changes

MorrisJobke approved these changes Nov 6, 2017

View reviewed changes

MorrisJobke merged commit 6c29ce4 into master Nov 6, 2017

MorrisJobke deleted the dav-email-customization branch November 6, 2017 14:14

MorrisJobke added this to the Nextcloud 13 milestone Nov 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance calendar invitation emails #6402

Enhance calendar invitation emails #6402

leonklingele-work commented Sep 7, 2017

codecov bot commented Sep 8, 2017 •

edited

Loading

leonklingele-work Sep 8, 2017

nickvergessen Sep 8, 2017

leonklingele-work Sep 8, 2017

LukasReschke Oct 19, 2017

nickvergessen Sep 8, 2017

leonklingele-work Sep 8, 2017

leonklingele-work Sep 8, 2017

leonklingele-work Sep 8, 2017

nickvergessen Sep 8, 2017 •

edited

Loading

leonklingele-work Sep 8, 2017 •

edited

Loading

nickvergessen Sep 8, 2017

nickvergessen Sep 8, 2017

leonklingele-work Sep 8, 2017

leonklingele-work Oct 9, 2017

nickvergessen Oct 9, 2017

leonklingele-work Oct 9, 2017

fancycode commented Oct 13, 2017

georgehrke commented Oct 15, 2017

leonklingele-work commented Oct 16, 2017

leonklingele-work commented Oct 16, 2017

georgehrke commented Oct 16, 2017

fancycode commented Oct 16, 2017

georgehrke commented Oct 31, 2017 •

edited

Loading

georgehrke commented Nov 2, 2017

georgehrke commented Nov 3, 2017

georgehrke commented Nov 3, 2017

Enhance calendar invitation emails #6402

Enhance calendar invitation emails #6402

Conversation

leonklingele-work commented Sep 7, 2017

codecov bot commented Sep 8, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickvergessen Sep 8, 2017 • edited Loading

Choose a reason for hiding this comment

leonklingele-work Sep 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fancycode commented Oct 13, 2017

georgehrke commented Oct 15, 2017

leonklingele-work commented Oct 16, 2017

leonklingele-work commented Oct 16, 2017

georgehrke commented Oct 16, 2017

fancycode commented Oct 16, 2017

georgehrke commented Oct 31, 2017 • edited Loading

georgehrke commented Nov 2, 2017

georgehrke commented Nov 3, 2017

georgehrke commented Nov 3, 2017

codecov bot commented Sep 8, 2017 •

edited

Loading

nickvergessen Sep 8, 2017 •

edited

Loading

leonklingele-work Sep 8, 2017 •

edited

Loading

georgehrke commented Oct 31, 2017 •

edited

Loading