Merge lp:~flacoste/launchpad/bug-781993-2 into lp:launchpad

[Francis J. Lacoste]

[r=bac][bug=781993] Allow filing bugs against source package with an official package branch.

= 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.

[Brad Crittenden (bac)]

This branch looks good Francis.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2011-06-17 21:17:24 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2011-06-17 21:17:26 +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/bug-views.txt'
--- lib/lp/bugs/browser/tests/bug-views.txt	2011-06-17 21:17:24 +0000
+++ lib/lp/bugs/browser/tests/bug-views.txt	2011-06-17 21:17:26 +0000
@@ -25,7 +25,8 @@
 if the user provides a valid package name when reporting the bug.
 
 If the package name entered by the user happens to be a binary package
-name, the bug is filed against the source package used to build that binary.
+name, that information is recorded in the description, and the first
+comment, of the bug report.
 
     >>> from zope.component import getMultiAdapter, getUtility
     >>> from canonical.launchpad.webapp.interfaces import (
@@ -69,7 +70,13 @@
 
     >>> latest_ubuntu_bugtask = ubuntu.searchTasks(search_params)[0]
 
-The source package from which the binary was built has been set on
+The user specified a binary package name, so that's been added to the
+bug description and the first comment:
+
+    >>> print latest_ubuntu_bugtask.bug.description
+    a bug in a bin pkg
+
+the source package from which the binary was built has been set on
 the bugtask.
 
     >>> print latest_ubuntu_bugtask.sourcepackagename.name
@@ -156,11 +163,7 @@
     3
     >>> [ (c.index, c.owner.name, c.text_contents)
     ...  for c in ubuntu_bugview.comments ]
-<<<<<<< TREE
-    [(0, u'name16', u'Binary package hint: linux-2.6.12...'),
-=======
     [(0, u'name16', u'a bug in a bin pkg'),
->>>>>>> MERGE-SOURCE
      (1, u'name16', u'I can reproduce this bug.'),
      (2, u'name16', u'I can reproduce this bug.')]
 

=== 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 21:17:26 +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/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-06-17 21:17:24 +0000
+++ lib/lp/bugs/model/bug.py	2011-06-17 21:17:26 +0000
@@ -51,18 +51,14 @@
     )
 from storm.expr import (
     And,
-    Count,
     Desc,
-    Exists,
     In,
-    Join,
     LeftJoin,
     Max,
     Not,
     Or,
     Select,
     SQL,
-    SQLRaw,
     Sum,
     Union,
     )
@@ -171,7 +167,6 @@
 from lp.bugs.model.bugtask import (
     BugTask,
     bugtask_sort_key,
-    get_bug_privacy_filter,
     )
 from lp.bugs.model.bugwatch import BugWatch
 from lp.bugs.model.structuralsubscription import (
@@ -300,6 +295,7 @@
                 ))
     sum_count = Sum(BugSummary.count)
     tag_count_columns = (BugSummary.tag, sum_count)
+
     # Always query for used
     def _query(*args):
         return store.find(tag_count_columns, *(where_conditions + list(args))
@@ -2605,7 +2601,7 @@
         # one source package, it will be returned more than one time. 4
         # is an arbitrary number that should be large enough.
         bugs = []
-        for bug_task in bug_tasks[:4*limit]:
+        for bug_task in bug_tasks[:4 * limit]:
             bug = bug_task.bug
             duplicateof = bug.duplicateof
             if duplicateof is not None:

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2011-06-17 21:17:24 +0000
+++ lib/lp/registry/interfaces/distribution.py	2011-06-17 21:17:26 +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 21:17:24 +0000
+++ lib/lp/registry/model/distribution.py	2011-06-17 21:17:26 +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 21:17:24 +0000
+++ lib/lp/registry/tests/test_distribution.py	2011-06-17 21:17:26 +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):