Launchpad itself

Merge lp:~james-w/launchpad/code-imports-for-source-packages into lp:launchpad/db-devel

  1. code-imports-for-source-packages
  2. Merge into db-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/code-imports-for-source-packages
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~james-w/launchpad/new-code-import-method-on-branchtarget
Diff against target: 269 lines (+145/-12)
7 files modified
lib/lp/code/mail/tests/test_codeimport.py (+28/-0)
lib/lp/code/model/branchtarget.py (+1/-1)
lib/lp/code/model/tests/test_branchtarget.py (+15/-8)
lib/lp/code/model/tests/test_codeimport.py (+21/-0)
lib/lp/code/stories/codeimport/xx-admin-codeimport.txt (+22/-1)
lib/lp/code/stories/webservice/xx-code-import.txt (+51/-2)
lib/lp/testing/factory.py (+7/-0)
To merge this branch: bzr merge lp:~james-w/launchpad/code-imports-for-source-packages
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+22191@code.launchpad.net

Commit message

Package branches are now a vaild target for code imports

Description of the change

Hi,

This flips the switch so that code imports can be registered
for package branches, though there's no way to actually create
them yet.

It's mainly adding the tests, as it's a single line change to
allow them to be created for source packages now.

Thanks,

James

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

Two minor whitespace tweaks please, or rather the same one in two
places. Other than that, awesome!

> === modified file 'lib/lp/code/interfaces/branchtarget.py'
> --- lib/lp/code/interfaces/branchtarget.py 2010-01-05 08:18:23 +0000
> +++ lib/lp/code/interfaces/branchtarget.py 2010-03-26 01:45:34 +0000
> @@ -88,6 +88,9 @@
> supports_merge_proposals = Attribute(
> "Does this target support merge proposals at all?")
>
> + supports_code_imports = Attribute(
> + "Does this target support code imports at all?")
> +
> def areBranchesMergeable(other_target):
> """Are branches from other_target mergeable into this target."""
>
> @@ -113,3 +116,18 @@
>
> def getBugTask(bug):
> """Get the BugTask for a given bug related to the branch target."""
> +
> + def newCodeImport(registrant, branch_name, rcs_type,
> + url=None, cvs_root=None, cvs_module=None):

This should be formatted like so:

    def newCodeImport(registrant, branch_name, rcs_type, url=None,
                      cvs_root=None, cvs_module=None):

