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
1=== modified file 'Makefile'
2--- Makefile 2010-03-15 17:46:45 +0000
3+++ Makefile 2010-03-18 17:28:01 +0000
4@@ -60,7 +60,7 @@
5 $(PY) ./utilities/make-dummy-hosted-branches
6
7 $(API_INDEX): $(BZR_VERSION_INFO)
8- mkdir $(APIDOC_DIR).tmp
9+ mkdir -p $(APIDOC_DIR).tmp
10 LPCONFIG=$(LPCONFIG) $(PY) ./utilities/create-lp-wadl-and-apidoc.py "$(WADL_TEMPLATE)"
11 mv $(APIDOC_DIR).tmp/* $(APIDOC_DIR)
12 rmdir $(APIDOC_DIR).tmp
13
14=== modified file 'lib/lp/code/browser/codeimport.py'
15--- lib/lp/code/browser/codeimport.py 2010-03-18 06:03:11 +0000
16+++ lib/lp/code/browser/codeimport.py 2010-03-18 17:28:01 +0000
17@@ -23,6 +23,7 @@
18 from zope.component import getUtility
19 from zope.formlib import form
20 from zope.interface import Interface
21+from zope.schema import Choice
22
23 from canonical.cachedproperty import cachedproperty
24 from canonical.launchpad import _
25@@ -117,13 +118,13 @@
26 """The default view for `ICodeImport`.
27
28 We present the CodeImport as a simple page listing all the details of the
29- import such as associated product and branch, who requested the import,
30+ import such as target and branch, who requested the import,
31 and so on.
32 """
33
34 def initialize(self):
35 """See `LaunchpadView.initialize`."""
36- self.title = "Code Import for %s" % (self.context.product.name,)
37+ self.title = "Code Import for %s" % (self.context.branch.target.name,)
38
39
40 class CodeImportBaseView(LaunchpadFormView):
41@@ -202,7 +203,7 @@
42
43 use_template(
44 ICodeImport,
45- ['product', 'rcs_type', 'cvs_root', 'cvs_module'])
46+ ['rcs_type', 'cvs_root', 'cvs_module'])
47
48 svn_branch_url = URIField(
49 title=_("Branch URL"), required=False,
50@@ -249,6 +250,12 @@
51 "imported branch. Examples: main, trunk."),
52 )
53
54+ product = Choice(
55+ title=_('Project'),
56+ description=_("The Project to associate the code import with."),
57+ vocabulary="Product",
58+ )
59+
60
61 class CodeImportNewView(CodeImportBaseView):
62 """The view to request a new code import."""
63@@ -343,8 +350,8 @@
64 <a href="%(product_url)s">%(product_name)s</a>
65 with the name of
66 <a href="%(branch_url)s">%(branch_name)s</a>.""",
67- product_url=canonical_url(existing_branch.product),
68- product_name=existing_branch.product.name,
69+ product_url=canonical_url(existing_branch.target),
70+ product_name=existing_branch.target.name,
71 branch_url=canonical_url(existing_branch),
72 branch_name=existing_branch.name))
73
74
75=== modified file 'lib/lp/code/interfaces/codeimport.py'
76--- lib/lp/code/interfaces/codeimport.py 2010-03-18 06:03:11 +0000
77+++ lib/lp/code/interfaces/codeimport.py 2010-03-18 17:28:01 +0000
78@@ -80,27 +80,6 @@
79 vocabulary='ValidPersonOrTeam',
80 description=_("The person who initially requested this import."))
81
82- owner = PublicPersonChoice(
83- title=_('Owner'), required=True, readonly=False,
84- vocabulary='ValidPersonOrTeam',
85- description=_("The community contact for this import."))
86-
87- assignee = PublicPersonChoice(
88- title=_('Assignee'), required=False, readonly=False,
89- vocabulary='ValidPersonOrTeam',
90- description=_("The person in charge of handling this import."))
91-
92- product = Choice(
93- title=_("Project"), required=True,
94- readonly=True, vocabulary='Product',
95- description=_("The project this code import belongs to."))
96-
97- series = Choice(
98- title=_("Series"),
99- readonly=True, vocabulary='ProductSeries',
100- description=_("The series this import is registered as the "
101- "code for, or None if there is no such series."))
102-
103 review_status = exported(
104 Choice(
105 title=_("Review Status"), vocabulary=CodeImportReviewStatus,
106
107=== modified file 'lib/lp/code/mail/codeimport.py'
108--- lib/lp/code/mail/codeimport.py 2010-03-09 07:38:04 +0000
109+++ lib/lp/code/mail/codeimport.py 2010-03-18 17:28:01 +0000
110@@ -31,7 +31,7 @@
111 return
112 user = IPerson(event.user)
113 subject = 'New code import: %s/%s' % (
114- code_import.product.name, code_import.branch.name)
115+ code_import.branch.target.name, code_import.branch.name)
116 if code_import.rcs_type == RevisionControlSystems.CVS:
117 location = '%s, %s' % (code_import.cvs_root, code_import.cvs_module)
118 else:
119@@ -147,7 +147,7 @@
120 headers = {'X-Launchpad-Branch': branch.unique_name}
121
122 subject = 'Code import %s/%s status: %s' % (
123- code_import.product.name, branch.name,
124+ code_import.branch.target.name, branch.name,
125 code_import.review_status.title)
126
127 email_template = get_email_template('code-import-status-updated.txt')
128
129=== modified file 'lib/lp/code/model/codeimport.py'
130--- lib/lp/code/model/codeimport.py 2010-03-05 03:51:59 +0000
131+++ lib/lp/code/model/codeimport.py 2010-03-18 17:28:01 +0000
132@@ -68,16 +68,6 @@
133 dbName='assignee', foreignKey='Person',
134 storm_validator=validate_public_person, notNull=False, default=None)
135
136- @property
137- def product(self):
138- """See `ICodeImport`."""
139- return self.branch.product
140-
141- @property
142- def series(self):
143- """See `ICodeImport`."""
144- return ProductSeries.selectOneBy(branch=self.branch)
145-
146 review_status = EnumCol(schema=CodeImportReviewStatus, notNull=True,
147 default=CodeImportReviewStatus.NEW)
148
149
150=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
151--- lib/lp/code/model/tests/test_codeimport.py 2010-02-23 02:31:48 +0000
152+++ lib/lp/code/model/tests/test_codeimport.py 2010-03-18 17:28:01 +0000
153@@ -637,10 +637,10 @@
154 # Deactivating a product means that code imports associated to it are
155 # no longer returned.
156 code_import = make_active_import(self.factory)
157- self.failUnless(code_import.product.active)
158+ self.failUnless(code_import.branch.product.active)
159 results = getUtility(ICodeImportSet).getActiveImports()
160 self.assertEquals(list(results), [code_import])
161- deactivate(code_import.product)
162+ deactivate(code_import.branch.product)
163 results = getUtility(ICodeImportSet).getActiveImports()
164 self.assertEquals(list(results), [])
165
166@@ -649,10 +649,10 @@
167 # products in it are no longer returned.
168 code_import = make_active_import(
169 self.factory, project_name="whatever")
170- self.failUnless(code_import.product.project.active)
171+ self.failUnless(code_import.branch.product.project.active)
172 results = getUtility(ICodeImportSet).getActiveImports()
173 self.assertEquals(list(results), [code_import])
174- deactivate(code_import.product.project)
175+ deactivate(code_import.branch.product.project)
176 results = getUtility(ICodeImportSet).getActiveImports()
177 self.assertEquals(list(results), [])
178
179@@ -663,7 +663,7 @@
180 prod2_a = make_active_import(
181 self.factory, product_name='prod2', branch_name='a')
182 prod1_b = self.factory.makeCodeImport(
183- product=prod1_a.product, branch_name='b')
184+ product=prod1_a.branch.product, branch_name='b')
185 make_import_active(self.factory, prod1_b)
186 results = getUtility(ICodeImportSet).getActiveImports()
187 self.assertEquals(
188
189=== modified file 'lib/lp/code/templates/codeimport-list.pt'
190--- lib/lp/code/templates/codeimport-list.pt 2010-03-08 01:15:46 +0000
191+++ lib/lp/code/templates/codeimport-list.pt 2010-03-18 17:28:01 +0000
192@@ -60,8 +60,8 @@
193 <td>
194 <a href="code-import"
195 tal:attributes="href codeimport/branch/fmt:url">
196- <span tal:replace="codeimport/product/name">
197- product-name
198+ <span tal:replace="codeimport/branch/target/name">
199+ target-name
200 </span>/<span tal:replace="codeimport/branch/name">
201 branch-name
202 </span>
203
204=== modified file 'lib/lp/code/templates/sources-list.pt'
205--- lib/lp/code/templates/sources-list.pt 2009-09-21 20:18:02 +0000
206+++ lib/lp/code/templates/sources-list.pt 2010-03-18 17:28:01 +0000
207@@ -43,8 +43,8 @@
208 <tbody>
209 <tr tal:repeat="code_import batch">
210 <td>
211- <a tal:attributes="href code_import/product/fmt:url"
212- tal:content="code_import/product/displayname"
213+ <a tal:attributes="href code_import/branch/target/fmt:url"
214+ tal:content="code_import/branch/target/displayname"
215 >$Product.name</a>
216 </td>
217 <td><a tal:attributes="href code_import/branch/fmt:url"