Merge lp:~jtv/launchpad/bug-427278 into lp:launchpad/db-devel
- bug-427278
- Merge into db-devel
Proposed by
Jeroen T. Vermeulen
Status: | Rejected | ||||
---|---|---|---|---|---|
Rejected by: | Jeroen T. Vermeulen | ||||
Proposed branch: | lp:~jtv/launchpad/bug-427278 | ||||
Merge into: | lp:launchpad/db-devel | ||||
Diff against target: |
2165 lines (+938/-380) 34 files modified
database/replication/Makefile (+7/-4) database/replication/repair-restored-db.py (+41/-22) lib/canonical/launchpad/icing/style.css (+9/-2) lib/canonical/launchpad/javascript/code/branchlinks.js (+4/-3) lib/canonical/launchpad/javascript/code/branchstatus.js (+2/-1) lib/canonical/launchpad/javascript/code/branchsubscription.js (+2/-1) lib/canonical/launchpad/javascript/code/codereview.js (+2/-1) lib/canonical/launchpad/javascript/code/popupdiff.js (+123/-0) lib/canonical/launchpad/webapp/configure.zcml (+0/-7) lib/canonical/launchpad/webapp/tales.py (+0/-62) lib/canonical/launchpad/webapp/tests/test_tales.py (+2/-135) lib/devscripts/ec2test/builtins.py (+1/-1) lib/lp/code/browser/branchlisting.py (+1/-8) lib/lp/code/browser/branchmergeproposal.py (+81/-33) lib/lp/code/browser/configure.zcml (+30/-5) lib/lp/code/browser/diff.py (+77/-0) lib/lp/code/browser/tests/test_branchmergeproposal.py (+55/-17) lib/lp/code/browser/tests/test_tales.py (+163/-0) lib/lp/code/model/branchmergeproposal.py (+11/-6) lib/lp/code/model/directbranchcommit.py (+17/-3) lib/lp/code/stories/branches/xx-branch-merge-proposals.txt (+43/-27) lib/lp/code/templates/branch-index.pt (+23/-1) lib/lp/code/templates/branch-pending-merges.pt (+1/-0) lib/lp/code/templates/branchmergeproposal-diff.pt (+33/-0) lib/lp/code/templates/branchmergeproposal-index.pt (+2/-28) lib/lp/code/templates/branchmergeproposal-link-summary.pt (+2/-1) lib/lp/code/templates/branchmergeproposal-vote-summary.pt (+52/-0) lib/lp/code/tests/helpers.py (+42/-0) lib/lp/code/tests/test_directbranchcommit.py (+1/-1) lib/lp/code/windmill/tests/test_popup_diff.py (+64/-0) lib/lp/services/browser_helpers.py (+8/-0) lib/lp/translations/doc/translations-export-to-branch.txt (+11/-3) lib/lp/translations/scripts/translations_to_branch.py (+13/-6) lib/lp/translations/utilities/gettext_po_exporter.py (+15/-2) |
||||
To merge this branch: | bzr merge lp:~jtv/launchpad/bug-427278 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Barry Warsaw (community) | Abstain | ||
Review via email:
|
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Jeroen T. Vermeulen (jtv) wrote : | # |
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Barry Warsaw (barry) wrote : | # |
Jeroen,
It looks like there's a lot more stuff in here than I'd expect from reading the cover letter. For example, why all the style and JavaScript changes? What's with the PreviewDiff changes? Something tells me that you've got a lot more changes in this merge proposal than you intend. Since I can't separate out what should be reviewed, I'm abstaining on this for now.
review:
Abstain
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Jeroen T. Vermeulen (jtv) wrote : | # |
Oops; forgot to MP for devel instead of the default db-devel. Filing new MP.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'database/replication/Makefile' | |||
2 | --- database/replication/Makefile 2009-11-13 10:41:58 +0000 | |||
3 | +++ database/replication/Makefile 2009-11-18 15:05:39 +0000 | |||
4 | @@ -36,6 +36,8 @@ | |||
5 | 36 | STAGING_CONFIG=staging # For swapping fresh db into place. | 36 | STAGING_CONFIG=staging # For swapping fresh db into place. |
6 | 37 | STAGING_DUMP=launchpad.dump # Dumpfile to build new staging from. | 37 | STAGING_DUMP=launchpad.dump # Dumpfile to build new staging from. |
7 | 38 | STAGING_TABLESPACE=pg_default # 'pg_default' for default | 38 | STAGING_TABLESPACE=pg_default # 'pg_default' for default |
8 | 39 | DOGFOOD_DBNAME=launchpad_dogfood | ||
9 | 40 | DOGFOOD_DUMP=launchpad.dump | ||
10 | 39 | 41 | ||
11 | 40 | _CONFIG=overridden-on-command-line | 42 | _CONFIG=overridden-on-command-line |
12 | 41 | _SLAVE_TABLESPACE=pg_default | 43 | _SLAVE_TABLESPACE=pg_default |
13 | @@ -98,12 +100,13 @@ | |||
14 | 98 | psql -q -d lpmain_staging_new -f authdb_drop.sql | 100 | psql -q -d lpmain_staging_new -f authdb_drop.sql |
15 | 99 | psql -q -d lpmain_staging_new -f authdb_create.sql \ | 101 | psql -q -d lpmain_staging_new -f authdb_create.sql \ |
16 | 100 | 2>&1 | grep -v _sl || true | 102 | 2>&1 | grep -v _sl || true |
17 | 103 | # Restore the data | ||
18 | 104 | pg_restore --dbname=lpmain_staging_new \ | ||
19 | 105 | --no-acl --no-owner --disable-triggers --data-only \ | ||
20 | 106 | --exit-on-error ${STAGING_DUMP} | ||
21 | 101 | # Uninstall Slony-I if it is installed - a pg_dump of a DB with | 107 | # Uninstall Slony-I if it is installed - a pg_dump of a DB with |
22 | 102 | # Slony-I installed isn't usable without this step. | 108 | # Slony-I installed isn't usable without this step. |
23 | 103 | LPCONFIG=${NEW_STAGING_CONFIG} ./repair-restored-db.py | 109 | LPCONFIG=${NEW_STAGING_CONFIG} ./repair-restored-db.py |
24 | 104 | pg_restore --dbname=lpmain_staging_new \ | ||
25 | 105 | --no-acl --no-owner --disable-triggers --data-only \ | ||
26 | 106 | --exit-on-error ${STAGING_DUMP} | ||
27 | 107 | # Setup replication | 110 | # Setup replication |
28 | 108 | make _replicate LPCONFIG=${NEW_STAGING_CONFIG} LAG="0 seconds" \ | 111 | make _replicate LPCONFIG=${NEW_STAGING_CONFIG} LAG="0 seconds" \ |
29 | 109 | _MASTER=lpmain_staging_new _SLAVE=lpmain_staging_slave_new \ | 112 | _MASTER=lpmain_staging_new _SLAVE=lpmain_staging_slave_new \ |
30 | @@ -138,10 +141,10 @@ | |||
31 | 138 | psql -q -d ${DOGFOOD_DBNAME} -f authdb_drop.sql | 141 | psql -q -d ${DOGFOOD_DBNAME} -f authdb_drop.sql |
32 | 139 | psql -q -d ${DOGFOOD_DBNAME} -f authdb_create.sql \ | 142 | psql -q -d ${DOGFOOD_DBNAME} -f authdb_create.sql \ |
33 | 140 | 2>&1 | grep -v _sl || true | 143 | 2>&1 | grep -v _sl || true |
34 | 141 | ./repair-restored-db.py -d ${DOGFOOD_DBNAME} | ||
35 | 142 | pg_restore --dbname=${DOGFOOD_DBNAME} --no-acl --no-owner \ | 144 | pg_restore --dbname=${DOGFOOD_DBNAME} --no-acl --no-owner \ |
36 | 143 | --disable-triggers --data-only \ | 145 | --disable-triggers --data-only \ |
37 | 144 | --exit-on-error ${DOGFOOD_DUMP} | 146 | --exit-on-error ${DOGFOOD_DUMP} |
38 | 147 | ./repair-restored-db.py -d ${DOGFOOD_DBNAME} | ||
39 | 145 | ../schema/upgrade.py -d ${DOGFOOD_DBNAME} | 148 | ../schema/upgrade.py -d ${DOGFOOD_DBNAME} |
40 | 146 | ../schema/fti.py -d ${DOGFOOD_DBNAME} | 149 | ../schema/fti.py -d ${DOGFOOD_DBNAME} |
41 | 147 | ../schema/security.py -d ${DOGFOOD_DBNAME} | 150 | ../schema/security.py -d ${DOGFOOD_DBNAME} |
42 | 148 | 151 | ||
43 | === modified file 'database/replication/repair-restored-db.py' | |||
44 | --- database/replication/repair-restored-db.py 2009-10-17 14:06:03 +0000 | |||
45 | +++ database/replication/repair-restored-db.py 2009-11-18 15:05:38 +0000 | |||
46 | @@ -27,7 +27,8 @@ | |||
47 | 27 | 27 | ||
48 | 28 | from canonical.config import config | 28 | from canonical.config import config |
49 | 29 | from canonical.database.postgresql import ConnectionString | 29 | from canonical.database.postgresql import ConnectionString |
51 | 30 | from canonical.database.sqlbase import connect, quote | 30 | from canonical.database.sqlbase import ( |
52 | 31 | connect, quote, ISOLATION_LEVEL_AUTOCOMMIT) | ||
53 | 31 | from canonical.launchpad.scripts import db_options, logger_options, logger | 32 | from canonical.launchpad.scripts import db_options, logger_options, logger |
54 | 32 | 33 | ||
55 | 33 | import replication.helpers | 34 | import replication.helpers |
56 | @@ -44,12 +45,23 @@ | |||
57 | 44 | 45 | ||
58 | 45 | log = logger(options) | 46 | log = logger(options) |
59 | 46 | 47 | ||
61 | 47 | con = connect(options.dbuser) | 48 | con = connect(options.dbuser, isolation=ISOLATION_LEVEL_AUTOCOMMIT) |
62 | 48 | 49 | ||
63 | 49 | if not replication.helpers.slony_installed(con): | 50 | if not replication.helpers.slony_installed(con): |
64 | 50 | log.info("Slony-I not installed. Nothing to do.") | 51 | log.info("Slony-I not installed. Nothing to do.") |
65 | 51 | return 0 | 52 | return 0 |
66 | 52 | 53 | ||
67 | 54 | if not repair_with_slonik(log, options, con): | ||
68 | 55 | repair_with_drop_schema(log, con) | ||
69 | 56 | |||
70 | 57 | return 0 | ||
71 | 58 | |||
72 | 59 | |||
73 | 60 | def repair_with_slonik(log, options, con): | ||
74 | 61 | """Attempt to uninstall Slony-I via 'UNINSTALL NODE' per best practice. | ||
75 | 62 | |||
76 | 63 | Returns True on success, False if unable to do so for any reason. | ||
77 | 64 | """ | ||
78 | 53 | cur = con.cursor() | 65 | cur = con.cursor() |
79 | 54 | 66 | ||
80 | 55 | # Determine the node id the database thinks it is. | 67 | # Determine the node id the database thinks it is. |
81 | @@ -60,27 +72,19 @@ | |||
82 | 60 | cur.execute(cmd) | 72 | cur.execute(cmd) |
83 | 61 | node_id = cur.fetchone()[0] | 73 | node_id = cur.fetchone()[0] |
84 | 62 | log.debug("Node Id is %d" % node_id) | 74 | log.debug("Node Id is %d" % node_id) |
85 | 75 | |||
86 | 76 | # Get a list of set ids in the database. | ||
87 | 77 | cur.execute( | ||
88 | 78 | "SELECT DISTINCT set_id FROM %s.sl_set" | ||
89 | 79 | % replication.helpers.CLUSTER_NAMESPACE) | ||
90 | 80 | set_ids = set(row[0] for row in cur.fetchall()) | ||
91 | 81 | log.debug("Set Ids are %s" % repr(set_ids)) | ||
92 | 82 | |||
93 | 63 | except psycopg2.InternalError: | 83 | except psycopg2.InternalError: |
94 | 64 | # Not enough information to determine node id. Possibly | 84 | # Not enough information to determine node id. Possibly |
114 | 65 | # this is an empty database. Just drop the _sl schema as | 85 | # this is an empty database. |
115 | 66 | # it is 'good enough' with Slony-I 1.2 - this mechanism | 86 | log.debug('Broken or no Slony-I install.') |
116 | 67 | # fails with Slony added primary keys, but we don't do that. | 87 | return False |
98 | 68 | con.rollback() | ||
99 | 69 | cur = con.cursor() | ||
100 | 70 | cur.execute("DROP SCHEMA _sl CASCADE") | ||
101 | 71 | con.commit() | ||
102 | 72 | return 0 | ||
103 | 73 | |||
104 | 74 | # Get a list of set ids in the database. | ||
105 | 75 | cur.execute( | ||
106 | 76 | "SELECT DISTINCT set_id FROM %s.sl_set" | ||
107 | 77 | % replication.helpers.CLUSTER_NAMESPACE) | ||
108 | 78 | set_ids = set(row[0] for row in cur.fetchall()) | ||
109 | 79 | log.debug("Set Ids are %s" % repr(set_ids)) | ||
110 | 80 | |||
111 | 81 | # Close so we don't block slonik(1) | ||
112 | 82 | del cur | ||
113 | 83 | con.close() | ||
117 | 84 | 88 | ||
118 | 85 | connection_string = ConnectionString(config.database.main_master) | 89 | connection_string = ConnectionString(config.database.main_master) |
119 | 86 | if options.dbname: | 90 | if options.dbname: |
120 | @@ -103,7 +107,22 @@ | |||
121 | 103 | log.debug(line) | 107 | log.debug(line) |
122 | 104 | script = '\n'.join(script) | 108 | script = '\n'.join(script) |
123 | 105 | 109 | ||
125 | 106 | replication.helpers.execute_slonik(script, auto_preamble=False) | 110 | return replication.helpers.execute_slonik( |
126 | 111 | script, auto_preamble=False, exit_on_fail=False) | ||
127 | 112 | |||
128 | 113 | |||
129 | 114 | def repair_with_drop_schema(log, con): | ||
130 | 115 | """ | ||
131 | 116 | Just drop the _sl schema as it is 'good enough' with Slony-I 1.2. | ||
132 | 117 | |||
133 | 118 | This mechanism fails with Slony added primary keys, but we don't | ||
134 | 119 | do that. | ||
135 | 120 | """ | ||
136 | 121 | log.info('Fallback mode - dropping _sl schema.') | ||
137 | 122 | cur = con.cursor() | ||
138 | 123 | cur.execute("DROP SCHEMA _sl CASCADE") | ||
139 | 124 | return True | ||
140 | 125 | |||
141 | 107 | 126 | ||
142 | 108 | if __name__ == '__main__': | 127 | if __name__ == '__main__': |
143 | 109 | sys.exit(main()) | 128 | sys.exit(main()) |
144 | 110 | 129 | ||
145 | === modified file 'lib/canonical/launchpad/icing/style.css' | |||
146 | --- lib/canonical/launchpad/icing/style.css 2009-11-10 22:09:08 +0000 | |||
147 | +++ lib/canonical/launchpad/icing/style.css 2009-11-18 15:05:38 +0000 | |||
148 | @@ -1753,10 +1753,13 @@ | |||
149 | 1753 | text-align: right; | 1753 | text-align: right; |
150 | 1754 | } | 1754 | } |
151 | 1755 | .codereviewcomment {font: 120% monospace;} | 1755 | .codereviewcomment {font: 120% monospace;} |
153 | 1756 | .codereviewcomment .attachment, #review-diff .attachment { | 1756 | .codereviewcomment .attachment, #review-diff .attachment, |
154 | 1757 | div.diff .attachmentBody { | ||
155 | 1757 | background-color: #f6f6f6; | 1758 | background-color: #f6f6f6; |
156 | 1758 | } | 1759 | } |
158 | 1759 | 1760 | div.diff .horizontal { | |
159 | 1761 | margin: 0; | ||
160 | 1762 | } | ||
161 | 1760 | body.tab-branches table.listing .section-heading { | 1763 | body.tab-branches table.listing .section-heading { |
162 | 1761 | color: #d18b39; | 1764 | color: #d18b39; |
163 | 1762 | } | 1765 | } |
164 | @@ -1765,12 +1768,16 @@ | |||
165 | 1765 | color: #666666; | 1768 | color: #666666; |
166 | 1766 | } | 1769 | } |
167 | 1767 | 1770 | ||
168 | 1771 | .yui-diff-overlay table.diff { | ||
169 | 1772 | font-size: 98%; | ||
170 | 1773 | } | ||
171 | 1768 | #proposal-summary td, #proposal-summary th { padding-bottom: 2px; } | 1774 | #proposal-summary td, #proposal-summary th { padding-bottom: 2px; } |
172 | 1769 | table.diff { | 1775 | table.diff { |
173 | 1770 | white-space: pre-wrap; | 1776 | white-space: pre-wrap; |
174 | 1771 | font: 120% monospace; | 1777 | font: 120% monospace; |
175 | 1772 | width: 100%; | 1778 | width: 100%; |
176 | 1773 | background-color: white; | 1779 | background-color: white; |
177 | 1780 | margin-bottom: 0.5em; | ||
178 | 1774 | } | 1781 | } |
179 | 1775 | table.diff .text { width: 99%; padding-left: 0.5em; } | 1782 | table.diff .text { width: 99%; padding-left: 0.5em; } |
180 | 1776 | table.diff .line-no { | 1783 | table.diff .line-no { |
181 | 1777 | 1784 | ||
182 | === modified file 'lib/canonical/launchpad/javascript/code/branchlinks.js' | |||
183 | --- lib/canonical/launchpad/javascript/code/branchlinks.js 2009-08-13 13:16:04 +0000 | |||
184 | +++ lib/canonical/launchpad/javascript/code/branchlinks.js 2009-11-18 15:05:39 +0000 | |||
185 | @@ -1,4 +1,5 @@ | |||
187 | 1 | /** Copyright (c) 2009, Canonical Ltd. All rights reserved. | 1 | /* Copyright 2009 Canonical Ltd. This software is licensed under the |
188 | 2 | * GNU Affero General Public License version 3 (see the file LICENSE). | ||
189 | 2 | * | 3 | * |
190 | 3 | * Code for handling links to branches from bugs and specs. | 4 | * Code for handling links to branches from bugs and specs. |
191 | 4 | * | 5 | * |
192 | @@ -24,11 +25,11 @@ | |||
193 | 24 | error_handler = new LP.client.ErrorHandler(); | 25 | error_handler = new LP.client.ErrorHandler(); |
194 | 25 | error_handler.clearProgressUI = function() { | 26 | error_handler.clearProgressUI = function() { |
195 | 26 | destroy_temporary_spinner(); | 27 | destroy_temporary_spinner(); |
197 | 27 | } | 28 | }; |
198 | 28 | error_handler.showError = function(error_message) { | 29 | error_handler.showError = function(error_message) { |
199 | 29 | alert('An unexpected error has occurred.'); | 30 | alert('An unexpected error has occurred.'); |
200 | 30 | Y.log(error_message); | 31 | Y.log(error_message); |
202 | 31 | } | 32 | }; |
203 | 32 | 33 | ||
204 | 33 | link_bug_overlay = new Y.lazr.FormOverlay({ | 34 | link_bug_overlay = new Y.lazr.FormOverlay({ |
205 | 34 | headerContent: '<h2>Link to a bug</h2>', | 35 | headerContent: '<h2>Link to a bug</h2>', |
206 | 35 | 36 | ||
207 | === modified file 'lib/canonical/launchpad/javascript/code/branchstatus.js' | |||
208 | --- lib/canonical/launchpad/javascript/code/branchstatus.js 2009-11-10 05:12:59 +0000 | |||
209 | +++ lib/canonical/launchpad/javascript/code/branchstatus.js 2009-11-18 15:05:38 +0000 | |||
210 | @@ -1,4 +1,5 @@ | |||
212 | 1 | /** Copyright (c) 2009, Canonical Ltd. All rights reserved. | 1 | /* Copyright 2009 Canonical Ltd. This software is licensed under the |
213 | 2 | * GNU Affero General Public License version 3 (see the file LICENSE). | ||
214 | 2 | * | 3 | * |
215 | 3 | * Code for handling the update of the branch status. | 4 | * Code for handling the update of the branch status. |
216 | 4 | * | 5 | * |
217 | 5 | 6 | ||
218 | === modified file 'lib/canonical/launchpad/javascript/code/branchsubscription.js' | |||
219 | --- lib/canonical/launchpad/javascript/code/branchsubscription.js 2009-08-13 13:47:24 +0000 | |||
220 | +++ lib/canonical/launchpad/javascript/code/branchsubscription.js 2009-11-18 15:05:39 +0000 | |||
221 | @@ -1,4 +1,5 @@ | |||
223 | 1 | /** Copyright (c) 2009, Canonical Ltd. All rights reserved. | 1 | /* Copyright 2009 Canonical Ltd. This software is licensed under the |
224 | 2 | * GNU Affero General Public License version 3 (see the file LICENSE). | ||
225 | 2 | * | 3 | * |
226 | 3 | * Subscription handling for branches. | 4 | * Subscription handling for branches. |
227 | 4 | * | 5 | * |
228 | 5 | 6 | ||
229 | === modified file 'lib/canonical/launchpad/javascript/code/codereview.js' | |||
230 | --- lib/canonical/launchpad/javascript/code/codereview.js 2009-10-28 16:49:59 +0000 | |||
231 | +++ lib/canonical/launchpad/javascript/code/codereview.js 2009-11-18 15:05:39 +0000 | |||
232 | @@ -1,4 +1,5 @@ | |||
234 | 1 | /** Copyright (c) 2009, Canonical Ltd. All rights reserved. | 1 | /* Copyright 2009 Canonical Ltd. This software is licensed under the |
235 | 2 | * GNU Affero General Public License version 3 (see the file LICENSE). | ||
236 | 2 | * | 3 | * |
237 | 3 | * Library for code review javascript. | 4 | * Library for code review javascript. |
238 | 4 | * | 5 | * |
239 | 5 | 6 | ||
240 | === added file 'lib/canonical/launchpad/javascript/code/popupdiff.js' | |||
241 | --- lib/canonical/launchpad/javascript/code/popupdiff.js 1970-01-01 00:00:00 +0000 | |||
242 | +++ lib/canonical/launchpad/javascript/code/popupdiff.js 2009-11-18 15:05:38 +0000 | |||
243 | @@ -0,0 +1,123 @@ | |||
244 | 1 | /* Copyright 2009 Canonical Ltd. This software is licensed under the | ||
245 | 2 | * GNU Affero General Public License version 3 (see the file LICENSE). | ||
246 | 3 | * | ||
247 | 4 | * Code for handling the popup diffs in the pretty overlays. | ||
248 | 5 | * | ||
249 | 6 | * @module popupdiff | ||
250 | 7 | * @requires node | ||
251 | 8 | */ | ||
252 | 9 | |||
253 | 10 | YUI.add('code.branchmergeproposal.popupdiff', function(Y) { | ||
254 | 11 | |||
255 | 12 | // The launchpad js client used. | ||
256 | 13 | var lp_client; | ||
257 | 14 | |||
258 | 15 | /* | ||
259 | 16 | * The DiffOverlay object inherits from the lazr-js PerttyOverlay. | ||
260 | 17 | * | ||
261 | 18 | * By sub-classing the DiffOverlay gets its own CSS class that is applied to | ||
262 | 19 | * the various HTML objeccts that are created. This allows styling of the | ||
263 | 20 | * overlay in a different way to any other PrettyOverlays. | ||
264 | 21 | */ | ||
265 | 22 | var DiffOverlay = function() { | ||
266 | 23 | DiffOverlay.superclass.constructor.apply(this, arguments); | ||
267 | 24 | }; | ||
268 | 25 | |||
269 | 26 | Y.extend(DiffOverlay, Y.lazr.PrettyOverlay, { | ||
270 | 27 | bindUI: function() { | ||
271 | 28 | // call PrettyOverlay's bindUI | ||
272 | 29 | this.constructor.superclass.bindUI.call(this); | ||
273 | 30 | } | ||
274 | 31 | }); | ||
275 | 32 | |||
276 | 33 | // The NAME gets appended to 'yui-' to give the class name 'yui-diff-overlay'. | ||
277 | 34 | DiffOverlay.NAME = 'diff-overlay'; | ||
278 | 35 | |||
279 | 36 | // A local page cache of the diff overlays that have been rendered. | ||
280 | 37 | // This makes subsequent views of an already loaded diff instantaneous. | ||
281 | 38 | var rendered_overlays = {}; | ||
282 | 39 | |||
283 | 40 | /* | ||
284 | 41 | * Display the diff for the specified api_url. | ||
285 | 42 | * | ||
286 | 43 | * If the specified api_url has already been rendered in an overlay, show it | ||
287 | 44 | * again. If it hasn't been loaded, show the spinner, and load the diff using | ||
288 | 45 | * the LP API. | ||
289 | 46 | * | ||
290 | 47 | * If the diff fails to load, the user is taken to the librarian url just as | ||
291 | 48 | * if Javascript was not enabled. | ||
292 | 49 | */ | ||
293 | 50 | function display_diff(node, api_url, librarian_url) { | ||
294 | 51 | |||
295 | 52 | // Look to see if we have rendered one already. | ||
296 | 53 | if (rendered_overlays[api_url] !== undefined) { | ||
297 | 54 | rendered_overlays[api_url].show(); | ||
298 | 55 | return; | ||
299 | 56 | } | ||
300 | 57 | |||
301 | 58 | // Show a spinner. | ||
302 | 59 | var html = [ | ||
303 | 60 | '<img src="/@@/spinner" alt="loading..." ', | ||
304 | 61 | ' style="padding-left: 0.5em"/>'].join(''); | ||
305 | 62 | var spinner = Y.Node.create(html); | ||
306 | 63 | node.appendChild(spinner); | ||
307 | 64 | |||
308 | 65 | // Load the diff. | ||
309 | 66 | var config = { | ||
310 | 67 | on: { | ||
311 | 68 | success: function(formatted_diff) { | ||
312 | 69 | node.removeChild(spinner); | ||
313 | 70 | var diff_overlay = new DiffOverlay({ | ||
314 | 71 | bodyContent: Y.Node.create(formatted_diff), | ||
315 | 72 | align: { | ||
316 | 73 | points: [Y.WidgetPositionExt.CC, | ||
317 | 74 | Y.WidgetPositionExt.CC] | ||
318 | 75 | }, | ||
319 | 76 | progressbar: false | ||
320 | 77 | }); | ||
321 | 78 | rendered_overlays[api_url] = diff_overlay; | ||
322 | 79 | diff_overlay.render(); | ||
323 | 80 | }, | ||
324 | 81 | failure: function() { | ||
325 | 82 | node.removeChild(spinner); | ||
326 | 83 | // Fail over to loading the librarian link. | ||
327 | 84 | document.location = librarian_url; | ||
328 | 85 | } | ||
329 | 86 | }, | ||
330 | 87 | accept: LP.client.XHTML | ||
331 | 88 | }; | ||
332 | 89 | lp_client.get(api_url, config); | ||
333 | 90 | } | ||
334 | 91 | |||
335 | 92 | |||
336 | 93 | // Grab the namespace in order to be able to expose the connect method. | ||
337 | 94 | var popupdiff = Y.namespace('code.branchmergeproposal.popupdiff'); | ||
338 | 95 | |||
339 | 96 | /* | ||
340 | 97 | * Connect the diff links to their pretty overlay function. | ||
341 | 98 | */ | ||
342 | 99 | popupdiff.connect_diff_links = function() { | ||
343 | 100 | Y.log('connect_diff_links called'); | ||
344 | 101 | // IE doesn't like pretty overlays. | ||
345 | 102 | if (Y.UA.ie) { | ||
346 | 103 | return; | ||
347 | 104 | } | ||
348 | 105 | |||
349 | 106 | // Setup the LP client. | ||
350 | 107 | lp_client = new LP.client.Launchpad(); | ||
351 | 108 | |||
352 | 109 | // var status_content = Y.get('#branch-details-status-value'); | ||
353 | 110 | var nl = Y.all('.popup-diff'); | ||
354 | 111 | nl.each(function(node, index, nodelist){ | ||
355 | 112 | var a = node.query('a'); | ||
356 | 113 | a.addClass('js-action'); | ||
357 | 114 | var librarian_url = a.getAttribute('href'); | ||
358 | 115 | var api_url = node.query('a.api-ref').getAttribute('href'); | ||
359 | 116 | a.on('click', function(e) { | ||
360 | 117 | e.preventDefault(); | ||
361 | 118 | display_diff(a, api_url, librarian_url); | ||
362 | 119 | }); | ||
363 | 120 | }); | ||
364 | 121 | }; | ||
365 | 122 | |||
366 | 123 | }, '0.1', {requires: ['io', 'node', 'lazr.overlay', 'lp.client']}); | ||
367 | 0 | 124 | ||
368 | === modified file 'lib/canonical/launchpad/webapp/configure.zcml' | |||
369 | --- lib/canonical/launchpad/webapp/configure.zcml 2009-10-20 22:13:21 +0000 | |||
370 | +++ lib/canonical/launchpad/webapp/configure.zcml 2009-11-18 15:05:39 +0000 | |||
371 | @@ -419,13 +419,6 @@ | |||
372 | 419 | /> | 419 | /> |
373 | 420 | 420 | ||
374 | 421 | <adapter | 421 | <adapter |
375 | 422 | for="lp.code.interfaces.diff.IPreviewDiff" | ||
376 | 423 | provides="zope.traversing.interfaces.IPathAdapter" | ||
377 | 424 | factory="canonical.launchpad.webapp.tales.PreviewDiffFormatterAPI" | ||
378 | 425 | name="fmt" | ||
379 | 426 | /> | ||
380 | 427 | |||
381 | 428 | <adapter | ||
382 | 429 | for="lp.code.interfaces.codeimport.ICodeImport" | 422 | for="lp.code.interfaces.codeimport.ICodeImport" |
383 | 430 | provides="zope.traversing.interfaces.IPathAdapter" | 423 | provides="zope.traversing.interfaces.IPathAdapter" |
384 | 431 | factory="canonical.launchpad.webapp.tales.CodeImportFormatterAPI" | 424 | factory="canonical.launchpad.webapp.tales.CodeImportFormatterAPI" |
385 | 432 | 425 | ||
386 | === modified file 'lib/canonical/launchpad/webapp/tales.py' | |||
387 | --- lib/canonical/launchpad/webapp/tales.py 2009-11-14 22:40:15 +0000 | |||
388 | +++ lib/canonical/launchpad/webapp/tales.py 2009-11-18 15:05:39 +0000 | |||
389 | @@ -1407,68 +1407,6 @@ | |||
390 | 1407 | '%(name)s</a>: %(title)s' % self._args(view_name)) | 1407 | '%(name)s</a>: %(title)s' % self._args(view_name)) |
391 | 1408 | 1408 | ||
392 | 1409 | 1409 | ||
393 | 1410 | class PreviewDiffFormatterAPI(ObjectFormatterAPI): | ||
394 | 1411 | """Formatter for preview diffs.""" | ||
395 | 1412 | |||
396 | 1413 | def url(self, view_name=None, rootsite=None): | ||
397 | 1414 | """Use the url of the librarian file containing the diff. | ||
398 | 1415 | """ | ||
399 | 1416 | librarian_alias = self._context.diff_text | ||
400 | 1417 | if librarian_alias is None: | ||
401 | 1418 | return None | ||
402 | 1419 | else: | ||
403 | 1420 | return librarian_alias.getURL() | ||
404 | 1421 | |||
405 | 1422 | def link(self, view_name): | ||
406 | 1423 | """The link to the diff should show the line count. | ||
407 | 1424 | |||
408 | 1425 | Stale diffs will have a stale-diff css class. | ||
409 | 1426 | Diffs with conflicts will have a conflict-diff css class. | ||
410 | 1427 | Diffs with neither will have clean-diff css class. | ||
411 | 1428 | |||
412 | 1429 | The title of the diff will show the number of lines added or removed | ||
413 | 1430 | if available. | ||
414 | 1431 | |||
415 | 1432 | :param view_name: If not None, the link will point to the page with | ||
416 | 1433 | that name on this object. | ||
417 | 1434 | """ | ||
418 | 1435 | title_words = [] | ||
419 | 1436 | if self._context.conflicts is not None: | ||
420 | 1437 | style = 'conflicts-diff' | ||
421 | 1438 | title_words.append(_('CONFLICTS')) | ||
422 | 1439 | else: | ||
423 | 1440 | style = 'clean-diff' | ||
424 | 1441 | # Stale style overrides conflicts or clean. | ||
425 | 1442 | if self._context.stale: | ||
426 | 1443 | style = 'stale-diff' | ||
427 | 1444 | title_words.append(_('Stale')) | ||
428 | 1445 | |||
429 | 1446 | if self._context.added_lines_count: | ||
430 | 1447 | title_words.append( | ||
431 | 1448 | _("%s added") % self._context.added_lines_count) | ||
432 | 1449 | |||
433 | 1450 | if self._context.removed_lines_count: | ||
434 | 1451 | title_words.append( | ||
435 | 1452 | _("%s removed") % self._context.removed_lines_count) | ||
436 | 1453 | |||
437 | 1454 | args = { | ||
438 | 1455 | 'line_count': _('%s lines') % self._context.diff_lines_count, | ||
439 | 1456 | 'style': style, | ||
440 | 1457 | 'title': ', '.join(title_words), | ||
441 | 1458 | 'url': self.url(view_name), | ||
442 | 1459 | } | ||
443 | 1460 | # Under normal circumstances, there will be an associated file, | ||
444 | 1461 | # however if the diff is empty, then there is no alias to link to. | ||
445 | 1462 | if args['url'] is None: | ||
446 | 1463 | return ( | ||
447 | 1464 | '<span title="%(title)s" class="%(style)s">' | ||
448 | 1465 | '%(line_count)s</span>' % args) | ||
449 | 1466 | else: | ||
450 | 1467 | return ( | ||
451 | 1468 | '<a href="%(url)s" title="%(title)s" class="%(style)s">' | ||
452 | 1469 | '<img src="/@@/download"/> %(line_count)s</a>' % args) | ||
453 | 1470 | |||
454 | 1471 | |||
455 | 1472 | class BranchSubscriptionFormatterAPI(CustomizableFormatter): | 1410 | class BranchSubscriptionFormatterAPI(CustomizableFormatter): |
456 | 1473 | """Adapter for IBranchSubscription objects to a formatted string.""" | 1411 | """Adapter for IBranchSubscription objects to a formatted string.""" |
457 | 1474 | 1412 | ||
458 | 1475 | 1413 | ||
459 | === modified file 'lib/canonical/launchpad/webapp/tests/test_tales.py' | |||
460 | --- lib/canonical/launchpad/webapp/tests/test_tales.py 2009-10-21 17:26:37 +0000 | |||
461 | +++ lib/canonical/launchpad/webapp/tests/test_tales.py 2009-11-18 15:05:39 +0000 | |||
462 | @@ -3,20 +3,15 @@ | |||
463 | 3 | 3 | ||
464 | 4 | """tales.py doctests.""" | 4 | """tales.py doctests.""" |
465 | 5 | 5 | ||
466 | 6 | from difflib import unified_diff | ||
467 | 7 | from textwrap import dedent | 6 | from textwrap import dedent |
468 | 8 | import unittest | 7 | import unittest |
469 | 9 | 8 | ||
470 | 10 | from storm.store import Store | ||
471 | 11 | from zope.security.proxy import removeSecurityProxy | ||
472 | 12 | from zope.testing.doctestunit import DocTestSuite | 9 | from zope.testing.doctestunit import DocTestSuite |
473 | 13 | 10 | ||
474 | 14 | from canonical.launchpad.ftests import test_tales | ||
475 | 15 | from canonical.launchpad.testing.pages import find_tags_by_class | 11 | from canonical.launchpad.testing.pages import find_tags_by_class |
476 | 16 | from canonical.launchpad.webapp.tales import FormattersAPI | 12 | from canonical.launchpad.webapp.tales import FormattersAPI |
480 | 17 | from canonical.testing import ( | 13 | from canonical.testing import DatabaseFunctionalLayer |
481 | 18 | DatabaseFunctionalLayer, LaunchpadFunctionalLayer) | 14 | from lp.testing import TestCase |
479 | 19 | from lp.testing import login, TestCase, TestCaseWithFactory | ||
482 | 20 | 15 | ||
483 | 21 | 16 | ||
484 | 22 | def test_requestapi(): | 17 | def test_requestapi(): |
485 | @@ -268,134 +263,6 @@ | |||
486 | 268 | self.assertEqual(3, line_count) | 263 | self.assertEqual(3, line_count) |
487 | 269 | 264 | ||
488 | 270 | 265 | ||
489 | 271 | class TestPreviewDiffFormatter(TestCaseWithFactory): | ||
490 | 272 | """Test the PreviewDiffFormatterAPI class.""" | ||
491 | 273 | |||
492 | 274 | layer = LaunchpadFunctionalLayer | ||
493 | 275 | |||
494 | 276 | def _createPreviewDiff(self, line_count=0, added=None, removed=None, | ||
495 | 277 | conflicts=None): | ||
496 | 278 | # Login an admin to avoid the launchpad.Edit requirements. | ||
497 | 279 | login('admin@canonical.com') | ||
498 | 280 | # Create a dummy preview diff, and make sure the branches have the | ||
499 | 281 | # correct last scanned ids to ensure that the new diff is not stale. | ||
500 | 282 | bmp = self.factory.makeBranchMergeProposal() | ||
501 | 283 | if line_count: | ||
502 | 284 | content = ''.join(unified_diff('', 'random content')) | ||
503 | 285 | else: | ||
504 | 286 | content = '' | ||
505 | 287 | preview = bmp.updatePreviewDiff( | ||
506 | 288 | content, u'rev-a', u'rev-b', conflicts=conflicts) | ||
507 | 289 | bmp.source_branch.last_scanned_id = preview.source_revision_id | ||
508 | 290 | bmp.target_branch.last_scanned_id = preview.target_revision_id | ||
509 | 291 | # Update the values directly sidestepping the security. | ||
510 | 292 | naked_diff = removeSecurityProxy(preview) | ||
511 | 293 | naked_diff.diff_lines_count = line_count | ||
512 | 294 | naked_diff.added_lines_count = added | ||
513 | 295 | naked_diff.removed_lines_count = removed | ||
514 | 296 | # In order to get the canonical url of the librarian file, we need to | ||
515 | 297 | # commit. | ||
516 | 298 | # transaction.commit() | ||
517 | 299 | # Make sure that the preview diff is in the db for the test. | ||
518 | 300 | # Storm bug: 324724 | ||
519 | 301 | Store.of(bmp).flush() | ||
520 | 302 | return preview | ||
521 | 303 | |||
522 | 304 | def _createStalePreviewDiff(self, line_count=0, added=None, removed=None, | ||
523 | 305 | conflicts=None): | ||
524 | 306 | preview = self._createPreviewDiff( | ||
525 | 307 | line_count, added, removed, conflicts) | ||
526 | 308 | preview.branch_merge_proposal.source_branch.last_scanned_id = 'other' | ||
527 | 309 | return preview | ||
528 | 310 | |||
529 | 311 | def test_creation_method(self): | ||
530 | 312 | # Just confirm that our helpers do what they say. | ||
531 | 313 | preview = self._createPreviewDiff(12, 45, 23) | ||
532 | 314 | self.assertEqual(12, preview.diff_lines_count) | ||
533 | 315 | self.assertEqual(45, preview.added_lines_count) | ||
534 | 316 | self.assertEqual(23, preview.removed_lines_count) | ||
535 | 317 | self.assertEqual(False, preview.stale) | ||
536 | 318 | self.assertEqual(True, self._createStalePreviewDiff().stale) | ||
537 | 319 | |||
538 | 320 | def test_fmt_no_diff(self): | ||
539 | 321 | # If there is no diff, there is no link. | ||
540 | 322 | preview = self._createPreviewDiff(0) | ||
541 | 323 | self.assertEqual( | ||
542 | 324 | '<span title="" class="clean-diff">0 lines</span>', | ||
543 | 325 | test_tales('preview/fmt:link', preview=preview)) | ||
544 | 326 | |||
545 | 327 | def test_fmt_lines_no_add_or_remove(self): | ||
546 | 328 | # If there is no diff, there is no link. | ||
547 | 329 | preview = self._createPreviewDiff(10, 0, 0) | ||
548 | 330 | self.assertEqual( | ||
549 | 331 | '<a href="%s" title="" class="clean-diff">' | ||
550 | 332 | '<img src="/@@/download"/> 10 lines</a>' | ||
551 | 333 | % preview.diff_text.getURL(), | ||
552 | 334 | test_tales('preview/fmt:link', preview=preview)) | ||
553 | 335 | |||
554 | 336 | def test_fmt_lines_some_added_no_removed(self): | ||
555 | 337 | # If there is no diff, there is no link. | ||
556 | 338 | preview = self._createPreviewDiff(10, 4, 0) | ||
557 | 339 | self.assertEqual( | ||
558 | 340 | '<a href="%s" title="4 added" class="clean-diff">' | ||
559 | 341 | '<img src="/@@/download"/> 10 lines</a>' | ||
560 | 342 | % preview.diff_text.getURL(), | ||
561 | 343 | test_tales('preview/fmt:link', preview=preview)) | ||
562 | 344 | |||
563 | 345 | def test_fmt_lines_no_added_some_removed(self): | ||
564 | 346 | # If there is no diff, there is no link. | ||
565 | 347 | preview = self._createPreviewDiff(10, 0, 4) | ||
566 | 348 | self.assertEqual( | ||
567 | 349 | '<a href="%s" title="4 removed" class="clean-diff">' | ||
568 | 350 | '<img src="/@@/download"/> 10 lines</a>' | ||
569 | 351 | % preview.diff_text.getURL(), | ||
570 | 352 | test_tales('preview/fmt:link', preview=preview)) | ||
571 | 353 | |||
572 | 354 | def test_fmt_lines_added_and_removed(self): | ||
573 | 355 | # If there is no diff, there is no link. | ||
574 | 356 | preview = self._createPreviewDiff(10, 6, 4) | ||
575 | 357 | self.assertEqual( | ||
576 | 358 | '<a href="%s" title="6 added, 4 removed" class="clean-diff">' | ||
577 | 359 | '<img src="/@@/download"/> 10 lines</a>' | ||
578 | 360 | % preview.diff_text.getURL(), | ||
579 | 361 | test_tales('preview/fmt:link', preview=preview)) | ||
580 | 362 | |||
581 | 363 | def test_fmt_simple_conflicts(self): | ||
582 | 364 | # If there is no diff, there is no link. | ||
583 | 365 | preview = self._createPreviewDiff(10, 2, 3, u'conflicts') | ||
584 | 366 | self.assertEqual( | ||
585 | 367 | '<a href="%s" title="CONFLICTS, 2 added, 3 removed" ' | ||
586 | 368 | 'class="conflicts-diff">' | ||
587 | 369 | '<img src="/@@/download"/> 10 lines</a>' | ||
588 | 370 | % preview.diff_text.getURL(), | ||
589 | 371 | test_tales('preview/fmt:link', preview=preview)) | ||
590 | 372 | |||
591 | 373 | def test_fmt_stale_empty_diff(self): | ||
592 | 374 | # If there is no diff, there is no link. | ||
593 | 375 | preview = self._createStalePreviewDiff(0) | ||
594 | 376 | self.assertEqual( | ||
595 | 377 | '<span title="Stale" class="stale-diff">0 lines</span>', | ||
596 | 378 | test_tales('preview/fmt:link', preview=preview)) | ||
597 | 379 | |||
598 | 380 | def test_fmt_stale_non_empty_diff(self): | ||
599 | 381 | # If there is no diff, there is no link. | ||
600 | 382 | preview = self._createStalePreviewDiff(500, 89, 340) | ||
601 | 383 | self.assertEqual( | ||
602 | 384 | '<a href="%s" title="Stale, 89 added, 340 removed" ' | ||
603 | 385 | 'class="stale-diff"><img src="/@@/download"/> 500 lines</a>' | ||
604 | 386 | % preview.diff_text.getURL(), | ||
605 | 387 | test_tales('preview/fmt:link', preview=preview)) | ||
606 | 388 | |||
607 | 389 | def test_fmt_stale_non_empty_diff_with_conflicts(self): | ||
608 | 390 | # If there is no diff, there is no link. | ||
609 | 391 | preview = self._createStalePreviewDiff(500, 89, 340, u'conflicts') | ||
610 | 392 | self.assertEqual( | ||
611 | 393 | '<a href="%s" title="CONFLICTS, Stale, 89 added, 340 removed" ' | ||
612 | 394 | 'class="stale-diff"><img src="/@@/download"/> 500 lines</a>' | ||
613 | 395 | % preview.diff_text.getURL(), | ||
614 | 396 | test_tales('preview/fmt:link', preview=preview)) | ||
615 | 397 | |||
616 | 398 | |||
617 | 399 | def test_suite(): | 266 | def test_suite(): |
618 | 400 | """Return this module's doctest Suite. Unit tests are also run.""" | 267 | """Return this module's doctest Suite. Unit tests are also run.""" |
619 | 401 | suite = unittest.TestSuite() | 268 | suite = unittest.TestSuite() |
620 | 402 | 269 | ||
621 | === modified file 'lib/devscripts/ec2test/builtins.py' | |||
622 | --- lib/devscripts/ec2test/builtins.py 2009-10-16 11:07:37 +0000 | |||
623 | +++ lib/devscripts/ec2test/builtins.py 2009-11-18 15:05:39 +0000 | |||
624 | @@ -557,7 +557,7 @@ | |||
625 | 557 | 'bzr pull -d /var/launchpad/download-cache ' | 557 | 'bzr pull -d /var/launchpad/download-cache ' |
626 | 558 | 'lp:lp-source-dependencies') | 558 | 'lp:lp-source-dependencies') |
627 | 559 | if public: | 559 | if public: |
629 | 560 | update_sourcecode_options = '--public-only' | 560 | update_sourcecode_options = ' --public-only' |
630 | 561 | else: | 561 | else: |
631 | 562 | update_sourcecode_options = '' | 562 | update_sourcecode_options = '' |
632 | 563 | user_connection.run_with_ssh_agent( | 563 | user_connection.run_with_ssh_agent( |
633 | 564 | 564 | ||
634 | === modified file 'lib/lp/code/browser/branchlisting.py' | |||
635 | --- lib/lp/code/browser/branchlisting.py 2009-10-26 18:40:04 +0000 | |||
636 | +++ lib/lp/code/browser/branchlisting.py 2009-11-18 15:05:39 +0000 | |||
637 | @@ -91,14 +91,7 @@ | |||
638 | 91 | from lp.registry.interfaces.product import IProduct | 91 | from lp.registry.interfaces.product import IProduct |
639 | 92 | from lp.registry.interfaces.sourcepackage import ISourcePackageFactory | 92 | from lp.registry.interfaces.sourcepackage import ISourcePackageFactory |
640 | 93 | from lp.registry.model.sourcepackage import SourcePackage | 93 | from lp.registry.model.sourcepackage import SourcePackage |
649 | 94 | 94 | from lp.services.browser_helpers import get_plural_text | |
642 | 95 | |||
643 | 96 | def get_plural_text(count, singular, plural): | ||
644 | 97 | """Return 'singular' if 'count' is 1, 'plural' otherwise.""" | ||
645 | 98 | if count == 1: | ||
646 | 99 | return singular | ||
647 | 100 | else: | ||
648 | 101 | return plural | ||
650 | 102 | 95 | ||
651 | 103 | 96 | ||
652 | 104 | class CodeVHostBreadcrumb(Breadcrumb): | 97 | class CodeVHostBreadcrumb(Breadcrumb): |
653 | 105 | 98 | ||
654 | === modified file 'lib/lp/code/browser/branchmergeproposal.py' | |||
655 | --- lib/lp/code/browser/branchmergeproposal.py 2009-11-10 22:57:42 +0000 | |||
656 | +++ lib/lp/code/browser/branchmergeproposal.py 2009-11-18 15:05:39 +0000 | |||
657 | @@ -34,7 +34,7 @@ | |||
658 | 34 | 34 | ||
659 | 35 | import simplejson | 35 | import simplejson |
660 | 36 | from zope.app.form.browser import TextAreaWidget | 36 | from zope.app.form.browser import TextAreaWidget |
662 | 37 | from zope.component import adapter, getUtility | 37 | from zope.component import adapter, adapts, getMultiAdapter, getUtility |
663 | 38 | from zope.event import notify as zope_notify | 38 | from zope.event import notify as zope_notify |
664 | 39 | from zope.formlib import form | 39 | from zope.formlib import form |
665 | 40 | from zope.interface import Interface, implementer, implements | 40 | from zope.interface import Interface, implementer, implements |
666 | @@ -60,7 +60,8 @@ | |||
667 | 60 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb | 60 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb |
668 | 61 | from canonical.launchpad.webapp.interfaces import IPrimaryContext | 61 | from canonical.launchpad.webapp.interfaces import IPrimaryContext |
669 | 62 | from canonical.launchpad.webapp.menu import NavigationMenu | 62 | from canonical.launchpad.webapp.menu import NavigationMenu |
671 | 63 | from canonical.launchpad.webapp.tales import FormattersAPI | 63 | from canonical.launchpad.webapp.tales import ( |
672 | 64 | DateTimeFormatterAPI, FormattersAPI) | ||
673 | 64 | from canonical.widgets.lazrjs import ( | 65 | from canonical.widgets.lazrjs import ( |
674 | 65 | TextAreaEditorWidget, vocabulary_to_choice_edit_items) | 66 | TextAreaEditorWidget, vocabulary_to_choice_edit_items) |
675 | 66 | 67 | ||
676 | @@ -74,6 +75,7 @@ | |||
677 | 74 | from lp.code.interfaces.codereviewcomment import ICodeReviewComment | 75 | from lp.code.interfaces.codereviewcomment import ICodeReviewComment |
678 | 75 | from lp.code.interfaces.codereviewvote import ( | 76 | from lp.code.interfaces.codereviewvote import ( |
679 | 76 | ICodeReviewVoteReference) | 77 | ICodeReviewVoteReference) |
680 | 78 | from lp.code.interfaces.diff import IPreviewDiff | ||
681 | 77 | from lp.registry.interfaces.person import IPersonSet | 79 | from lp.registry.interfaces.person import IPersonSet |
682 | 78 | from lp.services.comments.interfaces.conversation import IConversation | 80 | from lp.services.comments.interfaces.conversation import IConversation |
683 | 79 | 81 | ||
684 | @@ -138,6 +140,23 @@ | |||
685 | 138 | } | 140 | } |
686 | 139 | return friendly_texts[self.context.queue_status] | 141 | return friendly_texts[self.context.queue_status] |
687 | 140 | 142 | ||
688 | 143 | @property | ||
689 | 144 | def status_title(self): | ||
690 | 145 | """The title for the status text. | ||
691 | 146 | |||
692 | 147 | Only set if the status is approved or rejected. | ||
693 | 148 | """ | ||
694 | 149 | result = '' | ||
695 | 150 | if self.context.queue_status in ( | ||
696 | 151 | BranchMergeProposalStatus.CODE_APPROVED, | ||
697 | 152 | BranchMergeProposalStatus.REJECTED | ||
698 | 153 | ): | ||
699 | 154 | formatter = DateTimeFormatterAPI(self.context.date_reviewed) | ||
700 | 155 | result = '%s %s' % ( | ||
701 | 156 | self.context.reviewer.displayname, | ||
702 | 157 | formatter.displaydate()) | ||
703 | 158 | return result | ||
704 | 159 | |||
705 | 141 | 160 | ||
706 | 142 | class BranchMergeProposalMenuMixin: | 161 | class BranchMergeProposalMenuMixin: |
707 | 143 | """Mixin class for merge proposal menus.""" | 162 | """Mixin class for merge proposal menus.""" |
708 | @@ -427,10 +446,44 @@ | |||
709 | 427 | return SimpleVocabulary(terms) | 446 | return SimpleVocabulary(terms) |
710 | 428 | 447 | ||
711 | 429 | 448 | ||
712 | 449 | class DiffRenderingMixin: | ||
713 | 450 | """A mixin class for handling diff text.""" | ||
714 | 451 | |||
715 | 452 | @cachedproperty | ||
716 | 453 | def preview_diff_text(self): | ||
717 | 454 | """Return a (hopefully) intelligently encoded review diff.""" | ||
718 | 455 | preview_diff = self.preview_diff | ||
719 | 456 | if preview_diff is None: | ||
720 | 457 | return None | ||
721 | 458 | try: | ||
722 | 459 | diff = preview_diff.text.decode('utf-8') | ||
723 | 460 | except UnicodeDecodeError: | ||
724 | 461 | diff = preview_diff.text.decode('windows-1252', 'replace') | ||
725 | 462 | # Strip off the trailing carriage returns. | ||
726 | 463 | return diff.rstrip('\n') | ||
727 | 464 | |||
728 | 465 | @cachedproperty | ||
729 | 466 | def diff_oversized(self): | ||
730 | 467 | """Return True if the review_diff is over the configured size limit. | ||
731 | 468 | |||
732 | 469 | The diff can be over the limit in two ways. If the diff is oversized | ||
733 | 470 | in bytes it will be cut off at the Diff.text method. If the number of | ||
734 | 471 | lines is over the max_format_lines, then it is cut off at the fmt:diff | ||
735 | 472 | processing. | ||
736 | 473 | """ | ||
737 | 474 | review_diff = self.preview_diff | ||
738 | 475 | if review_diff is None: | ||
739 | 476 | return False | ||
740 | 477 | if review_diff.oversized: | ||
741 | 478 | return True | ||
742 | 479 | diff_text = self.preview_diff_text | ||
743 | 480 | return diff_text.count('\n') >= config.diff.max_format_lines | ||
744 | 481 | |||
745 | 430 | 482 | ||
746 | 431 | class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin, | 483 | class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin, |
747 | 432 | BranchMergeProposalRevisionIdMixin, | 484 | BranchMergeProposalRevisionIdMixin, |
749 | 433 | BranchMergeProposalStatusMixin): | 485 | BranchMergeProposalStatusMixin, |
750 | 486 | DiffRenderingMixin): | ||
751 | 434 | """A basic view used for the index page.""" | 487 | """A basic view used for the index page.""" |
752 | 435 | 488 | ||
753 | 436 | implements(IBranchMergeProposalActionMenu) | 489 | implements(IBranchMergeProposalActionMenu) |
754 | @@ -496,36 +549,6 @@ | |||
755 | 496 | return self.context.review_diff.diff | 549 | return self.context.review_diff.diff |
756 | 497 | return None | 550 | return None |
757 | 498 | 551 | ||
758 | 499 | @cachedproperty | ||
759 | 500 | def preview_diff_text(self): | ||
760 | 501 | """Return a (hopefully) intelligently encoded review diff.""" | ||
761 | 502 | preview_diff = self.preview_diff | ||
762 | 503 | if preview_diff is None: | ||
763 | 504 | return None | ||
764 | 505 | try: | ||
765 | 506 | diff = preview_diff.text.decode('utf-8') | ||
766 | 507 | except UnicodeDecodeError: | ||
767 | 508 | diff = preview_diff.text.decode('windows-1252', 'replace') | ||
768 | 509 | # Strip off the trailing carriage returns. | ||
769 | 510 | return diff.rstrip('\n') | ||
770 | 511 | |||
771 | 512 | @cachedproperty | ||
772 | 513 | def review_diff_oversized(self): | ||
773 | 514 | """Return True if the review_diff is over the configured size limit. | ||
774 | 515 | |||
775 | 516 | The diff can be over the limit in two ways. If the diff is oversized | ||
776 | 517 | in bytes it will be cut off at the Diff.text method. If the number of | ||
777 | 518 | lines is over the max_format_lines, then it is cut off at the fmt:diff | ||
778 | 519 | processing. | ||
779 | 520 | """ | ||
780 | 521 | review_diff = self.preview_diff | ||
781 | 522 | if review_diff is None: | ||
782 | 523 | return False | ||
783 | 524 | if review_diff.oversized: | ||
784 | 525 | return True | ||
785 | 526 | diff_text = self.preview_diff_text | ||
786 | 527 | return diff_text.count('\n') >= config.diff.max_format_lines | ||
787 | 528 | |||
788 | 529 | @property | 552 | @property |
789 | 530 | def has_bug_or_spec(self): | 553 | def has_bug_or_spec(self): |
790 | 531 | """Return whether or not the merge proposal has a linked bug or spec. | 554 | """Return whether or not the merge proposal has a linked bug or spec. |
791 | @@ -1290,3 +1313,28 @@ | |||
792 | 1290 | html = formatter(nomail).text_to_html() | 1313 | html = formatter(nomail).text_to_html() |
793 | 1291 | return html.encode('utf-8') | 1314 | return html.encode('utf-8') |
794 | 1292 | return renderer | 1315 | return renderer |
795 | 1316 | |||
796 | 1317 | |||
797 | 1318 | class FormatPreviewDiffView(LaunchpadView, DiffRenderingMixin): | ||
798 | 1319 | """A simple view to render a diff formatted nicely.""" | ||
799 | 1320 | |||
800 | 1321 | __used_for__ = IPreviewDiff | ||
801 | 1322 | |||
802 | 1323 | @property | ||
803 | 1324 | def preview_diff(self): | ||
804 | 1325 | return self.context | ||
805 | 1326 | |||
806 | 1327 | |||
807 | 1328 | class PreviewDiffHTMLRepresentation: | ||
808 | 1329 | adapts(IPreviewDiff, IWebServiceClientRequest) | ||
809 | 1330 | implements(Interface) | ||
810 | 1331 | |||
811 | 1332 | def __init__(self, diff, request): | ||
812 | 1333 | self.diff = diff | ||
813 | 1334 | self.request = request | ||
814 | 1335 | |||
815 | 1336 | def __call__(self): | ||
816 | 1337 | """Render `BugBranch` as XHTML using the webservice.""" | ||
817 | 1338 | diff_view = getMultiAdapter( | ||
818 | 1339 | (self.diff, self.request), name="+diff") | ||
819 | 1340 | return diff_view() | ||
820 | 1293 | 1341 | ||
821 | === modified file 'lib/lp/code/browser/configure.zcml' | |||
822 | --- lib/lp/code/browser/configure.zcml 2009-11-13 20:12:17 +0000 | |||
823 | +++ lib/lp/code/browser/configure.zcml 2009-11-18 15:05:38 +0000 | |||
824 | @@ -181,16 +181,24 @@ | |||
825 | 181 | name="+index" | 181 | name="+index" |
826 | 182 | template="../templates/branchmergeproposal-index.pt"/> | 182 | template="../templates/branchmergeproposal-index.pt"/> |
827 | 183 | <browser:page | 183 | <browser:page |
828 | 184 | name="++diff" | ||
829 | 185 | template="../templates/branchmergeproposal-diff.pt"/> | ||
830 | 186 | <browser:page | ||
831 | 184 | name="+pagelet-summary" | 187 | name="+pagelet-summary" |
832 | 185 | template="../templates/branchmergeproposal-pagelet-summary.pt"/> | 188 | template="../templates/branchmergeproposal-pagelet-summary.pt"/> |
833 | 186 | </browser:pages> | 189 | </browser:pages> |
836 | 187 | <browser:page | 190 | <browser:pages |
835 | 188 | name="+votes" | ||
837 | 189 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" | 191 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" |
838 | 190 | class="lp.code.browser.branchmergeproposal.BranchMergeProposalVoteView" | 192 | class="lp.code.browser.branchmergeproposal.BranchMergeProposalVoteView" |
839 | 191 | facet="branches" | 193 | facet="branches" |
842 | 192 | permission="launchpad.View" | 194 | permission="launchpad.View"> |
843 | 193 | template="../templates/branchmergeproposal-votes.pt"/> | 195 | <browser:page |
844 | 196 | name="+votes" | ||
845 | 197 | template="../templates/branchmergeproposal-votes.pt"/> | ||
846 | 198 | <browser:page | ||
847 | 199 | name="+vote-summary" | ||
848 | 200 | template="../templates/branchmergeproposal-vote-summary.pt"/> | ||
849 | 201 | </browser:pages> | ||
850 | 194 | <browser:pages | 202 | <browser:pages |
851 | 195 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" | 203 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" |
852 | 196 | class="lp.code.browser.branchmergeproposal.BranchMergeProposalEditView" | 204 | class="lp.code.browser.branchmergeproposal.BranchMergeProposalEditView" |
853 | @@ -276,7 +284,7 @@ | |||
854 | 276 | class="lp.code.browser.branchmergeproposal.BranchMergeCandidateView" | 284 | class="lp.code.browser.branchmergeproposal.BranchMergeCandidateView" |
855 | 277 | facet="branches" | 285 | facet="branches" |
856 | 278 | permission="zope.Public" | 286 | permission="zope.Public" |
858 | 279 | template="../templates/branch-merge-link-summary.pt"/> | 287 | template="../templates/branchmergeproposal-link-summary.pt"/> |
859 | 280 | <browser:page | 288 | <browser:page |
860 | 281 | name="+pagelet-subscribers" | 289 | name="+pagelet-subscribers" |
861 | 282 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" | 290 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" |
862 | @@ -898,6 +906,13 @@ | |||
863 | 898 | attribute_to_parent="branch_merge_proposal" | 906 | attribute_to_parent="branch_merge_proposal" |
864 | 899 | rootsite="code" /> | 907 | rootsite="code" /> |
865 | 900 | 908 | ||
866 | 909 | <browser:page | ||
867 | 910 | name="+diff" | ||
868 | 911 | for="lp.code.interfaces.diff.IPreviewDiff" | ||
869 | 912 | class="lp.code.browser.branchmergeproposal.FormatPreviewDiffView" | ||
870 | 913 | permission="launchpad.AnyPerson" | ||
871 | 914 | template="../templates/branchmergeproposal-diff.pt"/> | ||
872 | 915 | |||
873 | 901 | <browser:menus | 916 | <browser:menus |
874 | 902 | module="lp.code.browser.branchlisting" | 917 | module="lp.code.browser.branchlisting" |
875 | 903 | classes="PersonProductBranchesMenu"/> | 918 | classes="PersonProductBranchesMenu"/> |
876 | @@ -991,5 +1006,15 @@ | |||
877 | 991 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" | 1006 | for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal" |
878 | 992 | factory="lp.code.browser.branchmergeproposal.BranchMergeProposalBreadcrumb" | 1007 | factory="lp.code.browser.branchmergeproposal.BranchMergeProposalBreadcrumb" |
879 | 993 | permission="zope.Public"/> | 1008 | permission="zope.Public"/> |
880 | 1009 | <adapter | ||
881 | 1010 | factory="lp.code.browser.branchmergeproposal.PreviewDiffHTMLRepresentation" | ||
882 | 1011 | name="lazr.restful.EntryResource"/> | ||
883 | 1012 | |||
884 | 1013 | <adapter | ||
885 | 1014 | for="lp.code.interfaces.diff.IPreviewDiff" | ||
886 | 1015 | provides="zope.traversing.interfaces.IPathAdapter" | ||
887 | 1016 | factory="lp.code.browser.diff.PreviewDiffFormatterAPI" | ||
888 | 1017 | name="fmt" | ||
889 | 1018 | /> | ||
890 | 994 | 1019 | ||
891 | 995 | </configure> | 1020 | </configure> |
892 | 996 | 1021 | ||
893 | === added file 'lib/lp/code/browser/diff.py' | |||
894 | --- lib/lp/code/browser/diff.py 1970-01-01 00:00:00 +0000 | |||
895 | +++ lib/lp/code/browser/diff.py 2009-11-18 15:05:38 +0000 | |||
896 | @@ -0,0 +1,77 @@ | |||
897 | 1 | # Copyright 2009 Canonical Ltd. This software is licensed under the | ||
898 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
899 | 3 | |||
900 | 4 | """Display classes relating to diff objects of one sort or another.""" | ||
901 | 5 | |||
902 | 6 | __metaclass__ = type | ||
903 | 7 | __all__ = [ | ||
904 | 8 | 'PreviewDiffFormatterAPI', | ||
905 | 9 | ] | ||
906 | 10 | |||
907 | 11 | |||
908 | 12 | from canonical.launchpad import _ | ||
909 | 13 | from canonical.launchpad.webapp.tales import ObjectFormatterAPI | ||
910 | 14 | from lp.services.browser_helpers import get_plural_text | ||
911 | 15 | |||
912 | 16 | |||
913 | 17 | class PreviewDiffFormatterAPI(ObjectFormatterAPI): | ||
914 | 18 | """Formatter for preview diffs.""" | ||
915 | 19 | |||
916 | 20 | def url(self, view_name=None, rootsite=None): | ||
917 | 21 | """Use the url of the librarian file containing the diff. | ||
918 | 22 | """ | ||
919 | 23 | librarian_alias = self._context.diff_text | ||
920 | 24 | if librarian_alias is None: | ||
921 | 25 | return None | ||
922 | 26 | else: | ||
923 | 27 | return librarian_alias.getURL() | ||
924 | 28 | |||
925 | 29 | def link(self, view_name): | ||
926 | 30 | """The link to the diff should show the line count. | ||
927 | 31 | |||
928 | 32 | Stale diffs will have a stale-diff css class. | ||
929 | 33 | Diffs with conflicts will have a conflict-diff css class. | ||
930 | 34 | Diffs with neither will have clean-diff css class. | ||
931 | 35 | |||
932 | 36 | The title of the diff will show the number of lines added or removed | ||
933 | 37 | if available. | ||
934 | 38 | |||
935 | 39 | :param view_name: If not None, the link will point to the page with | ||
936 | 40 | that name on this object. | ||
937 | 41 | """ | ||
938 | 42 | diff = self._context | ||
939 | 43 | conflict_text = '' | ||
940 | 44 | if diff.conflicts is not None: | ||
941 | 45 | conflict_text = _(' (has conflicts)') | ||
942 | 46 | |||
943 | 47 | count_text = '' | ||
944 | 48 | added = diff.added_lines_count | ||
945 | 49 | removed = diff.removed_lines_count | ||
946 | 50 | if (added is not None and removed is not None): | ||
947 | 51 | count_text = ' (+%d/-%d)' % (added, removed) | ||
948 | 52 | |||
949 | 53 | file_text = '' | ||
950 | 54 | diffstat = diff.diffstat | ||
951 | 55 | if diffstat is not None: | ||
952 | 56 | file_count = len(diffstat) | ||
953 | 57 | file_text = get_plural_text( | ||
954 | 58 | file_count, _(' %d file modified'), _(' %d files modified')) | ||
955 | 59 | file_text = file_text % file_count | ||
956 | 60 | |||
957 | 61 | args = { | ||
958 | 62 | 'line_count': _('%s lines') % diff.diff_lines_count, | ||
959 | 63 | 'file_text': file_text, | ||
960 | 64 | 'conflict_text': conflict_text, | ||
961 | 65 | 'count_text': count_text, | ||
962 | 66 | 'url': self.url(view_name), | ||
963 | 67 | } | ||
964 | 68 | # Under normal circumstances, there will be an associated file, | ||
965 | 69 | # however if the diff is empty, then there is no alias to link to. | ||
966 | 70 | if args['url'] is None: | ||
967 | 71 | return ( | ||
968 | 72 | '<span class="empty-diff">' | ||
969 | 73 | '%(line_count)s</span>' % args) | ||
970 | 74 | else: | ||
971 | 75 | return ( | ||
972 | 76 | '<a href="%(url)s" class="diff-link">' | ||
973 | 77 | '%(line_count)s%(count_text)s%(file_text)s%(conflict_text)s</a>' % args) | ||
974 | 0 | 78 | ||
975 | === modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py' | |||
976 | --- lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-01 23:13:09 +0000 | |||
977 | +++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-18 15:05:38 +0000 | |||
978 | @@ -7,10 +7,11 @@ | |||
979 | 7 | 7 | ||
980 | 8 | __metaclass__ = type | 8 | __metaclass__ = type |
981 | 9 | 9 | ||
983 | 10 | from datetime import timedelta | 10 | from datetime import datetime, timedelta |
984 | 11 | from difflib import unified_diff | 11 | from difflib import unified_diff |
985 | 12 | import unittest | 12 | import unittest |
986 | 13 | 13 | ||
987 | 14 | import pytz | ||
988 | 14 | import transaction | 15 | import transaction |
989 | 15 | from zope.component import getMultiAdapter | 16 | from zope.component import getMultiAdapter |
990 | 16 | from zope.security.interfaces import Unauthorized | 17 | from zope.security.interfaces import Unauthorized |
991 | @@ -66,6 +67,7 @@ | |||
992 | 66 | link = menu.add_comment() | 67 | link = menu.add_comment() |
993 | 67 | self.assertTrue(menu.add_comment().enabled) | 68 | self.assertTrue(menu.add_comment().enabled) |
994 | 68 | 69 | ||
995 | 70 | |||
996 | 69 | class TestDecoratedCodeReviewVoteReference(TestCaseWithFactory): | 71 | class TestDecoratedCodeReviewVoteReference(TestCaseWithFactory): |
997 | 70 | 72 | ||
998 | 71 | layer = DatabaseFunctionalLayer | 73 | layer = DatabaseFunctionalLayer |
999 | @@ -459,13 +461,6 @@ | |||
1000 | 459 | self.bmp = self.factory.makeBranchMergeProposal(registrant=self.user) | 461 | self.bmp = self.factory.makeBranchMergeProposal(registrant=self.user) |
1001 | 460 | login_person(self.user) | 462 | login_person(self.user) |
1002 | 461 | 463 | ||
1003 | 462 | def _createView(self): | ||
1004 | 463 | # Construct the view and initialize it. | ||
1005 | 464 | view = BranchMergeProposalView( | ||
1006 | 465 | self.bmp, LaunchpadTestRequest()) | ||
1007 | 466 | view.initialize() | ||
1008 | 467 | return view | ||
1009 | 468 | |||
1010 | 469 | def makeTeamReview(self): | 464 | def makeTeamReview(self): |
1011 | 470 | owner = self.bmp.source_branch.owner | 465 | owner = self.bmp.source_branch.owner |
1012 | 471 | review_team = self.factory.makeTeam() | 466 | review_team = self.factory.makeTeam() |
1013 | @@ -477,7 +472,7 @@ | |||
1014 | 477 | albert = self.factory.makePerson() | 472 | albert = self.factory.makePerson() |
1015 | 478 | albert.join(review.reviewer) | 473 | albert.join(review.reviewer) |
1016 | 479 | login_person(albert) | 474 | login_person(albert) |
1018 | 480 | view = self._createView() | 475 | view = create_initialized_view(self.bmp, '+index') |
1019 | 481 | view.claim_action.success({'review_id': review.id}) | 476 | view.claim_action.success({'review_id': review.id}) |
1020 | 482 | self.assertEqual(albert, review.reviewer) | 477 | self.assertEqual(albert, review.reviewer) |
1021 | 483 | 478 | ||
1022 | @@ -486,13 +481,13 @@ | |||
1023 | 486 | review = self.makeTeamReview() | 481 | review = self.makeTeamReview() |
1024 | 487 | albert = self.factory.makePerson() | 482 | albert = self.factory.makePerson() |
1025 | 488 | login_person(albert) | 483 | login_person(albert) |
1027 | 489 | view = self._createView() | 484 | view = create_initialized_view(self.bmp, '+index') |
1028 | 490 | self.assertRaises(Unauthorized, view.claim_action.success, | 485 | self.assertRaises(Unauthorized, view.claim_action.success, |
1029 | 491 | {'review_id': review.id}) | 486 | {'review_id': review.id}) |
1030 | 492 | 487 | ||
1031 | 493 | def test_preview_diff_text_with_no_diff(self): | 488 | def test_preview_diff_text_with_no_diff(self): |
1032 | 494 | """review_diff should be None when there is no context.review_diff.""" | 489 | """review_diff should be None when there is no context.review_diff.""" |
1034 | 495 | view = self._createView() | 490 | view = create_initialized_view(self.bmp, '+index') |
1035 | 496 | self.assertIs(None, view.preview_diff_text) | 491 | self.assertIs(None, view.preview_diff_text) |
1036 | 497 | 492 | ||
1037 | 498 | def test_review_diff_utf8(self): | 493 | def test_review_diff_utf8(self): |
1038 | @@ -502,8 +497,9 @@ | |||
1039 | 502 | diff = StaticDiff.acquireFromText('x', 'y', diff_bytes) | 497 | diff = StaticDiff.acquireFromText('x', 'y', diff_bytes) |
1040 | 503 | transaction.commit() | 498 | transaction.commit() |
1041 | 504 | self.bmp.review_diff = diff | 499 | self.bmp.review_diff = diff |
1042 | 500 | view = create_initialized_view(self.bmp, '+index') | ||
1043 | 505 | self.assertEqual(diff_bytes.decode('utf-8'), | 501 | self.assertEqual(diff_bytes.decode('utf-8'), |
1045 | 506 | self._createView().preview_diff_text) | 502 | view.preview_diff_text) |
1046 | 507 | 503 | ||
1047 | 508 | def test_review_diff_all_chars(self): | 504 | def test_review_diff_all_chars(self): |
1048 | 509 | """review_diff should work on diffs containing all possible bytes.""" | 505 | """review_diff should work on diffs containing all possible bytes.""" |
1049 | @@ -512,8 +508,9 @@ | |||
1050 | 512 | diff = StaticDiff.acquireFromText('x', 'y', diff_bytes) | 508 | diff = StaticDiff.acquireFromText('x', 'y', diff_bytes) |
1051 | 513 | transaction.commit() | 509 | transaction.commit() |
1052 | 514 | self.bmp.review_diff = diff | 510 | self.bmp.review_diff = diff |
1053 | 511 | view = create_initialized_view(self.bmp, '+index') | ||
1054 | 515 | self.assertEqual(diff_bytes.decode('windows-1252', 'replace'), | 512 | self.assertEqual(diff_bytes.decode('windows-1252', 'replace'), |
1056 | 516 | self._createView().preview_diff_text) | 513 | view.preview_diff_text) |
1057 | 517 | 514 | ||
1058 | 518 | def addReviewDiff(self): | 515 | def addReviewDiff(self): |
1059 | 519 | review_diff_bytes = ''.join(unified_diff('', 'review')) | 516 | review_diff_bytes = ''.join(unified_diff('', 'review')) |
1060 | @@ -532,27 +529,31 @@ | |||
1061 | 532 | def test_preview_diff_prefers_preview_diff(self): | 529 | def test_preview_diff_prefers_preview_diff(self): |
1062 | 533 | """The preview will be used for BMP with both a review and preview.""" | 530 | """The preview will be used for BMP with both a review and preview.""" |
1063 | 534 | preview_diff = self.addBothDiffs() | 531 | preview_diff = self.addBothDiffs() |
1065 | 535 | self.assertEqual(preview_diff, self._createView().preview_diff) | 532 | view = create_initialized_view(self.bmp, '+index') |
1066 | 533 | self.assertEqual(preview_diff, view.preview_diff) | ||
1067 | 536 | 534 | ||
1068 | 537 | def test_preview_diff_uses_review_diff(self): | 535 | def test_preview_diff_uses_review_diff(self): |
1069 | 538 | """The review diff will be used if there is no preview.""" | 536 | """The review diff will be used if there is no preview.""" |
1070 | 539 | review_diff = self.addReviewDiff() | 537 | review_diff = self.addReviewDiff() |
1071 | 538 | view = create_initialized_view(self.bmp, '+index') | ||
1072 | 540 | self.assertEqual(review_diff.diff, | 539 | self.assertEqual(review_diff.diff, |
1074 | 541 | self._createView().preview_diff) | 540 | view.preview_diff) |
1075 | 542 | 541 | ||
1076 | 543 | def test_review_diff_text_prefers_preview_diff(self): | 542 | def test_review_diff_text_prefers_preview_diff(self): |
1077 | 544 | """The preview will be used for BMP with both a review and preview.""" | 543 | """The preview will be used for BMP with both a review and preview.""" |
1078 | 545 | preview_diff = self.addBothDiffs() | 544 | preview_diff = self.addBothDiffs() |
1079 | 546 | transaction.commit() | 545 | transaction.commit() |
1080 | 546 | view = create_initialized_view(self.bmp, '+index') | ||
1081 | 547 | self.assertEqual( | 547 | self.assertEqual( |
1083 | 548 | preview_diff.text, self._createView().preview_diff_text) | 548 | preview_diff.text, view.preview_diff_text) |
1084 | 549 | 549 | ||
1085 | 550 | def test_linked_bugs_excludes_mutual_bugs(self): | 550 | def test_linked_bugs_excludes_mutual_bugs(self): |
1086 | 551 | """List bugs that are linked to the source only.""" | 551 | """List bugs that are linked to the source only.""" |
1087 | 552 | bug = self.factory.makeBug() | 552 | bug = self.factory.makeBug() |
1088 | 553 | self.bmp.source_branch.linkBug(bug, self.bmp.registrant) | 553 | self.bmp.source_branch.linkBug(bug, self.bmp.registrant) |
1089 | 554 | self.bmp.target_branch.linkBug(bug, self.bmp.registrant) | 554 | self.bmp.target_branch.linkBug(bug, self.bmp.registrant) |
1091 | 555 | self.assertEqual([], self._createView().linked_bugs) | 555 | view = create_initialized_view(self.bmp, '+index') |
1092 | 556 | self.assertEqual([], view.linked_bugs) | ||
1093 | 556 | 557 | ||
1094 | 557 | 558 | ||
1095 | 558 | class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory): | 559 | class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory): |
1096 | @@ -692,5 +693,42 @@ | |||
1097 | 692 | self.assertEqual(u'\u2615', diff_attachment.diff_text) | 693 | self.assertEqual(u'\u2615', diff_attachment.diff_text) |
1098 | 693 | 694 | ||
1099 | 694 | 695 | ||
1100 | 696 | class TestBranchMergeCandidateView(TestCaseWithFactory): | ||
1101 | 697 | """Test the status title for the view.""" | ||
1102 | 698 | |||
1103 | 699 | layer = DatabaseFunctionalLayer | ||
1104 | 700 | |||
1105 | 701 | def test_needs_review_title(self): | ||
1106 | 702 | # No title is set for a proposal needing review. | ||
1107 | 703 | bmp = self.factory.makeBranchMergeProposal( | ||
1108 | 704 | set_state=BranchMergeProposalStatus.NEEDS_REVIEW) | ||
1109 | 705 | view = create_initialized_view(bmp, '+link-summary') | ||
1110 | 706 | self.assertEqual('', view.status_title) | ||
1111 | 707 | |||
1112 | 708 | def test_approved_shows_reviewer(self): | ||
1113 | 709 | # If the proposal is approved, the approver is shown in the title | ||
1114 | 710 | # along with when they approved it. | ||
1115 | 711 | bmp = self.factory.makeBranchMergeProposal() | ||
1116 | 712 | owner = bmp.target_branch.owner | ||
1117 | 713 | login_person(bmp.target_branch.owner) | ||
1118 | 714 | owner.displayname = 'Eric' | ||
1119 | 715 | bmp.approveBranch(owner, 'some-rev', datetime( | ||
1120 | 716 | year=2008, month=9, day=10, tzinfo=pytz.UTC)) | ||
1121 | 717 | view = create_initialized_view(bmp, '+link-summary') | ||
1122 | 718 | self.assertEqual('Eric on 2008-09-10', view.status_title) | ||
1123 | 719 | |||
1124 | 720 | def test_rejected_shows_reviewer(self): | ||
1125 | 721 | # If the proposal is rejected, the approver is shown in the title | ||
1126 | 722 | # along with when they approved it. | ||
1127 | 723 | bmp = self.factory.makeBranchMergeProposal() | ||
1128 | 724 | owner = bmp.target_branch.owner | ||
1129 | 725 | login_person(bmp.target_branch.owner) | ||
1130 | 726 | owner.displayname = 'Eric' | ||
1131 | 727 | bmp.rejectBranch(owner, 'some-rev', datetime( | ||
1132 | 728 | year=2008, month=9, day=10, tzinfo=pytz.UTC)) | ||
1133 | 729 | view = create_initialized_view(bmp, '+link-summary') | ||
1134 | 730 | self.assertEqual('Eric on 2008-09-10', view.status_title) | ||
1135 | 731 | |||
1136 | 732 | |||
1137 | 695 | def test_suite(): | 733 | def test_suite(): |
1138 | 696 | return unittest.TestLoader().loadTestsFromName(__name__) | 734 | return unittest.TestLoader().loadTestsFromName(__name__) |
1139 | 697 | 735 | ||
1140 | === added file 'lib/lp/code/browser/tests/test_tales.py' | |||
1141 | --- lib/lp/code/browser/tests/test_tales.py 1970-01-01 00:00:00 +0000 | |||
1142 | +++ lib/lp/code/browser/tests/test_tales.py 2009-11-18 15:05:39 +0000 | |||
1143 | @@ -0,0 +1,163 @@ | |||
1144 | 1 | # Copyright 2009 Canonical Ltd. This software is licensed under the | ||
1145 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
1146 | 3 | |||
1147 | 4 | """Tests for the tales formatters.""" | ||
1148 | 5 | |||
1149 | 6 | __metaclass__ = type | ||
1150 | 7 | |||
1151 | 8 | from difflib import unified_diff | ||
1152 | 9 | import unittest | ||
1153 | 10 | |||
1154 | 11 | from storm.store import Store | ||
1155 | 12 | from zope.security.proxy import removeSecurityProxy | ||
1156 | 13 | |||
1157 | 14 | from lp.testing import login, TestCaseWithFactory, test_tales | ||
1158 | 15 | from canonical.testing import LaunchpadFunctionalLayer | ||
1159 | 16 | |||
1160 | 17 | |||
1161 | 18 | class TestPreviewDiffFormatter(TestCaseWithFactory): | ||
1162 | 19 | """Test the PreviewDiffFormatterAPI class.""" | ||
1163 | 20 | |||
1164 | 21 | layer = LaunchpadFunctionalLayer | ||
1165 | 22 | |||
1166 | 23 | def _createPreviewDiff(self, line_count=0, added=None, removed=None, | ||
1167 | 24 | conflicts=None, diffstat=None): | ||
1168 | 25 | # Login an admin to avoid the launchpad.Edit requirements. | ||
1169 | 26 | login('admin@canonical.com') | ||
1170 | 27 | # Create a dummy preview diff, and make sure the branches have the | ||
1171 | 28 | # correct last scanned ids to ensure that the new diff is not stale. | ||
1172 | 29 | bmp = self.factory.makeBranchMergeProposal() | ||
1173 | 30 | if line_count: | ||
1174 | 31 | content = ''.join(unified_diff('', 'random content')) | ||
1175 | 32 | else: | ||
1176 | 33 | content = '' | ||
1177 | 34 | preview = bmp.updatePreviewDiff( | ||
1178 | 35 | content, u'rev-a', u'rev-b', conflicts=conflicts) | ||
1179 | 36 | bmp.source_branch.last_scanned_id = preview.source_revision_id | ||
1180 | 37 | bmp.target_branch.last_scanned_id = preview.target_revision_id | ||
1181 | 38 | # Update the values directly sidestepping the security. | ||
1182 | 39 | naked_diff = removeSecurityProxy(preview) | ||
1183 | 40 | naked_diff.diff_lines_count = line_count | ||
1184 | 41 | naked_diff.added_lines_count = added | ||
1185 | 42 | naked_diff.removed_lines_count = removed | ||
1186 | 43 | naked_diff.diffstat = diffstat | ||
1187 | 44 | # In order to get the canonical url of the librarian file, we need to | ||
1188 | 45 | # commit. | ||
1189 | 46 | # transaction.commit() | ||
1190 | 47 | # Make sure that the preview diff is in the db for the test. | ||
1191 | 48 | # Storm bug: 324724 | ||
1192 | 49 | Store.of(bmp).flush() | ||
1193 | 50 | return preview | ||
1194 | 51 | |||
1195 | 52 | def _createStalePreviewDiff(self, line_count=0, added=None, removed=None, | ||
1196 | 53 | conflicts=None, diffstat=None): | ||
1197 | 54 | preview = self._createPreviewDiff( | ||
1198 | 55 | line_count, added, removed, conflicts, diffstat) | ||
1199 | 56 | preview.branch_merge_proposal.source_branch.last_scanned_id = 'other' | ||
1200 | 57 | return preview | ||
1201 | 58 | |||
1202 | 59 | def test_creation_method(self): | ||
1203 | 60 | # Just confirm that our helpers do what they say. | ||
1204 | 61 | preview = self._createPreviewDiff( | ||
1205 | 62 | 12, 45, 23, u'conflicts', {'filename': (3,2)}) | ||
1206 | 63 | self.assertEqual(12, preview.diff_lines_count) | ||
1207 | 64 | self.assertEqual(45, preview.added_lines_count) | ||
1208 | 65 | self.assertEqual(23, preview.removed_lines_count) | ||
1209 | 66 | self.assertEqual(False, preview.stale) | ||
1210 | 67 | self.assertEqual(True, self._createStalePreviewDiff().stale) | ||
1211 | 68 | self.assertEqual(u'conflicts', preview.conflicts) | ||
1212 | 69 | self.assertEqual({'filename': (3,2)}, preview.diffstat) | ||
1213 | 70 | |||
1214 | 71 | def test_fmt_no_diff(self): | ||
1215 | 72 | # If there is no diff, there is no link. | ||
1216 | 73 | preview = self._createPreviewDiff(0) | ||
1217 | 74 | self.assertEqual( | ||
1218 | 75 | '<span class="empty-diff">0 lines</span>', | ||
1219 | 76 | test_tales('preview/fmt:link', preview=preview)) | ||
1220 | 77 | |||
1221 | 78 | def test_fmt_lines_no_add_or_remove(self): | ||
1222 | 79 | # If the lines added and removed are None, they are now shown. | ||
1223 | 80 | preview = self._createPreviewDiff(10, added=None, removed=None) | ||
1224 | 81 | self.assertEqual( | ||
1225 | 82 | '<a href="%s" class="diff-link">' | ||
1226 | 83 | '10 lines</a>' | ||
1227 | 84 | % preview.diff_text.getURL(), | ||
1228 | 85 | test_tales('preview/fmt:link', preview=preview)) | ||
1229 | 86 | |||
1230 | 87 | def test_fmt_lines_some_added_no_removed(self): | ||
1231 | 88 | # If the added and removed values are not None, they are shown. | ||
1232 | 89 | preview = self._createPreviewDiff(10, added=4, removed=0) | ||
1233 | 90 | self.assertEqual( | ||
1234 | 91 | '<a href="%s" class="diff-link">' | ||
1235 | 92 | '10 lines (+4/-0)</a>' | ||
1236 | 93 | % preview.diff_text.getURL(), | ||
1237 | 94 | test_tales('preview/fmt:link', preview=preview)) | ||
1238 | 95 | |||
1239 | 96 | def test_fmt_lines_files_modified(self): | ||
1240 | 97 | # If the diffstat has been set, the number of entries in the dict | ||
1241 | 98 | # defines the number of files modified. | ||
1242 | 99 | preview = self._createPreviewDiff( | ||
1243 | 100 | 10, added=4, removed=0, diffstat={ | ||
1244 | 101 | 'file1': (1, 0), | ||
1245 | 102 | 'file2': (3, 0)}) | ||
1246 | 103 | self.assertEqual( | ||
1247 | 104 | '<a href="%s" class="diff-link">' | ||
1248 | 105 | '10 lines (+4/-0) 2 files modified</a>' | ||
1249 | 106 | % preview.diff_text.getURL(), | ||
1250 | 107 | test_tales('preview/fmt:link', preview=preview)) | ||
1251 | 108 | |||
1252 | 109 | def test_fmt_lines_one_file_modified(self): | ||
1253 | 110 | # If only one file has been modified, a singular value is used. | ||
1254 | 111 | preview = self._createPreviewDiff( | ||
1255 | 112 | 10, added=4, removed=0, diffstat={ | ||
1256 | 113 | 'file': (3, 0)}) | ||
1257 | 114 | self.assertEqual( | ||
1258 | 115 | '<a href="%s" class="diff-link">' | ||
1259 | 116 | '10 lines (+4/-0) 1 file modified</a>' | ||
1260 | 117 | % preview.diff_text.getURL(), | ||
1261 | 118 | test_tales('preview/fmt:link', preview=preview)) | ||
1262 | 119 | |||
1263 | 120 | def test_fmt_simple_conflicts(self): | ||
1264 | 121 | # Conflicts are indicated using text in the link. | ||
1265 | 122 | preview = self._createPreviewDiff(10, 2, 3, u'conflicts') | ||
1266 | 123 | self.assertEqual( | ||
1267 | 124 | '<a href="%s" class="diff-link">' | ||
1268 | 125 | '10 lines (+2/-3) (has conflicts)</a>' | ||
1269 | 126 | % preview.diff_text.getURL(), | ||
1270 | 127 | test_tales('preview/fmt:link', preview=preview)) | ||
1271 | 128 | |||
1272 | 129 | def test_fmt_stale_empty_diff(self): | ||
1273 | 130 | # If there is no diff, there is no link. | ||
1274 | 131 | preview = self._createStalePreviewDiff(0) | ||
1275 | 132 | self.assertEqual( | ||
1276 | 133 | '<span class="empty-diff">0 lines</span>', | ||
1277 | 134 | test_tales('preview/fmt:link', preview=preview)) | ||
1278 | 135 | |||
1279 | 136 | def test_fmt_stale_non_empty_diff(self): | ||
1280 | 137 | # If there is no diff, there is no link. | ||
1281 | 138 | diffstat = dict( | ||
1282 | 139 | (self.factory.getUniqueString(), (2,3)) for x in range(23)) | ||
1283 | 140 | preview = self._createStalePreviewDiff( | ||
1284 | 141 | 500, 89, 340, diffstat=diffstat) | ||
1285 | 142 | self.assertEqual( | ||
1286 | 143 | '<a href="%s" class="diff-link">' | ||
1287 | 144 | '500 lines (+89/-340) 23 files modified</a>' | ||
1288 | 145 | % preview.diff_text.getURL(), | ||
1289 | 146 | test_tales('preview/fmt:link', preview=preview)) | ||
1290 | 147 | |||
1291 | 148 | def test_fmt_stale_non_empty_diff_with_conflicts(self): | ||
1292 | 149 | # If there is no diff, there is no link. | ||
1293 | 150 | diffstat = dict( | ||
1294 | 151 | (self.factory.getUniqueString(), (2,3)) for x in range(23)) | ||
1295 | 152 | preview = self._createStalePreviewDiff( | ||
1296 | 153 | 500, 89, 340, u'conflicts', diffstat=diffstat) | ||
1297 | 154 | self.assertEqual( | ||
1298 | 155 | '<a href="%s" class="diff-link">' | ||
1299 | 156 | '500 lines (+89/-340) 23 files modified (has conflicts)</a>' | ||
1300 | 157 | % preview.diff_text.getURL(), | ||
1301 | 158 | test_tales('preview/fmt:link', preview=preview)) | ||
1302 | 159 | |||
1303 | 160 | |||
1304 | 161 | def test_suite(): | ||
1305 | 162 | return unittest.TestLoader().loadTestsFromName(__name__) | ||
1306 | 163 | |||
1307 | 0 | 164 | ||
1308 | === modified file 'lib/lp/code/model/branchmergeproposal.py' | |||
1309 | --- lib/lp/code/model/branchmergeproposal.py 2009-10-05 14:17:48 +0000 | |||
1310 | +++ lib/lp/code/model/branchmergeproposal.py 2009-11-18 15:05:38 +0000 | |||
1311 | @@ -361,7 +361,8 @@ | |||
1312 | 361 | # or superseded, then it is valid to be merged. | 361 | # or superseded, then it is valid to be merged. |
1313 | 362 | return (self.queue_status not in FINAL_STATES) | 362 | return (self.queue_status not in FINAL_STATES) |
1314 | 363 | 363 | ||
1316 | 364 | def _reviewProposal(self, reviewer, next_state, revision_id): | 364 | def _reviewProposal(self, reviewer, next_state, revision_id, |
1317 | 365 | _date_reviewed=None): | ||
1318 | 365 | """Set the proposal to one of the two review statuses.""" | 366 | """Set the proposal to one of the two review statuses.""" |
1319 | 366 | # Check the reviewer can review the code for the target branch. | 367 | # Check the reviewer can review the code for the target branch. |
1320 | 367 | old_state = self.queue_status | 368 | old_state = self.queue_status |
1321 | @@ -371,21 +372,25 @@ | |||
1322 | 371 | self._transitionToState(next_state, reviewer) | 372 | self._transitionToState(next_state, reviewer) |
1323 | 372 | # Record the reviewer | 373 | # Record the reviewer |
1324 | 373 | self.reviewer = reviewer | 374 | self.reviewer = reviewer |
1326 | 374 | self.date_reviewed = UTC_NOW | 375 | if _date_reviewed is None: |
1327 | 376 | _date_reviewed = UTC_NOW | ||
1328 | 377 | self.date_reviewed = _date_reviewed | ||
1329 | 375 | # Record the reviewed revision id | 378 | # Record the reviewed revision id |
1330 | 376 | self.reviewed_revision_id = revision_id | 379 | self.reviewed_revision_id = revision_id |
1331 | 377 | notify(BranchMergeProposalStatusChangeEvent( | 380 | notify(BranchMergeProposalStatusChangeEvent( |
1332 | 378 | self, reviewer, old_state, next_state)) | 381 | self, reviewer, old_state, next_state)) |
1333 | 379 | 382 | ||
1335 | 380 | def approveBranch(self, reviewer, revision_id): | 383 | def approveBranch(self, reviewer, revision_id, _date_reviewed=None): |
1336 | 381 | """See `IBranchMergeProposal`.""" | 384 | """See `IBranchMergeProposal`.""" |
1337 | 382 | self._reviewProposal( | 385 | self._reviewProposal( |
1339 | 383 | reviewer, BranchMergeProposalStatus.CODE_APPROVED, revision_id) | 386 | reviewer, BranchMergeProposalStatus.CODE_APPROVED, revision_id, |
1340 | 387 | _date_reviewed) | ||
1341 | 384 | 388 | ||
1343 | 385 | def rejectBranch(self, reviewer, revision_id): | 389 | def rejectBranch(self, reviewer, revision_id, _date_reviewed=None): |
1344 | 386 | """See `IBranchMergeProposal`.""" | 390 | """See `IBranchMergeProposal`.""" |
1345 | 387 | self._reviewProposal( | 391 | self._reviewProposal( |
1347 | 388 | reviewer, BranchMergeProposalStatus.REJECTED, revision_id) | 392 | reviewer, BranchMergeProposalStatus.REJECTED, revision_id, |
1348 | 393 | _date_reviewed) | ||
1349 | 389 | 394 | ||
1350 | 390 | def enqueue(self, queuer, revision_id): | 395 | def enqueue(self, queuer, revision_id): |
1351 | 391 | """See `IBranchMergeProposal`.""" | 396 | """See `IBranchMergeProposal`.""" |
1352 | 392 | 397 | ||
1353 | === modified file 'lib/lp/code/model/directbranchcommit.py' | |||
1354 | --- lib/lp/code/model/directbranchcommit.py 2009-09-09 20:03:19 +0000 | |||
1355 | +++ lib/lp/code/model/directbranchcommit.py 2009-11-18 15:05:38 +0000 | |||
1356 | @@ -74,6 +74,8 @@ | |||
1357 | 74 | self.db_branch = db_branch | 74 | self.db_branch = db_branch |
1358 | 75 | self.to_mirror = to_mirror | 75 | self.to_mirror = to_mirror |
1359 | 76 | 76 | ||
1360 | 77 | self.last_scanned_id = self.db_branch.last_scanned_id | ||
1361 | 78 | |||
1362 | 77 | if committer is None: | 79 | if committer is None: |
1363 | 78 | committer = db_branch.owner | 80 | committer = db_branch.owner |
1364 | 79 | self.committer = committer | 81 | self.committer = committer |
1365 | @@ -172,12 +174,18 @@ | |||
1366 | 172 | assert self.is_locked, "Getting revision on un-locked branch." | 174 | assert self.is_locked, "Getting revision on un-locked branch." |
1367 | 173 | last_revision = None | 175 | last_revision = None |
1368 | 174 | last_revision = self.bzrbranch.last_revision() | 176 | last_revision = self.bzrbranch.last_revision() |
1370 | 175 | if last_revision != self.db_branch.last_scanned_id: | 177 | if last_revision != self.last_scanned_id: |
1371 | 176 | raise ConcurrentUpdateError( | 178 | raise ConcurrentUpdateError( |
1372 | 177 | "Branch has been changed. Not committing.") | 179 | "Branch has been changed. Not committing.") |
1373 | 178 | 180 | ||
1376 | 179 | def commit(self, commit_message): | 181 | def commit(self, commit_message, txn=None): |
1377 | 180 | """Commit to branch.""" | 182 | """Commit to branch. |
1378 | 183 | |||
1379 | 184 | :param commit_message: Message for branch's commit log. | ||
1380 | 185 | :param txn: Transaction to commit. Can be helpful in avoiding | ||
1381 | 186 | long idle times in database transactions. May be committed | ||
1382 | 187 | more than once. | ||
1383 | 188 | """ | ||
1384 | 181 | assert self.is_open, "Committing closed DirectBranchCommit." | 189 | assert self.is_open, "Committing closed DirectBranchCommit." |
1385 | 182 | assert self.is_locked, "Not locked at commit time." | 190 | assert self.is_locked, "Not locked at commit time." |
1386 | 183 | 191 | ||
1387 | @@ -185,6 +193,9 @@ | |||
1388 | 185 | try: | 193 | try: |
1389 | 186 | self._checkForRace() | 194 | self._checkForRace() |
1390 | 187 | 195 | ||
1391 | 196 | if txn: | ||
1392 | 197 | txn.commit() | ||
1393 | 198 | |||
1394 | 188 | rev_id = self.revision_tree.get_revision_id() | 199 | rev_id = self.revision_tree.get_revision_id() |
1395 | 189 | if rev_id == NULL_REVISION: | 200 | if rev_id == NULL_REVISION: |
1396 | 190 | if list(self.transform_preview.iter_changes()) == []: | 201 | if list(self.transform_preview.iter_changes()) == []: |
1397 | @@ -193,6 +204,9 @@ | |||
1398 | 193 | self.bzrbranch, commit_message) | 204 | self.bzrbranch, commit_message) |
1399 | 194 | IMasterObject(self.db_branch).requestMirror() | 205 | IMasterObject(self.db_branch).requestMirror() |
1400 | 195 | 206 | ||
1401 | 207 | if txn: | ||
1402 | 208 | txn.commit() | ||
1403 | 209 | |||
1404 | 196 | finally: | 210 | finally: |
1405 | 197 | self.unlock() | 211 | self.unlock() |
1406 | 198 | self.is_open = False | 212 | self.is_open = False |
1407 | 199 | 213 | ||
1408 | === modified file 'lib/lp/code/stories/branches/xx-branch-merge-proposals.txt' | |||
1409 | --- lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2009-10-28 19:31:47 +0000 | |||
1410 | +++ lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2009-11-18 15:05:39 +0000 | |||
1411 | @@ -29,6 +29,8 @@ | |||
1412 | 29 | >>> _unused = branch.subscribe(subscriber, | 29 | >>> _unused = branch.subscribe(subscriber, |
1413 | 30 | ... BranchSubscriptionNotificationLevel.NOEMAIL, None, | 30 | ... BranchSubscriptionNotificationLevel.NOEMAIL, None, |
1414 | 31 | ... CodeReviewNotificationLevel.FULL) | 31 | ... CodeReviewNotificationLevel.FULL) |
1415 | 32 | >>> from lp.code.tests.helpers import make_erics_fooix_project | ||
1416 | 33 | >>> locals().update(make_erics_fooix_project(factory)) | ||
1417 | 32 | >>> logout() | 34 | >>> logout() |
1418 | 33 | 35 | ||
1419 | 34 | Any logged in user can register a merge proposal. Registering | 36 | Any logged in user can register a merge proposal. Registering |
1420 | @@ -68,7 +70,8 @@ | |||
1421 | 68 | 70 | ||
1422 | 69 | Registering the merge proposal takes the user to the new merge proposal. | 71 | Registering the merge proposal takes the user to the new merge proposal. |
1423 | 70 | 72 | ||
1425 | 71 | >>> print nopriv_browser.url | 73 | >>> klingon_proposal = nopriv_browser.url |
1426 | 74 | >>> print klingon_proposal | ||
1427 | 72 | http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/... | 75 | http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/... |
1428 | 73 | 76 | ||
1429 | 74 | The summary reflects the selected target and prerequisite. | 77 | The summary reflects the selected target and prerequisite. |
1430 | @@ -97,9 +100,6 @@ | |||
1431 | 97 | ... 'Add more <b>mojo</b>') | 100 | ... 'Add more <b>mojo</b>') |
1432 | 98 | >>> nopriv_browser.getControl('Update').click() | 101 | >>> nopriv_browser.getControl('Update').click() |
1433 | 99 | 102 | ||
1434 | 100 | >>> print nopriv_browser.url | ||
1435 | 101 | http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1 | ||
1436 | 102 | |||
1437 | 103 | >>> print_tag_with_id(nopriv_browser.contents, 'edit-description') | 103 | >>> print_tag_with_id(nopriv_browser.contents, 'edit-description') |
1438 | 104 | Commit Message | 104 | Commit Message |
1439 | 105 | Add more <b>mojo</b> | 105 | Add more <b>mojo</b> |
1440 | @@ -113,7 +113,6 @@ | |||
1441 | 113 | for the source_branch. | 113 | for the source_branch. |
1442 | 114 | 114 | ||
1443 | 115 | >>> login('foo.bar@canonical.com') | 115 | >>> login('foo.bar@canonical.com') |
1444 | 116 | >>> eric = factory.makePerson(email="eric@example.com", password="test") | ||
1445 | 117 | >>> bmp = factory.makeBranchMergeProposal(registrant=eric) | 116 | >>> bmp = factory.makeBranchMergeProposal(registrant=eric) |
1446 | 118 | >>> bmp_url = canonical_url(bmp) | 117 | >>> bmp_url = canonical_url(bmp) |
1447 | 119 | >>> branch_url = canonical_url(bmp.source_branch) | 118 | >>> branch_url = canonical_url(bmp.source_branch) |
1448 | @@ -128,14 +127,13 @@ | |||
1449 | 128 | 127 | ||
1450 | 129 | >>> sample_browser = setupBrowser(auth="Basic test@canonical.com:test") | 128 | >>> sample_browser = setupBrowser(auth="Basic test@canonical.com:test") |
1451 | 130 | 129 | ||
1452 | 130 | |||
1453 | 131 | Requesting reviews | 131 | Requesting reviews |
1454 | 132 | ------------------ | 132 | ------------------ |
1455 | 133 | 133 | ||
1456 | 134 | You can request a review of a merge proposal. | 134 | You can request a review of a merge proposal. |
1457 | 135 | 135 | ||
1461 | 136 | >>> sample_browser.open( | 136 | >>> sample_browser.open(klingon_proposal) |
1459 | 137 | ... 'http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1' | ||
1460 | 138 | ... ) | ||
1462 | 139 | >>> sample_browser.getLink('Request a review').click() | 137 | >>> sample_browser.getLink('Request a review').click() |
1463 | 140 | >>> sample_browser.getControl('Reviewer').value = 'mark' | 138 | >>> sample_browser.getControl('Reviewer').value = 'mark' |
1464 | 141 | >>> sample_browser.getControl('Review type').value = 'first' | 139 | >>> sample_browser.getControl('Review type').value = 'first' |
1465 | @@ -169,7 +167,7 @@ | |||
1466 | 169 | Reviewer Review Type Date Requested Status | 167 | Reviewer Review Type Date Requested Status |
1467 | 170 | Sample Person Pending [Review] | 168 | Sample Person Pending [Review] |
1468 | 171 | Mark Shuttleworth second ... ago Pending | 169 | Mark Shuttleworth second ... ago Pending |
1470 | 172 | Review via email: mp+1@code.launchpad.dev | 170 | Review via email: mp+...@code.launchpad.dev |
1471 | 173 | Request another review | 171 | Request another review |
1472 | 174 | 172 | ||
1473 | 175 | 173 | ||
1474 | @@ -178,16 +176,14 @@ | |||
1475 | 178 | 176 | ||
1476 | 179 | People not logged in cannot perform reviews. | 177 | People not logged in cannot perform reviews. |
1477 | 180 | 178 | ||
1480 | 181 | >>> anon_browser.open('http://code.launchpad.dev/~name12/gnome-terminal' | 179 | >>> anon_browser.open(klingon_proposal) |
1479 | 182 | ... '/klingon/+merge/1') | ||
1481 | 183 | >>> link = anon_browser.getLink('[Review]') | 180 | >>> link = anon_browser.getLink('[Review]') |
1482 | 184 | Traceback (most recent call last): | 181 | Traceback (most recent call last): |
1483 | 185 | LinkNotFoundError | 182 | LinkNotFoundError |
1484 | 186 | 183 | ||
1485 | 187 | People who are logged in can perform reviews. | 184 | People who are logged in can perform reviews. |
1486 | 188 | 185 | ||
1489 | 189 | >>> nopriv_browser.open('http://code.launchpad.dev/~name12/gnome-terminal' | 186 | >>> nopriv_browser.open(klingon_proposal) |
1488 | 190 | ... '/klingon/+merge/1') | ||
1490 | 191 | >>> nopriv_browser.getLink('Add a review or comment').click() | 187 | >>> nopriv_browser.getLink('Add a review or comment').click() |
1491 | 192 | >>> nopriv_browser.getControl(name='field.comment').value = "Don't like it" | 188 | >>> nopriv_browser.getControl(name='field.comment').value = "Don't like it" |
1492 | 193 | >>> nopriv_browser.getControl(name='field.vote').getControl( | 189 | >>> nopriv_browser.getControl(name='field.vote').getControl( |
1493 | @@ -201,8 +197,7 @@ | |||
1494 | 201 | 197 | ||
1495 | 202 | People can claim reviews for teams of which they are a member. | 198 | People can claim reviews for teams of which they are a member. |
1496 | 203 | 199 | ||
1499 | 204 | >>> sample_browser.open('http://code.launchpad.dev/~name12/gnome-terminal' | 200 | >>> sample_browser.open(klingon_proposal) |
1498 | 205 | ... '/klingon/+merge/1') | ||
1500 | 206 | >>> sample_browser.getLink('Request another review').click() | 201 | >>> sample_browser.getLink('Request another review').click() |
1501 | 207 | >>> sample_browser.getControl('Reviewer').value = 'hwdb-team' | 202 | >>> sample_browser.getControl('Reviewer').value = 'hwdb-team' |
1502 | 208 | >>> sample_browser.getControl('Review type').value = 'claimable' | 203 | >>> sample_browser.getControl('Review type').value = 'claimable' |
1503 | @@ -213,8 +208,7 @@ | |||
1504 | 213 | Reviewer Review Type Date Requested Status... | 208 | Reviewer Review Type Date Requested Status... |
1505 | 214 | HWDB Team claimable ... ago Pending ... | 209 | HWDB Team claimable ... ago Pending ... |
1506 | 215 | >>> foobar_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test") | 210 | >>> foobar_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test") |
1509 | 216 | >>> foobar_browser.open('http://code.launchpad.dev/~name12/gnome-terminal' | 211 | >>> foobar_browser.open(klingon_proposal) |
1508 | 217 | ... '/klingon/+merge/1') | ||
1510 | 218 | >>> foobar_browser.getControl('Claim review').click() | 212 | >>> foobar_browser.getControl('Claim review').click() |
1511 | 219 | >>> pending = find_tag_by_id( | 213 | >>> pending = find_tag_by_id( |
1512 | 220 | ... foobar_browser.contents, 'code-review-votes') | 214 | ... foobar_browser.contents, 'code-review-votes') |
1513 | @@ -230,8 +224,7 @@ | |||
1514 | 230 | >>> foobar_browser.getLink('Reassign').click() | 224 | >>> foobar_browser.getLink('Reassign').click() |
1515 | 231 | >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team' | 225 | >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team' |
1516 | 232 | >>> foobar_browser.getControl('Reassign').click() | 226 | >>> foobar_browser.getControl('Reassign').click() |
1519 | 233 | >>> foobar_browser.open('http://code.launchpad.dev/~name12/gnome-terminal' | 227 | >>> foobar_browser.open(klingon_proposal) |
1518 | 234 | ... '/klingon/+merge/1') | ||
1520 | 235 | >>> pending = find_tag_by_id( | 228 | >>> pending = find_tag_by_id( |
1521 | 236 | ... foobar_browser.contents, 'code-review-votes') | 229 | ... foobar_browser.contents, 'code-review-votes') |
1522 | 237 | 230 | ||
1523 | @@ -248,8 +241,7 @@ | |||
1524 | 248 | When a branch has been merged into the target branch, the proposal should | 241 | When a branch has been merged into the target branch, the proposal should |
1525 | 249 | be marked as merged. | 242 | be marked as merged. |
1526 | 250 | 243 | ||
1529 | 251 | >>> nopriv_browser.open( | 244 | >>> nopriv_browser.open(klingon_proposal) |
1528 | 252 | ... 'http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1') | ||
1530 | 253 | 245 | ||
1531 | 254 | The edit icon at the end of the status allows the user to edit the status. | 246 | The edit icon at the end of the status allows the user to edit the status. |
1532 | 255 | 247 | ||
1533 | @@ -264,8 +256,6 @@ | |||
1534 | 264 | >>> nopriv_browser.getControl(name='field.queue_status').displayValue = ( | 256 | >>> nopriv_browser.getControl(name='field.queue_status').displayValue = ( |
1535 | 265 | ... ['Merged']) | 257 | ... ['Merged']) |
1536 | 266 | >>> nopriv_browser.getControl('Change Status').click() | 258 | >>> nopriv_browser.getControl('Change Status').click() |
1537 | 267 | >>> print nopriv_browser.url | ||
1538 | 268 | http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1 | ||
1539 | 269 | 259 | ||
1540 | 270 | The proposal is now shown as have being merged. | 260 | The proposal is now shown as have being merged. |
1541 | 271 | 261 | ||
1542 | @@ -282,6 +272,7 @@ | |||
1543 | 282 | >>> print extract_text(find_tag_by_id( | 272 | >>> print extract_text(find_tag_by_id( |
1544 | 283 | ... sample_browser.contents, 'landing-targets')) | 273 | ... sample_browser.contents, 'landing-targets')) |
1545 | 284 | Merged into lp://dev/~name12/gnome-terminal/main | 274 | Merged into lp://dev/~name12/gnome-terminal/main |
1546 | 275 | ... | ||
1547 | 285 | >>> sample_browser.getLink('Merged').click() | 276 | >>> sample_browser.getLink('Merged').click() |
1548 | 286 | >>> print_summary(sample_browser) | 277 | >>> print_summary(sample_browser) |
1549 | 287 | Status: Merged | 278 | Status: Merged |
1550 | @@ -303,6 +294,7 @@ | |||
1551 | 303 | >>> print extract_text(find_tag_by_id( | 294 | >>> print extract_text(find_tag_by_id( |
1552 | 304 | ... sample_browser.contents, 'landing-targets')) | 295 | ... sample_browser.contents, 'landing-targets')) |
1553 | 305 | Merged into lp://dev/~name12/gnome-terminal/main at revision 42 | 296 | Merged into lp://dev/~name12/gnome-terminal/main at revision 42 |
1554 | 297 | ... | ||
1555 | 306 | 298 | ||
1556 | 307 | 299 | ||
1557 | 308 | Resubmitting proposals | 300 | Resubmitting proposals |
1558 | @@ -315,9 +307,7 @@ | |||
1559 | 315 | branches but with the state set to work-in-progress. | 307 | branches but with the state set to work-in-progress. |
1560 | 316 | 308 | ||
1561 | 317 | >>> login('foo.bar@canonical.com') | 309 | >>> login('foo.bar@canonical.com') |
1565 | 318 | >>> fooix = factory.makeProduct(name='fooix') | 310 | >>> bmp = factory.makeBranchMergeProposal(target_branch=trunk) |
1563 | 319 | >>> target = factory.makeProductBranch(product=fooix, owner=eric) | ||
1564 | 320 | >>> bmp = factory.makeBranchMergeProposal(target_branch=target) | ||
1566 | 321 | >>> bmp_url = canonical_url(bmp) | 311 | >>> bmp_url = canonical_url(bmp) |
1567 | 322 | >>> logout() | 312 | >>> logout() |
1568 | 323 | >>> eric_browser.open(bmp_url) | 313 | >>> eric_browser.open(bmp_url) |
1569 | @@ -538,7 +528,6 @@ | |||
1570 | 538 | Bug #...: Bug for linking (Undecided – New) | 528 | Bug #...: Bug for linking (Undecided – New) |
1571 | 539 | 529 | ||
1572 | 540 | 530 | ||
1573 | 541 | |||
1574 | 542 | Target branch edge cases | 531 | Target branch edge cases |
1575 | 543 | ------------------------ | 532 | ------------------------ |
1576 | 544 | 533 | ||
1577 | @@ -626,3 +615,30 @@ | |||
1578 | 626 | >>> print extract_text(get_review_diff()) | 615 | >>> print extract_text(get_review_diff()) |
1579 | 627 | Preview Diff | 616 | Preview Diff |
1580 | 628 | Empty | 617 | Empty |
1581 | 618 | |||
1582 | 619 | |||
1583 | 620 | Merge proposal details shown on the branch page | ||
1584 | 621 | ----------------------------------------------- | ||
1585 | 622 | |||
1586 | 623 | A branch that has a merge proposal, but no requested reviews shows this on the | ||
1587 | 624 | branch page. | ||
1588 | 625 | |||
1589 | 626 | >>> nopriv_browser.open('http://code.launchpad.dev/~fred/fooix/feature') | ||
1590 | 627 | >>> nopriv_browser.getLink('Propose for merging').click() | ||
1591 | 628 | >>> nopriv_browser.getControl('Reviewer').value = '' | ||
1592 | 629 | >>> nopriv_browser.getControl('Propose Merge').click() | ||
1593 | 630 | >>> nopriv_browser.getLink('lp://dev/~fred/fooix/feature').click() | ||
1594 | 631 | >>> print_tag_with_id(nopriv_browser.contents, 'landing-targets') | ||
1595 | 632 | Ready for review for merging into lp://dev/fooix | ||
1596 | 633 | No reviews requested | ||
1597 | 634 | |||
1598 | 635 | If there are reviews either pending or completed these are also shown. | ||
1599 | 636 | |||
1600 | 637 | The api tag is a hidden anchor that holds the URL for the merge proposal api | ||
1601 | 638 | access. | ||
1602 | 639 | |||
1603 | 640 | >>> nopriv_browser.open('http://code.launchpad.dev/~fred/fooix/proposed') | ||
1604 | 641 | >>> print_tag_with_id(nopriv_browser.contents, 'landing-targets') | ||
1605 | 642 | Ready for review for merging into lp://dev/fooix | ||
1606 | 643 | Eric the Viking: Pending (code) requested ... ago | ||
1607 | 644 | Diff: 47 lines (+7/-0) 2 files modified api | ||
1608 | 629 | 645 | ||
1609 | === modified file 'lib/lp/code/templates/branch-index.pt' | |||
1610 | --- lib/lp/code/templates/branch-index.pt 2009-10-11 02:33:30 +0000 | |||
1611 | +++ lib/lp/code/templates/branch-index.pt 2009-11-18 15:05:39 +0000 | |||
1612 | @@ -25,6 +25,20 @@ | |||
1613 | 25 | #download-url dd span.branch-url, #upload-url dd span.branch-url { | 25 | #download-url dd span.branch-url, #upload-url dd span.branch-url { |
1614 | 26 | font-weight: normal; | 26 | font-weight: normal; |
1615 | 27 | } | 27 | } |
1616 | 28 | dl.reviews dd { | ||
1617 | 29 | padding-left: 2em; | ||
1618 | 30 | margin-bottom: 0; | ||
1619 | 31 | } | ||
1620 | 32 | .yui-diff-overlay-hidden { | ||
1621 | 33 | display: none; | ||
1622 | 34 | } | ||
1623 | 35 | .yui-diff-overlay { | ||
1624 | 36 | width: 80%; | ||
1625 | 37 | } | ||
1626 | 38 | .yui-diff-overlay #yui-pretty-overlay-modal { | ||
1627 | 39 | width: auto; | ||
1628 | 40 | color: black; | ||
1629 | 41 | } | ||
1630 | 28 | 42 | ||
1631 | 29 | </style> | 43 | </style> |
1632 | 30 | <script type="text/javascript" | 44 | <script type="text/javascript" |
1633 | @@ -43,12 +57,20 @@ | |||
1634 | 43 | tal:attributes="src string:${lp_js}/code/branchstatus.js"> | 57 | tal:attributes="src string:${lp_js}/code/branchstatus.js"> |
1635 | 44 | </script> | 58 | </script> |
1636 | 45 | <script type="text/javascript" | 59 | <script type="text/javascript" |
1637 | 60 | tal:condition="devmode" | ||
1638 | 61 | tal:define="lp_js string:${icingroot}/build" | ||
1639 | 62 | tal:attributes="src string:${lp_js}/code/popupdiff.js"> | ||
1640 | 63 | </script> | ||
1641 | 64 | <script type="text/javascript" | ||
1642 | 46 | tal:content="string: | 65 | tal:content="string: |
1643 | 47 | YUI().use('node', 'event', 'widget', 'plugin', 'overlay', | 66 | YUI().use('node', 'event', 'widget', 'plugin', 'overlay', |
1645 | 48 | 'lazr.choiceedit', 'code.branchstatus', function(Y) { | 67 | 'lazr.choiceedit', 'code.branchstatus', |
1646 | 68 | 'code.branchmergeproposal.popupdiff', | ||
1647 | 69 | function(Y) { | ||
1648 | 49 | Y.on('load', | 70 | Y.on('load', |
1649 | 50 | function(e) { | 71 | function(e) { |
1650 | 51 | Y.branchstatus.connect_status(${view/status_config}); | 72 | Y.branchstatus.connect_status(${view/status_config}); |
1651 | 73 | Y.code.branchmergeproposal.popupdiff.connect_diff_links(); | ||
1652 | 52 | }, | 74 | }, |
1653 | 53 | window); | 75 | window); |
1654 | 54 | }); | 76 | }); |
1655 | 55 | 77 | ||
1656 | === modified file 'lib/lp/code/templates/branch-pending-merges.pt' | |||
1657 | --- lib/lp/code/templates/branch-pending-merges.pt 2009-09-18 21:20:42 +0000 | |||
1658 | +++ lib/lp/code/templates/branch-pending-merges.pt 2009-11-18 15:05:38 +0000 | |||
1659 | @@ -36,6 +36,7 @@ | |||
1660 | 36 | <div> | 36 | <div> |
1661 | 37 | <tal:merge-fragment tal:replace="structure mergeproposal/@@+link-summary" /> | 37 | <tal:merge-fragment tal:replace="structure mergeproposal/@@+link-summary" /> |
1662 | 38 | </div> | 38 | </div> |
1663 | 39 | <tal:merge-fragment tal:replace="structure mergeproposal/@@+vote-summary" /> | ||
1664 | 39 | </tal:landing-candidates> | 40 | </tal:landing-candidates> |
1665 | 40 | </div> | 41 | </div> |
1666 | 41 | 42 | ||
1667 | 42 | 43 | ||
1668 | === added file 'lib/lp/code/templates/branchmergeproposal-diff.pt' | |||
1669 | --- lib/lp/code/templates/branchmergeproposal-diff.pt 1970-01-01 00:00:00 +0000 | |||
1670 | +++ lib/lp/code/templates/branchmergeproposal-diff.pt 2009-11-18 15:05:39 +0000 | |||
1671 | @@ -0,0 +1,33 @@ | |||
1672 | 1 | <tal:root | ||
1673 | 2 | xmlns:tal="http://xml.zope.org/namespaces/tal" | ||
1674 | 3 | > | ||
1675 | 4 | |||
1676 | 5 | <div class="boardComment attachment diff" | ||
1677 | 6 | tal:define="attachment view/preview_diff/diff_text"> | ||
1678 | 7 | <tal:real-diff condition="attachment"> | ||
1679 | 8 | <div class="boardCommentDetails filename"> | ||
1680 | 9 | <ul class="horizontal"> | ||
1681 | 10 | <li> | ||
1682 | 11 | <a tal:attributes="href attachment/getURL"> | ||
1683 | 12 | <img src="/@@/download"/> | ||
1684 | 13 | Download diff | ||
1685 | 14 | </a> | ||
1686 | 15 | </li> | ||
1687 | 16 | </ul> | ||
1688 | 17 | </div> | ||
1689 | 18 | <div class="boardCommentBody attachmentBody"> | ||
1690 | 19 | <tal:diff replace="structure view/preview_diff_text/fmt:diff" /> | ||
1691 | 20 | </div> | ||
1692 | 21 | <div class="boardCommentFooter" | ||
1693 | 22 | tal:condition="view/diff_oversized"> | ||
1694 | 23 | The diff has been truncated for viewing. | ||
1695 | 24 | </div> | ||
1696 | 25 | </tal:real-diff> | ||
1697 | 26 | <tal:empty-diff condition="not: attachment"> | ||
1698 | 27 | <div class="boardCommentBody attachmentBody"> | ||
1699 | 28 | <em>Empty</em> | ||
1700 | 29 | </div> | ||
1701 | 30 | </tal:empty-diff> | ||
1702 | 31 | </div> | ||
1703 | 32 | |||
1704 | 33 | </tal:root> | ||
1705 | 0 | \ No newline at end of file | 34 | \ No newline at end of file |
1706 | 1 | 35 | ||
1707 | === modified file 'lib/lp/code/templates/branchmergeproposal-index.pt' | |||
1708 | --- lib/lp/code/templates/branchmergeproposal-index.pt 2009-11-10 21:49:38 +0000 | |||
1709 | +++ lib/lp/code/templates/branchmergeproposal-index.pt 2009-11-18 15:05:38 +0000 | |||
1710 | @@ -128,34 +128,8 @@ | |||
1711 | 128 | </div> | 128 | </div> |
1712 | 129 | 129 | ||
1713 | 130 | <div id="review-diff" tal:condition="view/preview_diff"> | 130 | <div id="review-diff" tal:condition="view/preview_diff"> |
1742 | 131 | <h2>Preview Diff</h2> | 131 | <h2>Preview Diff</h2> |
1743 | 132 | <div class="boardComment attachment" | 132 | <div tal:replace="structure context/@@++diff"/> |
1716 | 133 | tal:define="attachment view/preview_diff/diff_text"> | ||
1717 | 134 | <tal:real-diff condition="attachment"> | ||
1718 | 135 | <div class="boardCommentDetails filename"> | ||
1719 | 136 | <ul class="horizontal"> | ||
1720 | 137 | <li> | ||
1721 | 138 | <a tal:attributes="href attachment/getURL"> | ||
1722 | 139 | <img src="/@@/download"/> | ||
1723 | 140 | Download diff | ||
1724 | 141 | </a> | ||
1725 | 142 | </li> | ||
1726 | 143 | </ul> | ||
1727 | 144 | </div> | ||
1728 | 145 | <div class="boardCommentBody attachmentBody"> | ||
1729 | 146 | <tal:diff replace="structure view/preview_diff_text/fmt:diff" /> | ||
1730 | 147 | </div> | ||
1731 | 148 | <div class="boardCommentFooter" | ||
1732 | 149 | tal:condition="view/review_diff_oversized"> | ||
1733 | 150 | The diff has been truncated for viewing. | ||
1734 | 151 | </div> | ||
1735 | 152 | </tal:real-diff> | ||
1736 | 153 | <tal:empty-diff condition="not: attachment"> | ||
1737 | 154 | <div class="boardCommentBody attachmentBody"> | ||
1738 | 155 | <em>Empty</em> | ||
1739 | 156 | </div> | ||
1740 | 157 | </tal:empty-diff> | ||
1741 | 158 | </div> | ||
1744 | 159 | </div> | 133 | </div> |
1745 | 160 | 134 | ||
1746 | 161 | <tal:script | 135 | <tal:script |
1747 | 162 | 136 | ||
1748 | === renamed file 'lib/lp/code/templates/branch-merge-link-summary.pt' => 'lib/lp/code/templates/branchmergeproposal-link-summary.pt' | |||
1749 | --- lib/lp/code/templates/branch-merge-link-summary.pt 2009-09-22 16:31:42 +0000 | |||
1750 | +++ lib/lp/code/templates/branchmergeproposal-link-summary.pt 2009-11-18 15:05:39 +0000 | |||
1751 | @@ -4,7 +4,8 @@ | |||
1752 | 4 | xmlns:i18n="http://xml.zope.org/namespaces/i18n"> | 4 | xmlns:i18n="http://xml.zope.org/namespaces/i18n"> |
1753 | 5 | 5 | ||
1754 | 6 | <img src="/@@/merge-proposal-icon" /> | 6 | <img src="/@@/merge-proposal-icon" /> |
1756 | 7 | <a tal:attributes="href context/fmt:url" | 7 | <a tal:attributes="href context/fmt:url; |
1757 | 8 | title view/status_title" | ||
1758 | 8 | tal:content="view/friendly_text">Approved</a> | 9 | tal:content="view/friendly_text">Approved</a> |
1759 | 9 | <tal:for-merging condition="not: context/queue_status/enumvalue:MERGED"> | 10 | <tal:for-merging condition="not: context/queue_status/enumvalue:MERGED"> |
1760 | 10 | for merging | 11 | for merging |
1761 | 11 | 12 | ||
1762 | === added file 'lib/lp/code/templates/branchmergeproposal-vote-summary.pt' | |||
1763 | --- lib/lp/code/templates/branchmergeproposal-vote-summary.pt 1970-01-01 00:00:00 +0000 | |||
1764 | +++ lib/lp/code/templates/branchmergeproposal-vote-summary.pt 2009-11-18 15:05:38 +0000 | |||
1765 | @@ -0,0 +1,52 @@ | |||
1766 | 1 | <tal:root | ||
1767 | 2 | xmlns:tal="http://xml.zope.org/namespaces/tal" | ||
1768 | 3 | xmlns:metal="http://xml.zope.org/namespaces/metal" | ||
1769 | 4 | xmlns:i18n="http://xml.zope.org/namespaces/i18n"> | ||
1770 | 5 | |||
1771 | 6 | <tal:comment condition="nothing"> | ||
1772 | 7 | <!-- | ||
1773 | 8 | Yet again we are bitten by white space issues, so some tags in this | ||
1774 | 9 | template have closing brackets on following lines, and not breaks | ||
1775 | 10 | between some tags. | ||
1776 | 11 | --> | ||
1777 | 12 | </tal:comment> | ||
1778 | 13 | |||
1779 | 14 | <dl class="reviews"> | ||
1780 | 15 | <dd tal:condition="not: view/reviews"> | ||
1781 | 16 | No reviews requested | ||
1782 | 17 | </dd> | ||
1783 | 18 | <dd tal:repeat="review view/current_reviews"> | ||
1784 | 19 | <tal:reviewer replace="structure review/reviewer/fmt:link:mainsite"> | ||
1785 | 20 | Eric the Reviewer</tal:reviewer | ||
1786 | 21 | ><tal:community condition="not: review/trusted"> | ||
1787 | 22 | (community)</tal:community>: | ||
1788 | 23 | <span tal:attributes="class string:vote${review/comment/vote/name}" | ||
1789 | 24 | tal:content="review/status_text"> | ||
1790 | 25 | Approved | ||
1791 | 26 | </span> | ||
1792 | 27 | <tal:vote-tags condition="review/review_type_str"> | ||
1793 | 28 | (<tal:tag replace="review/review_type_str"/>) | ||
1794 | 29 | </tal:vote-tags> | ||
1795 | 30 | <tal:date replace="review/date_of_comment/fmt:displaydate" /> | ||
1796 | 31 | </dd> | ||
1797 | 32 | <dd tal:repeat="review view/requested_reviews" | ||
1798 | 33 | tal:attributes="id string:review-${review/reviewer/name}"> | ||
1799 | 34 | <tal:reviewer | ||
1800 | 35 | tal:replace="structure review/reviewer/fmt:link:mainsite" />: | ||
1801 | 36 | |||
1802 | 37 | <span class="votePENDING">Pending</span> | ||
1803 | 38 | <tal:vote-tags condition="review/review_type_str"> | ||
1804 | 39 | (<tal:tag replace="review/review_type_str"/>) | ||
1805 | 40 | </tal:vote-tags> | ||
1806 | 41 | requested | ||
1807 | 42 | <tal:date replace="review/date_requested/fmt:approximatedate"/> | ||
1808 | 43 | </dd> | ||
1809 | 44 | <dd tal:condition="context/preview_diff" | ||
1810 | 45 | tal:attributes="class string:popup-diff mp-${context/id}"> | ||
1811 | 46 | Diff: <tal:diff replace="structure context/preview_diff/fmt:link"/> | ||
1812 | 47 | <a class="api-ref" style="display:none" | ||
1813 | 48 | tal:attributes="href context/preview_diff/fmt:api_url">api</a> | ||
1814 | 49 | </dd> | ||
1815 | 50 | |||
1816 | 51 | </dl> | ||
1817 | 52 | </tal:root> | ||
1818 | 0 | 53 | ||
1819 | === modified file 'lib/lp/code/tests/helpers.py' | |||
1820 | --- lib/lp/code/tests/helpers.py 2009-10-26 18:40:04 +0000 | |||
1821 | +++ lib/lp/code/tests/helpers.py 2009-11-18 15:05:38 +0000 | |||
1822 | @@ -6,10 +6,12 @@ | |||
1823 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
1824 | 7 | __all__ = [ | 7 | __all__ = [ |
1825 | 8 | 'make_linked_package_branch', | 8 | 'make_linked_package_branch', |
1826 | 9 | 'make_erics_fooix_project', | ||
1827 | 9 | ] | 10 | ] |
1828 | 10 | 11 | ||
1829 | 11 | 12 | ||
1830 | 12 | from datetime import timedelta | 13 | from datetime import timedelta |
1831 | 14 | from difflib import unified_diff | ||
1832 | 13 | from itertools import count | 15 | from itertools import count |
1833 | 14 | 16 | ||
1834 | 15 | from zope.component import getUtility | 17 | from zope.component import getUtility |
1835 | @@ -23,6 +25,46 @@ | |||
1836 | 23 | from lp.testing import time_counter | 25 | from lp.testing import time_counter |
1837 | 24 | 26 | ||
1838 | 25 | 27 | ||
1839 | 28 | def make_erics_fooix_project(factory): | ||
1840 | 29 | """Make Eric, the Fooix project, and some branches. | ||
1841 | 30 | |||
1842 | 31 | :return: a dict of objects to put into local scope. | ||
1843 | 32 | """ | ||
1844 | 33 | result = {} | ||
1845 | 34 | eric = factory.makePerson( | ||
1846 | 35 | name='eric', displayname='Eric the Viking', | ||
1847 | 36 | email='eric@example.com', password='test') | ||
1848 | 37 | fooix = factory.makeProduct( | ||
1849 | 38 | name='fooix', displayname='Fooix', owner=eric) | ||
1850 | 39 | trunk = factory.makeProductBranch( | ||
1851 | 40 | owner=eric, product=fooix, name='trunk') | ||
1852 | 41 | removeSecurityProxy(fooix.development_focus).branch = trunk | ||
1853 | 42 | # Development is done by Fred. | ||
1854 | 43 | fred = factory.makePerson( | ||
1855 | 44 | name='fred', displayname='Fred Flintstone', | ||
1856 | 45 | email='fred@example.com', password='test') | ||
1857 | 46 | feature = factory.makeProductBranch( | ||
1858 | 47 | owner=fred, product=fooix, name='feature') | ||
1859 | 48 | proposed = factory.makeProductBranch( | ||
1860 | 49 | owner=fred, product=fooix, name='proposed') | ||
1861 | 50 | bmp = proposed.addLandingTarget( | ||
1862 | 51 | registrant=fred, target_branch=trunk, needs_review=True, | ||
1863 | 52 | review_requests=[(eric, 'code')]) | ||
1864 | 53 | # And fake a diff. | ||
1865 | 54 | naked_bmp = removeSecurityProxy(bmp) | ||
1866 | 55 | preview = removeSecurityProxy(naked_bmp.updatePreviewDiff( | ||
1867 | 56 | ''.join(unified_diff('', 'random content')), u'rev-a', u'rev-b')) | ||
1868 | 57 | naked_bmp.source_branch.last_scanned_id = preview.source_revision_id | ||
1869 | 58 | naked_bmp.target_branch.last_scanned_id = preview.target_revision_id | ||
1870 | 59 | preview.diff_lines_count = 47 | ||
1871 | 60 | preview.added_lines_count = 7 | ||
1872 | 61 | preview.remvoed_lines_count = 13 | ||
1873 | 62 | preview.diffstat = {'file1': (3, 8), 'file2': (4, 5)} | ||
1874 | 63 | return { | ||
1875 | 64 | 'eric': eric, 'fooix': fooix, 'trunk':trunk, 'feature': feature, | ||
1876 | 65 | 'proposed': proposed, 'fred': fred} | ||
1877 | 66 | |||
1878 | 67 | |||
1879 | 26 | def make_linked_package_branch(factory, distribution=None, | 68 | def make_linked_package_branch(factory, distribution=None, |
1880 | 27 | sourcepackagename=None): | 69 | sourcepackagename=None): |
1881 | 28 | """Make a new package branch and make it official.""" | 70 | """Make a new package branch and make it official.""" |
1882 | 29 | 71 | ||
1883 | === modified file 'lib/lp/code/tests/test_directbranchcommit.py' | |||
1884 | --- lib/lp/code/tests/test_directbranchcommit.py 2009-10-02 09:48:30 +0000 | |||
1885 | +++ lib/lp/code/tests/test_directbranchcommit.py 2009-11-18 15:05:39 +0000 | |||
1886 | @@ -37,7 +37,7 @@ | |||
1887 | 37 | 37 | ||
1888 | 38 | self.committer = DirectBranchCommit(self.db_branch) | 38 | self.committer = DirectBranchCommit(self.db_branch) |
1889 | 39 | if update_last_scanned_id: | 39 | if update_last_scanned_id: |
1891 | 40 | self.db_branch.last_scanned_id = ( | 40 | self.committer.last_scanned_id = ( |
1892 | 41 | self.committer.bzrbranch.last_revision()) | 41 | self.committer.bzrbranch.last_revision()) |
1893 | 42 | 42 | ||
1894 | 43 | def _tearDownCommitter(self): | 43 | def _tearDownCommitter(self): |
1895 | 44 | 44 | ||
1896 | === added file 'lib/lp/code/windmill/tests/test_popup_diff.py' | |||
1897 | --- lib/lp/code/windmill/tests/test_popup_diff.py 1970-01-01 00:00:00 +0000 | |||
1898 | +++ lib/lp/code/windmill/tests/test_popup_diff.py 2009-11-18 15:05:39 +0000 | |||
1899 | @@ -0,0 +1,64 @@ | |||
1900 | 1 | # Copyright 2009 Canonical Ltd. This software is licensed under the | ||
1901 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
1902 | 3 | |||
1903 | 4 | """Test for the popup diff.""" | ||
1904 | 5 | |||
1905 | 6 | __metaclass__ = type | ||
1906 | 7 | __all__ = [] | ||
1907 | 8 | |||
1908 | 9 | import transaction | ||
1909 | 10 | import unittest | ||
1910 | 11 | |||
1911 | 12 | import windmill | ||
1912 | 13 | from windmill.authoring import WindmillTestClient | ||
1913 | 14 | |||
1914 | 15 | from canonical.launchpad.windmill.testing.constants import PAGE_LOAD | ||
1915 | 16 | from lp.code.tests.helpers import make_erics_fooix_project | ||
1916 | 17 | from lp.code.windmill.testing import CodeWindmillLayer | ||
1917 | 18 | from lp.testing import TestCaseWithFactory | ||
1918 | 19 | |||
1919 | 20 | |||
1920 | 21 | POPUP_DIFF = ( | ||
1921 | 22 | u'//dd[contains(@class, "popup-diff")]' | ||
1922 | 23 | '/a[contains(@class, "js-action")]') | ||
1923 | 24 | VISIBLE_DIFF = ( | ||
1924 | 25 | u'//table[contains(@class, "yui-diff-overlay") and ' | ||
1925 | 26 | 'not(contains(@class, "yui-diff-overlay-hidden"))]') | ||
1926 | 27 | CLOSE_VISIBLE_DIFF = ( | ||
1927 | 28 | u'//table[contains(@class, "yui-diff-overlay")]' | ||
1928 | 29 | '//a[@class="close-button"]') | ||
1929 | 30 | JS_ONLOAD_EXECUTE_DELAY = 1000 | ||
1930 | 31 | |||
1931 | 32 | |||
1932 | 33 | class TestPopupOnBranchPage(TestCaseWithFactory): | ||
1933 | 34 | """Test the popup diff.""" | ||
1934 | 35 | |||
1935 | 36 | layer = CodeWindmillLayer | ||
1936 | 37 | |||
1937 | 38 | def test_branch_popup_diff(self): | ||
1938 | 39 | """Test branch diff popups.""" | ||
1939 | 40 | client = WindmillTestClient("Branch popup diffs") | ||
1940 | 41 | make_erics_fooix_project(self.factory) | ||
1941 | 42 | transaction.commit() | ||
1942 | 43 | |||
1943 | 44 | start_url = ( | ||
1944 | 45 | windmill.settings['TEST_URL'] + '~fred/fooix/proposed') | ||
1945 | 46 | client.open(url=start_url) | ||
1946 | 47 | client.waits.forPageLoad(timeout=PAGE_LOAD) | ||
1947 | 48 | # Sleep for a bit to make sure that the JS onload has had time to execute. | ||
1948 | 49 | client.waits.sleep(milliseconds=JS_ONLOAD_EXECUTE_DELAY) | ||
1949 | 50 | |||
1950 | 51 | # Make sure that the link anchor has the js-action class. | ||
1951 | 52 | client.asserts.assertNode(xpath=POPUP_DIFF) | ||
1952 | 53 | client.click(xpath=POPUP_DIFF) | ||
1953 | 54 | |||
1954 | 55 | # Wait for the diff to show. | ||
1955 | 56 | client.waits.forElement(xpath=VISIBLE_DIFF) | ||
1956 | 57 | # Click on the close button. | ||
1957 | 58 | client.click(xpath=CLOSE_VISIBLE_DIFF) | ||
1958 | 59 | # Make sure that the diff has gone. | ||
1959 | 60 | client.asserts.assertNotNode(xpath=VISIBLE_DIFF) | ||
1960 | 61 | |||
1961 | 62 | |||
1962 | 63 | def test_suite(): | ||
1963 | 64 | return unittest.TestLoader().loadTestsFromName(__name__) | ||
1964 | 0 | 65 | ||
1965 | === modified file 'lib/lp/services/browser_helpers.py' | |||
1966 | --- lib/lp/services/browser_helpers.py 2009-09-07 14:54:34 +0000 | |||
1967 | +++ lib/lp/services/browser_helpers.py 2009-11-18 15:05:39 +0000 | |||
1968 | @@ -6,6 +6,7 @@ | |||
1969 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
1970 | 7 | __all__ = [ | 7 | __all__ = [ |
1971 | 8 | 'get_user_agent_distroseries', | 8 | 'get_user_agent_distroseries', |
1972 | 9 | 'get_plural_text', | ||
1973 | 9 | ] | 10 | ] |
1974 | 10 | 11 | ||
1975 | 11 | import re | 12 | import re |
1976 | @@ -28,3 +29,10 @@ | |||
1977 | 28 | else: | 29 | else: |
1978 | 29 | return None | 30 | return None |
1979 | 30 | 31 | ||
1980 | 32 | |||
1981 | 33 | def get_plural_text(count, singular, plural): | ||
1982 | 34 | """Return 'singular' if 'count' is 1, 'plural' otherwise.""" | ||
1983 | 35 | if count == 1: | ||
1984 | 36 | return singular | ||
1985 | 37 | else: | ||
1986 | 38 | return plural | ||
1987 | 31 | 39 | ||
1988 | === modified file 'lib/lp/translations/doc/translations-export-to-branch.txt' | |||
1989 | --- lib/lp/translations/doc/translations-export-to-branch.txt 2009-10-20 06:03:46 +0000 | |||
1990 | +++ lib/lp/translations/doc/translations-export-to-branch.txt 2009-11-18 15:05:39 +0000 | |||
1991 | @@ -45,7 +45,7 @@ | |||
1992 | 45 | ... if self.simulate_race: | 45 | ... if self.simulate_race: |
1993 | 46 | ... raise ConcurrentUpdateError("Simulated race condition.") | 46 | ... raise ConcurrentUpdateError("Simulated race condition.") |
1994 | 47 | ... | 47 | ... |
1996 | 48 | ... def commit(self, message=None): | 48 | ... def commit(self, message=None, txn=None): |
1997 | 49 | ... self._checkForRace() | 49 | ... self._checkForRace() |
1998 | 50 | ... self.logger.info("Committed %d file(s)." % self.written_files) | 50 | ... self.logger.info("Committed %d file(s)." % self.written_files) |
1999 | 51 | ... | 51 | ... |
2000 | @@ -129,11 +129,11 @@ | |||
2001 | 129 | 129 | ||
2002 | 130 | >>> transaction.commit() | 130 | >>> transaction.commit() |
2003 | 131 | >>> script = MockExportTranslationsToBranch( | 131 | >>> script = MockExportTranslationsToBranch( |
2005 | 132 | ... 'export-to-branch', test_args=['-vv']) | 132 | ... 'export-to-branch', test_args=[]) |
2006 | 133 | >>> script.main() | 133 | >>> script.main() |
2007 | 134 | INFO Exporting to translations branches. | 134 | INFO Exporting to translations branches. |
2008 | 135 | INFO Exporting Gazblachko trunk series. | 135 | INFO Exporting Gazblachko trunk series. |
2010 | 136 | DEBUG No previous translations commit found. | 136 | DEBUG ... |
2011 | 137 | INFO Writing file 'po/main/nl.po': | 137 | INFO Writing file 'po/main/nl.po': |
2012 | 138 | INFO # ... | 138 | INFO # ... |
2013 | 139 | msgid "" | 139 | msgid "" |
2014 | @@ -143,6 +143,7 @@ | |||
2015 | 143 | msgid "maingazpot msgid" | 143 | msgid "maingazpot msgid" |
2016 | 144 | msgstr "maingazpot msgstr" | 144 | msgstr "maingazpot msgstr" |
2017 | 145 | <BLANKLINE> | 145 | <BLANKLINE> |
2018 | 146 | DEBUG ... | ||
2019 | 146 | INFO Writing file 'po/module/nl.po': | 147 | INFO Writing file 'po/module/nl.po': |
2020 | 147 | INFO # ... | 148 | INFO # ... |
2021 | 148 | msgid "" | 149 | msgid "" |
2022 | @@ -152,6 +153,8 @@ | |||
2023 | 152 | <BLANKLINE> | 153 | <BLANKLINE> |
2024 | 153 | msgid "gazmod msgid" | 154 | msgid "gazmod msgid" |
2025 | 154 | msgstr "gazmod msgstr" | 155 | msgstr "gazmod msgstr" |
2026 | 156 | <BLANKLINE> | ||
2027 | 157 | DEBUG ... | ||
2028 | 155 | INFO Committed 2 file(s). | 158 | INFO Committed 2 file(s). |
2029 | 156 | INFO Unlock. | 159 | INFO Unlock. |
2030 | 157 | INFO Processed 1 item(s); 0 failure(s). | 160 | INFO Processed 1 item(s); 0 failure(s). |
2031 | @@ -186,6 +189,7 @@ | |||
2032 | 186 | >>> script.main() | 189 | >>> script.main() |
2033 | 187 | INFO Exporting to translations branches. | 190 | INFO Exporting to translations branches. |
2034 | 188 | INFO Exporting Gazblachko trunk series. | 191 | INFO Exporting Gazblachko trunk series. |
2035 | 192 | DEBUG .... | ||
2036 | 189 | DEBUG Last commit was at .... | 193 | DEBUG Last commit was at .... |
2037 | 190 | INFO Unlock. | 194 | INFO Unlock. |
2038 | 191 | INFO Processed 1 item(s); 0 failure(s). | 195 | INFO Processed 1 item(s); 0 failure(s). |
2039 | @@ -197,6 +201,7 @@ | |||
2040 | 197 | >>> script.main() | 201 | >>> script.main() |
2041 | 198 | INFO Exporting to translations branches. | 202 | INFO Exporting to translations branches. |
2042 | 199 | INFO Exporting Gazblachko trunk series. | 203 | INFO Exporting Gazblachko trunk series. |
2043 | 204 | DEBUG .... | ||
2044 | 200 | DEBUG Last commit was at ... | 205 | DEBUG Last commit was at ... |
2045 | 201 | INFO Writing file 'po/main/nl.po': | 206 | INFO Writing file 'po/main/nl.po': |
2046 | 202 | INFO ... | 207 | INFO ... |
2047 | @@ -228,11 +233,14 @@ | |||
2048 | 228 | >>> script.main() | 233 | >>> script.main() |
2049 | 229 | INFO Exporting to translations branches. | 234 | INFO Exporting to translations branches. |
2050 | 230 | INFO Exporting Gazblachko trunk series. | 235 | INFO Exporting Gazblachko trunk series. |
2051 | 236 | DEBUG .... | ||
2052 | 231 | DEBUG No previous translations commit found. | 237 | DEBUG No previous translations commit found. |
2053 | 238 | DEBUG .... | ||
2054 | 232 | INFO Writing file 'po/main/nl.po': | 239 | INFO Writing file 'po/main/nl.po': |
2055 | 233 | ... | 240 | ... |
2056 | 234 | msgstr "gazmod msgstr" | 241 | msgstr "gazmod msgstr" |
2057 | 235 | <BLANKLINE> | 242 | <BLANKLINE> |
2058 | 243 | DEBUG ... | ||
2059 | 236 | INFO Unlock. | 244 | INFO Unlock. |
2060 | 237 | ERROR Failure: Simulated race condition. | 245 | ERROR Failure: Simulated race condition. |
2061 | 238 | INFO Processed 1 item(s); 1 failure(s). | 246 | INFO Processed 1 item(s); 1 failure(s). |
2062 | 239 | 247 | ||
2063 | === modified file 'lib/lp/translations/scripts/translations_to_branch.py' | |||
2064 | --- lib/lp/translations/scripts/translations_to_branch.py 2009-10-20 06:03:46 +0000 | |||
2065 | +++ lib/lp/translations/scripts/translations_to_branch.py 2009-11-18 15:05:39 +0000 | |||
2066 | @@ -119,7 +119,7 @@ | |||
2067 | 119 | def _commit(self, source, committer): | 119 | def _commit(self, source, committer): |
2068 | 120 | """Commit changes to branch. Check for race conditions.""" | 120 | """Commit changes to branch. Check for race conditions.""" |
2069 | 121 | self._checkForObjections(source) | 121 | self._checkForObjections(source) |
2071 | 122 | committer.commit(self.commit_message) | 122 | committer.commit(self.commit_message, txn=self.txn) |
2072 | 123 | 123 | ||
2073 | 124 | def _isTranslationsCommit(self, revision): | 124 | def _isTranslationsCommit(self, revision): |
2074 | 125 | """Is `revision` an automatic translations commit?""" | 125 | """Is `revision` an automatic translations commit?""" |
2075 | @@ -159,6 +159,9 @@ | |||
2076 | 159 | self._checkForObjections(source) | 159 | self._checkForObjections(source) |
2077 | 160 | 160 | ||
2078 | 161 | committer = self._prepareBranchCommit(source.translations_branch) | 161 | committer = self._prepareBranchCommit(source.translations_branch) |
2079 | 162 | self.logger.debug("Created DirectBranchCommit.") | ||
2080 | 163 | if self.txn: | ||
2081 | 164 | self.txn.commit() | ||
2082 | 162 | 165 | ||
2083 | 163 | bzr_branch = committer.bzrbranch | 166 | bzr_branch = committer.bzrbranch |
2084 | 164 | 167 | ||
2085 | @@ -185,15 +188,17 @@ | |||
2086 | 185 | base_path = os.path.dirname(template.path) | 188 | base_path = os.path.dirname(template.path) |
2087 | 186 | 189 | ||
2088 | 187 | for pofile in template.pofiles: | 190 | for pofile in template.pofiles: |
2089 | 188 | |||
2090 | 189 | has_changed = ( | 191 | has_changed = ( |
2091 | 190 | changed_since is None or | 192 | changed_since is None or |
2092 | 191 | pofile.date_changed > changed_since) | 193 | pofile.date_changed > changed_since) |
2093 | 192 | if not has_changed: | 194 | if not has_changed: |
2094 | 193 | continue | 195 | continue |
2095 | 194 | 196 | ||
2096 | 197 | language_code = pofile.getFullLanguageCode() | ||
2097 | 198 | self.logger.debug("Exporting %s." % language_code) | ||
2098 | 199 | |||
2099 | 195 | pofile_path = os.path.join( | 200 | pofile_path = os.path.join( |
2101 | 196 | base_path, pofile.getFullLanguageCode() + '.po') | 201 | base_path, language_code + '.po') |
2102 | 197 | pofile_contents = pofile.export() | 202 | pofile_contents = pofile.export() |
2103 | 198 | 203 | ||
2104 | 199 | committer.writeFile(pofile_path, pofile_contents) | 204 | committer.writeFile(pofile_path, pofile_contents) |
2105 | @@ -210,6 +215,7 @@ | |||
2106 | 210 | template.clearPOFileCache() | 215 | template.clearPOFileCache() |
2107 | 211 | 216 | ||
2108 | 212 | if change_count > 0: | 217 | if change_count > 0: |
2109 | 218 | self.logger.debug("Writing to branch.") | ||
2110 | 213 | self._commit(source, committer) | 219 | self._commit(source, committer) |
2111 | 214 | finally: | 220 | finally: |
2112 | 215 | committer.unlock() | 221 | committer.unlock() |
2113 | @@ -231,13 +237,14 @@ | |||
2114 | 231 | raise | 237 | raise |
2115 | 232 | except Exception, e: | 238 | except Exception, e: |
2116 | 233 | items_failed += 1 | 239 | items_failed += 1 |
2118 | 234 | self.logger.error("Failure: %s" % e) | 240 | message = unicode(e) |
2119 | 241 | if message == u'': | ||
2120 | 242 | message = e.__class__.__name__ | ||
2121 | 243 | self.logger.error("Failure: %s" % message) | ||
2122 | 235 | if self.txn: | 244 | if self.txn: |
2123 | 236 | self.txn.abort() | 245 | self.txn.abort() |
2124 | 237 | 246 | ||
2125 | 238 | items_done += 1 | 247 | items_done += 1 |
2126 | 239 | if self.txn: | ||
2127 | 240 | self.txn.begin() | ||
2128 | 241 | 248 | ||
2129 | 242 | self.logger.info("Processed %d item(s); %d failure(s)." % ( | 249 | self.logger.info("Processed %d item(s); %d failure(s)." % ( |
2130 | 243 | items_done, items_failed)) | 250 | items_done, items_failed)) |
2131 | 244 | 251 | ||
2132 | === modified file 'lib/lp/translations/utilities/gettext_po_exporter.py' | |||
2133 | --- lib/lp/translations/utilities/gettext_po_exporter.py 2009-11-02 00:10:05 +0000 | |||
2134 | +++ lib/lp/translations/utilities/gettext_po_exporter.py 2009-11-18 15:05:39 +0000 | |||
2135 | @@ -14,6 +14,7 @@ | |||
2136 | 14 | 'GettextPOExporter', | 14 | 'GettextPOExporter', |
2137 | 15 | ] | 15 | ] |
2138 | 16 | 16 | ||
2139 | 17 | import logging | ||
2140 | 17 | import os | 18 | import os |
2141 | 18 | 19 | ||
2142 | 19 | from zope.interface import implements | 20 | from zope.interface import implements |
2143 | @@ -344,8 +345,20 @@ | |||
2144 | 344 | raise UnicodeEncodeError( | 345 | raise UnicodeEncodeError( |
2145 | 345 | '%s:\n%s' % (file_path, str(error))) | 346 | '%s:\n%s' % (file_path, str(error))) |
2146 | 346 | 347 | ||
2149 | 347 | # This message cannot be represented in current encoding, | 348 | # This message cannot be represented in the current |
2150 | 348 | # change to UTF-8 and try again. | 349 | # encoding. |
2151 | 350 | if translation_file.path: | ||
2152 | 351 | file_description = translation_file.path | ||
2153 | 352 | elif translation_file.language_code: | ||
2154 | 353 | file_description = ( | ||
2155 | 354 | "%s translation" % translation_file.language_code) | ||
2156 | 355 | else: | ||
2157 | 356 | file_description = "template" | ||
2158 | 357 | logging.info( | ||
2159 | 358 | "Can't represent %s as %s; using UTF-8 instead." % ( | ||
2160 | 359 | file_description, | ||
2161 | 360 | translation_file.header.charset.upper())) | ||
2162 | 361 | |||
2163 | 349 | old_charset = translation_file.header.charset | 362 | old_charset = translation_file.header.charset |
2164 | 350 | translation_file.header.charset = 'UTF-8' | 363 | translation_file.header.charset = 'UTF-8' |
2165 | 351 | # We need to update the header too. | 364 | # We need to update the header too. |
= Bug 427278 =
Translations exports to bzr branches can sometimes keep transactions open for a while. There's no particular reason to go that far, and it's Not Nice. We've had the script idle-killed a few times.
We've done several things that prevented a repetition:
1. We no longer export all translations for a template; now we only do ones that have changed since the last export. Makes the exports a lot faster.
2. The deadline for long transactions was extended, and obviously that's pretty effective at stopping the alarm bells from ringing. But ideal it is not.
3. The branch you see here makes the script commit more often around the likely pain points.
There's really only one catch: DirectBranchCommit checks the branch for conflicting changes. For that it needs to compare the latest revision in the branch to the last_scanned_id as stored in the database when the DirectBranchCommit was constructed. So instead of getting the last_scanned_id at the point where it's referenced, DirectBranchCommit will now retrieve and store it at construction time. This should also close off a race condition that may or may not exist in the case where database transactions are committed while a DirectBranchCommit is active.
Passing a transaction into DirectBranchCom mit.commit, as I do here, is a bit weird. There are reasons for it:
* Operations that access branch storage can take a long time, but don't need database access. That makes them excellent points to close off any ongoing transaction. The framework starts new transactions on demand, and that's going to happen only after the bzr operation completes. Until that time, the committer avoids being "idle in transaction" on the database.
* Nowadays txn is global, so we could just "import transaction" and "transaction. commit( )". But passing it as an object makes it easy to mock transactions in tests. Also, passing a boolean is slightly more error-prone. Named parameters are a big improvement- -"dbc.commit( message, commit=True)"--but still relatively sensitive to mistakes.
* Passing the transaction object to commit rather than the constructor makes it clear that you're not giving out a blanket license to commit. The DirectBranchCommit can commit only inside that one particular method call. Don't hide transaction boundaries in the program flow.
A lot of the actual diff went into nicer logging.
== Test, demo & Q/A ==
Test: *to.*branch' -t 'direct. *branch. *commit'
{{{
./bin/test -vv -t 'translations.
}}}
Create a branch on staging, and set it as the productseries branch and the translations export branch for some project. Enable translations and two-way bzr synchronization. Push a template to the branch.
Watch the project's import queue; wait for the template to be imported. Then translate a string in the template. Watch the revision log for the branch; an automatic translations commit should appear there within the hour.
No lint.
Jeroen