← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~pappacena/launchpad:ocirecipe-list-on-person into launchpad:master

 

Review: Approve



Diff comments:

> diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
> index 5eee7e3..4d526ff 100644
> --- a/lib/lp/oci/browser/ocirecipe.py
> +++ b/lib/lp/oci/browser/ocirecipe.py
> @@ -204,7 +204,9 @@ class OCIRecipeContextMenu(ContextMenu):
>  
>  
>  class OCIProjectRecipesView(LaunchpadView):
> -    """Default view for the list of OCI recipes of an OCI project."""
> +    """Default view for the list of OCI recipes of a context (OCI project
> +    or Person).
> +    """

This seems like a reasonable repurposing, but the view class name no longer quite makes sense.  Maybe something like `OCIRecipeListingView` (and similarly rename the template to `ocirecipe-listing.pt`, and the associated test class)?

>      page_title = 'Recipes'
>  
>      @property
> diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
> index 2a5598a..1397837 100644
> --- a/lib/lp/oci/model/ocirecipe.py
> +++ b/lib/lp/oci/model/ocirecipe.py
> @@ -875,6 +877,14 @@ class OCIRecipeSet:
>              OCIRecipe.oci_project == oci_project,
>              get_ocirecipe_privacy_filter(visible_by_user))
>  
> +    def findByContext(self, context, visible_by_user):
> +        if IPerson.providedBy(context):
> +            return self.findByOwner(context).find(
> +                get_ocirecipe_privacy_filter(visible_by_user))
> +        if IOCIProject.providedBy(context):

Really minor style point, but I'd slightly prefer if/elif/else for this sort of thing rather than if-fallthrough - it just makes it easier to see at a glance that exactly one branch is taken.

> +            return self.findByOCIProject(context, visible_by_user)
> +        raise NotImplementedError("Unknown OCI recipe context: %s" % context)
> +
>      def findByGitRepository(self, repository, paths=None):
>          """See `IOCIRecipeSet`."""
>          clauses = [OCIRecipe.git_repository == repository]


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400214
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-filter-private.


References