Merge lp:~stevenk/launchpad/squash-archivesubscriptionerror into lp:launchpad

[Steve Kowalik]

Stop SourcesListEntriesWidget assuming that the currently logged in user is the only one who activates tokens.

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.

[William Grant (wgrant)]

review: Approve (code)

=== 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 05:30:25 +0000
@@ -586,9 +586,9 @@
                     "being dispatched.")
             self.request.response.addNotification(structured(notification))
         super(ArchiveViewBase, self).initialize()
-        # Set the archive attribute so SourcesListEntriesWidget can be built
-        # correctly.
+        # Set properties for SourcesListEntriesWidget.
         self.archive = self.context
+        self.sources_list_user = 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 05:30:25 +0000
@@ -1,8 +1,6 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 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,25 @@
     def initialize(self):
         """Process any posted actions."""
         super(PersonArchiveSubscriptionView, self).initialize()
-        # Set the archive attribute so SourcesListEntriesWidget can be built
-        # correctly.
+        # Set properties for SourcesListEntriesWidget.
         self.archive = self.context.archive
+        self.sources_list_user = 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 05:30:25 +0000
@@ -1,8 +1,6 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 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,7 @@
 
 
 class SourcesListEntriesWidget:
-    """Setup the sources list entries widget.
-
-    This class assumes self.user is set in child classes.
-    """
+    """Setup the sources list entries widget."""
 
     @cachedproperty
     def sources_list_entries(self):
@@ -153,12 +148,10 @@
             return SourcesListEntriesView(entries, self.request)
         else:
             comment = "Personal access of %s (%s) to %s" % (
-                self.user.displayname,
-                self.user.name,
-                self.archive.displayname)
+                self.sources_list_user.displayname,
+                self.sources_list_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 +161,7 @@
         """Return the corresponding current token for this subscription."""
         token_set = getUtility(IArchiveAuthTokenSet)
         return token_set.getActiveTokenForArchiveAndPerson(
-            self.archive, self.user)
+            self.archive, self.sources_list_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 05:30:25 +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