(https://dev.launchpad.net/PythonStyleGuide#Wrapping%20long%20arguments%20in%20function%20definitions)

> + """Create a new code import for this target.
> +
> + :param registrant: the `IPerson` who should be recorded as creating
> + the import and will own the resulting branch.
> + :param branch_name: the name the resulting branch should have.
> + :param rcs_type: the type of the foreign VCS.
> + :param url: the url to import from if the import isn't CVS.
> + :param cvs_root: if the import is from CVS the CVSROOT to import from.
> + :param cvs_module: if the import is from CVS the module to import.
> + :returns: an `ICodeImport`.
> + :raises AssertionError: if supports_code_imports is False.
> + """

> === modified file 'lib/lp/code/model/branchtarget.py'
> --- lib/lp/code/model/branchtarget.py 2010-01-05 08:18:23 +0000
> +++ lib/lp/code/model/branchtarget.py 2010-03-26 01:45:34 +0000
> @@ -19,6 +19,7 @@
> from lp.code.interfaces.branchcollection import IAllBranches
> from lp.code.interfaces.branchtarget import (
> check_default_stacked_on, IBranchTarget)
> +from lp.code.interfaces.codeimport import ICodeImportSet
> from lp.registry.interfaces.pocket import PackagePublishingPocket
> from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
>
> @@ -36,6 +37,12 @@
> def __ne__(self, other):
> return self.context != other.context
>
> + def newCodeImport(self, registrant, branch_name, rcs_type, url=None,
> + cvs_root=None, cvs_module=None):

Similarly.

> + return getUtility(ICodeImportSet).new(
> + registrant, self, branch_name, rcs_type, url=url,
> + cvs_root=cvs_root, cvs_module=cvs_module)
> +
>
> class PackageBranchTarget(_BaseBranchTarget):
> implements(IBranchTarget)
> @@ -95,6 +102,11 @@
> """See `IBranchTarget`."""
> return True
>
> + @property
> + def supports_code_imports(self):
> + """See `IBranchTarget`."""
> + return False
> +
> def areBranchesMergeable(self, other_target)...

Read more...

review: Approve
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

That was a review of the other proposal, of course :/

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

The only problem I have with this is that

Header
++++++

lines should be preceded by two blank lines. There are two in the diff ('Approving an import' and 'Package Branches') that are not (one of these not being your fault, but please fix anyway).

Otherwise, great.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/mail/tests/test_codeimport.py'
--- lib/lp/code/mail/tests/test_codeimport.py 2010-03-18 15:39:58 +0000
+++ lib/lp/code/mail/tests/test_codeimport.py 2010-04-02 02:46:34 +0000
@@ -111,6 +111,34 @@
111 '-- \nYou are getting this email because you are a member of the '111 '-- \nYou are getting this email because you are a member of the '
112 'vcs-imports team.\n', msg.get_payload(decode=True))112 'vcs-imports team.\n', msg.get_payload(decode=True))
113113
114 def test_new_source_package_import(self):
115 # Test the email for a new sourcepackage import.
116 eric = self.factory.makePerson(name='eric')
117 distro = self.factory.makeDistribution(name='foobuntu')
118 series = self.factory.makeDistroSeries(
119 name='manic', distribution=distro)
120 fooix = self.factory.makeSourcePackage(
121 sourcename='fooix', distroseries=series)
122 # Eric needs to be logged in for the mail to be sent.
123 login_person(eric)
124 code_import = self.factory.makePackageCodeImport(
125 hg_repo_url='http://hg.example.com/fooix.hg',
126 branch_name='master', sourcepackage=fooix, registrant=eric)
127 transaction.commit()
128 msg = message_from_string(stub.test_emails[0][2])
129 self.assertEqual('code-import', msg['X-Launchpad-Notification-Type'])
130 self.assertEqual(
131 '~eric/foobuntu/manic/fooix/master', msg['X-Launchpad-Branch'])
132 self.assertEqual(
133 'A new mercurial code import has been requested '
134 'by Eric:\n'
135 ' http://code.launchpad.dev/~eric/foobuntu/manic/fooix/master\n'
136 'from\n'
137 ' http://hg.example.com/fooix.hg\n'
138 '\n'
139 '-- \nYou are getting this email because you are a member of the '
140 'vcs-imports team.\n', msg.get_payload(decode=True))
141
114142
115def test_suite():143def test_suite():
116 return TestLoader().loadTestsFromName(__name__)144 return TestLoader().loadTestsFromName(__name__)
117145
=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py 2010-04-02 02:46:31 +0000
+++ lib/lp/code/model/branchtarget.py 2010-04-02 02:46:34 +0000
@@ -105,7 +105,7 @@
105 @property105 @property
106 def supports_code_imports(self):106 def supports_code_imports(self):
107 """See `IBranchTarget`."""107 """See `IBranchTarget`."""
108 return False108 return True
109109
110 def areBranchesMergeable(self, other_target):110 def areBranchesMergeable(self, other_target):
111 """See `IBranchTarget`."""111 """See `IBranchTarget`."""
112112
=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py 2010-04-02 02:46:31 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py 2010-04-02 02:46:34 +0000
@@ -177,15 +177,22 @@
177 ubuntu_branches.teamowner)177 ubuntu_branches.teamowner)
178 self.assertEqual(branch, self.target.default_merge_target)178 self.assertEqual(branch, self.target.default_merge_target)
179179
180 def test_doesnt_support_code_imports(self):180 def test_supports_code_imports(self):
181 self.assertFalse(self.target.supports_code_imports)181 self.assertTrue(self.target.supports_code_imports)
182182
183 def test_creating_code_import_fails(self):183 def test_creating_code_import_succeeds(self):
184 self.assertRaises(184 target_url = self.factory.getUniqueURL()
185 AssertionError, self.target.newCodeImport,185 branch_name = self.factory.getUniqueString("name-")
186 self.factory.makePerson(),186 owner = self.factory.makePerson()
187 self.factory.getUniqueString("name-"),187 code_import = self.target.newCodeImport(
188 RevisionControlSystems.GIT, url=self.factory.getUniqueURL())188 owner, branch_name, RevisionControlSystems.GIT, url=target_url)
189 code_import = removeSecurityProxy(code_import)
190 self.assertProvides(code_import, ICodeImport)
191 self.assertEqual(target_url, code_import.url)
192 self.assertEqual(branch_name, code_import.branch.name)
193 self.assertEqual(owner, code_import.registrant)
194 self.assertEqual(owner, code_import.branch.owner)
195 self.assertEqual(self.target, code_import.branch.target)
189196
190197
191class TestPersonBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):198class TestPersonBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
192199
=== modified file 'lib/lp/code/model/tests/test_codeimport.py'
--- lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:46:31 +0000
+++ lib/lp/code/model/tests/test_codeimport.py 2010-04-02 02:46:34 +0000
@@ -10,6 +10,7 @@
10from sqlobject import SQLObjectNotFound10from sqlobject import SQLObjectNotFound
11from storm.store import Store11from storm.store import Store
12from zope.component import getUtility12from zope.component import getUtility
13from zope.security.proxy import removeSecurityProxy
1314
14from lp.code.model.codeimport import CodeImportSet15from lp.code.model.codeimport import CodeImportSet
15from lp.code.model.codeimportevent import CodeImportEvent16from lp.code.model.codeimportevent import CodeImportEvent
@@ -134,6 +135,26 @@
134 url=self.factory.getUniqueURL(),135 url=self.factory.getUniqueURL(),
135 review_status=None)136 review_status=None)
136137
138 def test_create_source_package_import(self):
139 """Test that we can create an import targetting a source package."""
140 registrant = self.factory.makePerson()
141 source_package = self.factory.makeSourcePackage()
142 target = IBranchTarget(source_package)
143 code_import = CodeImportSet().new(
144 registrant=registrant,
145 target=target,
146 branch_name='imported',
147 rcs_type=RevisionControlSystems.HG,
148 url=self.factory.getUniqueURL(),
149 review_status=None)
150 code_import = removeSecurityProxy(code_import)
151 self.assertEqual(registrant, code_import.registrant)
152 self.assertEqual(registrant, code_import.branch.owner)
153 self.assertEqual(target, code_import.branch.target)
154 self.assertEqual(source_package, code_import.branch.sourcepackage)
155 # And a job is still created
156 self.assertIsNot(None, code_import.import_job)
157
137158
138class TestCodeImportDeletion(TestCaseWithFactory):159class TestCodeImportDeletion(TestCaseWithFactory):
139 """Test the deletion of CodeImports."""160 """Test the deletion of CodeImports."""
140161
=== modified file 'lib/lp/code/stories/codeimport/xx-admin-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2010-03-18 15:39:58 +0000
+++ lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2010-04-02 02:46:34 +0000
@@ -38,6 +38,11 @@
38 >>> hg_import_location = str(canonical_url(hg_import.branch))38 >>> hg_import_location = str(canonical_url(hg_import.branch))
39 >>> hg_import_branch_unique_name = hg_import.branch.unique_name39 >>> hg_import_branch_unique_name = hg_import.branch.unique_name
4040
41 >>> package_import = factory.makePackageCodeImport(
42 ... hg_repo_url="http://hg.example.org/zap")
43 >>> package_import_location = str(canonical_url(package_import.branch))
44 >>> package_import_branch_unique_name = package_import.branch.unique_name
45
41 >>> logout()46 >>> logout()
4247
43 >>> import_browser = setupBrowser(48 >>> import_browser = setupBrowser(
@@ -109,6 +114,11 @@
109 >>> print_form_fields(import_browser)114 >>> print_form_fields(import_browser)
110 field.url: http://hg.example.org/bar115 field.url: http://hg.example.org/bar
111116
117 >>> import_browser.open(package_import_location)
118 >>> import_browser.getLink('Edit import source or review import').click()
119 >>> print_form_fields(import_browser)
120 field.url: http://hg.example.org/zap
121
112122
113Editing the import location123Editing the import location
114+++++++++++++++++++++++++++124+++++++++++++++++++++++++++
@@ -158,7 +168,7 @@
158 ... print extract_text(message)168 ... print extract_text(message)
159 The code import has been updated.169 The code import has been updated.
160170
161and Mercurial imports.171Mercurial imports,
162172
163 >>> import_browser.open(hg_import_location + '/+edit-import')173 >>> import_browser.open(hg_import_location + '/+edit-import')
164 >>> import_browser.getControl('URL').value = \174 >>> import_browser.getControl('URL').value = \
@@ -168,6 +178,17 @@
168 ... print extract_text(message)178 ... print extract_text(message)
169 The code import has been updated.179 The code import has been updated.
170180
181and imports targetting source packages.
182
183 >>> import_browser.open(package_import_location + '/+edit-import')
184 >>> import_browser.getControl('URL').value = \
185 ... 'http://metal.example.org/zap'
186 >>> import_browser.getControl('Update').click()
187 >>> for message in get_feedback_messages(import_browser.contents):
188 ... print extract_text(message)
189 The code import has been updated.
190
191
171Approving an import192Approving an import
172+++++++++++++++++++193+++++++++++++++++++
173194
174195
=== modified file 'lib/lp/code/stories/webservice/xx-code-import.txt'
--- lib/lp/code/stories/webservice/xx-code-import.txt 2010-03-18 15:39:58 +0000
+++ lib/lp/code/stories/webservice/xx-code-import.txt 2010-04-02 02:46:34 +0000
@@ -12,7 +12,8 @@
12 >>> person = factory.makePerson(name='import-owner')12 >>> person = factory.makePerson(name='import-owner')
13 >>> product = factory.makeProduct(name='scruff')13 >>> product = factory.makeProduct(name='scruff')
14 >>> code_import = removeSecurityProxy(factory.makeProductCodeImport(14 >>> code_import = removeSecurityProxy(factory.makeProductCodeImport(
15 ... registrant=person, product=product, branch_name='import'))15 ... registrant=person, product=product, branch_name='import',
16 ... svn_branch_url="http://svn.domain.com/source"))
16 >>> no_import_branch = removeSecurityProxy(factory.makeProductBranch(17 >>> no_import_branch = removeSecurityProxy(factory.makeProductBranch(
17 ... owner=person, product=product, name='no-import'))18 ... owner=person, product=product, name='no-import'))
18 >>> logout()19 >>> logout()
@@ -49,7 +50,55 @@
49 >>> print representation['rcs_type']50 >>> print representation['rcs_type']
50 Subversion via CSCVS51 Subversion via CSCVS
51 >>> print representation['url']52 >>> print representation['url']
52 http://domain9.domain.com/path1053 http://svn.domain.com/source
54 >>> print representation['cvs_root']
55 None
56 >>> print representation['cvs_module']
57 None
58 >>> print representation['date_last_successful']
59 None
60
61
62Package Branches
63----------------
64
65The same is true for package branches.
66
67 >>> login(ANONYMOUS)
68 >>> distribution = factory.makeDistribution(name='scruffbuntu')
69 >>> distroseries = factory.makeDistroSeries(
70 ... name='manic', distribution=distribution)
71 >>> source_package = factory.makeSourcePackage(
72 ... sourcename='scruff', distroseries=distroseries)
73 >>> code_import = removeSecurityProxy(factory.makePackageCodeImport(
74 ... registrant=person, sourcepackage=source_package,
75 ... branch_name='import',
76 ... svn_branch_url="http://svn.domain.com/package_source"))
77 >>> logout()
78
79There is a link on the branch object
80
81 >>> branch_url = '/' + code_import.branch.unique_name
82 >>> response = webservice.get(branch_url)
83 >>> representation = response.jsonBody()
84 >>> print representation['code_import_link']
85 http://.../~import-owner/scruffbuntu/manic/scruff/import/+code-import
86
87and there is information available about the import itsef.
88
89 >>> import_url = representation['code_import_link']
90 >>> response = webservice.get(import_url)
91 >>> representation = response.jsonBody()
92 >>> print representation['self_link'] == import_url
93 True
94 >>> print representation['branch_link']
95 http://.../~import-owner/scruffbuntu/manic/scruff/import
96 >>> print representation['review_status']
97 Pending Review
98 >>> print representation['rcs_type']
99 Subversion via CSCVS
100 >>> print representation['url']
101 http://svn.domain.com/package_source
53 >>> print representation['cvs_root']102 >>> print representation['cvs_root']
54 None103 None
55 >>> print representation['cvs_module']104 >>> print representation['cvs_module']
56105
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-03-26 21:03:30 +0000
+++ lib/lp/testing/factory.py 2010-04-02 02:46:34 +0000
@@ -1406,6 +1406,13 @@
1406 return target.newFAQ(1406 return target.newFAQ(
1407 owner=target.owner, title=title, content='content')1407 owner=target.owner, title=title, content='content')
14081408
1409 def makePackageCodeImport(self, sourcepackage=None, **kwargs):
1410 """Make a code import targetting a sourcepackage."""
1411 if sourcepackage is None:
1412 sourcepackage = self.makeSourcePackage()
1413 target = IBranchTarget(sourcepackage)
1414 return self.makeCodeImport(target=target, **kwargs)
1415
1409 def makeProductCodeImport(self, product=None, **kwargs):1416 def makeProductCodeImport(self, product=None, **kwargs):
1410 """Make a code import targetting a product."""1417 """Make a code import targetting a product."""
1411 if product is None:1418 if product is None:

Subscribers

People subscribed via source and target branches

to status/vote changes: