launchpad-reviewers team mailing list archive

["Francis J. Lacoste" <francis.lacoste@xxxxxxxxxx>]

Francis J. Lacoste has proposed merging lp:~flacoste/launchpad/bug-781993-2 into lp:launchpad with lp:~flacoste/launchpad/bug-781993-1 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~flacoste/launchpad/bug-781993-2/+merge/65058

= Summary =

We want to be able to file bugs in the Ensemble Principia distribution
(https://launchpad.net/principia). That distribution doesn't have published
source package, but they have official source package branch.

It builds on the previous guessPublishedSourcePackageName refactoring.

== Proposed fix ==

Allow filing a bug on a source package if it has official source package
branch.

== Pre-implementation notes ==

== Implementation details ==

 * If no published source package is found, it looks for official source
 package branches for it.

== Tests ==

In addition to the unit test for guessPublishedSourcePackageName, I added an
integration test just to make sure that we don't regress that functionality.
(By changing the vocabulary for example, they are currently very lax and will
allow any valid source or binary package  names. If we were to make them more
context sensitive, it might be a problem if official package branches weren't
considered.)


 ./bin/test -vvt 'test_guessPublished|TestFileBugSourcePackage'

== Demo and Q/A ==

Try filing a sourcepackage bug on qastaging on the the principia distribution.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt
  lib/lp/bugs/browser/bugtarget.py
  lib/lp/bugs/doc/bugzilla-import.txt
  lib/lp/bugs/browser/widgets/bugtask.py
  lib/lp/bugs/browser/tests/bug-views.txt
  lib/canonical/launchpad/interfaces/validation.py
  lib/lp/testing/factory.py
  lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
  lib/lp/bugs/model/bug.py
  lib/lp/soyuz/doc/distribution.txt
  cronscripts/update-debwatches.py
  lib/lp/bugs/doc/bugtask-package-widget.txt
  lib/lp/registry/interfaces/distribution.py
  lib/lp/app/widgets/launchpadtarget.py
  lib/canonical/launchpad/scripts/debsync.py
  lib/lp/registry/tests/test_distribution.py
  lib/lp/bugs/scripts/bugzilla.py
  lib/lp/registry/model/distribution.py
  lib/lp/bugs/xmlrpc/bug.py
  lib/lp/bugs/interfaces/bug.py

./lib/lp/bugs/browser/tests/bug-views.txt
     195: source has trailing whitespace.
./lib/lp/bugs/model/bug.py
      52: 'SQLRaw' imported but unused
      52: 'Join' imported but unused
      52: 'Exists' imported but unused
     171: 'get_bug_privacy_filter' imported but unused
      52: 'Count' imported but unused
     304: E301 expected 1 blank line, found 0
    2590: E225 missing whitespace around operator
./cronscripts/update-debwatches.py
       9: '_pythonpath' imported but unused
./lib/lp/bugs/scripts/bugzilla.py
       8: Line exceeds 78 characters.

I'll fix this post-review.
-- 
https://code.launchpad.net/~flacoste/launchpad/bug-781993-2/+merge/65058
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~flacoste/launchpad/bug-781993-2 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2011-06-17 19:58:36 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2011-06-17 19:58:45 +0000
@@ -329,6 +329,17 @@
         self.extra_data = FileBugData()
 
     def initialize(self):
+        # redirect_ubuntu_filebug is a cached_property.
+        # Access it first just to compute its value. Because it
+        # makes a DB access to get the bug supervisor, it causes
+        # trouble in tests when form validation errors occur. Because the
+        # transaction is doomed, the storm cache is invalidated and accessing
+        # the property will result in a a LostObjectError, because
+        # the created objects disappeared. Not likely a problem in production
+        # since the objects will still be in the DB, but doesn't hurt there
+        # either. It makes for better diagnosis of failing tests.
+        if self.redirect_ubuntu_filebug:
+            pass
         LaunchpadFormView.initialize(self)
         if (not self.redirect_ubuntu_filebug and
             self.extra_data_token is not None and

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2011-04-29 01:55:28 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2011-06-17 19:58:45 +0000
@@ -8,6 +8,7 @@
     TooLong,
     TooShort,
     )
+from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.ftests import login
 from canonical.launchpad.testing.pages import (
@@ -21,6 +22,7 @@
     FileBugViewBase,
     )
 from lp.bugs.interfaces.bug import IBugAddForm
+from lp.bugs.publisher import BugsLayer
 from lp.testing import (
     login_person,
     TestCaseWithFactory,
@@ -394,3 +396,33 @@
             "source": product.displayname, "content": u"Include bug details",
             }]
         self.assertEqual(expected_guidelines, view.bug_reporting_guidelines)
