launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
> index 2ee8802..90b55e7 100644
> --- a/lib/lp/snappy/model/snap.py
> +++ b/lib/lp/snappy/model/snap.py
> @@ -1478,9 +1477,14 @@ class SnapSet:
>                  collection.visibleByUser(visible_by_user),
>                  visible_by_user=visible_by_user)
>  
> +        snaps_for_project = IStore(Snap).find(
> +            Snap,
> +            Snap.project == project,
> +            get_snap_privacy_filter(visible_by_user))

Could we have a test that shows that snaps associated with this project are returned by `findByProject`?

>          bzr_collection = removeSecurityProxy(IBranchCollection(project))
>          git_collection = removeSecurityProxy(IGitCollection(project))
> -        return _getSnaps(bzr_collection).union(_getSnaps(git_collection))
> +        return snaps_for_project.union(
> +            _getSnaps(bzr_collection)).union(_getSnaps(git_collection))
>  
>      def findByBranch(self, branch, visible_by_user=None):
>          """See `ISnapSet`."""


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399224
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:snap-create-on-project.