launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/squash-archivesubscriptionerror into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #971587 in Launchpad itself: "ArchiveSubscriptionError raised on PPA +archivesubscription page"
  https://bugs.launchpad.net/launchpad/+bug/971587

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/squash-archivesubscriptionerror/+merge/139382

Fix the cause of the random ArchiveSubscriptionError's we were seeing.

SourcesListEntriesWidget was assuming that self.user was always the user, so when an admin went to activate a token for a user that already had one, the code would check that the admin didn't have an token, which they didn't, so would immediately try and create a token for the user. Which would fail.

I have put a stop to SourcesListEntriesWidget's assumptions.
-- 
https://code.launchpad.net/~stevenk/launchpad/squash-archivesubscriptionerror/+merge/139382
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/squash-archivesubscriptionerror into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2012-11-26 08:40:20 +0000
+++ lib/lp/soyuz/browser/archive.py	2012-12-12 04:56:22 +0000
@@ -586,9 +586,8 @@
                     "being dispatched.")
             self.request.response.addNotification(structured(notification))
         super(ArchiveViewBase, self).initialize()
-        # Set the archive attribute so SourcesListEntriesWidget can be built
-        # correctly.
-        self.archive = self.context
+        super(ArchiveViewBase, self).set_archive_and_user(
+            self.context, self.user)
 
     @cachedproperty
     def private(self):

=== modified file 'lib/lp/soyuz/browser/archivesubscription.py'
--- lib/lp/soyuz/browser/archivesubscription.py	2012-04-17 22:09:28 +0000
+++ lib/lp/soyuz/browser/archivesubscription.py	2012-12-12 04:56:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=F0401
-
 """Browser views related to archive subscriptions."""
 
 __metaclass__ = type
@@ -362,30 +360,24 @@
     def initialize(self):
         """Process any posted actions."""
         super(PersonArchiveSubscriptionView, self).initialize()
-        # Set the archive attribute so SourcesListEntriesWidget can be built
-        # correctly.
-        self.archive = self.context.archive
+        super(PersonArchiveSubscriptionView, self).set_archive_and_user(
+            self.context.archive, self.context.subscriber)
 
         # If an activation was requested and there isn't a currently
         # active token, then create a token, provide a notification
         # and redirect.
         if self.request.form.get('activate') and not self.active_token:
             self.context.archive.newAuthToken(self.context.subscriber)
-
             self.request.response.redirect(self.request.getURL())
-
         # Otherwise, if a regeneration was requested and there is an
         # active token, then cancel the old token, create a new one,
         # provide a notification and redirect.
         elif self.request.form.get('regenerate') and self.active_token:
             self.active_token.deactivate()
-
             self.context.archive.newAuthToken(self.context.subscriber)
-
             self.request.response.addNotification(
                 "Launchpad has generated the new password you requested "
                 "for your access to the archive %s. Please follow "
                 "the instructions below to update your custom "
                 "\"sources.list\"." % self.context.archive.displayname)
-
             self.request.response.redirect(self.request.getURL())

=== modified file 'lib/lp/soyuz/browser/sourceslist.py'
--- lib/lp/soyuz/browser/sourceslist.py	2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/browser/sourceslist.py	2012-12-12 04:56:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=F0401
-
 """Browser views for sources list entries."""
 
 from z3c.ptcompat import ViewPageTemplateFile
@@ -138,10 +136,15 @@
 
 
 class SourcesListEntriesWidget:
-    """Setup the sources list entries widget.
-
-    This class assumes self.user is set in child classes.
-    """
+    """Setup the sources list entries widget."""
+
+    def __init__(self):
+        self.archive = None
+        self.current_user = None
+
+    def set_archive_and_user(self, archive, user):
+        self.archive = archive
+        self.current_user = user
 
     @cachedproperty
     def sources_list_entries(self):
@@ -153,12 +156,10 @@
             return SourcesListEntriesView(entries, self.request)
         else:
             comment = "Personal access of %s (%s) to %s" % (
-                self.user.displayname,
-                self.user.name,
+                self.user.displayname, self.current_user.name,
                 self.archive.displayname)
             entries = SourcesListEntries(
-                self.archive.distribution,
-                self.active_token.archive_url,
+                self.archive.distribution, self.active_token.archive_url,
                 self.archive.series_with_sources)
             return SourcesListEntriesView(
                 entries, self.request, comment=comment)
@@ -168,7 +169,7 @@
         """Return the corresponding current token for this subscription."""
         token_set = getUtility(IArchiveAuthTokenSet)
         return token_set.getActiveTokenForArchiveAndPerson(
-            self.archive, self.user)
+            self.archive, self.current_user)
 
     @property
     def archive_url(self):

=== modified file 'lib/lp/soyuz/browser/tests/archivesubscription-views.txt'
--- lib/lp/soyuz/browser/tests/archivesubscription-views.txt	2012-12-10 13:43:47 +0000
+++ lib/lp/soyuz/browser/tests/archivesubscription-views.txt	2012-12-12 04:56:22 +0000
@@ -363,6 +363,7 @@
     >>> view = create_initialized_view(spiv_subscription, name="+index")
     >>> view.active_token.token == original_token
     False
+    >>> current_token = view.active_token.token
 
 The security for the PersonalArchiveSubscription provides spiv, as the
 subscriber, with view access:
@@ -384,3 +385,9 @@
     >>> check_permission('launchpad.View', spiv_subscription)
     True
 
+Now that we're logged in as Celso, the view still realizes the user in question
+is spiv:
+
+    >>> view = create_initialized_view(spiv_subscription, name="+index")
+    >>> view.active_token.token == current_token
+    True