Merge lp:~james-w/launchpad/nuke-code-import-warts into lp:launchpad

[James Westby]

Some attributes have been removed from ICodeImport in favour of accessing ICodeImport.branch.

Hi,

This deletes some unneeded things from ICodeImport.

owner, assignee and series were all unused, so we drop them
from the interface. The former two are still in the model as they
are in the db, and we still set owner as it has a non-NULL constraint.
If that's the wrong choice then I will need some assistance in
dropping them from the db or dropping the constraint.

product is used, but it's bad form to expose it directly. The code
import doesn't actually need to know, and furthermore, we would like
to support non-project imports one day. The interface for that is IBranchTarget,
so calling code should just use code_import.branch.target.

Thanks,

James

[Michael Hudson-Doyle (mwhudson)]

Yay, all looks good to me.

review: Approve

[Tim Penhey (thumper)]

I agree. I'm happy to see cruft being removed.

review: Approve

=== modified file 'Makefile'
--- Makefile	2010-03-15 17:46:45 +0000
+++ Makefile	2010-03-18 17:28:01 +0000
@@ -60,7 +60,7 @@
 	$(PY) ./utilities/make-dummy-hosted-branches
 
 $(API_INDEX): $(BZR_VERSION_INFO)
-	mkdir $(APIDOC_DIR).tmp
+	mkdir -p $(APIDOC_DIR).tmp
 	LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py "$(WADL_TEMPLATE)"
 	mv $(APIDOC_DIR).tmp/* $(APIDOC_DIR)
 	rmdir $(APIDOC_DIR).tmp

=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py	2010-03-18 06:03:11 +0000
+++ lib/lp/code/browser/codeimport.py	2010-03-18 17:28:01 +0000
@@ -23,6 +23,7 @@
 from zope.component import getUtility
 from zope.formlib import form
 from zope.interface import Interface
+from zope.schema import Choice
 
 from canonical.cachedproperty import cachedproperty
 from canonical.launchpad import _
@@ -117,13 +118,13 @@
     """The default view for `ICodeImport`.
 
     We present the CodeImport as a simple page listing all the details of the
-    import such as associated product and branch, who requested the import,
+    import such as target and branch, who requested the import,
     and so on.
     """
 
     def initialize(self):
         """See `LaunchpadView.initialize`."""
-        self.title = "Code Import for %s" % (self.context.product.name,)
+        self.title = "Code Import for %s" % (self.context.branch.target.name,)
 
 
 class CodeImportBaseView(LaunchpadFormView):
@@ -202,7 +203,7 @@
 
     use_template(
         ICodeImport,
-        ['product', 'rcs_type', 'cvs_root', 'cvs_module'])
+        ['rcs_type', 'cvs_root', 'cvs_module'])
 
     svn_branch_url = URIField(
         title=_("Branch URL"), required=False,
@@ -249,6 +250,12 @@
             "imported branch.  Examples: main, trunk."),
         )
 
+    product = Choice(
+        title=_('Project'),
+        description=_("The Project to associate the code import with."),
+        vocabulary="Product",
+        )
+
 
 class CodeImportNewView(CodeImportBaseView):
     """The view to request a new code import."""
@@ -343,8 +350,8 @@
             <a href="%(product_url)s">%(product_name)s</a>
             with the name of
             <a href="%(branch_url)s">%(branch_name)s</a>.""",
-                       product_url=canonical_url(existing_branch.product),
-                       product_name=existing_branch.product.name,
+                       product_url=canonical_url(existing_branch.target),
+                       product_name=existing_branch.target.name,
                        branch_url=canonical_url(existing_branch),
                        branch_name=existing_branch.name))
 

=== modified file 'lib/lp/code/interfaces/codeimport.py'
--- lib/lp/code/interfaces/codeimport.py	2010-03-18 06:03:11 +0000
+++ lib/lp/code/interfaces/codeimport.py	2010-03-18 17:28:01 +0000
@@ -80,27 +80,6 @@
         vocabulary='ValidPersonOrTeam',
         description=_("The person who initially requested this import."))
 
-    owner = PublicPersonChoice(
-        title=_('Owner'), required=True, readonly=False,
-        vocabulary='ValidPersonOrTeam',
-        description=_("The community contact for this import."))
-
-    assignee = PublicPersonChoice(
-        title=_('Assignee'), required=False, readonly=False,
-        vocabulary='ValidPersonOrTeam',
-        description=_("The person in charge of handling this import."))
-
-    product = Choice(
-        title=_("Project"), required=True,
-        readonly=True, vocabulary='Product',
-        description=_("The project this code import belongs to."))
-
-    series = Choice(
-        title=_("Series"),
-        readonly=True, vocabulary='ProductSeries',
-        description=_("The series this import is registered as the "
-                      "code for, or None if there is no such series."))
-
     review_status = exported(
         Choice(
             title=_("Review Status"), vocabulary=CodeImportReviewStatus,

=== modified file 'lib/lp/code/mail/codeimport.py'
--- lib/lp/code/mail/codeimport.py	2010-03-09 07:38:04 +0000
+++ lib/lp/code/mail/codeimport.py	2010-03-18 17:28:01 +0000
@@ -31,7 +31,7 @@
         return
     user = IPerson(event.user)
     subject = 'New code import: %s/%s' % (
-        code_import.product.name, code_import.branch.name)
+        code_import.branch.target.name, code_import.branch.name)
     if code_import.rcs_type == RevisionControlSystems.CVS:
         location = '%s, %s' % (code_import.cvs_root, code_import.cvs_module)
     else:
@@ -147,7 +147,7 @@
     headers = {'X-Launchpad-Branch': branch.unique_name}
 
     subject = 'Code import %s/%s status: %s' % (
-        code_import.product.name, branch.name,
+        code_import.branch.target.name, branch.name,
         code_import.review_status.title)
 
     email_template = get_email_template('code-import-status-updated.txt')

=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py	2010-03-05 03:51:59 +0000
+++ lib/lp/code/model/codeimport.py	2010-03-18 17:28:01 +0000
@@ -68,16 +68,6 @@
         dbName='assignee', foreignKey='Person',
         storm_validator=validate_public_person, notNull=False, default=None)
 
-    @property
-    def product(self):
-        """See `ICodeImport`."""
-        return self.branch.product
-
-    @property
-    def series(self):
-        """See `ICodeImport`."""
-        return ProductSeries.selectOneBy(branch=self.branch)
-
     review_status = EnumCol(schema=CodeImportReviewStatus, notNull=True,
         default=CodeImportReviewStatus.NEW)
 

=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py	2010-02-23 02:31:48 +0000
+++ lib/lp/code/model/tests/test_codeimport.py	2010-03-18 17:28:01 +0000
@@ -637,10 +637,10 @@
         # Deactivating a product means that code imports associated to it are
         # no longer returned.
         code_import = make_active_import(self.factory)
-        self.failUnless(code_import.product.active)
+        self.failUnless(code_import.branch.product.active)
         results = getUtility(ICodeImportSet).getActiveImports()
         self.assertEquals(list(results), [code_import])
-        deactivate(code_import.product)
+        deactivate(code_import.branch.product)
         results = getUtility(ICodeImportSet).getActiveImports()
         self.assertEquals(list(results), [])
 
@@ -649,10 +649,10 @@
         # products in it are no longer returned.
         code_import = make_active_import(
             self.factory, project_name="whatever")
-        self.failUnless(code_import.product.project.active)
+        self.failUnless(code_import.branch.product.project.active)
         results = getUtility(ICodeImportSet).getActiveImports()
         self.assertEquals(list(results), [code_import])
-        deactivate(code_import.product.project)
+        deactivate(code_import.branch.product.project)
         results = getUtility(ICodeImportSet).getActiveImports()
         self.assertEquals(list(results), [])
 
@@ -663,7 +663,7 @@
         prod2_a = make_active_import(
             self.factory, product_name='prod2', branch_name='a')
         prod1_b = self.factory.makeCodeImport(
-            product=prod1_a.product, branch_name='b')
+            product=prod1_a.branch.product, branch_name='b')
         make_import_active(self.factory, prod1_b)
         results = getUtility(ICodeImportSet).getActiveImports()
         self.assertEquals(

=== modified file 'lib/lp/code/templates/codeimport-list.pt'
--- lib/lp/code/templates/codeimport-list.pt	2010-03-08 01:15:46 +0000
+++ lib/lp/code/templates/codeimport-list.pt	2010-03-18 17:28:01 +0000
@@ -60,8 +60,8 @@
           <td>
             <a href="code-import"
                tal:attributes="href codeimport/branch/fmt:url">
-              <span tal:replace="codeimport/product/name">
-                product-name
+              <span tal:replace="codeimport/branch/target/name">
+                target-name
               </span>/<span tal:replace="codeimport/branch/name">
                 branch-name
               </span>

=== modified file 'lib/lp/code/templates/sources-list.pt'
--- lib/lp/code/templates/sources-list.pt	2009-09-21 20:18:02 +0000
+++ lib/lp/code/templates/sources-list.pt	2010-03-18 17:28:01 +0000
@@ -43,8 +43,8 @@
       <tbody>
         <tr tal:repeat="code_import batch">
           <td>
-            <a tal:attributes="href code_import/product/fmt:url"
-               tal:content="code_import/product/displayname"
+            <a tal:attributes="href code_import/branch/target/fmt:url"
+               tal:content="code_import/branch/target/displayname"
               >$Product.name</a>
           </td>
           <td><a tal:attributes="href code_import/branch/fmt:url"