launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:fix-pillar-person-oops into launchpad:master.

Commit message:
Fixing OOPS on shared artifacts listing page

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400607

This feature was properly implemented in another MP (https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400059), where we actually show the shared snaps and OCI recipes.

But after adding private snaps, we have introduced a regression causing this page to OOPS. This MP fixes the OOPS, until we have the feature implementation merged.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:fix-pillar-person-oops into launchpad:master.

diff --git a/lib/lp/registry/browser/tests/test_pillar_sharing.py b/lib/lp/registry/browser/tests/test_pillar_sharing.py
index 493b42b..432c473 100644
--- a/lib/lp/registry/browser/tests/test_pillar_sharing.py
+++ b/lib/lp/registry/browser/tests/test_pillar_sharing.py
@@ -20,6 +20,7 @@ from zope.traversing.browser.absoluteurl import absoluteURL
 
 from lp.app.enums import InformationType
 from lp.app.interfaces.services import IService
+from lp.oci.interfaces.ocirecipe import OCI_RECIPE_ALLOW_CREATE
 from lp.registry.enums import (
     BranchSharingPolicy,
     BugSharingPolicy,
@@ -29,8 +30,10 @@ from lp.registry.interfaces.accesspolicy import IAccessPolicyGrantFlatSource
 from lp.registry.model.pillar import PillarPerson
 from lp.services.beautifulsoup import BeautifulSoup
 from lp.services.config import config
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.interfaces import StormRangeFactoryError
 from lp.services.webapp.publisher import canonical_url
+from lp.snappy.interfaces.snap import SNAP_PRIVATE_FEATURE_FLAG
 from lp.testing import (
     admin_logged_in,
     login_person,
@@ -43,7 +46,11 @@ from lp.testing import (
     )
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.matchers import HasQueryCount
-from lp.testing.pages import setupBrowserForUser
+from lp.testing.pages import (
+    extract_text,
+    find_tag_by_id,
+    setupBrowserForUser,
+    )
 from lp.testing.views import (
     create_initialized_view,
     create_view,
@@ -379,6 +386,41 @@ class PillarSharingViewTestMixin:
                 team_name,
                 [grantee['name'] for grantee in cache.objects['grantee_data']])
 
+    def test_pillar_person_sharing(self):
+        self.useFixture(FeatureFixture({
+            SNAP_PRIVATE_FEATURE_FLAG: 'on',
+            OCI_RECIPE_ALLOW_CREATE: 'on'}))
+        totals = {"oci_recipes": 1, "snaps": 0}
+        items = [
+            self.factory.makeOCIRecipe(
+                owner=self.owner, registrant=self.owner,
+                information_type=InformationType.USERDATA,
+                oci_project=self.factory.makeOCIProject(pillar=self.pillar))]
+        if self.pillar_type == 'product':
+            totals["snaps"] = 1
+            items.append(self.factory.makeSnap(
+                information_type=InformationType.USERDATA,
+                owner=self.owner, registrant=self.owner, project=self.pillar))
+
+        person = self.factory.makePerson()
+        with person_logged_in(self.owner):
+            for item in items:
+                item.subscribe(person, self.owner)
+
+        pillarperson = PillarPerson(self.pillar, person)
+        url = 'http://launchpad.test/%s/+sharing/%s' % (
+            pillarperson.pillar.name, pillarperson.person.name)
+        browser = self.getUserBrowser(user=self.owner, url=url)
+        content = extract_text(
+            find_tag_by_id(browser.contents, "observer-summary"))
+        self.assertTextMatchesExpressionIgnoreWhitespace("""
+            0 bugs,
+            0 Bazaar branches,
+            0 Git repositories,
+            %(snaps)s snaps,
+            and 0 blueprints shared
+            """ % totals, content)
+
 
 class TestProductSharingView(PillarSharingViewTestMixin,
                                  SharingBaseTestCase):
diff --git a/lib/lp/registry/services/sharingservice.py b/lib/lp/registry/services/sharingservice.py
index a0e755b..062c7ba 100644
--- a/lib/lp/registry/services/sharingservice.py
+++ b/lib/lp/registry/services/sharingservice.py
@@ -250,7 +250,7 @@ class SharingService:
         snaps = []
         if snap_ids:
             all_snaps = getUtility(ISnapSet)
-            snaps = all_snaps.findByIds(snap_ids)
+            snaps = list(all_snaps.findByIds(snap_ids))
         specifications = []
         if specification_ids:
             specifications = load(Specification, specification_ids)
diff --git a/lib/lp/registry/templates/pillar-sharing-details.pt b/lib/lp/registry/templates/pillar-sharing-details.pt
index a6f2710..dc907dd 100644
--- a/lib/lp/registry/templates/pillar-sharing-details.pt
+++ b/lib/lp/registry/templates/pillar-sharing-details.pt
@@ -30,6 +30,7 @@
       <tal:bugs replace="view/shared_bugs_count">0</tal:bugs> bugs,
       <tal:branches replace="view/shared_branches_count">0</tal:branches> Bazaar branches,
       <tal:gitrepositories replace="view/shared_gitrepositories_count">0</tal:gitrepositories> Git repositories,
+      <tal:snaps replace="view/shared_snaps_count">0</tal:snaps> snaps,
       and  <tal:specifications
       replace="view/shared_specifications_count">0</tal:specifications>
       blueprints shared with <tal:name replace="view/person/displayname">