Merge lp://staging/~jamalta/launchpad/changesfile-253525 into lp://staging/launchpad

Proposed by Jamal Fanaian
Status: Merged
Approved by: Henning Eggers
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://staging/~jamalta/launchpad/changesfile-253525
Merge into: lp://staging/launchpad
Diff against target: 229 lines (+27/-46)
7 files modified
lib/lp/soyuz/stories/ppa/xx-copy-packages.txt (+4/-4)
lib/lp/soyuz/stories/ppa/xx-delete-packages.txt (+4/-4)
lib/lp/soyuz/stories/ppa/xx-ppa-files.txt (+1/-1)
lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt (+3/-3)
lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt (+12/-31)
lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt (+2/-2)
lib/lp/soyuz/templates/sourcepackagepublishinghistory-listing-archive-detailed.pt (+1/-1)
To merge this branch: bzr merge lp://staging/~jamalta/launchpad/changesfile-253525
Reviewer Review Type Date Requested Status
Jamal Fanaian (community) Approve
Henning Eggers (community) code Approve
Review via email: mp+17596@code.staging.launchpad.net

This proposal supersedes a proposal from 2010-01-18.

Commit message

Renamed "changesfile" throughout various pages in soyuz to "sources.changes" since it is not a word. Updated tests that referenced the term "changesfile".

To post a comment you must log in.
Revision history for this message
Jamal Fanaian (jamalta) wrote : Posted in a previous version of this proposal

Summary

Bug #253525 explains that the word changesfile does not exist and should be changed for something that makes sense. The PPA page was changed to read sources.changes, so the same example was followed in the other templates that still read changesfile.

Proposed fix

Replace changesfile in templates so that it reads sources.changes instead.

Pre-implementation notes

Discussed with Curtis Hovey who agreed that sources.changes was the best term to use. He also pointed out stories for soyuz that will have to be modified to work with this change.

Implementation details

lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt
lib/lp/soyuz/templates/sourcepackagepublishinghistory-listing-archive-detailed.pt
 * Changes changesfile term in the content to sources.changes

lib/lp/soyuz/stories/ppa/xx-copy-packages.txt
lib/lp/soyuz/stories/ppa/xx-delete-packages.txt
lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt
lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt
 * Update stories that referenced pages with the above templates to read sources.changes instead of changesfile.

Tests

bin/test -vvct xx-distroseries-sources
bin/test -vvct xx-copy-packages
bin/test -vvct xx-delete-packages
bin/test -vvct xx-ppa-packages

Demo and Q/A

https://launchpad.dev/~cprov/+archive/ppa/+packages
https://launchpad.dev/ubuntu/hoary/+source/pmount/0.1-1

Revision history for this message
Henning Eggers (henninge) wrote : Posted in a previous version of this proposal

As discussed on irc, please apply extract_text to the tests in xx-distroseries-sources.txt to remove the HTML from the text. Find all occurances in that file for bonus points.

Come back with an incremental diff for approval, please.

review: Needs Fixing (code)
Revision history for this message
Jamal Fanaian (jamalta) wrote :

Summary

Bug #253525 explains that the word changesfile does not exist and should be changed for something that makes sense. The PPA page was changed to read sources.changes, so the same example was followed in the other templates that still read changesfile.

Proposed fix

Replace changesfile in templates so that it reads sources.changes instead.

Pre-implementation notes

Discussed with Curtis Hovey who agreed that sources.changes was the best term to use. He also pointed out stories for soyuz that will have to be modified to work with this change.

Implementation details

lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt
lib/lp/soyuz/templates/sourcepackagepublishinghistory-listing-archive-detailed.pt
 * Changes changesfile term in the content to sources.changes

lib/lp/soyuz/stories/ppa/xx-copy-packages.txt
lib/lp/soyuz/stories/ppa/xx-delete-packages.txt
lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt
lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt
 * Update stories that referenced pages with the above templates to read sources.changes instead of changesfile.

Tests

