launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-fix-series-rendering into lp:launchpad.

Commit message:
Don't try to render the store series as a link on Snap:+index.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1599673 in Launchpad itself: "https://launchpad.net/+snappy-series/16 broken link"
  https://bugs.launchpad.net/launchpad/+bug/1599673

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-fix-series-rendering/+merge/300205

Don't try to render the store series as a link on Snap:+index.

While I was here, I noticed that SnappySeries wasn't viewable by anonymous users as it was supposed to be, which made the test behave confusingly before the template fix.  I fixed this up too.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-fix-series-rendering into lp:launchpad.

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2016-05-05 20:02:01 +0000
+++ lib/lp/security.py	2016-07-15 15:12:49 +0000
@@ -3197,6 +3197,11 @@
     usedfor = ISnapBuild
 
 
+class ViewSnappySeries(AnonymousAuthorization):
+    """Anyone can view an `ISnappySeries`."""
+    usedfor = ISnappySeries
+
+
 class EditSnappySeries(EditByRegistryExpertsOrAdmins):
     usedfor = ISnappySeries
 

=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py	2016-07-12 13:38:58 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py	2016-07-15 15:12:49 +0000
@@ -422,7 +422,7 @@
 
     def test_create_new_snap_display_processors(self):
         branch = self.factory.makeAnyBranch()
-        distroseries = self.setUpDistroSeries()
+        self.setUpDistroSeries()
         browser = self.getViewBrowser(
             branch, view_name="+new-snap", user=self.person)
         processors = browser.getControl(name="field.processors")
@@ -449,7 +449,7 @@
 
     def test_create_new_snap_processors(self):
         branch = self.factory.makeAnyBranch()
-        distroseries = self.setUpDistroSeries()
+        self.setUpDistroSeries()
         browser = self.getViewBrowser(
             branch, view_name="+new-snap", user=self.person)
         processors = browser.getControl(name="field.processors")
@@ -1092,13 +1092,12 @@
             name="test-person", displayname="Test Person")
         self.factory.makeBuilder(virtualized=True)
 
-    def makeSnap(self, branch=None, git_ref=None):
-        if branch is None and git_ref is None:
-            branch = self.factory.makeAnyBranch()
+    def makeSnap(self, **kwargs):
+        if kwargs.get("branch") is None and kwargs.get("git_ref") is None:
+            kwargs["branch"] = self.factory.makeAnyBranch()
         return self.factory.makeSnap(
             registrant=self.person, owner=self.person,
-            distroseries=self.distroseries, name=u"snap-name", branch=branch,
-            git_ref=git_ref)
+            distroseries=self.distroseries, name=u"snap-name", **kwargs)
 
     def makeBuild(self, snap=None, archive=None, date_created=None, **kwargs):
         if snap is None:
@@ -1229,6 +1228,27 @@
             Primary Archive for Ubuntu Linux
             """, self.getMainText(build.snap))
 
+    def test_index_store_upload(self):
+        # If the snap package is to be automatically uploaded to the store,
+        # the index page shows details of this.
+        with admin_logged_in():
+            snappyseries = self.factory.makeSnappySeries(
+                usable_distro_series=[self.distroseries])
+        snap = self.makeSnap(
+            store_upload=True, store_series=snappyseries,
+            store_name=self.getUniqueString(u"store-name"))
+        view = create_initialized_view(snap, "+index")
+        store_upload_tag = soupmatchers.Tag(
+            "store upload", "div", attrs={"id": "store_upload"})
+        self.assertThat(view(), soupmatchers.HTMLContains(
+            soupmatchers.Within(
+                store_upload_tag,
+                soupmatchers.Tag(
+                    "store series name", "span", text=snappyseries.title)),
+            soupmatchers.Within(
+                store_upload_tag,
+                soupmatchers.Tag("store name", "span", text=snap.store_name))))
+
     def setStatus(self, build, status):
         build.updateStatus(
             BuildStatus.BUILDING, date_started=build.date_created)

=== modified file 'lib/lp/snappy/templates/snap-index.pt'
--- lib/lp/snappy/templates/snap-index.pt	2016-06-20 21:17:58 +0000
+++ lib/lp/snappy/templates/snap-index.pt	2016-07-15 15:12:49 +0000
@@ -93,7 +93,7 @@
       <dl id="store_series">
         <dt>Store series:</dt>
         <dd>
-          <a tal:replace="structure context/store_series/fmt:link"/>
+          <span tal:content="context/store_series/title"/>
           <a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
         </dd>
       </dl>

=== modified file 'lib/lp/snappy/tests/test_snappyseries.py'
--- lib/lp/snappy/tests/test_snappyseries.py	2016-06-28 21:10:18 +0000
+++ lib/lp/snappy/tests/test_snappyseries.py	2016-07-15 15:12:49 +0000
@@ -11,8 +11,12 @@
     MatchesSetwise,
     MatchesStructure,
     )
-from zope.component import getUtility
+from zope.component import (
+    getAdapter,
+    getUtility,
+    )
 
+from lp.app.interfaces.security import IAuthorization
 from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.interfaces import OAuthPermission
@@ -68,6 +72,12 @@
         snappy_series.usable_distro_series = []
         self.assertContentEqual([], snappy_series.usable_distro_series)
 
+    def test_anonymous(self):
+        # Anyone can view an `ISnappySeries`.
+        series = self.factory.makeSnappySeries(name="dummy")
+        authz = getAdapter(series, IAuthorization, name="launchpad.View")
+        self.assertTrue(authz.checkUnauthenticated())
+
 
 class TestSnappySeriesSet(TestCaseWithFactory):