launchpad-reviewers team mailing list archive

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

Pushed the requested changes.

About withdrawing the push hint, maybe it would make sense to move it to the "Code" tab instead of removing it completely? The hint itself seems to be valuable. Also, I guess we should do it in a separate MP.

Diff comments:

> diff --git a/lib/lp/app/widgets/launchpadtarget.py b/lib/lp/app/widgets/launchpadtarget.py
> index 00f0b37..e2fe87c 100644
> --- a/lib/lp/app/widgets/launchpadtarget.py
> +++ b/lib/lp/app/widgets/launchpadtarget.py
> @@ -55,15 +55,17 @@ class LaunchpadTargetWidget(BrowserWidget, InputWidget):
>      def getProductVocabulary(self):
>          return 'Product'
>  
> -    def setUpSubWidgets(self):
> -        if self._widgets_set_up:
> -            return
> +    def getPackageVocabulary(self):

Ok!

>          if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
>              # Replace the default field with a field that uses the better
>              # vocabulary.
> -            package_vocab = 'DistributionSourcePackage'
> +            return 'DistributionSourcePackage'
>          else:
> -            package_vocab = 'BinaryAndSourcePackageName'
> +            return 'BinaryAndSourcePackageName'
> +
> +    def setUpSubWidgets(self):
> +        if self._widgets_set_up:
> +            return
>          fields = [
>              Choice(
>                  __name__='product', title=u'Project',
> diff --git a/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt b/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt
> index 49341d1..174a261 100644
> --- a/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt
> +++ b/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt
> @@ -49,6 +49,7 @@ respective bug task.
>      Target
>      Distribution
>      ...
> +    Package (Find...)
>      Project (Find…)

Not really. It works fine either way, so I'll make the new one use the same style as the existing one.

>      Status  Importance  Milestone
>      New...  Low...      (nothing selected)...
> diff --git a/lib/lp/registry/browser/ociproject.py b/lib/lp/registry/browser/ociproject.py
> index 3f938d8..3fada35 100644
> --- a/lib/lp/registry/browser/ociproject.py
> +++ b/lib/lp/registry/browser/ociproject.py
> @@ -175,8 +178,23 @@ class OCIProjectFacets(StandardLaunchpadFacets):
>      enable_only = [
>          'overview',
>          'branches',
> +        'bugs',
>          ]
>  
> +    def makeLink(self, text, context, view_name, site):
> +        site = 'mainsite' if self.mainsite_only else site
> +        target = canonical_url(context, view_name=view_name, rootsite=site)
> +        return Link(target, text, site=site)
> +
> +    def branches(self):
> +        return self.makeLink('Code', self.context.pillar, '+branches', 'code')

Ok!

> +
> +    def bugs(self):
> +        """Override bugs link to show the OCIProject's bug page, instead of
> +        the pillar's bug page.
> +        """
> +        return self.makeLink('Bugs', self.context, '+bugs', 'bugs')
> +
>  
>  class OCIProjectNavigationMenu(NavigationMenu):
>      """Navigation menu for OCI projects."""
> diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
> index 85fb588..d2cabc8 100644
> --- a/lib/lp/registry/interfaces/ociproject.py
> +++ b/lib/lp/registry/interfaces/ociproject.py
> @@ -67,7 +72,9 @@ from lp.services.fields import (
>  OCI_PROJECT_ALLOW_CREATE = 'oci.project.create.enabled'
>  
>  
> -class IOCIProjectView(IHasGitRepositories, Interface):
> +class IOCIProjectView(IHasGitRepositories, IServiceUsage,

By having its own "Overview", "Code" and "Bugs" pages, OCIProject behaves a bit more like a Distribution or a Product itself, isn't it? DistributionSourcePacakges would be a bit more like OCIRecipes here.

Bugs page, for example, tries to adapt OCIProject to IServiceUsage at some point (to control the facet links at the top), and this seems to be the easier way to make that work.

> +                      IHasOfficialBugTags, IStructuralSubscriptionTarget,
> +                      Interface):
>      """IOCIProject attributes that require launchpad.View permission."""
>  
>      id = Int(title=_("ID"), required=True, readonly=True)
> diff --git a/lib/lp/registry/vocabularies.py b/lib/lp/registry/vocabularies.py
> index 02e3158..e59028e 100644
> --- a/lib/lp/registry/vocabularies.py
> +++ b/lib/lp/registry/vocabularies.py
> @@ -2229,5 +2240,75 @@ class OCIProjectVocabulary(StormVocabularyBase):
>      def search(self, query, vocab_filter=None):
>          return getUtility(IOCIProjectSet).searchByName(query)
>  
> +    @property

It is probably not being used at all, but I'll change it to property in this MP anyway, and leave tests around it to be done after.

>      def _entries(self):
> -        return getUtility(IOCIProjectSet).searchByName('')
> +        return getUtility(IOCIProjectSet).searchByName(six.ensure_text(''))

Ok!

> +
> +    def __contains__(self, obj):
> +        found_obj = IStore(self._table).find(
> +            self._table,
> +            self._table.id == obj.id).one()
> +        return found_obj is not None and found_obj == obj
> +
> +
> +@implementer(IHugeVocabulary)
> +class DistributionPackageVocabulary:
> +    """A simple wrapper to automatically select package vocabulary
> +    (BinaryAndSourcePackageNameVocabulary or OCIProjectVocabulary) depending

Ok!

> +    on which type of distribution we are dealing with.
> +    """
> +
> +    def __init__(self, context=None):
> +        super(DistributionPackageVocabulary, self).__init__()
> +        if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
> +            # Replace the default field with a field that uses the better
> +            # vocabulary.
> +            self.packages_vocabulary = DistributionSourcePackageVocabulary(
> +                context)
> +        else:
> +            self.packages_vocabulary = BinaryAndSourcePackageNameVocabulary(
> +                context)
> +
> +        self.oci_projects_vocabulary = OCIProjectVocabulary(context)
> +        self.distribution = None
> +        self.context = context
> +
> +    def setDistribution(self, distribution):
> +        self.distribution = distribution
> +        self.oci_projects_vocabulary.setPillar(distribution)
> +        if isinstance(
> +                self.packages_vocabulary, DistributionSourcePackageVocabulary):
> +            self.packages_vocabulary.setDistribution(distribution)
> +
> +    @property
> +    def is_oci_distribution(self):
> +        distribution = self.distribution
> +        if distribution is None and self.context is not None:
> +            # If distribution was not set yet, try to guess it from context.
> +            distribution = getattr(self.context, 'distribution', None)
> +        oci_traversal_policy = DistributionDefaultTraversalPolicy.OCI_PROJECT
> +        return (
> +            distribution is not None and
> +            distribution.default_traversal_policy == oci_traversal_policy)
> +
> +    def __getattribute__(self, item):

Indeed. Fixing it.

> +        local_attrs = {
> +            'is_oci_distribution', 'oci_projects_vocabulary',
> +            'packages_vocabulary', 'distribution', 'setDistribution',
> +            'context'}
> +        if item in local_attrs:
> +            return object.__getattribute__(self, item)
> +        if self.is_oci_distribution:
> +            vocabulary = self.oci_projects_vocabulary
> +        else:
> +            vocabulary = self.packages_vocabulary
> +        return getattr(vocabulary, item)
> +
> +    # Special methods should be declared at the class declaration,
> +    # even if self.__special_method__ will be forwarded to the correct
> +    # vocabulary after, by __getattribute__.
> +    def __iter__(self):
> +        return self.__iter__()
> +
> +    def __contains__(self, obj):
> +        return self.__contains__(obj)


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394181
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:bugtask-oci-project.