bin/test -vvct xx-distroseries-sources
bin/test -vvct xx-copy-packages
bin/test -vvct xx-delete-packages
bin/test -vvct xx-ppa-packages

Demo and Q/A

https://launchpad.dev/~cprov/+archive/ppa/+packages
https://launchpad.dev/ubuntu/hoary/+source/pmount/0.1-1

Revision history for this message
Jamal Fanaian (jamalta) wrote :

=== modified file 'lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt'
--- lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt 2010-01-18 16:42:09 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt 2010-01-18 17:02:54 +0000
@@ -243,11 +243,8 @@
   >>> user_browser.open(
   ... "http://launchpad.dev/ubuntu/breezy-autotest/+source/"
   ... "commercialpackage/1.0-1")
- >>> changelog = find_tag_by_id(
- ... user_browser.contents, 'commercialpackage_1.0-1')
- >>> print extract_text(changelog.find('a'))
- <email address hidden>
-
+ >>> print user_browser.getLink('<email address hidden>').url
+ http://launchpad.dev/~name16

 Let's check how the page behaves if we no files are present:

Revision history for this message
Henning Eggers (henninge) wrote :

Thank you very much for the fix and for taking the time to improve the test as well. BIG BONUS POINTS for that! ;-) Good job!

I will land tha branch for you. Thank you for your contribution-

review: Approve (code)
Revision history for this message
Jamal Fanaian (jamalta) wrote :

