Launchpad itself

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

  1. nuke-code-import-warts
  2. Merge into devel
Proposed by James Westby
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~james-w/launchpad/nuke-code-import-warts
Merge into: lp:launchpad
Prerequisite: lp:~james-w/launchpad/export-code-import
Diff against target: 217 lines (+24/-48)
8 files modified
Makefile (+1/-1)
lib/lp/code/browser/codeimport.py (+12/-5)
lib/lp/code/interfaces/codeimport.py (+0/-21)
lib/lp/code/mail/codeimport.py (+2/-2)
lib/lp/code/model/codeimport.py (+0/-10)
lib/lp/code/model/tests/test_codeimport.py (+5/-5)
lib/lp/code/templates/codeimport-list.pt (+2/-2)
lib/lp/code/templates/sources-list.pt (+2/-2)
To merge this branch: bzr merge lp:~james-w/launchpad/nuke-code-import-warts
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Michael Hudson-Doyle Approve
Review via email: mp+21587@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Yay, all looks good to me.

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Makefile'
--- Makefile 2010-03-15 17:46:45 +0000
+++ Makefile 2010-03-18 17:28:01 +0000
@@ -60,7 +60,7 @@
60 $(PY) ./utilities/make-dummy-hosted-branches60 $(PY) ./utilities/make-dummy-hosted-branches
6161
62$(API_INDEX): $(BZR_VERSION_INFO)62$(API_INDEX): $(BZR_VERSION_INFO)
63 mkdir $(APIDOC_DIR).tmp63 mkdir -p $(APIDOC_DIR).tmp
64 LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py "$(WADL_TEMPLATE)"64 LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py "$(WADL_TEMPLATE)"
65 mv $(APIDOC_DIR).tmp/* $(APIDOC_DIR)65 mv $(APIDOC_DIR).tmp/* $(APIDOC_DIR)
66 rmdir $(APIDOC_DIR).tmp66 rmdir $(APIDOC_DIR).tmp
6767
=== 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 @@
23from zope.component import getUtility23from zope.component import getUtility
24from zope.formlib import form24from zope.formlib import form
25from zope.interface import Interface25from zope.interface import Interface
26from zope.schema import Choice
2627
27from canonical.cachedproperty import cachedproperty28from canonical.cachedproperty import cachedproperty
28from canonical.launchpad import _29from canonical.launchpad import _
@@ -117,13 +118,13 @@
117 """The default view for `ICodeImport`.118 """The default view for `ICodeImport`.
118119
119 We present the CodeImport as a simple page listing all the details of the120 We present the CodeImport as a simple page listing all the details of the
120 import such as associated product and branch, who requested the import,121 import such as target and branch, who requested the import,
121 and so on.122 and so on.
122 """123 """
123124
124 def initialize(self):125 def initialize(self):
125 """See `LaunchpadView.initialize`."""126 """See `LaunchpadView.initialize`."""
126 self.title = "Code Import for %s" % (self.context.product.name,)127 self.title = "Code Import for %s" % (self.context.branch.target.name,)
127128
128129
129class CodeImportBaseView(LaunchpadFormView):130class CodeImportBaseView(LaunchpadFormView):
@@ -202,7 +203,7 @@
202203
203 use_template(204 use_template(
204 ICodeImport,205 ICodeImport,
205 ['product', 'rcs_type', 'cvs_root', 'cvs_module'])206 ['rcs_type', 'cvs_root', 'cvs_module'])
206207
207 svn_branch_url = URIField(208 svn_branch_url = URIField(
208 title=_("Branch URL"), required=False,209 title=_("Branch URL"), required=False,
@@ -249,6 +250,12 @@
249 "imported branch. Examples: main, trunk."),250 "imported branch. Examples: main, trunk."),
250 )251 )
251252
253 product = Choice(
254 title=_('Project'),
255 description=_("The Project to associate the code import with."),
256 vocabulary="Product",
257 )
258
252259
253class CodeImportNewView(CodeImportBaseView):260class CodeImportNewView(CodeImportBaseView):
254 """The view to request a new code import."""261 """The view to request a new code import."""
@@ -343,8 +350,8 @@
343 <a href="%(product_url)s">%(product_name)s</a>350 <a href="%(product_url)s">%(product_name)s</a>
344 with the name of351 with the name of
345 <a href="%(branch_url)s">%(branch_name)s</a>.""",352 <a href="%(branch_url)s">%(branch_name)s</a>.""",
346 product_url=canonical_url(existing_branch.product),353 product_url=canonical_url(existing_branch.target),
347 product_name=existing_branch.product.name,354 product_name=existing_branch.target.name,
348 branch_url=canonical_url(existing_branch),355 branch_url=canonical_url(existing_branch),
349 branch_name=existing_branch.name))356 branch_name=existing_branch.name))
350357
351358
=== 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 @@
80 vocabulary='ValidPersonOrTeam',80 vocabulary='ValidPersonOrTeam',
81 description=_("The person who initially requested this import."))81 description=_("The person who initially requested this import."))
8282
83 owner = PublicPersonChoice(
84 title=_('Owner'), required=True, readonly=False,
85 vocabulary='ValidPersonOrTeam',
86 description=_("The community contact for this import."))
87
88 assignee = PublicPersonChoice(
89 title=_('Assignee'), required=False, readonly=False,
90 vocabulary='ValidPersonOrTeam',
91 description=_("The person in charge of handling this import."))
92
93 product = Choice(
94 title=_("Project"), required=True,
95 readonly=True, vocabulary='Product',
96 description=_("The project this code import belongs to."))
97
98 series = Choice(
99 title=_("Series"),
100 readonly=True, vocabulary='ProductSeries',
101 description=_("The series this import is registered as the "
102 "code for, or None if there is no such series."))
103
104 review_status = exported(83 review_status = exported(
105 Choice(84 Choice(
106 title=_("Review Status"), vocabulary=CodeImportReviewStatus,85 title=_("Review Status"), vocabulary=CodeImportReviewStatus,
10786
=== 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 @@
31 return31 return
32 user = IPerson(event.user)32 user = IPerson(event.user)
33 subject = 'New code import: %s/%s' % (33 subject = 'New code import: %s/%s' % (
34 code_import.product.name, code_import.branch.name)34 code_import.branch.target.name, code_import.branch.name)
35 if code_import.rcs_type == RevisionControlSystems.CVS:35 if code_import.rcs_type == RevisionControlSystems.CVS:
36 location = '%s, %s' % (code_import.cvs_root, code_import.cvs_module)36 location = '%s, %s' % (code_import.cvs_root, code_import.cvs_module)
37 else:37 else:
@@ -147,7 +147,7 @@
147 headers = {'X-Launchpad-Branch': branch.unique_name}147 headers = {'X-Launchpad-Branch': branch.unique_name}
148148
149 subject = 'Code import %s/%s status: %s' % (149 subject = 'Code import %s/%s status: %s' % (
150 code_import.product.name, branch.name,150 code_import.branch.target.name, branch.name,
151 code_import.review_status.title)151 code_import.review_status.title)
152152
153 email_template = get_email_template('code-import-status-updated.txt')153 email_template = get_email_template('code-import-status-updated.txt')
154154
=== 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 @@
68 dbName='assignee', foreignKey='Person',68 dbName='assignee', foreignKey='Person',
69 storm_validator=validate_public_person, notNull=False, default=None)69 storm_validator=validate_public_person, notNull=False, default=None)
7070
71 @property
72 def product(self):
73 """See `ICodeImport`."""
74 return self.branch.product
75
76 @property
77 def series(self):
78 """See `ICodeImport`."""
79 return ProductSeries.selectOneBy(branch=self.branch)
80
81 review_status = EnumCol(schema=CodeImportReviewStatus, notNull=True,71 review_status = EnumCol(schema=CodeImportReviewStatus, notNull=True,
82 default=CodeImportReviewStatus.NEW)72 default=CodeImportReviewStatus.NEW)
8373
8474
=== 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 @@
637 # Deactivating a product means that code imports associated to it are637 # Deactivating a product means that code imports associated to it are
638 # no longer returned.638 # no longer returned.
639 code_import = make_active_import(self.factory)639 code_import = make_active_import(self.factory)
640 self.failUnless(code_import.product.active)640 self.failUnless(code_import.branch.product.active)
641 results = getUtility(ICodeImportSet).getActiveImports()641 results = getUtility(ICodeImportSet).getActiveImports()
642 self.assertEquals(list(results), [code_import])642 self.assertEquals(list(results), [code_import])
643 deactivate(code_import.product)643 deactivate(code_import.branch.product)
644 results = getUtility(ICodeImportSet).getActiveImports()644 results = getUtility(ICodeImportSet).getActiveImports()
645 self.assertEquals(list(results), [])645 self.assertEquals(list(results), [])
646646
@@ -649,10 +649,10 @@
649 # products in it are no longer returned.649 # products in it are no longer returned.
650 code_import = make_active_import(650 code_import = make_active_import(
651 self.factory, project_name="whatever")651 self.factory, project_name="whatever")
652 self.failUnless(code_import.product.project.active)652 self.failUnless(code_import.branch.product.project.active)
653 results = getUtility(ICodeImportSet).getActiveImports()653 results = getUtility(ICodeImportSet).getActiveImports()
654 self.assertEquals(list(results), [code_import])654 self.assertEquals(list(results), [code_import])
655 deactivate(code_import.product.project)655 deactivate(code_import.branch.product.project)
656 results = getUtility(ICodeImportSet).getActiveImports()656 results = getUtility(ICodeImportSet).getActiveImports()
657 self.assertEquals(list(results), [])657 self.assertEquals(list(results), [])
658658
@@ -663,7 +663,7 @@
663 prod2_a = make_active_import(663 prod2_a = make_active_import(
664 self.factory, product_name='prod2', branch_name='a')664 self.factory, product_name='prod2', branch_name='a')
665 prod1_b = self.factory.makeCodeImport(665 prod1_b = self.factory.makeCodeImport(
666 product=prod1_a.product, branch_name='b')666 product=prod1_a.branch.product, branch_name='b')
667 make_import_active(self.factory, prod1_b)667 make_import_active(self.factory, prod1_b)
668 results = getUtility(ICodeImportSet).getActiveImports()668 results = getUtility(ICodeImportSet).getActiveImports()
669 self.assertEquals(669 self.assertEquals(
670670
=== 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 @@
60 <td>60 <td>
61 <a href="code-import"61 <a href="code-import"
62 tal:attributes="href codeimport/branch/fmt:url">62 tal:attributes="href codeimport/branch/fmt:url">
63 <span tal:replace="codeimport/product/name">63 <span tal:replace="codeimport/branch/target/name">
64 product-name64 target-name
65 </span>/<span tal:replace="codeimport/branch/name">65 </span>/<span tal:replace="codeimport/branch/name">
66 branch-name66 branch-name
67 </span>67 </span>
6868
=== 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 @@
43 <tbody>43 <tbody>
44 <tr tal:repeat="code_import batch">44 <tr tal:repeat="code_import batch">
45 <td>45 <td>
46 <a tal:attributes="href code_import/product/fmt:url"46 <a tal:attributes="href code_import/branch/target/fmt:url"
47 tal:content="code_import/product/displayname"47 tal:content="code_import/branch/target/displayname"
48 >$Product.name</a>48 >$Product.name</a>
49 </td>49 </td>
50 <td><a tal:attributes="href code_import/branch/fmt:url"50 <td><a tal:attributes="href code_import/branch/fmt:url"