Review: Approve
Diff comments: > diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py > index c3ba48a..4418bcb 100644 > --- a/lib/lp/snappy/model/snap.py > +++ b/lib/lp/snappy/model/snap.py > @@ -1084,6 +1091,23 @@ class Snap(Storm, WebhookTargetMixin): > order_by = Desc(SnapBuild.id) > return self._getBuilds(filter_term, order_by) > > + def visibleByUser(self, user): > + """See `IGitRepository`.""" > + if not self.private: > + return True > + if user is None: > + return False > + if user.inTeam(self.owner): > + return True This check should be dropped in favour of the owner having an access grant. > + > + store = IStore(self) > + visibility_clause = removeSecurityProxy(getUtility( > + ISnapSet))._findSnapVisibilityClause(user) This seems like another good reason to prefer putting all the logic in a top-level function, as I suggested in comments on the previous branch in this stack. > + return not store.find( > + Snap, > + Snap.id == self.id, > + visibility_clause).is_empty() > + > def subscribe(self, person, subscribed_by): > """See `ISnap`.""" > # XXX pappacena 2021-02-05: We may need a "SnapSubscription" here. -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397693 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:snap-pillar-accesspolicy. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