+
+
+class TestFileBugSourcePackage(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_filebug_works_on_official_package_branch(self):
+        # It should be possible to file a bug against a source package
+        # when there is an official package branch.
+        user = self.factory.makePerson()
+        sourcepackage = self.factory.makeSourcePackage('my-package')
+        self.factory.makeRelatedBranchesForSourcePackage(
+            sourcepackage=sourcepackage)
+        removeSecurityProxy(sourcepackage.distribution).official_malone = True
+        login_person(user)
+
+        view = create_initialized_view(
+            context=sourcepackage.distribution, name='+filebug',
+            form={
+                'field.title': 'A bug',
+                'field.comment': 'A comment',
+                'field.bugtarget.distribution': (
+                    sourcepackage.distribution.name),
+                'field.packagename': 'my-package',
+                'field.actions.submit_bug': 'Submit Bug Request',
+            }, layer=BugsLayer, principal=user)
+        msg = "\n".join([
+            notification.message
+            for notification in view.request.response.notifications])
+        self.assertIn("Thank you for your bug report.", msg)

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2011-06-17 19:58:36 +0000
+++ lib/lp/registry/interfaces/distribution.py	2011-06-17 19:58:45 +0000
@@ -570,8 +570,12 @@
     def guessPublishedSourcePackageName(pkgname):
         """Return the "published" SourcePackageName related to pkgname.
 
-        If pkgname is corresponds to a source package that was published in
-        any of the distribution series.
+        If pkgname corresponds to a source package that was published in
+        any of the distribution series, that's the SourcePackageName that is
+        returned.
+
+        If there is any official source package branch linked, then that
+        source package name is returned.
 
         Otherwise, try to find a published binary package name and then return
         the source package name from which it comes from.

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2011-06-17 19:58:36 +0000
+++ lib/lp/registry/model/distribution.py	2011-06-17 19:58:45 +0000
@@ -119,10 +119,12 @@
     HasBugHeatMixin,
     OfficialBugTagTargetMixin,
     )
-from lp.bugs.model.bugtask import BugTask
 from lp.bugs.model.structuralsubscription import (
     StructuralSubscriptionTargetMixin,
     )
+from lp.code.interfaces.seriessourcepackagebranch import (
+    IFindOfficialBranchLinks,
+    )
 from lp.registry.errors import NoSuchDistroSeries
 from lp.registry.interfaces.distribution import (
     IBaseDistribution,
@@ -649,7 +651,8 @@
         """See `IBugTarget`."""
         return get_bug_tags("BugTask.distribution = %s" % sqlvalues(self))
 
-    def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0, include_tags=None):
+    def getUsedBugTagsWithOpenCounts(self, user, tag_limit=0,
+                                     include_tags=None):
         """See IBugTarget."""
         # Circular fail.
         from lp.bugs.model.bugsummary import BugSummary
@@ -1407,6 +1410,14 @@
             if publishing is not None:
                 return sourcepackagename
 
+            # Look to see if there is an official source package branch.
+            # That's considered "published" enough.
+            branch_links = getUtility(IFindOfficialBranchLinks)
+            results = branch_links.findForDistributionSourcePackage(
+                self.getSourcePackage(sourcepackagename))
+            if results.any() is not None:
+                return sourcepackagename
+
         # At this point we don't have a published source package by
         # that name, so let's try to find a binary package and work
         # back from there.

=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py	2011-06-17 19:58:36 +0000
+++ lib/lp/registry/tests/test_distribution.py	2011-06-17 19:58:45 +0000
@@ -168,6 +168,18 @@
             distroseries.distribution.guessPublishedSourcePackageName(
                 'my-package').name)
 
+    def test_guessPublishedSourcePackageName_official_package_branch(self):
+        # It consider that a sourcepackage that has an official package
+        # branch is published.
+        sourcepackage = self.factory.makeSourcePackage(
+            sourcepackagename='my-package')
+        self.factory.makeRelatedBranchesForSourcePackage(
+            sourcepackage=sourcepackage)
+        self.assertEquals(
+            'my-package',
+            sourcepackage.distribution.guessPublishedSourcePackageName(
+                'my-package').name)
+
 
 class TestDistributionCurrentSourceReleases(
     TestDistroSeriesCurrentSourceReleases):