launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/code-import-bug-link into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #620790 ability to link to a bug for failing code imports
  https://bugs.launchpad.net/bugs/620790


This adds a field to CodeImport that can be used to link the bug that causes the import to be failing. This field can only be set if the import is marked failing.

Pre-implementation call
=======================

I haven't had a pre-implementation call.

Tests
=====
-- 
https://code.launchpad.net/~jelmer/launchpad/code-import-bug-link/+merge/33595
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/code-import-bug-link into lp:launchpad.

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-08-23 04:51:48 +0000
+++ database/schema/comments.sql	2010-08-24 22:49:44 +0000
@@ -365,6 +365,7 @@
 COMMENT ON COLUMN CodeImport.date_last_successful IS 'When this code import last succeeded. NULL if this import has never succeeded.';
 COMMENT ON COLUMN CodeImport.assignee IS 'The person in charge of delivering this code import and interacting with the owner.';
 COMMENT ON COLUMN Codeimport.update_interval IS 'How often should this import be updated. If NULL, defaults to a system-wide value set by the Launchpad administrators.';
+COMMENT ON COLUMN CodeImport.failure_bug IS 'The bug that causes this code import to fail.';
 --COMMENT ON COLUMN CodeImport.modified_by IS 'The user modifying the CodeImport.  This column is never actually set in the database -- it is only present to communicate to the trigger that creates the event, which will intercept and remove the value for this column.';
 
 -- CodeImportEvent

=== added file 'database/schema/patch-2208-00-2.sql'
--- database/schema/patch-2208-00-2.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-00-2.sql	2010-08-24 22:49:44 +0000
@@ -0,0 +1,16 @@
+-- Copyright 2010 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+SET client_min_messages=ERROR;
+
+-- Add reference to bugs to code imports.
+ALTER TABLE CodeImport
+    ADD COLUMN failure_bug integer;
+ALTER TABLE CodeImport
+    ADD CONSTRAINT codeimport__failure_bug__fk
+        FOREIGN KEY (failure_bug) REFERENCES Bug;
+-- A bug for the failure can only be linked if the import is failing (40).
+ALTER TABLE CodeImport
+	ADD CONSTRAINT codeimport__failure_bug_requires_failing
+	CHECK (review_status = 40 OR failure_bug IS NULL);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 00, 2);

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-08-19 03:01:51 +0000
+++ lib/lp/code/configure.zcml	2010-08-24 22:49:44 +0000
@@ -736,6 +736,7 @@
                        product
                        series
                        review_status
+                       failure_bug
                        rcs_type
                        cvs_root
                        cvs_module
@@ -754,7 +755,8 @@
                    requestImport"/>
     <require
        permission="launchpad.Edit"
-       attributes="updateFromData"/>
+       attributes="updateFromData
+                   linkFailureBug"/>
 
     <!-- ICodeImport has no set_schema, because all modifications should be
          done through methods that create CodeImportEvent objects when

=== modified file 'lib/lp/code/interfaces/codeimport.py'
--- lib/lp/code/interfaces/codeimport.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/interfaces/codeimport.py	2010-08-24 22:49:44 +0000
@@ -40,10 +40,15 @@
 
 from canonical.launchpad import _
 from canonical.launchpad.validators import LaunchpadValidationError
+<<<<<<< TREE
 from lp.code.enums import (
     CodeImportReviewStatus,
     RevisionControlSystems,
     )
+=======
+from lp.bugs.interfaces.bug import IBug
+from lp.code.enums import CodeImportReviewStatus, RevisionControlSystems
+>>>>>>> MERGE-SOURCE
 from lp.code.interfaces.branch import IBranch
 from lp.services.fields import (
     PublicPersonChoice,
@@ -100,6 +105,11 @@
             description=_("The Bazaar branch produced by the "
                 "import system.")))
 
+    failure_bug = ReferenceChoice(
+            title=_('Failure bug'), required=False, readonly=False,
+            vocabulary='Bug', schema=IBug,
+            description=_("The bug that is causing this import to fail."))
+
     registrant = PublicPersonChoice(
         title=_('Registrant'), required=True, readonly=True,
         vocabulary='ValidPersonOrTeam',
@@ -188,6 +198,14 @@
             None if no changes were made.
         """
 
+    def linkFailureBug(bug):
+        """Link the bug that causes this import to fail.
+
+        This method requires the review_status to be FAILING.
+
+        :param bug: The bug that is causing the import to fail.
+        """
+
     def tryFailingImportAgain(user):
         """Try a failing import again.
 

=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/codeimport.py	2010-08-24 22:49:44 +0000
@@ -85,6 +85,9 @@
         dbName='assignee', foreignKey='Person',
         storm_validator=validate_public_person, notNull=False, default=None)
 
+    failure_bug = ForeignKey(
+        dbName='failure_bug', foreignKey='Bug', notNull=False, default=None)
+
     review_status = EnumCol(schema=CodeImportReviewStatus, notNull=True,
         default=CodeImportReviewStatus.NEW)
 
@@ -191,6 +194,8 @@
                 else:
                     new_whiteboard = whiteboard
                 self.branch.whiteboard = whiteboard
+        if data.get('review_status', None) != CodeImportReviewStatus.FAILING:
+            self.failure_bug = None
         token = event_set.beginModify(self)
         for name, value in data.items():
             setattr(self, name, value)
@@ -207,6 +212,13 @@
     def __repr__(self):
         return "<CodeImport for %s>" % self.branch.unique_name
 
+    def linkFailureBug(self, bug):
+        """See `ICodeImport`."""
+        if self.review_status != CodeImportReviewStatus.FAILING:
+            raise AssertionError(
+                "review_status is %s not FAILING" % self.review_status.name)
+        self.failure_bug = bug
+
     def tryFailingImportAgain(self, user):
         """See `ICodeImport`."""
         if self.review_status != CodeImportReviewStatus.FAILING:

=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/tests/test_codeimport.py	2010-08-24 22:49:44 +0000
@@ -391,6 +391,31 @@
         self.assertEqual(
             CodeImportReviewStatus.FAILING, code_import.review_status)
 
+    def test_mark_failing_with_bug(self):
+        # Marking an import as failing and linking to a bug.
+        code_import = self.factory.makeCodeImport()
+        code_import.updateFromData(
+            {'review_status':CodeImportReviewStatus.FAILING},
+            self.import_operator)
+        self.assertEquals(None, code_import.failure_bug)
+        bug = self.factory.makeBug()
+        code_import.linkFailureBug(bug)
+        self.assertEqual(
+            CodeImportReviewStatus.FAILING, code_import.review_status)
+        self.assertEquals(bug, code_import.failure_bug)
+
+    def test_mark_no_longer_failing_with_bug(self):
+        # Marking an import as no longer failing removes the failure bug link.
+        code_import = self.factory.makeCodeImport()
+        code_import.updateFromData(
+            {'review_status':CodeImportReviewStatus.FAILING},
+            self.import_operator)
+        code_import.linkFailureBug(self.factory.makeBug())
+        code_import.updateFromData(
+            {'review_status':CodeImportReviewStatus.REVIEWED},
+            self.import_operator)
+        self.assertEquals(None, code_import.failure_bug)
+
 
 class TestCodeImportResultsAttribute(TestCaseWithFactory):
     """Test the results attribute of a CodeImport."""