Merge lp:~jcsackett/launchpad/deprecate-official_codehosting into lp:launchpad

[j.c.sackett]

Replaces use of official_codehosting with the codehosting usage enum where possible.

= Summary =

Replaces use of official_codehosting with codehosting_attributes where possible.

== Proposed fix ==

Where official_codehosting is used, replace it with codehosting_usage and the ServiceUsage enums.

== Pre-implementation notes ==

Spoke with Curtis.

== Implementation details ==

As in Proposed fix.

== Tests ==

bin/test -vvc -t productseries-views.txt
bin/test -vvc -t pillar-views.txt
bin/test -vvc -t distribution-views.txt

== Demo and Q/A ==

Check out a project, product and distribution on launchpad.dev.

Everything should function as before--this shouldn't have introduced
any visible changes.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/adapters.py
  lib/lp/registry/configure.zcml
  lib/lp/registry/browser/distribution.py
  lib/lp/registry/browser/pillar.py
  lib/lp/registry/browser/productseries.py
  lib/lp/registry/browser/tests/pillar-views.txt
  lib/lp/registry/browser/tests/productseries-views.txt

[Brad Crittenden (bac)]

This branch looks nice Jon. What would you think of creating some helper functions like:

uses_Launchpad(thing)

You could then collapse this:

175 + if self.codehosting_usage == ServiceUsage.LAUNCHPAD:
176 + configured = True
177 + else:
178 + configured = False
179 return [dict(link=set_branch,
181 + configured=configured)]

to:

179 return [dict(link=set_branch,
181 + configured=uses_launchpad(self.codehosting_usage)

I guess you could do the same with:

configured = (self.codehosting_usage == ServiceUsage.LAUNCHPAD)

with less overhead.

review: Approve (code)

[j.c.sackett (jcsackett)]

Brad,

I actually deliberately expanded some clauses into the if constructs because I found them easier to parse.

I'm not opposed to a helper function, since the inside of that would be fairly clear, but assigning a boolean
expression to a variable is sort of difficult to scan in a large block of code.

Thoughts?

On Aug 27, 2010, at 4:49 PM, Brad Crittenden wrote:

> Review: Approve code
> This branch looks nice Jon. What would you think of creating some helper functions like:
>
> uses_Launchpad(thing)
>
> You could then collapse this:
>
> 175 + if self.codehosting_usage == ServiceUsage.LAUNCHPAD:
> 176 + configured = True
> 177 + else:
> 178 + configured = False
> 179 return [dict(link=set_branch,
> 181 + configured=configured)]
>
> to:
>
> 179 return [dict(link=set_branch,
> 181 + configured=uses_launchpad(self.codehosting_usage)
>
> I guess you could do the same with:
>
> configured = (self.codehosting_usage == ServiceUsage.LAUNCHPAD)
>
> with less overhead.
> --
> https://code.launchpad.net/~jcsackett/launchpad/deprecate-official_codehosting/+merge/33953
> Your team Launchpad code reviewers from Canonical is subscribed to branch lp:launchpad/devel.
>
> --
> launchpad-reviews mailing list
> <email address hidden>
> https://lists.ubuntu.com/mailman/listinfo/launchpad-reviews

[Brad Crittenden (bac)]

On Aug 27, 2010, at 19:25, "j.c.sackett" <email address hidden> wrote:

> Brad,
>
> I actually deliberately expanded some clauses into the if constructs because I found them easier to parse.
>
> I'm not opposed to a helper function, since the inside of that would be fairly clear, but assigning a boolean
> expression to a variable is sort of difficult to scan in a large block of code.
>
> Thoughts?

What you have now is fine to land. If we find ourselves repeating that logic elsewhere then we can create a helper.

Brad

>

=== modified file 'lib/lp/registry/adapters.py'
--- lib/lp/registry/adapters.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/adapters.py	2010-08-31 14:49:44 +0000
@@ -7,16 +7,25 @@
 
 __all__ = [
     'distroseries_to_launchpadusage',
+    'distroseries_to_serviceusage',
     'PollSubset',
     'productseries_to_product',
     ]
 
 
-from zope.component import getUtility
+from zope.component import (
+    adapter,
+    getUtility,
+    )
 from zope.component.interfaces import ComponentLookupError
-from zope.interface import implements
+from zope.interface import (
+    implementer,
+    implements,
+    )
 
 from canonical.launchpad.webapp.interfaces import ILaunchpadPrincipal
+from lp.app.interfaces.launchpad import IServiceUsage
+from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.poll import (
     IPollSet,
     IPollSubset,
@@ -25,6 +34,13 @@
     )
 
 
+@implementer(IServiceUsage)
+@adapter(IDistroSeries)
+def distroseries_to_serviceusage(distroseries):
+    """Adapts `IDistroSeries` object to `IServiceUsage`."""
+    return distroseries.distribution
+
+
 def distroseries_to_launchpadusage(distroseries):
     """Adapts `IDistroSeries` object to `ILaunchpadUsage`."""
     return distroseries.distribution

=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2010-08-26 22:44:30 +0000
+++ lib/lp/registry/browser/distribution.py	2010-08-31 14:49:44 +0000
@@ -80,6 +80,7 @@
     QuestionTargetFacetMixin,
     QuestionTargetTraversalMixin,
     )
+from lp.app.enums import ServiceUsage
 from lp.app.errors import NotFoundError
 from lp.blueprints.browser.specificationtarget import (
     HasSpecificationsMenuMixin,
@@ -133,7 +134,7 @@
             url = canonical_url(self.context, rootsite='bugs')
             uses.append(href_template % (url, 'Bug Tracking'))
         if IProduct.providedBy(self.context):
-            if self.context.official_codehosting:
+            if self.context.codehosting_usage == ServiceUsage.LAUNCHPAD:
                 url = canonical_url(self.context, rootsite='code')
                 uses.append(href_template % (url, 'Branches'))
         if self.context.official_rosetta:

=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/browser/pillar.py	2010-08-31 14:49:44 +0000
@@ -31,6 +31,8 @@
     nearest,
     )
 from canonical.launchpad.webapp.tales import MenuAPI
+from lp.app.enums import ServiceUsage
+from lp.app.interfaces.launchpad import IServiceUsage
 from lp.registry.browser.structuralsubscription import (
     StructuralSubscriptionMenuMixin,
     )
@@ -73,9 +75,16 @@
             enabled=self.pillar.official_rosetta)
 
     def submit_code(self):
+        if self.pillar.codehosting_usage in [
+                ServiceUsage.LAUNCHPAD, 
+                ServiceUsage.EXTERNAL,
+                ]:
+            enabled = True
+        else:
+            enabled = False
         return Link(
             '+addbranch', 'Submit code', site='code', icon='code',
-            enabled=self.pillar.official_codehosting)
+            enabled=enabled)
 
     def register_blueprint(self):
         return Link(
@@ -96,19 +105,20 @@
         self.official_answers = False
         self.official_blueprints = False
         self.official_rosetta = False
-        self.official_codehosting = False
+        self.codehosting_usage = ServiceUsage.UNKNOWN
         pillar = nearest(self.context, IPillar)
         if IProjectGroup.providedBy(pillar):
             for product in pillar.products:
                 self._set_official_launchpad(product)
             # Project groups do not support submit code, override the
             # default.
-            self.official_codehosting = False
+            self.codehosting_usage = ServiceUsage.NOT_APPLICABLE
         else:
             self._set_official_launchpad(pillar)
             if IDistroSeries.providedBy(self.context):
                 self.official_answers = False
-                self.official_codehosting = False
+                distribution = self.context.distribution
+                self.codehosting_usage = distribution.codehosting_usage
             elif IDistributionSourcePackage.providedBy(self.context):
                 self.official_blueprints = False
                 self.official_rosetta = False
@@ -128,8 +138,7 @@
             self.official_blueprints = True
         if pillar.official_rosetta:
             self.official_rosetta = True
-        if pillar.official_codehosting:
-            self.official_codehosting = True
+        self.codehosting_usage = IServiceUsage(pillar).codehosting_usage
 
     @property
     def has_involvement(self):
@@ -137,7 +146,7 @@
         return (
             self.official_malone or self.official_answers
             or self.official_blueprints or self.official_rosetta
-            or self.official_codehosting)
+            or self.codehosting_usage == ServiceUsage.LAUNCHPAD)
 
     @property
     def enabled_links(self):

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2010-08-23 04:48:17 +0000
+++ lib/lp/registry/browser/productseries.py	2010-08-31 14:49:44 +0000
@@ -83,6 +83,7 @@
 from canonical.launchpad.webapp.tales import MenuAPI
 from canonical.widgets.itemswidgets import LaunchpadRadioWidget
 from canonical.widgets.textwidgets import StrippedTextWidget
+from lp.app.enums import ServiceUsage
 from lp.app.errors import (
     NotFoundError,
     UnexpectedFormData,
@@ -237,7 +238,7 @@
     def submit_code(self):
         target = canonical_url(
             self.pillar, view_name='+addbranch', rootsite='code')
-        enabled = self.view.official_codehosting
+        enabled = self.view.codehosting_usage == ServiceUsage.LAUNCHPAD
         return Link(
             target, 'Submit code', icon='code', enabled=enabled)
 
@@ -251,7 +252,10 @@
 
     def __init__(self, context, request):
         super(ProductSeriesInvolvementView, self).__init__(context, request)
-        self.official_codehosting = self.context.branch is not None
+        if self.context.branch is not None:
+            self.codehosting_usage = ServiceUsage.LAUNCHPAD
+        else:
+            self.codehosting_usage = ServiceUsage.UNKNOWN
         self.official_answers = False
 
     @property
@@ -260,8 +264,12 @@
         series_menu = MenuAPI(self.context).overview
         set_branch = series_menu['set_branch']
         set_branch.text = 'Configure series branch'
+        if self.codehosting_usage == ServiceUsage.LAUNCHPAD:
+            configured = True
+        else:
+            configured = False
         return [dict(link=set_branch,
-                     configured=self.official_codehosting)]
+                     configured=configured)]
 
 
 class ProductSeriesOverviewMenu(

=== modified file 'lib/lp/registry/browser/tests/pillar-views.txt'
--- lib/lp/registry/browser/tests/pillar-views.txt	2010-08-19 20:11:11 +0000
+++ lib/lp/registry/browser/tests/pillar-views.txt	2010-08-31 14:49:44 +0000
@@ -39,8 +39,8 @@
     False
     >>> view.official_blueprints
     False
-    >>> view.official_codehosting
-    False
+    >>> view.codehosting_usage.name
+    'NOT_APPLICABLE'
 
 The view provides a list of enabled links that is rendered by the template.
 
@@ -200,23 +200,23 @@
     >>> product.official_codehosting
     False
     >>> view = create_view(product, '+get-involved')
-    >>> view.official_codehosting
-    False
+    >>> view.codehosting_usage.name
+    'UNKNOWN'
 
     >>> product.development_focus.branch = factory.makeBranch(
     ...     product=product)
     >>> product.official_codehosting
     True
     >>> view = create_view(product, '+get-involved')
-    >>> view.official_codehosting
-    True
+    >>> view.codehosting_usage.name
+    'LAUNCHPAD' 
 
 Project groups cannot make links to register a branch, so
 official_codehosting is always false.
 
     >>> view = create_view(project_group, '+get-involved')
-    >>> view.official_codehosting
-    False
+    >>> view.codehosting_usage.name
+    'NOT_APPLICABLE'
 
 DistroSeries can use this view. The distribution is used to set the links.
 

=== modified file 'lib/lp/registry/browser/tests/productseries-views.txt'
--- lib/lp/registry/browser/tests/productseries-views.txt	2010-08-23 04:48:17 +0000
+++ lib/lp/registry/browser/tests/productseries-views.txt	2010-08-31 14:49:44 +0000
@@ -39,8 +39,8 @@
     True
     >>> print view.official_rosetta
     True
-    >>> print view.official_codehosting
-    False
+    >>> print view.codehosting_usage.name
+    UNKNOWN
     >>> for link in view.enabled_links:
     ...     print link.url
     http://bugs.launchpad.dev/app/simple/+filebug
@@ -54,8 +54,8 @@
 
     >>> series.branch = factory.makeBranch()
     >>> view = create_view(series, '+get-involved')
-    >>> print view.official_codehosting
-    True
+    >>> print view.codehosting_usage.name
+    LAUNCHPAD
     >>> for link in view.enabled_links:
     ...     print link.url
     http://bugs.launchpad.dev/app/simple/+filebug

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2010-08-23 03:25:20 +0000
+++ lib/lp/registry/configure.zcml	2010-08-31 14:49:44 +0000
@@ -171,6 +171,8 @@
         factory="lp.registry.adapters.distroseries_to_launchpadusage"
         permission="zope.Public"/>
     <adapter
+        factory="lp.registry.adapters.distroseries_to_serviceusage" />
+    <adapter
         provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
         for="lp.registry.interfaces.distroseries.IDistroSeries"
         factory="lp.registry.browser.distroseries.DistroSeriesBreadcrumb"
@@ -1379,6 +1381,11 @@
         factory="lp.registry.adapters.productseries_to_product"
         permission="zope.Public"/>
     <adapter
+        provides="lp.app.interfaces.launchpad.IServiceUsage"
+        for="lp.registry.interfaces.productseries.IProductSeries"
+        factory="lp.registry.adapters.productseries_to_product"
+        permission="zope.Public"/>
+    <adapter
         provides="lp.app.interfaces.launchpad.ILaunchpadUsage"
         for="lp.registry.interfaces.productseries.IProductSeries"
         factory="lp.registry.adapters.productseries_to_product"

=== modified file 'lib/lp/registry/stories/product/xx-product-launchpad-usage.txt'
--- lib/lp/registry/stories/product/xx-product-launchpad-usage.txt	2010-08-25 00:01:57 +0000
+++ lib/lp/registry/stories/product/xx-product-launchpad-usage.txt	2010-08-31 14:49:44 +0000
@@ -47,7 +47,8 @@
     >>> registrant_browser.getControl('Somewhere else').selected = True
     >>> registrant_browser.getControl('Change').click()
 
-We'll also set it as officially using codehosting.
+We'll also set up a development focus branch. Because it's a mirrored
+branch, codehosting won't show up as a service the product uses.
 
     >>> registrant_browser.getLink('Configure project branch').click()
     >>> registrant_browser.getControl(name='field.branch_location').value = (
@@ -89,7 +90,7 @@
 
     >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')
     >>> print extract_text(uses)
-    Uses Launchpad for: Branches and Translations.
+    Uses Launchpad for: Translations.
 
 Blueprints can be enabled too, though it isn't easy.
 
@@ -110,12 +111,12 @@
 
 
 On the product page, we can see that the product doesn't use any bug
-tracker, not even Launchpad, but does use other apps.
+tracker, not even Launchpad, but does use Translations.
 
     >>> registrant_browser.open('http://launchpad.dev/firefox')
     >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')
     >>> print extract_text(uses)
-    Uses Launchpad for: Blueprints, Branches, and Translations.
+    Uses Launchpad for: Blueprints and Translations.
 
 Setting a usage to external is reflected in the "Uses" message.
 Change translations to external.
@@ -127,7 +128,7 @@
 
     >>> uses = find_tag_by_id(registrant_browser.contents, id='uses')
     >>> print extract_text(uses)
-    Uses Launchpad for: Blueprints and Branches.
+    Uses Launchpad for: Blueprints.
 
 
 Tracking bugs by email