Merge lp:~daker/launchpad/fix.1600499 into lp:launchpad

Proposed by Adnane Belmadiaf on 2016-07-09
Status: Needs review
Proposed branch: lp:~daker/launchpad/fix.1600499
Merge into: lp:launchpad
Diff against target: 119 lines (+72/-0)
6 files modified
lib/lp/bugs/emailtemplates/bug-notification-verbose.txt (+12/-0)
lib/lp/bugs/emailtemplates/bug-notification.txt (+12/-0)
lib/lp/code/emailtemplates/branch-merge-proposal-created.txt (+12/-0)
lib/lp/code/emailtemplates/branch-merge-proposal-updated.txt (+12/-0)
lib/lp/code/emailtemplates/branch-modified.txt (+12/-0)
lib/lp/code/emailtemplates/review-requested.txt (+12/-0)
To merge this branch: bzr merge lp:~daker/launchpad/fix.1600499
Reviewer Review Type Date Requested Status
Colin Watson 2016-07-09 Needs Fixing on 2016-07-10
Review via email: mp+299616@code.launchpad.net

Description of the change

Add support for Gmail go-to actions

To post a comment you must log in.
Colin Watson (cjwatson) wrote :

Thanks for the patch.

These were previously plain-text emails sent to non-Gmail users as well. Dumping HTML-like markup into the start of them without even a Content-Type change is not going to look good at all to anyone who doesn't use Gmail; and I would be wary of making them anything other than text/plain anyway.

Can you point to the specification for this actions markup? We need to find some approach that's a bit less horribly intrusive, if indeed it's worth doing. As a last resort it may require a new PersonSettings column, but it will depend on what options the spec affords us. For example, if it were possible to do this by way of additional message headers, that would be *much* better than dumping JSON into the message body.

Whatever approach ends up being workable, this needs to include tests.

review: Needs Fixing
Adnane Belmadiaf (daker) wrote :

Thanks for reply, i just checked LP emails, indeed it's sent as text/plain, to solve this the email need to be sent as multipart/alternative, which then will include both versions the text/plain & text/html, the email client will choose which version to display depending on the user settings, here is the markup reference https://developers.google.com/gmail/markup/reference/go-to-action#view_action

lp:~daker/launchpad/fix.1600499 updated on 2016-07-10
18127. By Adnane Belmadiaf on 2016-07-10

Restore trailing space

18128. By Adnane Belmadiaf on 2016-07-10

Restore more trailing space

Colin Watson (cjwatson) wrote :

It appears that Gmail doesn't support any header-based method, which is deeply unfortunate. I'm not at all sure that it's worth cluttering our emails with multipart/alternative cruft just for this, but if it is then you'd probably need to start somewhere in BaseMailer.

Unmerged revisions

18128. By Adnane Belmadiaf on 2016-07-10

Restore more trailing space

18127. By Adnane Belmadiaf on 2016-07-10

Restore trailing space

18126. By Adnane Belmadiaf on 2016-07-09

Add more gmail actions

18125. By Adnane Belmadiaf on 2016-07-09

Add Gmail actions markup

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/emailtemplates/bug-notification-verbose.txt'
2--- lib/lp/bugs/emailtemplates/bug-notification-verbose.txt 2011-06-15 11:19:21 +0000
3+++ lib/lp/bugs/emailtemplates/bug-notification-verbose.txt 2016-07-10 20:33:39 +0000
4@@ -1,3 +1,15 @@
5+<script type="application/ld+json">
6+{
7+ "@context": "http://schema.org",
8+ "@type": "EmailMessage",
9+ "potentialAction": {
10+ "@type": "ViewAction",
11+ "name": "View Bug",
12+ "url": "%(bug_url)s"
13+ },
14+ "description": ""
15+}
16+</script>
17 %(content)s
18
19 --
20
21=== modified file 'lib/lp/bugs/emailtemplates/bug-notification.txt'
22--- lib/lp/bugs/emailtemplates/bug-notification.txt 2011-06-15 11:19:21 +0000
23+++ lib/lp/bugs/emailtemplates/bug-notification.txt 2016-07-10 20:33:39 +0000
24@@ -1,3 +1,15 @@
25+<script type="application/ld+json">
26+{
27+ "@context": "http://schema.org",
28+ "@type": "EmailMessage",
29+ "potentialAction": {
30+ "@type": "ViewAction",
31+ "name": "View Bug",
32+ "url": "%(bug_url)s"
33+ },
34+ "description": ""
35+}
36+</script>
37 %(content)s
38
39 --
40
41=== modified file 'lib/lp/code/emailtemplates/branch-merge-proposal-created.txt'
42--- lib/lp/code/emailtemplates/branch-merge-proposal-created.txt 2014-11-14 12:30:28 +0000
43+++ lib/lp/code/emailtemplates/branch-merge-proposal-created.txt 2016-07-10 20:33:39 +0000
44@@ -1,3 +1,15 @@
45+<script type="application/ld+json">
46+{
47+ "@context": "http://schema.org",
48+ "@type": "EmailMessage",
49+ "potentialAction": {
50+ "@type": "ViewAction",
51+ "name": "View Merge Request",
52+ "url": "%(proposal_url)s"
53+ },
54+ "description": ""
55+}
56+</script>
57 %(proposal_registrant)s has proposed merging %(source_branch)s into %(target_branch)s%(prerequisite)s.
58
59 %(commit_message)s%(reviews)s%(related_bugtasks)s
60
61=== modified file 'lib/lp/code/emailtemplates/branch-merge-proposal-updated.txt'
62--- lib/lp/code/emailtemplates/branch-merge-proposal-updated.txt 2014-11-14 12:30:28 +0000
63+++ lib/lp/code/emailtemplates/branch-merge-proposal-updated.txt 2016-07-10 20:33:39 +0000
64@@ -1,3 +1,15 @@
65+<script type="application/ld+json">
66+{
67+ "@context": "http://schema.org",
68+ "@type": "EmailMessage",
69+ "potentialAction": {
70+ "@type": "ViewAction",
71+ "name": "View Merge Request",
72+ "url": "%(proposal_url)s"
73+ },
74+ "description": ""
75+}
76+</script>
77 The proposal to merge %(source_branch)s into %(target_branch)s has been updated.
78
79 %(delta)s
80
81=== modified file 'lib/lp/code/emailtemplates/branch-modified.txt'
82--- lib/lp/code/emailtemplates/branch-modified.txt 2011-12-18 22:31:46 +0000
83+++ lib/lp/code/emailtemplates/branch-modified.txt 2016-07-10 20:33:39 +0000
84@@ -1,3 +1,15 @@
85+<script type="application/ld+json">
86+{
87+ "@context": "http://schema.org",
88+ "@type": "EmailMessage",
89+ "potentialAction": {
90+ "@type": "ViewAction",
91+ "name": "View Branch",
92+ "url": "%(branch_url)s"
93+ },
94+ "description": ""
95+}
96+</script>
97 %(diff)s%(delta)s
98
99 --
100
101=== modified file 'lib/lp/code/emailtemplates/review-requested.txt'
102--- lib/lp/code/emailtemplates/review-requested.txt 2014-11-14 12:30:28 +0000
103+++ lib/lp/code/emailtemplates/review-requested.txt 2016-07-10 20:33:39 +0000
104@@ -1,3 +1,15 @@
105+<script type="application/ld+json">
106+{
107+ "@context": "http://schema.org",
108+ "@type": "EmailMessage",
109+ "potentialAction": {
110+ "@type": "ViewAction",
111+ "name": "View Merge Request",
112+ "url": "%(proposal_url)s"
113+ },
114+ "description": ""
115+}
116+</script>
117 You have been requested to review the proposed merge of %(source_branch)s into %(target_branch)s.
118
119 For more details, see: