Launchpad itself

Merge lp:~michael.nelson/launchpad/509370-access-non-unique-ppa-name into lp:launchpad

  1. 509370-access-non-unique-ppa-name
  2. Merge into devel
Proposed by Michael Nelson
Status: Merged
Approved by: Michael Nelson
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~michael.nelson/launchpad/509370-access-non-unique-ppa-name
Merge into: lp:launchpad
Diff against target: 382 lines (+90/-54)
12 files modified
lib/canonical/launchpad/doc/tales.txt (+17/-2)
lib/canonical/launchpad/emailtemplates/ppa-subscription-new.txt (+3/-1)
lib/canonical/launchpad/mailnotification.py (+9/-3)
lib/canonical/launchpad/webapp/tales.py (+27/-1)
lib/lp/soyuz/browser/archive.py (+0/-11)
lib/lp/soyuz/browser/tests/archive-views.txt (+1/-13)
lib/lp/soyuz/doc/archivesubscriber.txt (+4/-1)
lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt (+11/-11)
lib/lp/soyuz/stories/ppa/xx-private-ppa-subscriptions.txt (+3/-3)
lib/lp/soyuz/templates/archive-index.pt (+2/-2)
lib/lp/soyuz/templates/person-archive-subscription.pt (+2/-3)
lib/lp/soyuz/templates/person-archive-subscriptions.pt (+11/-3)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/509370-access-non-unique-ppa-name
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Curtis Hovey (community) ui Approve
Matthew Revell (community) archivesubscriber.txt Approve
Review via email: mp+19022@code.launchpad.net

Commit message

Ensures that email sent regarding private ppa subscriptions uniquely identifies the ppa by including the ppa ref (ppa:cprov/myppa), and additionally the description (to help the person receiving the email to understand what the purpose of the PPA is.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

This branch addresses the issues in bug 509370, by ensuring that when a person is added as a subscriber to a private PPA, the email they receive informing them will include not only the display name of the PPA, but also the ppa name, and the description of the PPA.

It also updates the archive subscriptions page to include the PPA name (in addition to the display name) and a link to the owner (but not currently the description of the PPA as it's not viewable by the subscriber, but this could be addressed by bug 336779).

Matt: can you check the wording change to the email template? You can see the change in expected output in doc/archivesubscriber.txt

An example of the archivesubscriptions page:
http://people.canonical.com/~michaeln/tmp/Screenshot-Private%20PPA%20access%20:%20Joe%20Smith%20-%20Chromium.png

== Demo ==
To setup the subscription, run the following in a harness:

http://pastebin.ubuntu.com/373304/

and then login as <email address hidden>:test and browse toz;

https://launchpad.dev/~joesmith

then click on "View your private PPA subscriptions"

== Test ==
bin/test -vv -t xx-private-ppa-subscription -t archivesubscriber.txt

== Pylint notices ==

lib/canonical/launchpad/mailnotification.py
    15: [F0401] Unable to import 'email.Header' (No module named Header)
    16: [F0401] Unable to import 'email.MIMEText' (No module named MIMEText)
    17: [F0401] Unable to import 'email.MIMEMultipart' (No module named MIMEMultipart)
    18: [F0401] Unable to import 'email.MIMEMessage' (No module named MIMEMessage)
    19: [F0401] Unable to import 'email.Utils' (No module named Utils)

Revision history for this message
Matthew Revell (matthew.revell) wrote :

> Matt: can you check the wording change to the email template? You can see the
> change in expected output in doc/archivesubscriber.txt

Looks good to me. +1

As an aside, should we link to some help in the email?

review: Approve (archivesubscriber.txt)
Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Michael.

This looks nice. I have a few remarks about the reset password button.
1) It should be in title case: [Reset Password]
2) Why is it missing padding between the text and the border in webkit?
   I see a very narrow button in Epiphany, looks fine in Firefox.

review: Approve (ui)
Revision history for this message
Abel Deuring (adeuring) :
review: Approve (code)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

> Hi Michael.
>
> This looks nice. I have a few remarks about the reset password button.
> 1) It should be in title case: [Reset Password]

Thanks Curtis, I updated this one.
> 2) Why is it missing padding between the text and the border in webkit?
> I see a very narrow button in Epiphany, looks fine in Firefox.

Chromium is fine, but yeah, I installed Epiphany and took a look. Eek.

So we've got a button {padding:0} in our style.css.

Take a look at:
http://people.canonical.com/~michaeln/tmp/button_default_padding.png

Both images are chromium, the first with the default 0 padding, the second with 8px padding on the button. As you can see, Chromium (and FF, etc.) even with zero padding, the content of the button (text in this case) has its own padding or margin, whereas epiphany doesn't. I couldn't find a quick solution (modifying the button padding obviously affects the other browsers), so I've switched the button, replacing it with a hidden input (for the value passed to the view) and a normal <input type="submit"..> for the button. This ensures that Epiphany appears just like Chromium/FF now.

Thanks.
http://pastebin.ubuntu.com/374707/

Revision history for this message
Curtis Hovey (sinzui) wrote :

On Fri, 2010-02-12 at 12:43 +0000, Michael Nelson wrote:
> > Hi Michael.
 ...
> Take a look at:
> http://people.canonical.com/~michaeln/tmp/button_default_padding.png
>
> Both images are chromium, the first with the default 0 padding, the
> second with 8px padding on the button. As you can see, Chromium (and
> FF, etc.) even with zero padding, the content of the button (text in
> this case) has its own padding or margin, whereas epiphany doesn't. I
> couldn't find a quick solution (modifying the button padding obviously
> affects the other browsers), so I've switched the button, replacing it
> with a hidden input (for the value passed to the view) and a normal
> <input type="submit"..> for the button. This ensures that Epiphany
> appears just like Chromium/FF now.
>
> Thanks.
> http://pastebin.ubuntu.com/374707/

Your solution explains other bug reports if have seen about narrow
buttons, such as in the help pane.

I see that webkit has a plethora of styles, and I suspect the wrong one
is chosen by default. I only recently discovered that MacOS supports
extra narrow buttons and scrollbars:

    -webkit-appearance: <push-button|square-button|button|button-bevel>

May investigate this if I do another round of webkitisms.

--
__Curtis C. Hovey_________
http://launchpad.net/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/doc/tales.txt'
2--- lib/canonical/launchpad/doc/tales.txt 2010-01-14 12:09:34 +0000
3+++ lib/canonical/launchpad/doc/tales.txt 2010-02-12 13:51:23 +0000
4@@ -123,11 +123,14 @@
5 <img ... src="/@@/distribution" />
6
7 PPAs have a 'link' formatter, which returns the appropriate HTML used
8-for referring to them in other pages.
9+for referring to them in other pages and a 'reference' formatter which
10+displays the unique ppa reference.
11
12 >>> print test_tales("ppa/fmt:link", ppa=mark.archive)
13 <a href="/~mark/+archive/ppa"
14 class="sprite ppa-icon">PPA for Mark Shuttleworth</a>
15+ >>> print test_tales("ppa/fmt:reference", ppa=mark.archive)
16+ ppa:mark/ppa
17
18 Disabled PPAs links use a different icon and are only linkified for
19 users with launchpad.View on them.
20@@ -160,10 +163,22 @@
21
22 >>> print test_tales("ppa/fmt:link", ppa=mark.archive)
23
24+Similarly, references to private PPAs are not rendered unless the user has
25+a subscription to the PPA.
26+
27+ >>> login('foo.bar@canonical.com')
28+ >>> print test_tales("ppa/fmt:reference", ppa=mark.archive)
29+
30+ >>> login('mark@example.com')
31+ >>> foo = getUtility(IPersonSet).getByName('name16')
32+ >>> ignore = mark.archive.newSubscription(foo, mark)
33+ >>> login('foo.bar@canonical.com')
34+ >>> print test_tales("ppa/fmt:reference", ppa=mark.archive)
35+ ppa:mark/ppa
36+
37 We also have icons for builds which may have different dimensions.
38
39 >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
40- >>> login('foo.bar@canonical.com')
41 >>> stp = SoyuzTestPublisher()
42 >>> stp.prepareBreezyAutotest()
43 >>> source = stp.getPubSource()
44
45=== modified file 'lib/canonical/launchpad/emailtemplates/ppa-subscription-new.txt'
46--- lib/canonical/launchpad/emailtemplates/ppa-subscription-new.txt 2009-07-13 09:22:52 +0000
47+++ lib/canonical/launchpad/emailtemplates/ppa-subscription-new.txt 2010-02-12 13:51:23 +0000
48@@ -3,7 +3,9 @@
49 Launchpad: access to a private archive
50 --------------------------------------
51
52-%(registrant_name)s has granted you access to a private software archive "%(ppa_name)s", which is hosted by Launchpad.
53+%(registrant_name)s has granted you access to a private software archive "%(ppa_displayname)s" (%(ppa_reference)s), which is hosted by Launchpad and has the following description:
54+
55+%(ppa_description)s
56
57 To start downloading and using software from this archive you need to view your access details by visiting this link:
58
59
60=== modified file 'lib/canonical/launchpad/mailnotification.py'
61--- lib/canonical/launchpad/mailnotification.py 2010-01-14 17:53:22 +0000
62+++ lib/canonical/launchpad/mailnotification.py 2010-02-12 13:51:23 +0000
63@@ -1203,9 +1203,13 @@
64 """Notification that a new PPA subscription can be activated."""
65 non_active_subscribers = subscription.getNonActiveSubscribers()
66
67+ archive = subscription.archive
68 registrant_name = subscription.registrant.displayname
69- ppa_name = subscription.archive.displayname
70- subject = 'PPA access granted for ' + ppa_name
71+ ppa_displayname = archive.displayname
72+ ppa_reference = "ppa:%s/%s" % (
73+ archive.owner.name, archive.name)
74+ ppa_description = archive.description
75+ subject = 'PPA access granted for ' + ppa_displayname
76
77 template = get_email_template('ppa-subscription-new.txt')
78
79@@ -1222,7 +1226,9 @@
80 'recipient_name': person.displayname,
81 'registrant_name': registrant_name,
82 'registrant_profile_url': canonical_url(subscription.registrant),
83- 'ppa_name': ppa_name,
84+ 'ppa_displayname': ppa_displayname,
85+ 'ppa_reference': ppa_reference,
86+ 'ppa_description': ppa_description,
87 'recipient_subscriptions_url': recipient_subscriptions_url,
88 }
89 body = MailWrapper(72).format(template % replacements,
90
91=== modified file 'lib/canonical/launchpad/webapp/tales.py'
92--- lib/canonical/launchpad/webapp/tales.py 2010-01-25 14:50:42 +0000
93+++ lib/canonical/launchpad/webapp/tales.py 2010-02-12 13:51:23 +0000
94@@ -45,6 +45,7 @@
95 from lp.blueprints.interfaces.specification import ISpecification
96 from lp.code.interfaces.branch import IBranch
97 from lp.soyuz.interfaces.archive import ArchivePurpose, IPPA
98+from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
99 from canonical.launchpad.interfaces.launchpad import (
100 IHasIcon, IHasLogo, IHasMugshot, IPrivacy)
101 from lp.registry.interfaces.person import IPerson, IPersonSet
102@@ -1589,6 +1590,13 @@
103
104 _link_summary_template = '%(display_name)s'
105 _link_permission = 'launchpad.View'
106+ _reference_template = "ppa:%(owner_name)s/%(ppa_name)s"
107+
108+ final_traversable_names = {
109+ 'reference': 'reference',
110+ }
111+ final_traversable_names.update(
112+ CustomizableFormatter.final_traversable_names)
113
114 def _link_summary_values(self):
115 """See CustomizableFormatter._link_summary_values."""
116@@ -1618,6 +1626,24 @@
117 else:
118 return ''
119
120+ def reference(self, view_name=None, rootsite=None):
121+ """Return the text PPA reference for a PPA."""
122+ # XXX: noodles 2010-02-11 bug=336779: This following check
123+ # should be replaced with the normal check_permission once
124+ # permissions for archive subscribers has been resolved.
125+ if self._context.private:
126+ request = get_current_browser_request()
127+ person = IPerson(request.principal)
128+ subscriptions = getUtility(IArchiveSubscriberSet).getBySubscriber(
129+ person, self._context)
130+ if subscriptions.is_empty():
131+ return ''
132+
133+ return self._reference_template % {
134+ 'owner_name': self._context.owner.name,
135+ 'ppa_name': self._context.name,
136+ }
137+
138
139 class SpecificationBranchFormatterAPI(CustomizableFormatter):
140 """Adapter for ISpecificationBranch objects to a formatted string."""
141@@ -3112,7 +3138,7 @@
142
143 traversable_names = {
144 'link': 'link',
145- 'url': 'url',
146+ 'url': 'url',
147 'displayname': 'displayname',
148 }
149
150
151=== modified file 'lib/lp/soyuz/browser/archive.py'
152--- lib/lp/soyuz/browser/archive.py 2010-02-02 21:28:50 +0000
153+++ lib/lp/soyuz/browser/archive.py 2010-02-12 13:51:23 +0000
154@@ -561,17 +561,6 @@
155 can_edit = check_permission('launchpad.Edit', self.context)
156 return can_edit and len(disabled_dependencies) > 0
157
158- @property
159- def ppa_reference(self):
160- """PPA reference as supported by `dput` and `software-properties`.
161-
162- :raises AssertionError: if the context `IArchive` is not a PPA.
163- :return: a `str` as 'ppa:%(ppa.owner.name)/%(ppa.name)'
164- """
165- assert self.context.is_ppa, (
166- 'PPA reference should not be used for non-PPA archives.')
167- return 'ppa:%s/%s' % (self.context.owner.name, self.context.name)
168-
169 @cachedproperty
170 def package_copy_requests(self):
171 """Return any package copy requests associated with this archive."""
172
173=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
174--- lib/lp/soyuz/browser/tests/archive-views.txt 2010-02-02 21:28:50 +0000
175+++ lib/lp/soyuz/browser/tests/archive-views.txt 2010-02-12 13:51:23 +0000
176@@ -46,18 +46,6 @@
177 >>> print copy_archive_view.archive_label
178 Archive
179
180-There is also the 'ppa_reference' property, which mimics the syntax
181-supported by `dput` and `software-properties` Ubuntu tools.
182-
183- >>> print ppa_archive_view.ppa_reference
184- ppa:cprov/ppa
185-
186- >>> print copy_archive_view.ppa_reference
187- Traceback (most recent call last):
188- ...
189- AssertionError: PPA reference should not be used for non-PPA
190- archives.
191-
192 The ArchiveView is provides the html for the inline description
193 editing widget.
194
195@@ -492,7 +480,7 @@
196 Packages in ...PPA for Celso Providelo...
197
198 >>> print copy_archive_view.page_title
199- Packages in ...Copy archive intrepid-security-rebuild...
200+ Packages in ...Copy archive intrepid-security-rebuild...
201
202 This view inherits from ArchiveViewBase and has all the
203 corresponding properties such as archive_url, build_counters etc.
204
205=== modified file 'lib/lp/soyuz/doc/archivesubscriber.txt'
206--- lib/lp/soyuz/doc/archivesubscriber.txt 2009-12-24 01:41:54 +0000
207+++ lib/lp/soyuz/doc/archivesubscriber.txt 2010-02-12 13:51:23 +0000
208@@ -116,7 +116,10 @@
209 --------------------------------------
210 <BLANKLINE>
211 Celso Providelo has granted you access to a private software archive
212- "PPA for Celso Providelo", which is hosted by Launchpad.
213+ "PPA for Celso Providelo" (ppa:cprov/ppa), which is hosted by Launchpad
214+ and has the following description:
215+ <BLANKLINE>
216+ packages to help my friends.
217 <BLANKLINE>
218 To start downloading and using software from this archive you need to
219 view your access details by visiting this link:
220
221=== modified file 'lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt'
222--- lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt 2009-12-24 01:41:54 +0000
223+++ lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt 2010-02-12 13:51:23 +0000
224@@ -197,8 +197,8 @@
225 >>> joe_browser.open("http://launchpad.dev/~joesmith")
226 >>> joe_browser.getLink('View your private PPA subscriptions').click()
227 >>> print_subscriptions_for_person(joe_browser.contents)
228- Archive
229- PPA for Celso Providelo View
230+ Archive Owner
231+ PPA for ... Celso Providelo View
232
233 When Joe clicks on the View button for Celso's PPA then the
234 details of the subscription are displayed with the newly created
235@@ -223,8 +223,9 @@
236 >>> joe_browser.open(
237 ... "http://launchpad.dev/~joesmith/+archivesubscriptions")
238 >>> print_subscriptions_for_person(joe_browser.contents)
239- Archive
240- PPA for Celso Providelo View
241+ Archive Owner
242+ PPA for ... Celso Providelo View
243+
244 >>> joe_browser.getLink('View').click()
245 >>> print(extract_text(joe_browser.contents))
246 Access to PPA for Celso Providelo...
247@@ -244,13 +245,12 @@
248 >>> print(extract_text(regeneration_info))
249 Reset password
250 If you believe...
251- Reset password
252 Note: after ...
253
254 When Joe clicks on the 'Generate new personal subscription' link then
255 the page is redisplayed with new sources.list entries and a notification.
256
257- >>> joe_browser.getControl(name='regenerate').click()
258+ >>> joe_browser.getControl(name='regenerate_btn').click()
259 >>> for msg in get_feedback_messages(joe_browser.contents):
260 ... print msg
261 Launchpad has generated the new password you requested for your
262@@ -278,7 +278,7 @@
263 >>> transaction.commit()
264 >>> logout()
265
266-When Mark, a member of the Launchpad team, visits his profile and clicks
267+When Mark, a member of the Launchpad team, visits his profile and clicks
268 'View your private PPA subscriptions', then he'll see a list of his current
269 subscriptions.
270
271@@ -288,8 +288,8 @@
272
273 >>> mark_browser.getLink('View your private PPA subscriptions').click()
274 >>> print_subscriptions_for_person(mark_browser.contents)
275- Archive
276- PPA for Celso Providelo View
277+ Archive Owner
278+ PPA for ... Celso Providelo View
279
280 When Mark clicks on the view button, then he is taken to the page for
281 his personal subscription for Celso's private PPA and the newly-created
282@@ -314,8 +314,8 @@
283 >>> mark_browser.open(
284 ... "http://launchpad.dev/~mark/+archivesubscriptions")
285 >>> print_subscriptions_for_person(mark_browser.contents)
286- Archive
287- PPA for Celso Providelo View
288+ Archive Owner
289+ PPA for ... Celso Providelo View
290
291 >>> mark_browser.getLink('View').click()
292 >>> print(extract_text(mark_browser.contents))
293
294=== modified file 'lib/lp/soyuz/stories/ppa/xx-private-ppa-subscriptions.txt'
295--- lib/lp/soyuz/stories/ppa/xx-private-ppa-subscriptions.txt 2009-09-30 14:15:39 +0000
296+++ lib/lp/soyuz/stories/ppa/xx-private-ppa-subscriptions.txt 2010-02-12 13:51:23 +0000
297@@ -138,9 +138,9 @@
298 >>> for row in find_tags_by_class(joe_browser.contents,
299 ... 'archive-subscription-row'):
300 ... print extract_text(row)
301- Archive
302- PPA for Mark Shuttleworth View
303- PPA for Celso Providelo View
304+ Archive Owner
305+ PPA for Mark Shuttleworth (ppa:mark/ppa) Mark Shuttleworth View
306+ PPA for Celso Providelo (ppa:cprov/ppa) Celso Providelo View
307
308 == Confirming a subscription ==
309
310
311=== modified file 'lib/lp/soyuz/templates/archive-index.pt'
312--- lib/lp/soyuz/templates/archive-index.pt 2010-02-10 14:19:23 +0000
313+++ lib/lp/soyuz/templates/archive-index.pt 2010-02-12 13:51:23 +0000
314@@ -84,7 +84,7 @@
315 <p>You can update your system with unsupported packages from
316 this untrusted PPA by <a href="/+help/soyuz/ppa-sources-list.html"
317 target="help">adding</a> <strong
318- tal:content="view/ppa_reference">ppa:cprov/ppa</strong>
319+ tal:content="context/fmt:reference">ppa:cprov/ppa</strong>
320 to your system's Software Sources.
321 <span id="pre-karmic-systems-slide-trigger"
322 >Not using Ubuntu 9.10 (Karmic)?</span></p>
323@@ -316,7 +316,7 @@
324 spurious build failures or binaries with unexpected
325 contents.</p>
326 <p>You can upload packages to this PPA using:</p>
327- <p><tt>dput <tal:ppa-ref replace="view/ppa_reference"
328+ <p><tt>dput <tal:ppa-ref replace="context/fmt:reference"
329 >ppa:cprov/ppa</tal:ppa-ref> &lt;source.changes&gt;</tt>
330 (<a href="https://help.launchpad.net/Packaging/PPA/Uploading"
331 >Read about uploading</a>)</p>
332
333=== modified file 'lib/lp/soyuz/templates/person-archive-subscription.pt'
334--- lib/lp/soyuz/templates/person-archive-subscription.pt 2009-09-14 08:44:08 +0000
335+++ lib/lp/soyuz/templates/person-archive-subscription.pt 2010-02-12 13:51:23 +0000
336@@ -55,9 +55,8 @@
337 on this page. You'll need to update them on your computer.
338 </p>
339 <form action="" method="post">
340- <button type="submit" name="regenerate" value="1">
341- Reset password
342- </button>
343+ <input type="hidden" name="regenerate" value="1" />
344+ <input type="submit" name="regenerate_btn" value="Reset Password" />
345 </form>
346 <p class="discreet">
347 Note: after resetting the password for your personal
348
349=== modified file 'lib/lp/soyuz/templates/person-archive-subscriptions.pt'
350--- lib/lp/soyuz/templates/person-archive-subscriptions.pt 2009-11-02 21:44:55 +0000
351+++ lib/lp/soyuz/templates/person-archive-subscriptions.pt 2010-02-12 13:51:23 +0000
352@@ -16,10 +16,11 @@
353 been granted access are listed below.
354 </p>
355 <table summary="CONTEXTS/fmt:pagetitle" id="archive-subscriptions"
356- class="listing" style="width:70%">
357+ class="listing">
358 <thead>
359 <tr class="archive-subscription-row">
360 <th>Archive</th>
361+ <th>Owner</th>
362 <th></th>
363 </tr>
364 </thead>
365@@ -29,8 +30,15 @@
366 <tal:subscription_and_token
367 define="subscription subscription_with_token/subscription;
368 token subscription_with_token/token">
369- <td tal:content="subscription/archive/displayname"
370- class="ppa_display_name">Private PPA for Celso
371+ <td class="ppa_display_name">
372+ <tal:display_name content="subscription/archive/displayname">
373+ Private PPA for Celso
374+ </tal:display_name>
375+ (<tal:reference content="subscription/archive/fmt:reference">
376+ ppa:cprov/ppa</tal:reference>)
377+ </td>
378+ <td><tal:owner content="structure subscription/archive/owner/fmt:link">
379+ Celso Providelo</tal:owner>
380 </td>
381 <td>
382 <tal:active condition="token">