Looks like I missed a test. Fixed errors from the test.

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-files.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2009-11-27 13:09:06 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2010-01-18 21:41:12 +0000
@@ -91,7 +91,7 @@
 Links to files accessible via +files/ proxy in the PPA page.

     >>> ppa_links = [
- ... ('(changesfile)',
+ ... ('(sources.changes)',
     ... another_test_source.sourcepackagerelease.upload_changesfile),
     ... ]

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/soyuz/stories/ppa/xx-copy-packages.txt'
--- lib/lp/soyuz/stories/ppa/xx-copy-packages.txt 2009-12-11 15:57:14 +0000
+++ lib/lp/soyuz/stories/ppa/xx-copy-packages.txt 2010-01-18 21:44:16 +0000
@@ -988,7 +988,7 @@
988 >>> print_ppa_packages(jblack_browser.contents)988 >>> print_ppa_packages(jblack_browser.contents)
989 Source Published Status Series Section Build989 Source Published Status Series Section Build
990 Status990 Status
991 foo - 2.0 (changesfile) ... Published Hoary Base991 foo - 2.0 (sources.changes) ... Published Hoary Base
992992
993 >>> foo_pub_id = getPPAPubIDsFor('no-priv', 'foo')[0]993 >>> foo_pub_id = getPPAPubIDsFor('no-priv', 'foo')[0]
994 >>> jblack_browser.getControl(994 >>> jblack_browser.getControl(
@@ -1063,8 +1063,8 @@
1063 >>> print_ppa_packages(jblack_browser.contents)1063 >>> print_ppa_packages(jblack_browser.contents)
1064 Source Published Status Series Section Build1064 Source Published Status Series Section Build
1065 Status1065 Status
1066 foo - 2.0 (changesfile) Pending Hoary Base i3861066 foo - 2.0 (sources.changes) Pending Hoary Base i386
1067 foo - 1.1 (changesfile) Pending Warty Base1067 foo - 1.1 (sources.changes) Pending Warty Base
1068 pmount - 0.1-1 (Newer...) Pending Hoary Editors1068 pmount - 0.1-1 (Newer...) Pending Hoary Editors
1069 pmount - 0.1-1 Pending Warty Editors1069 pmount - 0.1-1 Pending Warty Editors
1070 pmount - 0.1-1 Pending Grumpy Editors1070 pmount - 0.1-1 Pending Grumpy Editors
@@ -1080,7 +1080,7 @@
1080 >>> print_ppa_packages(jblack_browser.contents)1080 >>> print_ppa_packages(jblack_browser.contents)
1081 Source Published Status Series Section Build1081 Source Published Status Series Section Build
1082 Status1082 Status
1083 foo - 1.1 (changesfile) ... Published Hoary Base1083 foo - 1.1 (sources.changes) ... Published Hoary Base
1084 iceweasel...(...) 2007-07-09 Published Breezy-autotest Editors1084 iceweasel...(...) 2007-07-09 Published Breezy-autotest Editors
10851085
1086 >>> foo_pub_id = getPPAPubIDsFor('mark', 'foo')[0]1086 >>> foo_pub_id = getPPAPubIDsFor('mark', 'foo')[0]
10871087
=== modified file 'lib/lp/soyuz/stories/ppa/xx-delete-packages.txt'
--- lib/lp/soyuz/stories/ppa/xx-delete-packages.txt 2009-09-22 16:39:28 +0000
+++ lib/lp/soyuz/stories/ppa/xx-delete-packages.txt 2010-01-18 21:44:16 +0000
@@ -371,7 +371,7 @@
371 >>> print_ppa_packages(user_browser.contents)371 >>> print_ppa_packages(user_browser.contents)
372 Source Published Status Series Section Build372 Source Published Status Series Section Build
373 Status373 Status
374 foo - 1.0 (changesfile) Superseded Hoary Base374 foo - 1.0 (sources.changes) Superseded Hoary Base
375375
376We don't show the publishing details for binary packages, but the376We don't show the publishing details for binary packages, but the
377presence of 'Built packages' and the binary filename in the 'Files'377presence of 'Built packages' and the binary filename in the 'Files'
@@ -401,7 +401,7 @@
401 >>> print_ppa_packages(user_browser.contents)401 >>> print_ppa_packages(user_browser.contents)
402 Source Published Status Series Section Build402 Source Published Status Series Section Build
403 Status403 Status
404 foo - 1.0 (changesfile) Superseded Hoary Base404 foo - 1.0 (sources.changes) Superseded Hoary Base
405405
406 >>> expander_url = user_browser.getLink(406 >>> expander_url = user_browser.getLink(
407 ... id='pub%s-expander' % foo_pub_src.id).url407 ... id='pub%s-expander' % foo_pub_src.id).url
@@ -440,7 +440,7 @@
440 >>> print_ppa_packages(user_browser.contents)440 >>> print_ppa_packages(user_browser.contents)
441 Source Published Status Series Section Build441 Source Published Status Series Section Build
442 Status442 Status
443 foo - 1.0 (changesfile) Superseded Hoary Base443 foo - 1.0 (sources.changes) Superseded Hoary Base
444444
445The deletion works exactly as it does for PUBLISHED sources, both,445The deletion works exactly as it does for PUBLISHED sources, both,
446source and binaries are marked as DELETED and the corresponding446source and binaries are marked as DELETED and the corresponding
@@ -479,7 +479,7 @@
479 >>> print_ppa_packages(user_browser.contents)479 >>> print_ppa_packages(user_browser.contents)
480 Source Published Status Series Section Build480 Source Published Status Series Section Build
481 Status481 Status
482 foo - 1.0 (changesfile) Deleted Hoary Base482 foo - 1.0 (sources.changes) Deleted Hoary Base
483483
484 >>> expander_url = user_browser.getLink(484 >>> expander_url = user_browser.getLink(
485 ... id='pub%s-expander' % foo_pub_src.id).url485 ... id='pub%s-expander' % foo_pub_src.id).url
486486
=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-files.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2009-11-27 13:09:06 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2010-01-18 21:44:16 +0000
@@ -91,7 +91,7 @@
91Links to files accessible via +files/ proxy in the PPA page.91Links to files accessible via +files/ proxy in the PPA page.
9292
93 >>> ppa_links = [93 >>> ppa_links = [
94 ... ('(changesfile)',94 ... ('(sources.changes)',
95 ... another_test_source.sourcepackagerelease.upload_changesfile),95 ... another_test_source.sourcepackagerelease.upload_changesfile),
96 ... ]96 ... ]
9797
9898
=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt 2009-12-14 13:49:03 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt 2010-01-18 21:44:16 +0000
@@ -96,7 +96,7 @@
96 Source Published Status Series Section Build96 Source Published Status Series Section Build
97 Status97 Status
98 cdrkit - 1.0 2007-07-09 Published Breezy-a... Editors i38698 cdrkit - 1.0 2007-07-09 Published Breezy-a... Editors i386
99 ice...(changesfile) 2007-07-09 Published Warty Editors i38699 ice...(sources.changes) 2007-07-09 Published Warty Editors i386
100 pmount - 0.1-1 2007-07-09 Published Warty Editors100 pmount - 0.1-1 2007-07-09 Published Warty Editors
101101
102Each data row is expandable to contain some sections containing:102Each data row is expandable to contain some sections containing:
@@ -289,7 +289,7 @@
289 >>> print_archive_package_rows(anon_browser.contents)289 >>> print_archive_package_rows(anon_browser.contents)
290 Source Published Status Series Section Build290 Source Published Status Series Section Build
291 Status291 Status
292 i...(changesfile) 2007-07-09 Superseded Warty Editors292 i...(sources.changes) 2007-07-09 Superseded Warty Editors
293 pmount - 0.1-1 2007-07-09 Deleted Warty Editors293 pmount - 0.1-1 2007-07-09 Deleted Warty Editors
294294
295The 'Any Status' filter is also available, so the user can search over295The 'Any Status' filter is also available, so the user can search over
@@ -301,7 +301,7 @@
301 Source Published Status Series Section Build301 Source Published Status Series Section Build
302 Status302 Status
303 cdrkit - 1.0 2007-07-09 Published Breezy-a... Editors i386303 cdrkit - 1.0 2007-07-09 Published Breezy-a... Editors i386
304 ic...(changesfile) 2007-07-09 Superseded Warty Editors304 ic...(sources.changes) 2007-07-09 Superseded Warty Editors
305 pmount - 0.1-1 2007-07-09 Deleted Warty Editors305 pmount - 0.1-1 2007-07-09 Deleted Warty Editors
306306
307307
308308
=== modified file 'lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt'
--- lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt 2009-11-18 02:58:23 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-distroseries-sources.txt 2010-01-18 21:44:16 +0000
@@ -212,14 +212,8 @@
212212
213With the possibility to download the entire changesfile (if available):213With the possibility to download the entire changesfile (if available):
214214
215 >>> print find_tag_by_id(browser.contents, 'changesfile')215 >>> print extract_text(find_tag_by_id(browser.contents, 'changesfile'))
216 <div id="changesfile">216 View sources.changes
217 <p>
218 <a href="http://localhost:58000/52/mozilla-firefox_0.9_i386.changes">
219 View changesfile
220 </a>
221 </p>
222 </div>
223217
224And also download the files contained in this source, like '.orig',218And also download the files contained in this source, like '.orig',
225'.diff' and the DSC:219'.diff' and the DSC:
@@ -249,11 +243,8 @@
249 >>> user_browser.open(243 >>> user_browser.open(
250 ... "http://launchpad.dev/ubuntu/breezy-autotest/+source/"244 ... "http://launchpad.dev/ubuntu/breezy-autotest/+source/"
251 ... "commercialpackage/1.0-1")245 ... "commercialpackage/1.0-1")
252 >>> changelog = find_tag_by_id(246 >>> print user_browser.getLink('foo.bar@canonical.com').url
253 ... user_browser.contents, 'commercialpackage_1.0-1')247 http://launchpad.dev/~name16
254 >>> changelog.find('a')
255 <a href="http://launchpad.dev/~name16" class="sprite person">&nbsp;foo.bar@canonical.com</a>
256
257248
258Let's check how the page behaves if we no files are present:249Let's check how the page behaves if we no files are present:
259250
@@ -264,15 +255,11 @@
264A string is presented in both 'changesfile' and 'files' sections,255A string is presented in both 'changesfile' and 'files' sections,
265warning the user that no file is available:256warning the user that no file is available:
266257
267 >>> print find_tag_by_id(browser.contents, 'changesfile')258 >>> print extract_text(find_tag_by_id(browser.contents, 'changesfile'))
268 <div id="changesfile">259 No sources.changes available.
269 <p>No changesfile available.</p>
270 </div>
271260
272 >>> print find_tag_by_id(browser.contents, 'files')261 >>> print extract_text(find_tag_by_id(browser.contents, 'files'))
273 <div id="files">262 No files available for download.
274 <p>No files available for download.</p>
275 </div>
276263
277264
278= DistroSeries Partner Source Package Pages =265= DistroSeries Partner Source Package Pages =
@@ -291,11 +278,8 @@
291This page provides its versions publications organised by pocket.278This page provides its versions publications organised by pocket.
292We can see 'commercialpackage' is published once in pocket RELEASE:279We can see 'commercialpackage' is published once in pocket RELEASE:
293280
294 >>> print find_tag_by_id(browser.contents, 'publishing_history')281 >>> print browser.getLink('commercialpackage 1.0-1').url
295 <dl id="publishing_history">282 http://launchpad.dev/ubuntu/breezy-autotest/+source/commercialpackage/1.0-1
296 ...
297 <a href="/ubuntu/breezy-autotest/+source/commercialpackage/1.0-1">commercialpackage 1.0-1</a>
298 ...
299283
300The user can also download the files for the "currentrelease" (last284The user can also download the files for the "currentrelease" (last
301published version) if they are available:285published version) if they are available:
@@ -372,11 +356,8 @@
372356
373With the possibility to download the entire changesfile (if available):357With the possibility to download the entire changesfile (if available):
374358
375 >>> print find_tag_by_id(browser.contents, 'changesfile')359 >>> print browser.getLink('View sources.changes').url
376 <div id="changesfile">360 http://localhost:58000/65/commercialpackage_1.0-1_source.changes
377 ...
378 <a href="http://localhost:58000/65/commercialpackage_1.0-1_source.changes">
379 ...
380361
381And also download the files contained in this source, like '.orig',362And also download the files contained in this source, like '.orig',
382'.diff' and the DSC:363'.diff' and the DSC:
383364
=== modified file 'lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt'
--- lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2009-10-27 03:23:05 +0000
+++ lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2010-01-18 21:44:16 +0000
@@ -50,10 +50,10 @@
50 <div id="changesfile" tal:define="changesfile context/changesfile">50 <div id="changesfile" tal:define="changesfile context/changesfile">
51 <p tal:condition="changesfile">51 <p tal:condition="changesfile">
52 <a tal:attributes="href changesfile/http_url">52 <a tal:attributes="href changesfile/http_url">
53 View changesfile53 View sources.changes
54 </a>54 </a>
55 </p>55 </p>
56 <p tal:condition="not: changesfile">No changesfile available.</p>56 <p tal:condition="not: changesfile">No sources.changes available.</p>
57 </div>57 </div>
5858
59</div>59</div>
6060
=== modified file 'lib/lp/soyuz/templates/sourcepackagepublishinghistory-listing-archive-detailed.pt'
--- lib/lp/soyuz/templates/sourcepackagepublishinghistory-listing-archive-detailed.pt 2009-12-10 15:19:03 +0000
+++ lib/lp/soyuz/templates/sourcepackagepublishinghistory-listing-archive-detailed.pt 2010-01-18 21:44:16 +0000
@@ -35,7 +35,7 @@
35 <a tal:condition="changesfile"35 <a tal:condition="changesfile"
36 tal:attributes="href changesfile/http_url;36 tal:attributes="href changesfile/http_url;
37 title changesfile/filename"37 title changesfile/filename"
38 >(changesfile)</a>38 >(sources.changes)</a>
39 </tal:view_changesfile>39 </tal:view_changesfile>
40 </td>40 </td>
41 <td tal:condition="context/archive/owner/isTeam"41 <td tal:condition="context/archive/owner/isTeam"