Merge lp:~jtv/launchpad/bug-427278 into lp:launchpad/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
Reviewer Review Type Date Requested Status
Barry Warsaw (community) Abstain
Review via email: mp+14994@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= 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 DirectBranchCommit.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:
{{{
./bin/test -vv -t 'translations.*to.*branch' -t 'direct.*branch.*commit'
}}}

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

Revision history for this message
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
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
=== modified file 'database/replication/Makefile'
--- database/replication/Makefile 2009-11-13 10:41:58 +0000
+++ database/replication/Makefile 2009-11-18 15:05:39 +0000
@@ -36,6 +36,8 @@
36STAGING_CONFIG=staging # For swapping fresh db into place.36STAGING_CONFIG=staging # For swapping fresh db into place.
37STAGING_DUMP=launchpad.dump # Dumpfile to build new staging from.37STAGING_DUMP=launchpad.dump # Dumpfile to build new staging from.
38STAGING_TABLESPACE=pg_default # 'pg_default' for default38STAGING_TABLESPACE=pg_default # 'pg_default' for default
39DOGFOOD_DBNAME=launchpad_dogfood
40DOGFOOD_DUMP=launchpad.dump
3941
40_CONFIG=overridden-on-command-line42_CONFIG=overridden-on-command-line
41_SLAVE_TABLESPACE=pg_default43_SLAVE_TABLESPACE=pg_default
@@ -98,12 +100,13 @@
98 psql -q -d lpmain_staging_new -f authdb_drop.sql100 psql -q -d lpmain_staging_new -f authdb_drop.sql
99 psql -q -d lpmain_staging_new -f authdb_create.sql \101 psql -q -d lpmain_staging_new -f authdb_create.sql \
100 2>&1 | grep -v _sl || true102 2>&1 | grep -v _sl || true
103 # Restore the data
104 pg_restore --dbname=lpmain_staging_new \
105 --no-acl --no-owner --disable-triggers --data-only \
106 --exit-on-error ${STAGING_DUMP}
101 # Uninstall Slony-I if it is installed - a pg_dump of a DB with107 # Uninstall Slony-I if it is installed - a pg_dump of a DB with
102 # Slony-I installed isn't usable without this step.108 # Slony-I installed isn't usable without this step.
103 LPCONFIG=${NEW_STAGING_CONFIG} ./repair-restored-db.py109 LPCONFIG=${NEW_STAGING_CONFIG} ./repair-restored-db.py
104 pg_restore --dbname=lpmain_staging_new \
105 --no-acl --no-owner --disable-triggers --data-only \
106 --exit-on-error ${STAGING_DUMP}
107 # Setup replication110 # Setup replication
108 make _replicate LPCONFIG=${NEW_STAGING_CONFIG} LAG="0 seconds" \111 make _replicate LPCONFIG=${NEW_STAGING_CONFIG} LAG="0 seconds" \
109 _MASTER=lpmain_staging_new _SLAVE=lpmain_staging_slave_new \112 _MASTER=lpmain_staging_new _SLAVE=lpmain_staging_slave_new \
@@ -138,10 +141,10 @@
138 psql -q -d ${DOGFOOD_DBNAME} -f authdb_drop.sql141 psql -q -d ${DOGFOOD_DBNAME} -f authdb_drop.sql
139 psql -q -d ${DOGFOOD_DBNAME} -f authdb_create.sql \142 psql -q -d ${DOGFOOD_DBNAME} -f authdb_create.sql \
140 2>&1 | grep -v _sl || true143 2>&1 | grep -v _sl || true
141 ./repair-restored-db.py -d ${DOGFOOD_DBNAME}
142 pg_restore --dbname=${DOGFOOD_DBNAME} --no-acl --no-owner \144 pg_restore --dbname=${DOGFOOD_DBNAME} --no-acl --no-owner \
143 --disable-triggers --data-only \145 --disable-triggers --data-only \
144 --exit-on-error ${DOGFOOD_DUMP}146 --exit-on-error ${DOGFOOD_DUMP}
147 ./repair-restored-db.py -d ${DOGFOOD_DBNAME}
145 ../schema/upgrade.py -d ${DOGFOOD_DBNAME}148 ../schema/upgrade.py -d ${DOGFOOD_DBNAME}
146 ../schema/fti.py -d ${DOGFOOD_DBNAME}149 ../schema/fti.py -d ${DOGFOOD_DBNAME}
147 ../schema/security.py -d ${DOGFOOD_DBNAME}150 ../schema/security.py -d ${DOGFOOD_DBNAME}
148151
=== modified file 'database/replication/repair-restored-db.py'
--- database/replication/repair-restored-db.py 2009-10-17 14:06:03 +0000
+++ database/replication/repair-restored-db.py 2009-11-18 15:05:38 +0000
@@ -27,7 +27,8 @@
2727
28from canonical.config import config28from canonical.config import config
29from canonical.database.postgresql import ConnectionString29from canonical.database.postgresql import ConnectionString
30from canonical.database.sqlbase import connect, quote30from canonical.database.sqlbase import (
31 connect, quote, ISOLATION_LEVEL_AUTOCOMMIT)
31from canonical.launchpad.scripts import db_options, logger_options, logger32from canonical.launchpad.scripts import db_options, logger_options, logger
3233
33import replication.helpers34import replication.helpers
@@ -44,12 +45,23 @@
4445
45 log = logger(options)46 log = logger(options)
4647
47 con = connect(options.dbuser)48 con = connect(options.dbuser, isolation=ISOLATION_LEVEL_AUTOCOMMIT)
4849
49 if not replication.helpers.slony_installed(con):50 if not replication.helpers.slony_installed(con):
50 log.info("Slony-I not installed. Nothing to do.")51 log.info("Slony-I not installed. Nothing to do.")
51 return 052 return 0
5253
54 if not repair_with_slonik(log, options, con):
55 repair_with_drop_schema(log, con)
56
57 return 0
58
59
60def repair_with_slonik(log, options, con):
61 """Attempt to uninstall Slony-I via 'UNINSTALL NODE' per best practice.
62
63 Returns True on success, False if unable to do so for any reason.
64 """
53 cur = con.cursor()65 cur = con.cursor()
5466
55 # Determine the node id the database thinks it is.67 # Determine the node id the database thinks it is.
@@ -60,27 +72,19 @@
60 cur.execute(cmd)72 cur.execute(cmd)
61 node_id = cur.fetchone()[0]73 node_id = cur.fetchone()[0]
62 log.debug("Node Id is %d" % node_id)74 log.debug("Node Id is %d" % node_id)
75
76 # Get a list of set ids in the database.
77 cur.execute(
78 "SELECT DISTINCT set_id FROM %s.sl_set"
79 % replication.helpers.CLUSTER_NAMESPACE)
80 set_ids = set(row[0] for row in cur.fetchall())
81 log.debug("Set Ids are %s" % repr(set_ids))
82
63 except psycopg2.InternalError:83 except psycopg2.InternalError:
64 # Not enough information to determine node id. Possibly84 # Not enough information to determine node id. Possibly
65 # this is an empty database. Just drop the _sl schema as85 # this is an empty database.
66 # it is 'good enough' with Slony-I 1.2 - this mechanism86 log.debug('Broken or no Slony-I install.')
67 # fails with Slony added primary keys, but we don't do that.87 return False
68 con.rollback()
69 cur = con.cursor()
70 cur.execute("DROP SCHEMA _sl CASCADE")
71 con.commit()
72 return 0
73
74 # Get a list of set ids in the database.
75 cur.execute(
76 "SELECT DISTINCT set_id FROM %s.sl_set"
77 % replication.helpers.CLUSTER_NAMESPACE)
78 set_ids = set(row[0] for row in cur.fetchall())
79 log.debug("Set Ids are %s" % repr(set_ids))
80
81 # Close so we don't block slonik(1)
82 del cur
83 con.close()
8488
85 connection_string = ConnectionString(config.database.main_master)89 connection_string = ConnectionString(config.database.main_master)
86 if options.dbname:90 if options.dbname:
@@ -103,7 +107,22 @@
103 log.debug(line)107 log.debug(line)
104 script = '\n'.join(script)108 script = '\n'.join(script)
105109
106 replication.helpers.execute_slonik(script, auto_preamble=False)110 return replication.helpers.execute_slonik(
111 script, auto_preamble=False, exit_on_fail=False)
112
113
114def repair_with_drop_schema(log, con):
115 """
116 Just drop the _sl schema as it is 'good enough' with Slony-I 1.2.
117
118 This mechanism fails with Slony added primary keys, but we don't
119 do that.
120 """
121 log.info('Fallback mode - dropping _sl schema.')
122 cur = con.cursor()
123 cur.execute("DROP SCHEMA _sl CASCADE")
124 return True
125
107126
108if __name__ == '__main__':127if __name__ == '__main__':
109 sys.exit(main())128 sys.exit(main())
110129
=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css 2009-11-10 22:09:08 +0000
+++ lib/canonical/launchpad/icing/style.css 2009-11-18 15:05:38 +0000
@@ -1753,10 +1753,13 @@
1753 text-align: right;1753 text-align: right;
1754}1754}
1755.codereviewcomment {font: 120% monospace;}1755.codereviewcomment {font: 120% monospace;}
1756.codereviewcomment .attachment, #review-diff .attachment {1756.codereviewcomment .attachment, #review-diff .attachment,
1757div.diff .attachmentBody {
1757 background-color: #f6f6f6;1758 background-color: #f6f6f6;
1758}1759}
17591760div.diff .horizontal {
1761 margin: 0;
1762}
1760body.tab-branches table.listing .section-heading {1763body.tab-branches table.listing .section-heading {
1761 color: #d18b39;1764 color: #d18b39;
1762}1765}
@@ -1765,12 +1768,16 @@
1765 color: #666666;1768 color: #666666;
1766}1769}
17671770
1771.yui-diff-overlay table.diff {
1772 font-size: 98%;
1773}
1768#proposal-summary td, #proposal-summary th { padding-bottom: 2px; }1774#proposal-summary td, #proposal-summary th { padding-bottom: 2px; }
1769table.diff {1775table.diff {
1770 white-space: pre-wrap;1776 white-space: pre-wrap;
1771 font: 120% monospace;1777 font: 120% monospace;
1772 width: 100%;1778 width: 100%;
1773 background-color: white;1779 background-color: white;
1780 margin-bottom: 0.5em;
1774}1781}
1775table.diff .text { width: 99%; padding-left: 0.5em; }1782table.diff .text { width: 99%; padding-left: 0.5em; }
1776table.diff .line-no {1783table.diff .line-no {
17771784
=== modified file 'lib/canonical/launchpad/javascript/code/branchlinks.js'
--- lib/canonical/launchpad/javascript/code/branchlinks.js 2009-08-13 13:16:04 +0000
+++ lib/canonical/launchpad/javascript/code/branchlinks.js 2009-11-18 15:05:39 +0000
@@ -1,4 +1,5 @@
1/** Copyright (c) 2009, Canonical Ltd. All rights reserved.1/* Copyright 2009 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE).
2 *3 *
3 * Code for handling links to branches from bugs and specs.4 * Code for handling links to branches from bugs and specs.
4 *5 *
@@ -24,11 +25,11 @@
24 error_handler = new LP.client.ErrorHandler();25 error_handler = new LP.client.ErrorHandler();
25 error_handler.clearProgressUI = function() {26 error_handler.clearProgressUI = function() {
26 destroy_temporary_spinner();27 destroy_temporary_spinner();
27 }28 };
28 error_handler.showError = function(error_message) {29 error_handler.showError = function(error_message) {
29 alert('An unexpected error has occurred.');30 alert('An unexpected error has occurred.');
30 Y.log(error_message);31 Y.log(error_message);
31 }32 };
3233
33 link_bug_overlay = new Y.lazr.FormOverlay({34 link_bug_overlay = new Y.lazr.FormOverlay({
34 headerContent: '<h2>Link to a bug</h2>',35 headerContent: '<h2>Link to a bug</h2>',
3536
=== modified file 'lib/canonical/launchpad/javascript/code/branchstatus.js'
--- lib/canonical/launchpad/javascript/code/branchstatus.js 2009-11-10 05:12:59 +0000
+++ lib/canonical/launchpad/javascript/code/branchstatus.js 2009-11-18 15:05:38 +0000
@@ -1,4 +1,5 @@
1/** Copyright (c) 2009, Canonical Ltd. All rights reserved.1/* Copyright 2009 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE).
2 *3 *
3 * Code for handling the update of the branch status.4 * Code for handling the update of the branch status.
4 *5 *
56
=== modified file 'lib/canonical/launchpad/javascript/code/branchsubscription.js'
--- lib/canonical/launchpad/javascript/code/branchsubscription.js 2009-08-13 13:47:24 +0000
+++ lib/canonical/launchpad/javascript/code/branchsubscription.js 2009-11-18 15:05:39 +0000
@@ -1,4 +1,5 @@
1/** Copyright (c) 2009, Canonical Ltd. All rights reserved.1/* Copyright 2009 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE).
2 *3 *
3 * Subscription handling for branches.4 * Subscription handling for branches.
4 *5 *
56
=== modified file 'lib/canonical/launchpad/javascript/code/codereview.js'
--- lib/canonical/launchpad/javascript/code/codereview.js 2009-10-28 16:49:59 +0000
+++ lib/canonical/launchpad/javascript/code/codereview.js 2009-11-18 15:05:39 +0000
@@ -1,4 +1,5 @@
1/** Copyright (c) 2009, Canonical Ltd. All rights reserved.1/* Copyright 2009 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE).
2 *3 *
3 * Library for code review javascript.4 * Library for code review javascript.
4 *5 *
56
=== added file 'lib/canonical/launchpad/javascript/code/popupdiff.js'
--- lib/canonical/launchpad/javascript/code/popupdiff.js 1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/code/popupdiff.js 2009-11-18 15:05:38 +0000
@@ -0,0 +1,123 @@
1/* Copyright 2009 Canonical Ltd. This software is licensed under the
2 * GNU Affero General Public License version 3 (see the file LICENSE).
3 *
4 * Code for handling the popup diffs in the pretty overlays.
5 *
6 * @module popupdiff
7 * @requires node
8 */
9
10YUI.add('code.branchmergeproposal.popupdiff', function(Y) {
11
12// The launchpad js client used.
13var lp_client;
14
15/*
16 * The DiffOverlay object inherits from the lazr-js PerttyOverlay.
17 *
18 * By sub-classing the DiffOverlay gets its own CSS class that is applied to
19 * the various HTML objeccts that are created. This allows styling of the
20 * overlay in a different way to any other PrettyOverlays.
21 */
22var DiffOverlay = function() {
23 DiffOverlay.superclass.constructor.apply(this, arguments);
24};
25
26Y.extend(DiffOverlay, Y.lazr.PrettyOverlay, {
27 bindUI: function() {
28 // call PrettyOverlay's bindUI
29 this.constructor.superclass.bindUI.call(this);
30 }
31 });
32
33// The NAME gets appended to 'yui-' to give the class name 'yui-diff-overlay'.
34DiffOverlay.NAME = 'diff-overlay';
35
36// A local page cache of the diff overlays that have been rendered.
37// This makes subsequent views of an already loaded diff instantaneous.
38var rendered_overlays = {};
39
40/*
41 * Display the diff for the specified api_url.
42 *
43 * If the specified api_url has already been rendered in an overlay, show it
44 * again. If it hasn't been loaded, show the spinner, and load the diff using
45 * the LP API.
46 *
47 * If the diff fails to load, the user is taken to the librarian url just as
48 * if Javascript was not enabled.
49 */
50function display_diff(node, api_url, librarian_url) {
51
52 // Look to see if we have rendered one already.
53 if (rendered_overlays[api_url] !== undefined) {
54 rendered_overlays[api_url].show();
55 return;
56 }
57
58 // Show a spinner.
59 var html = [
60 '<img src="/@@/spinner" alt="loading..." ',
61 ' style="padding-left: 0.5em"/>'].join('');
62 var spinner = Y.Node.create(html);
63 node.appendChild(spinner);
64
65 // Load the diff.
66 var config = {
67 on: {
68 success: function(formatted_diff) {
69 node.removeChild(spinner);
70 var diff_overlay = new DiffOverlay({
71 bodyContent: Y.Node.create(formatted_diff),
72 align: {
73 points: [Y.WidgetPositionExt.CC,
74 Y.WidgetPositionExt.CC]
75 },
76 progressbar: false
77 });
78 rendered_overlays[api_url] = diff_overlay;
79 diff_overlay.render();
80 },
81 failure: function() {
82 node.removeChild(spinner);
83 // Fail over to loading the librarian link.
84 document.location = librarian_url;
85 }
86 },
87 accept: LP.client.XHTML
88 };
89 lp_client.get(api_url, config);
90}
91
92
93// Grab the namespace in order to be able to expose the connect method.
94var popupdiff = Y.namespace('code.branchmergeproposal.popupdiff');
95
96/*
97 * Connect the diff links to their pretty overlay function.
98 */
99popupdiff.connect_diff_links = function() {
100 Y.log('connect_diff_links called');
101 // IE doesn't like pretty overlays.
102 if (Y.UA.ie) {
103 return;
104 }
105
106 // Setup the LP client.
107 lp_client = new LP.client.Launchpad();
108
109 // var status_content = Y.get('#branch-details-status-value');
110 var nl = Y.all('.popup-diff');
111 nl.each(function(node, index, nodelist){
112 var a = node.query('a');
113 a.addClass('js-action');
114 var librarian_url = a.getAttribute('href');
115 var api_url = node.query('a.api-ref').getAttribute('href');
116 a.on('click', function(e) {
117 e.preventDefault();
118 display_diff(a, api_url, librarian_url);
119 });
120 });
121};
122
123 }, '0.1', {requires: ['io', 'node', 'lazr.overlay', 'lp.client']});
0124
=== modified file 'lib/canonical/launchpad/webapp/configure.zcml'
--- lib/canonical/launchpad/webapp/configure.zcml 2009-10-20 22:13:21 +0000
+++ lib/canonical/launchpad/webapp/configure.zcml 2009-11-18 15:05:39 +0000
@@ -419,13 +419,6 @@
419 />419 />
420420
421 <adapter421 <adapter
422 for="lp.code.interfaces.diff.IPreviewDiff"
423 provides="zope.traversing.interfaces.IPathAdapter"
424 factory="canonical.launchpad.webapp.tales.PreviewDiffFormatterAPI"
425 name="fmt"
426 />
427
428 <adapter
429 for="lp.code.interfaces.codeimport.ICodeImport"422 for="lp.code.interfaces.codeimport.ICodeImport"
430 provides="zope.traversing.interfaces.IPathAdapter"423 provides="zope.traversing.interfaces.IPathAdapter"
431 factory="canonical.launchpad.webapp.tales.CodeImportFormatterAPI"424 factory="canonical.launchpad.webapp.tales.CodeImportFormatterAPI"
432425
=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py 2009-11-14 22:40:15 +0000
+++ lib/canonical/launchpad/webapp/tales.py 2009-11-18 15:05:39 +0000
@@ -1407,68 +1407,6 @@
1407 '%(name)s</a>: %(title)s' % self._args(view_name))1407 '%(name)s</a>: %(title)s' % self._args(view_name))
14081408
14091409
1410class PreviewDiffFormatterAPI(ObjectFormatterAPI):
1411 """Formatter for preview diffs."""
1412
1413 def url(self, view_name=None, rootsite=None):
1414 """Use the url of the librarian file containing the diff.
1415 """
1416 librarian_alias = self._context.diff_text
1417 if librarian_alias is None:
1418 return None
1419 else:
1420 return librarian_alias.getURL()
1421
1422 def link(self, view_name):
1423 """The link to the diff should show the line count.
1424
1425 Stale diffs will have a stale-diff css class.
1426 Diffs with conflicts will have a conflict-diff css class.
1427 Diffs with neither will have clean-diff css class.
1428
1429 The title of the diff will show the number of lines added or removed
1430 if available.
1431
1432 :param view_name: If not None, the link will point to the page with
1433 that name on this object.
1434 """
1435 title_words = []
1436 if self._context.conflicts is not None:
1437 style = 'conflicts-diff'
1438 title_words.append(_('CONFLICTS'))
1439 else:
1440 style = 'clean-diff'
1441 # Stale style overrides conflicts or clean.
1442 if self._context.stale:
1443 style = 'stale-diff'
1444 title_words.append(_('Stale'))
1445
1446 if self._context.added_lines_count:
1447 title_words.append(
1448 _("%s added") % self._context.added_lines_count)
1449
1450 if self._context.removed_lines_count:
1451 title_words.append(
1452 _("%s removed") % self._context.removed_lines_count)
1453
1454 args = {
1455 'line_count': _('%s lines') % self._context.diff_lines_count,
1456 'style': style,
1457 'title': ', '.join(title_words),
1458 'url': self.url(view_name),
1459 }
1460 # Under normal circumstances, there will be an associated file,
1461 # however if the diff is empty, then there is no alias to link to.
1462 if args['url'] is None:
1463 return (
1464 '<span title="%(title)s" class="%(style)s">'
1465 '%(line_count)s</span>' % args)
1466 else:
1467 return (
1468 '<a href="%(url)s" title="%(title)s" class="%(style)s">'
1469 '<img src="/@@/download"/>&nbsp;%(line_count)s</a>' % args)
1470
1471
1472class BranchSubscriptionFormatterAPI(CustomizableFormatter):1410class BranchSubscriptionFormatterAPI(CustomizableFormatter):
1473 """Adapter for IBranchSubscription objects to a formatted string."""1411 """Adapter for IBranchSubscription objects to a formatted string."""
14741412
14751413
=== modified file 'lib/canonical/launchpad/webapp/tests/test_tales.py'
--- lib/canonical/launchpad/webapp/tests/test_tales.py 2009-10-21 17:26:37 +0000
+++ lib/canonical/launchpad/webapp/tests/test_tales.py 2009-11-18 15:05:39 +0000
@@ -3,20 +3,15 @@
33
4"""tales.py doctests."""4"""tales.py doctests."""
55
6from difflib import unified_diff
7from textwrap import dedent6from textwrap import dedent
8import unittest7import unittest
98
10from storm.store import Store
11from zope.security.proxy import removeSecurityProxy
12from zope.testing.doctestunit import DocTestSuite9from zope.testing.doctestunit import DocTestSuite
1310
14from canonical.launchpad.ftests import test_tales
15from canonical.launchpad.testing.pages import find_tags_by_class11from canonical.launchpad.testing.pages import find_tags_by_class
16from canonical.launchpad.webapp.tales import FormattersAPI12from canonical.launchpad.webapp.tales import FormattersAPI
17from canonical.testing import (13from canonical.testing import DatabaseFunctionalLayer
18 DatabaseFunctionalLayer, LaunchpadFunctionalLayer)14from lp.testing import TestCase
19from lp.testing import login, TestCase, TestCaseWithFactory
2015
2116
22def test_requestapi():17def test_requestapi():
@@ -268,134 +263,6 @@
268 self.assertEqual(3, line_count)263 self.assertEqual(3, line_count)
269264
270265
271class TestPreviewDiffFormatter(TestCaseWithFactory):
272 """Test the PreviewDiffFormatterAPI class."""
273
274 layer = LaunchpadFunctionalLayer
275
276 def _createPreviewDiff(self, line_count=0, added=None, removed=None,
277 conflicts=None):
278 # Login an admin to avoid the launchpad.Edit requirements.
279 login('admin@canonical.com')
280 # Create a dummy preview diff, and make sure the branches have the
281 # correct last scanned ids to ensure that the new diff is not stale.
282 bmp = self.factory.makeBranchMergeProposal()
283 if line_count:
284 content = ''.join(unified_diff('', 'random content'))
285 else:
286 content = ''
287 preview = bmp.updatePreviewDiff(
288 content, u'rev-a', u'rev-b', conflicts=conflicts)
289 bmp.source_branch.last_scanned_id = preview.source_revision_id
290 bmp.target_branch.last_scanned_id = preview.target_revision_id
291 # Update the values directly sidestepping the security.
292 naked_diff = removeSecurityProxy(preview)
293 naked_diff.diff_lines_count = line_count
294 naked_diff.added_lines_count = added
295 naked_diff.removed_lines_count = removed
296 # In order to get the canonical url of the librarian file, we need to
297 # commit.
298 # transaction.commit()
299 # Make sure that the preview diff is in the db for the test.
300 # Storm bug: 324724
301 Store.of(bmp).flush()
302 return preview
303
304 def _createStalePreviewDiff(self, line_count=0, added=None, removed=None,
305 conflicts=None):
306 preview = self._createPreviewDiff(
307 line_count, added, removed, conflicts)
308 preview.branch_merge_proposal.source_branch.last_scanned_id = 'other'
309 return preview
310
311 def test_creation_method(self):
312 # Just confirm that our helpers do what they say.
313 preview = self._createPreviewDiff(12, 45, 23)
314 self.assertEqual(12, preview.diff_lines_count)
315 self.assertEqual(45, preview.added_lines_count)
316 self.assertEqual(23, preview.removed_lines_count)
317 self.assertEqual(False, preview.stale)
318 self.assertEqual(True, self._createStalePreviewDiff().stale)
319
320 def test_fmt_no_diff(self):
321 # If there is no diff, there is no link.
322 preview = self._createPreviewDiff(0)
323 self.assertEqual(
324 '<span title="" class="clean-diff">0 lines</span>',
325 test_tales('preview/fmt:link', preview=preview))
326
327 def test_fmt_lines_no_add_or_remove(self):
328 # If there is no diff, there is no link.
329 preview = self._createPreviewDiff(10, 0, 0)
330 self.assertEqual(
331 '<a href="%s" title="" class="clean-diff">'
332 '<img src="/@@/download"/>&nbsp;10 lines</a>'
333 % preview.diff_text.getURL(),
334 test_tales('preview/fmt:link', preview=preview))
335
336 def test_fmt_lines_some_added_no_removed(self):
337 # If there is no diff, there is no link.
338 preview = self._createPreviewDiff(10, 4, 0)
339 self.assertEqual(
340 '<a href="%s" title="4 added" class="clean-diff">'
341 '<img src="/@@/download"/>&nbsp;10 lines</a>'
342 % preview.diff_text.getURL(),
343 test_tales('preview/fmt:link', preview=preview))
344
345 def test_fmt_lines_no_added_some_removed(self):
346 # If there is no diff, there is no link.
347 preview = self._createPreviewDiff(10, 0, 4)
348 self.assertEqual(
349 '<a href="%s" title="4 removed" class="clean-diff">'
350 '<img src="/@@/download"/>&nbsp;10 lines</a>'
351 % preview.diff_text.getURL(),
352 test_tales('preview/fmt:link', preview=preview))
353
354 def test_fmt_lines_added_and_removed(self):
355 # If there is no diff, there is no link.
356 preview = self._createPreviewDiff(10, 6, 4)
357 self.assertEqual(
358 '<a href="%s" title="6 added, 4 removed" class="clean-diff">'
359 '<img src="/@@/download"/>&nbsp;10 lines</a>'
360 % preview.diff_text.getURL(),
361 test_tales('preview/fmt:link', preview=preview))
362
363 def test_fmt_simple_conflicts(self):
364 # If there is no diff, there is no link.
365 preview = self._createPreviewDiff(10, 2, 3, u'conflicts')
366 self.assertEqual(
367 '<a href="%s" title="CONFLICTS, 2 added, 3 removed" '
368 'class="conflicts-diff">'
369 '<img src="/@@/download"/>&nbsp;10 lines</a>'
370 % preview.diff_text.getURL(),
371 test_tales('preview/fmt:link', preview=preview))
372
373 def test_fmt_stale_empty_diff(self):
374 # If there is no diff, there is no link.
375 preview = self._createStalePreviewDiff(0)
376 self.assertEqual(
377 '<span title="Stale" class="stale-diff">0 lines</span>',
378 test_tales('preview/fmt:link', preview=preview))
379
380 def test_fmt_stale_non_empty_diff(self):
381 # If there is no diff, there is no link.
382 preview = self._createStalePreviewDiff(500, 89, 340)
383 self.assertEqual(
384 '<a href="%s" title="Stale, 89 added, 340 removed" '
385 'class="stale-diff"><img src="/@@/download"/>&nbsp;500 lines</a>'
386 % preview.diff_text.getURL(),
387 test_tales('preview/fmt:link', preview=preview))
388
389 def test_fmt_stale_non_empty_diff_with_conflicts(self):
390 # If there is no diff, there is no link.
391 preview = self._createStalePreviewDiff(500, 89, 340, u'conflicts')
392 self.assertEqual(
393 '<a href="%s" title="CONFLICTS, Stale, 89 added, 340 removed" '
394 'class="stale-diff"><img src="/@@/download"/>&nbsp;500 lines</a>'
395 % preview.diff_text.getURL(),
396 test_tales('preview/fmt:link', preview=preview))
397
398
399def test_suite():266def test_suite():
400 """Return this module's doctest Suite. Unit tests are also run."""267 """Return this module's doctest Suite. Unit tests are also run."""
401 suite = unittest.TestSuite()268 suite = unittest.TestSuite()
402269
=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py 2009-10-16 11:07:37 +0000
+++ lib/devscripts/ec2test/builtins.py 2009-11-18 15:05:39 +0000
@@ -557,7 +557,7 @@
557 'bzr pull -d /var/launchpad/download-cache '557 'bzr pull -d /var/launchpad/download-cache '
558 'lp:lp-source-dependencies')558 'lp:lp-source-dependencies')
559 if public:559 if public:
560 update_sourcecode_options = '--public-only'560 update_sourcecode_options = ' --public-only'
561 else:561 else:
562 update_sourcecode_options = ''562 update_sourcecode_options = ''
563 user_connection.run_with_ssh_agent(563 user_connection.run_with_ssh_agent(
564564
=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py 2009-10-26 18:40:04 +0000
+++ lib/lp/code/browser/branchlisting.py 2009-11-18 15:05:39 +0000
@@ -91,14 +91,7 @@
91from lp.registry.interfaces.product import IProduct91from lp.registry.interfaces.product import IProduct
92from lp.registry.interfaces.sourcepackage import ISourcePackageFactory92from lp.registry.interfaces.sourcepackage import ISourcePackageFactory
93from lp.registry.model.sourcepackage import SourcePackage93from lp.registry.model.sourcepackage import SourcePackage
9494from lp.services.browser_helpers import get_plural_text
95
96def get_plural_text(count, singular, plural):
97 """Return 'singular' if 'count' is 1, 'plural' otherwise."""
98 if count == 1:
99 return singular
100 else:
101 return plural
10295
10396
104class CodeVHostBreadcrumb(Breadcrumb):97class CodeVHostBreadcrumb(Breadcrumb):
10598
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2009-11-10 22:57:42 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2009-11-18 15:05:39 +0000
@@ -34,7 +34,7 @@
3434
35import simplejson35import simplejson
36from zope.app.form.browser import TextAreaWidget36from zope.app.form.browser import TextAreaWidget
37from zope.component import adapter, getUtility37from zope.component import adapter, adapts, getMultiAdapter, getUtility
38from zope.event import notify as zope_notify38from zope.event import notify as zope_notify
39from zope.formlib import form39from zope.formlib import form
40from zope.interface import Interface, implementer, implements40from zope.interface import Interface, implementer, implements
@@ -60,7 +60,8 @@
60from canonical.launchpad.webapp.breadcrumb import Breadcrumb60from canonical.launchpad.webapp.breadcrumb import Breadcrumb
61from canonical.launchpad.webapp.interfaces import IPrimaryContext61from canonical.launchpad.webapp.interfaces import IPrimaryContext
62from canonical.launchpad.webapp.menu import NavigationMenu62from canonical.launchpad.webapp.menu import NavigationMenu
63from canonical.launchpad.webapp.tales import FormattersAPI63from canonical.launchpad.webapp.tales import (
64 DateTimeFormatterAPI, FormattersAPI)
64from canonical.widgets.lazrjs import (65from canonical.widgets.lazrjs import (
65 TextAreaEditorWidget, vocabulary_to_choice_edit_items)66 TextAreaEditorWidget, vocabulary_to_choice_edit_items)
6667
@@ -74,6 +75,7 @@
74from lp.code.interfaces.codereviewcomment import ICodeReviewComment75from lp.code.interfaces.codereviewcomment import ICodeReviewComment
75from lp.code.interfaces.codereviewvote import (76from lp.code.interfaces.codereviewvote import (
76 ICodeReviewVoteReference)77 ICodeReviewVoteReference)
78from lp.code.interfaces.diff import IPreviewDiff
77from lp.registry.interfaces.person import IPersonSet79from lp.registry.interfaces.person import IPersonSet
78from lp.services.comments.interfaces.conversation import IConversation80from lp.services.comments.interfaces.conversation import IConversation
7981
@@ -138,6 +140,23 @@
138 }140 }
139 return friendly_texts[self.context.queue_status]141 return friendly_texts[self.context.queue_status]
140142
143 @property
144 def status_title(self):
145 """The title for the status text.
146
147 Only set if the status is approved or rejected.
148 """
149 result = ''
150 if self.context.queue_status in (
151 BranchMergeProposalStatus.CODE_APPROVED,
152 BranchMergeProposalStatus.REJECTED
153 ):
154 formatter = DateTimeFormatterAPI(self.context.date_reviewed)
155 result = '%s %s' % (
156 self.context.reviewer.displayname,
157 formatter.displaydate())
158 return result
159
141160
142class BranchMergeProposalMenuMixin:161class BranchMergeProposalMenuMixin:
143 """Mixin class for merge proposal menus."""162 """Mixin class for merge proposal menus."""
@@ -427,10 +446,44 @@
427 return SimpleVocabulary(terms)446 return SimpleVocabulary(terms)
428447
429448
449class DiffRenderingMixin:
450 """A mixin class for handling diff text."""
451
452 @cachedproperty
453 def preview_diff_text(self):
454 """Return a (hopefully) intelligently encoded review diff."""
455 preview_diff = self.preview_diff
456 if preview_diff is None:
457 return None
458 try:
459 diff = preview_diff.text.decode('utf-8')
460 except UnicodeDecodeError:
461 diff = preview_diff.text.decode('windows-1252', 'replace')
462 # Strip off the trailing carriage returns.
463 return diff.rstrip('\n')
464
465 @cachedproperty
466 def diff_oversized(self):
467 """Return True if the review_diff is over the configured size limit.
468
469 The diff can be over the limit in two ways. If the diff is oversized
470 in bytes it will be cut off at the Diff.text method. If the number of
471 lines is over the max_format_lines, then it is cut off at the fmt:diff
472 processing.
473 """
474 review_diff = self.preview_diff
475 if review_diff is None:
476 return False
477 if review_diff.oversized:
478 return True
479 diff_text = self.preview_diff_text
480 return diff_text.count('\n') >= config.diff.max_format_lines
481
430482
431class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,483class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,
432 BranchMergeProposalRevisionIdMixin,484 BranchMergeProposalRevisionIdMixin,
433 BranchMergeProposalStatusMixin):485 BranchMergeProposalStatusMixin,
486 DiffRenderingMixin):
434 """A basic view used for the index page."""487 """A basic view used for the index page."""
435488
436 implements(IBranchMergeProposalActionMenu)489 implements(IBranchMergeProposalActionMenu)
@@ -496,36 +549,6 @@
496 return self.context.review_diff.diff549 return self.context.review_diff.diff
497 return None550 return None
498551
499 @cachedproperty
500 def preview_diff_text(self):
501 """Return a (hopefully) intelligently encoded review diff."""
502 preview_diff = self.preview_diff
503 if preview_diff is None:
504 return None
505 try:
506 diff = preview_diff.text.decode('utf-8')
507 except UnicodeDecodeError:
508 diff = preview_diff.text.decode('windows-1252', 'replace')
509 # Strip off the trailing carriage returns.
510 return diff.rstrip('\n')
511
512 @cachedproperty
513 def review_diff_oversized(self):
514 """Return True if the review_diff is over the configured size limit.
515
516 The diff can be over the limit in two ways. If the diff is oversized
517 in bytes it will be cut off at the Diff.text method. If the number of
518 lines is over the max_format_lines, then it is cut off at the fmt:diff
519 processing.
520 """
521 review_diff = self.preview_diff
522 if review_diff is None:
523 return False
524 if review_diff.oversized:
525 return True
526 diff_text = self.preview_diff_text
527 return diff_text.count('\n') >= config.diff.max_format_lines
528
529 @property552 @property
530 def has_bug_or_spec(self):553 def has_bug_or_spec(self):
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.
@@ -1290,3 +1313,28 @@
1290 html = formatter(nomail).text_to_html()1313 html = formatter(nomail).text_to_html()
1291 return html.encode('utf-8')1314 return html.encode('utf-8')
1292 return renderer1315 return renderer
1316
1317
1318class FormatPreviewDiffView(LaunchpadView, DiffRenderingMixin):
1319 """A simple view to render a diff formatted nicely."""
1320
1321 __used_for__ = IPreviewDiff
1322
1323 @property
1324 def preview_diff(self):
1325 return self.context
1326
1327
1328class PreviewDiffHTMLRepresentation:
1329 adapts(IPreviewDiff, IWebServiceClientRequest)
1330 implements(Interface)
1331
1332 def __init__(self, diff, request):
1333 self.diff = diff
1334 self.request = request
1335
1336 def __call__(self):
1337 """Render `BugBranch` as XHTML using the webservice."""
1338 diff_view = getMultiAdapter(
1339 (self.diff, self.request), name="+diff")
1340 return diff_view()
12931341
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2009-11-13 20:12:17 +0000
+++ lib/lp/code/browser/configure.zcml 2009-11-18 15:05:38 +0000
@@ -181,16 +181,24 @@
181 name="+index"181 name="+index"
182 template="../templates/branchmergeproposal-index.pt"/>182 template="../templates/branchmergeproposal-index.pt"/>
183 <browser:page183 <browser:page
184 name="++diff"
185 template="../templates/branchmergeproposal-diff.pt"/>
186 <browser:page
184 name="+pagelet-summary"187 name="+pagelet-summary"
185 template="../templates/branchmergeproposal-pagelet-summary.pt"/>188 template="../templates/branchmergeproposal-pagelet-summary.pt"/>
186 </browser:pages>189 </browser:pages>
187 <browser:page190 <browser:pages
188 name="+votes"
189 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"191 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
190 class="lp.code.browser.branchmergeproposal.BranchMergeProposalVoteView"192 class="lp.code.browser.branchmergeproposal.BranchMergeProposalVoteView"
191 facet="branches"193 facet="branches"
192 permission="launchpad.View"194 permission="launchpad.View">
193 template="../templates/branchmergeproposal-votes.pt"/>195 <browser:page
196 name="+votes"
197 template="../templates/branchmergeproposal-votes.pt"/>
198 <browser:page
199 name="+vote-summary"
200 template="../templates/branchmergeproposal-vote-summary.pt"/>
201 </browser:pages>
194 <browser:pages202 <browser:pages
195 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"203 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
196 class="lp.code.browser.branchmergeproposal.BranchMergeProposalEditView"204 class="lp.code.browser.branchmergeproposal.BranchMergeProposalEditView"
@@ -276,7 +284,7 @@
276 class="lp.code.browser.branchmergeproposal.BranchMergeCandidateView"284 class="lp.code.browser.branchmergeproposal.BranchMergeCandidateView"
277 facet="branches"285 facet="branches"
278 permission="zope.Public"286 permission="zope.Public"
279 template="../templates/branch-merge-link-summary.pt"/>287 template="../templates/branchmergeproposal-link-summary.pt"/>
280 <browser:page288 <browser:page
281 name="+pagelet-subscribers"289 name="+pagelet-subscribers"
282 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"290 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
@@ -898,6 +906,13 @@
898 attribute_to_parent="branch_merge_proposal"906 attribute_to_parent="branch_merge_proposal"
899 rootsite="code" />907 rootsite="code" />
900908
909 <browser:page
910 name="+diff"
911 for="lp.code.interfaces.diff.IPreviewDiff"
912 class="lp.code.browser.branchmergeproposal.FormatPreviewDiffView"
913 permission="launchpad.AnyPerson"
914 template="../templates/branchmergeproposal-diff.pt"/>
915
901 <browser:menus916 <browser:menus
902 module="lp.code.browser.branchlisting"917 module="lp.code.browser.branchlisting"
903 classes="PersonProductBranchesMenu"/>918 classes="PersonProductBranchesMenu"/>
@@ -991,5 +1006,15 @@
991 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"1006 for="lp.code.interfaces.branchmergeproposal.IBranchMergeProposal"
992 factory="lp.code.browser.branchmergeproposal.BranchMergeProposalBreadcrumb"1007 factory="lp.code.browser.branchmergeproposal.BranchMergeProposalBreadcrumb"
993 permission="zope.Public"/>1008 permission="zope.Public"/>
1009 <adapter
1010 factory="lp.code.browser.branchmergeproposal.PreviewDiffHTMLRepresentation"
1011 name="lazr.restful.EntryResource"/>
1012
1013 <adapter
1014 for="lp.code.interfaces.diff.IPreviewDiff"
1015 provides="zope.traversing.interfaces.IPathAdapter"
1016 factory="lp.code.browser.diff.PreviewDiffFormatterAPI"
1017 name="fmt"
1018 />
9941019
995</configure>1020</configure>
9961021
=== added file 'lib/lp/code/browser/diff.py'
--- lib/lp/code/browser/diff.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/diff.py 2009-11-18 15:05:38 +0000
@@ -0,0 +1,77 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Display classes relating to diff objects of one sort or another."""
5
6__metaclass__ = type
7__all__ = [
8 'PreviewDiffFormatterAPI',
9 ]
10
11
12from canonical.launchpad import _
13from canonical.launchpad.webapp.tales import ObjectFormatterAPI
14from lp.services.browser_helpers import get_plural_text
15
16
17class PreviewDiffFormatterAPI(ObjectFormatterAPI):
18 """Formatter for preview diffs."""
19
20 def url(self, view_name=None, rootsite=None):
21 """Use the url of the librarian file containing the diff.
22 """
23 librarian_alias = self._context.diff_text
24 if librarian_alias is None:
25 return None
26 else:
27 return librarian_alias.getURL()
28
29 def link(self, view_name):
30 """The link to the diff should show the line count.
31
32 Stale diffs will have a stale-diff css class.
33 Diffs with conflicts will have a conflict-diff css class.
34 Diffs with neither will have clean-diff css class.
35
36 The title of the diff will show the number of lines added or removed
37 if available.
38
39 :param view_name: If not None, the link will point to the page with
40 that name on this object.
41 """
42 diff = self._context
43 conflict_text = ''
44 if diff.conflicts is not None:
45 conflict_text = _(' (has conflicts)')
46
47 count_text = ''
48 added = diff.added_lines_count
49 removed = diff.removed_lines_count
50 if (added is not None and removed is not None):
51 count_text = ' (+%d/-%d)' % (added, removed)
52
53 file_text = ''
54 diffstat = diff.diffstat
55 if diffstat is not None:
56 file_count = len(diffstat)
57 file_text = get_plural_text(
58 file_count, _(' %d file modified'), _(' %d files modified'))
59 file_text = file_text % file_count
60
61 args = {
62 'line_count': _('%s lines') % diff.diff_lines_count,
63 'file_text': file_text,
64 'conflict_text': conflict_text,
65 'count_text': count_text,
66 'url': self.url(view_name),
67 }
68 # Under normal circumstances, there will be an associated file,
69 # however if the diff is empty, then there is no alias to link to.
70 if args['url'] is None:
71 return (
72 '<span class="empty-diff">'
73 '%(line_count)s</span>' % args)
74 else:
75 return (
76 '<a href="%(url)s" class="diff-link">'
77 '%(line_count)s%(count_text)s%(file_text)s%(conflict_text)s</a>' % args)
078
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-01 23:13:09 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2009-11-18 15:05:38 +0000
@@ -7,10 +7,11 @@
77
8__metaclass__ = type8__metaclass__ = type
99
10from datetime import timedelta10from datetime import datetime, timedelta
11from difflib import unified_diff11from difflib import unified_diff
12import unittest12import unittest
1313
14import pytz
14import transaction15import transaction
15from zope.component import getMultiAdapter16from zope.component import getMultiAdapter
16from zope.security.interfaces import Unauthorized17from zope.security.interfaces import Unauthorized
@@ -66,6 +67,7 @@
66 link = menu.add_comment()67 link = menu.add_comment()
67 self.assertTrue(menu.add_comment().enabled)68 self.assertTrue(menu.add_comment().enabled)
6869
70
69class TestDecoratedCodeReviewVoteReference(TestCaseWithFactory):71class TestDecoratedCodeReviewVoteReference(TestCaseWithFactory):
7072
71 layer = DatabaseFunctionalLayer73 layer = DatabaseFunctionalLayer
@@ -459,13 +461,6 @@
459 self.bmp = self.factory.makeBranchMergeProposal(registrant=self.user)461 self.bmp = self.factory.makeBranchMergeProposal(registrant=self.user)
460 login_person(self.user)462 login_person(self.user)
461463
462 def _createView(self):
463 # Construct the view and initialize it.
464 view = BranchMergeProposalView(
465 self.bmp, LaunchpadTestRequest())
466 view.initialize()
467 return view
468
469 def makeTeamReview(self):464 def makeTeamReview(self):
470 owner = self.bmp.source_branch.owner465 owner = self.bmp.source_branch.owner
471 review_team = self.factory.makeTeam()466 review_team = self.factory.makeTeam()
@@ -477,7 +472,7 @@
477 albert = self.factory.makePerson()472 albert = self.factory.makePerson()
478 albert.join(review.reviewer)473 albert.join(review.reviewer)
479 login_person(albert)474 login_person(albert)
480 view = self._createView()475 view = create_initialized_view(self.bmp, '+index')
481 view.claim_action.success({'review_id': review.id})476 view.claim_action.success({'review_id': review.id})
482 self.assertEqual(albert, review.reviewer)477 self.assertEqual(albert, review.reviewer)
483478
@@ -486,13 +481,13 @@
486 review = self.makeTeamReview()481 review = self.makeTeamReview()
487 albert = self.factory.makePerson()482 albert = self.factory.makePerson()
488 login_person(albert)483 login_person(albert)
489 view = self._createView()484 view = create_initialized_view(self.bmp, '+index')
490 self.assertRaises(Unauthorized, view.claim_action.success,485 self.assertRaises(Unauthorized, view.claim_action.success,
491 {'review_id': review.id})486 {'review_id': review.id})
492487
493 def test_preview_diff_text_with_no_diff(self):488 def test_preview_diff_text_with_no_diff(self):
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."""
495 view = self._createView()490 view = create_initialized_view(self.bmp, '+index')
496 self.assertIs(None, view.preview_diff_text)491 self.assertIs(None, view.preview_diff_text)
497492
498 def test_review_diff_utf8(self):493 def test_review_diff_utf8(self):
@@ -502,8 +497,9 @@
502 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)497 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)
503 transaction.commit()498 transaction.commit()
504 self.bmp.review_diff = diff499 self.bmp.review_diff = diff
500 view = create_initialized_view(self.bmp, '+index')
505 self.assertEqual(diff_bytes.decode('utf-8'),501 self.assertEqual(diff_bytes.decode('utf-8'),
506 self._createView().preview_diff_text)502 view.preview_diff_text)
507503
508 def test_review_diff_all_chars(self):504 def test_review_diff_all_chars(self):
509 """review_diff should work on diffs containing all possible bytes."""505 """review_diff should work on diffs containing all possible bytes."""
@@ -512,8 +508,9 @@
512 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)508 diff = StaticDiff.acquireFromText('x', 'y', diff_bytes)
513 transaction.commit()509 transaction.commit()
514 self.bmp.review_diff = diff510 self.bmp.review_diff = diff
511 view = create_initialized_view(self.bmp, '+index')
515 self.assertEqual(diff_bytes.decode('windows-1252', 'replace'),512 self.assertEqual(diff_bytes.decode('windows-1252', 'replace'),
516 self._createView().preview_diff_text)513 view.preview_diff_text)
517514
518 def addReviewDiff(self):515 def addReviewDiff(self):
519 review_diff_bytes = ''.join(unified_diff('', 'review'))516 review_diff_bytes = ''.join(unified_diff('', 'review'))
@@ -532,27 +529,31 @@
532 def test_preview_diff_prefers_preview_diff(self):529 def test_preview_diff_prefers_preview_diff(self):
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."""
534 preview_diff = self.addBothDiffs()531 preview_diff = self.addBothDiffs()
535 self.assertEqual(preview_diff, self._createView().preview_diff)532 view = create_initialized_view(self.bmp, '+index')
533 self.assertEqual(preview_diff, view.preview_diff)
536534
537 def test_preview_diff_uses_review_diff(self):535 def test_preview_diff_uses_review_diff(self):
538 """The review diff will be used if there is no preview."""536 """The review diff will be used if there is no preview."""
539 review_diff = self.addReviewDiff()537 review_diff = self.addReviewDiff()
538 view = create_initialized_view(self.bmp, '+index')
540 self.assertEqual(review_diff.diff,539 self.assertEqual(review_diff.diff,
541 self._createView().preview_diff)540 view.preview_diff)
542541
543 def test_review_diff_text_prefers_preview_diff(self):542 def test_review_diff_text_prefers_preview_diff(self):
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."""
545 preview_diff = self.addBothDiffs()544 preview_diff = self.addBothDiffs()
546 transaction.commit()545 transaction.commit()
546 view = create_initialized_view(self.bmp, '+index')
547 self.assertEqual(547 self.assertEqual(
548 preview_diff.text, self._createView().preview_diff_text)548 preview_diff.text, view.preview_diff_text)
549549
550 def test_linked_bugs_excludes_mutual_bugs(self):550 def test_linked_bugs_excludes_mutual_bugs(self):
551 """List bugs that are linked to the source only."""551 """List bugs that are linked to the source only."""
552 bug = self.factory.makeBug()552 bug = self.factory.makeBug()
553 self.bmp.source_branch.linkBug(bug, self.bmp.registrant)553 self.bmp.source_branch.linkBug(bug, self.bmp.registrant)
554 self.bmp.target_branch.linkBug(bug, self.bmp.registrant)554 self.bmp.target_branch.linkBug(bug, self.bmp.registrant)
555 self.assertEqual([], self._createView().linked_bugs)555 view = create_initialized_view(self.bmp, '+index')
556 self.assertEqual([], view.linked_bugs)
556557
557558
558class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory):559class TestBranchMergeProposalChangeStatusOptions(TestCaseWithFactory):
@@ -692,5 +693,42 @@
692 self.assertEqual(u'\u2615', diff_attachment.diff_text)693 self.assertEqual(u'\u2615', diff_attachment.diff_text)
693694
694695
696class TestBranchMergeCandidateView(TestCaseWithFactory):
697 """Test the status title for the view."""
698
699 layer = DatabaseFunctionalLayer
700
701 def test_needs_review_title(self):
702 # No title is set for a proposal needing review.
703 bmp = self.factory.makeBranchMergeProposal(
704 set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
705 view = create_initialized_view(bmp, '+link-summary')
706 self.assertEqual('', view.status_title)
707
708 def test_approved_shows_reviewer(self):
709 # If the proposal is approved, the approver is shown in the title
710 # along with when they approved it.
711 bmp = self.factory.makeBranchMergeProposal()
712 owner = bmp.target_branch.owner
713 login_person(bmp.target_branch.owner)
714 owner.displayname = 'Eric'
715 bmp.approveBranch(owner, 'some-rev', datetime(
716 year=2008, month=9, day=10, tzinfo=pytz.UTC))
717 view = create_initialized_view(bmp, '+link-summary')
718 self.assertEqual('Eric on 2008-09-10', view.status_title)
719
720 def test_rejected_shows_reviewer(self):
721 # If the proposal is rejected, the approver is shown in the title
722 # along with when they approved it.
723 bmp = self.factory.makeBranchMergeProposal()
724 owner = bmp.target_branch.owner
725 login_person(bmp.target_branch.owner)
726 owner.displayname = 'Eric'
727 bmp.rejectBranch(owner, 'some-rev', datetime(
728 year=2008, month=9, day=10, tzinfo=pytz.UTC))
729 view = create_initialized_view(bmp, '+link-summary')
730 self.assertEqual('Eric on 2008-09-10', view.status_title)
731
732
695def test_suite():733def test_suite():
696 return unittest.TestLoader().loadTestsFromName(__name__)734 return unittest.TestLoader().loadTestsFromName(__name__)
697735
=== added file 'lib/lp/code/browser/tests/test_tales.py'
--- lib/lp/code/browser/tests/test_tales.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/tests/test_tales.py 2009-11-18 15:05:39 +0000
@@ -0,0 +1,163 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for the tales formatters."""
5
6__metaclass__ = type
7
8from difflib import unified_diff
9import unittest
10
11from storm.store import Store
12from zope.security.proxy import removeSecurityProxy
13
14from lp.testing import login, TestCaseWithFactory, test_tales
15from canonical.testing import LaunchpadFunctionalLayer
16
17
18class TestPreviewDiffFormatter(TestCaseWithFactory):
19 """Test the PreviewDiffFormatterAPI class."""
20
21 layer = LaunchpadFunctionalLayer
22
23 def _createPreviewDiff(self, line_count=0, added=None, removed=None,
24 conflicts=None, diffstat=None):
25 # Login an admin to avoid the launchpad.Edit requirements.
26 login('admin@canonical.com')
27 # Create a dummy preview diff, and make sure the branches have the
28 # correct last scanned ids to ensure that the new diff is not stale.
29 bmp = self.factory.makeBranchMergeProposal()
30 if line_count:
31 content = ''.join(unified_diff('', 'random content'))
32 else:
33 content = ''
34 preview = bmp.updatePreviewDiff(
35 content, u'rev-a', u'rev-b', conflicts=conflicts)
36 bmp.source_branch.last_scanned_id = preview.source_revision_id
37 bmp.target_branch.last_scanned_id = preview.target_revision_id
38 # Update the values directly sidestepping the security.
39 naked_diff = removeSecurityProxy(preview)
40 naked_diff.diff_lines_count = line_count
41 naked_diff.added_lines_count = added
42 naked_diff.removed_lines_count = removed
43 naked_diff.diffstat = diffstat
44 # In order to get the canonical url of the librarian file, we need to
45 # commit.
46 # transaction.commit()
47 # Make sure that the preview diff is in the db for the test.
48 # Storm bug: 324724
49 Store.of(bmp).flush()
50 return preview
51
52 def _createStalePreviewDiff(self, line_count=0, added=None, removed=None,
53 conflicts=None, diffstat=None):
54 preview = self._createPreviewDiff(
55 line_count, added, removed, conflicts, diffstat)
56 preview.branch_merge_proposal.source_branch.last_scanned_id = 'other'
57 return preview
58
59 def test_creation_method(self):
60 # Just confirm that our helpers do what they say.
61 preview = self._createPreviewDiff(
62 12, 45, 23, u'conflicts', {'filename': (3,2)})
63 self.assertEqual(12, preview.diff_lines_count)
64 self.assertEqual(45, preview.added_lines_count)
65 self.assertEqual(23, preview.removed_lines_count)
66 self.assertEqual(False, preview.stale)
67 self.assertEqual(True, self._createStalePreviewDiff().stale)
68 self.assertEqual(u'conflicts', preview.conflicts)
69 self.assertEqual({'filename': (3,2)}, preview.diffstat)
70
71 def test_fmt_no_diff(self):
72 # If there is no diff, there is no link.
73 preview = self._createPreviewDiff(0)
74 self.assertEqual(
75 '<span class="empty-diff">0 lines</span>',
76 test_tales('preview/fmt:link', preview=preview))
77
78 def test_fmt_lines_no_add_or_remove(self):
79 # If the lines added and removed are None, they are now shown.
80 preview = self._createPreviewDiff(10, added=None, removed=None)
81 self.assertEqual(
82 '<a href="%s" class="diff-link">'
83 '10 lines</a>'
84 % preview.diff_text.getURL(),
85 test_tales('preview/fmt:link', preview=preview))
86
87 def test_fmt_lines_some_added_no_removed(self):
88 # If the added and removed values are not None, they are shown.
89 preview = self._createPreviewDiff(10, added=4, removed=0)
90 self.assertEqual(
91 '<a href="%s" class="diff-link">'
92 '10 lines (+4/-0)</a>'
93 % preview.diff_text.getURL(),
94 test_tales('preview/fmt:link', preview=preview))
95
96 def test_fmt_lines_files_modified(self):
97 # If the diffstat has been set, the number of entries in the dict
98 # defines the number of files modified.
99 preview = self._createPreviewDiff(
100 10, added=4, removed=0, diffstat={
101 'file1': (1, 0),
102 'file2': (3, 0)})
103 self.assertEqual(
104 '<a href="%s" class="diff-link">'
105 '10 lines (+4/-0) 2 files modified</a>'
106 % preview.diff_text.getURL(),
107 test_tales('preview/fmt:link', preview=preview))
108
109 def test_fmt_lines_one_file_modified(self):
110 # If only one file has been modified, a singular value is used.
111 preview = self._createPreviewDiff(
112 10, added=4, removed=0, diffstat={
113 'file': (3, 0)})
114 self.assertEqual(
115 '<a href="%s" class="diff-link">'
116 '10 lines (+4/-0) 1 file modified</a>'
117 % preview.diff_text.getURL(),
118 test_tales('preview/fmt:link', preview=preview))
119
120 def test_fmt_simple_conflicts(self):
121 # Conflicts are indicated using text in the link.
122 preview = self._createPreviewDiff(10, 2, 3, u'conflicts')
123 self.assertEqual(
124 '<a href="%s" class="diff-link">'
125 '10 lines (+2/-3) (has conflicts)</a>'
126 % preview.diff_text.getURL(),
127 test_tales('preview/fmt:link', preview=preview))
128
129 def test_fmt_stale_empty_diff(self):
130 # If there is no diff, there is no link.
131 preview = self._createStalePreviewDiff(0)
132 self.assertEqual(
133 '<span class="empty-diff">0 lines</span>',
134 test_tales('preview/fmt:link', preview=preview))
135
136 def test_fmt_stale_non_empty_diff(self):
137 # If there is no diff, there is no link.
138 diffstat = dict(
139 (self.factory.getUniqueString(), (2,3)) for x in range(23))
140 preview = self._createStalePreviewDiff(
141 500, 89, 340, diffstat=diffstat)
142 self.assertEqual(
143 '<a href="%s" class="diff-link">'
144 '500 lines (+89/-340) 23 files modified</a>'
145 % preview.diff_text.getURL(),
146 test_tales('preview/fmt:link', preview=preview))
147
148 def test_fmt_stale_non_empty_diff_with_conflicts(self):
149 # If there is no diff, there is no link.
150 diffstat = dict(
151 (self.factory.getUniqueString(), (2,3)) for x in range(23))
152 preview = self._createStalePreviewDiff(
153 500, 89, 340, u'conflicts', diffstat=diffstat)
154 self.assertEqual(
155 '<a href="%s" class="diff-link">'
156 '500 lines (+89/-340) 23 files modified (has conflicts)</a>'
157 % preview.diff_text.getURL(),
158 test_tales('preview/fmt:link', preview=preview))
159
160
161def test_suite():
162 return unittest.TestLoader().loadTestsFromName(__name__)
163
0164
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2009-10-05 14:17:48 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2009-11-18 15:05:38 +0000
@@ -361,7 +361,8 @@
361 # or superseded, then it is valid to be merged.361 # or superseded, then it is valid to be merged.
362 return (self.queue_status not in FINAL_STATES)362 return (self.queue_status not in FINAL_STATES)
363363
364 def _reviewProposal(self, reviewer, next_state, revision_id):364 def _reviewProposal(self, reviewer, next_state, revision_id,
365 _date_reviewed=None):
365 """Set the proposal to one of the two review statuses."""366 """Set the proposal to one of the two review statuses."""
366 # Check the reviewer can review the code for the target branch.367 # Check the reviewer can review the code for the target branch.
367 old_state = self.queue_status368 old_state = self.queue_status
@@ -371,21 +372,25 @@
371 self._transitionToState(next_state, reviewer)372 self._transitionToState(next_state, reviewer)
372 # Record the reviewer373 # Record the reviewer
373 self.reviewer = reviewer374 self.reviewer = reviewer
374 self.date_reviewed = UTC_NOW375 if _date_reviewed is None:
376 _date_reviewed = UTC_NOW
377 self.date_reviewed = _date_reviewed
375 # Record the reviewed revision id378 # Record the reviewed revision id
376 self.reviewed_revision_id = revision_id379 self.reviewed_revision_id = revision_id
377 notify(BranchMergeProposalStatusChangeEvent(380 notify(BranchMergeProposalStatusChangeEvent(
378 self, reviewer, old_state, next_state))381 self, reviewer, old_state, next_state))
379382
380 def approveBranch(self, reviewer, revision_id):383 def approveBranch(self, reviewer, revision_id, _date_reviewed=None):
381 """See `IBranchMergeProposal`."""384 """See `IBranchMergeProposal`."""
382 self._reviewProposal(385 self._reviewProposal(
383 reviewer, BranchMergeProposalStatus.CODE_APPROVED, revision_id)386 reviewer, BranchMergeProposalStatus.CODE_APPROVED, revision_id,
387 _date_reviewed)
384388
385 def rejectBranch(self, reviewer, revision_id):389 def rejectBranch(self, reviewer, revision_id, _date_reviewed=None):
386 """See `IBranchMergeProposal`."""390 """See `IBranchMergeProposal`."""
387 self._reviewProposal(391 self._reviewProposal(
388 reviewer, BranchMergeProposalStatus.REJECTED, revision_id)392 reviewer, BranchMergeProposalStatus.REJECTED, revision_id,
393 _date_reviewed)
389394
390 def enqueue(self, queuer, revision_id):395 def enqueue(self, queuer, revision_id):
391 """See `IBranchMergeProposal`."""396 """See `IBranchMergeProposal`."""
392397
=== modified file 'lib/lp/code/model/directbranchcommit.py'
--- lib/lp/code/model/directbranchcommit.py 2009-09-09 20:03:19 +0000
+++ lib/lp/code/model/directbranchcommit.py 2009-11-18 15:05:38 +0000
@@ -74,6 +74,8 @@
74 self.db_branch = db_branch74 self.db_branch = db_branch
75 self.to_mirror = to_mirror75 self.to_mirror = to_mirror
7676
77 self.last_scanned_id = self.db_branch.last_scanned_id
78
77 if committer is None:79 if committer is None:
78 committer = db_branch.owner80 committer = db_branch.owner
79 self.committer = committer81 self.committer = committer
@@ -172,12 +174,18 @@
172 assert self.is_locked, "Getting revision on un-locked branch."174 assert self.is_locked, "Getting revision on un-locked branch."
173 last_revision = None175 last_revision = None
174 last_revision = self.bzrbranch.last_revision()176 last_revision = self.bzrbranch.last_revision()
175 if last_revision != self.db_branch.last_scanned_id:177 if last_revision != self.last_scanned_id:
176 raise ConcurrentUpdateError(178 raise ConcurrentUpdateError(
177 "Branch has been changed. Not committing.")179 "Branch has been changed. Not committing.")
178180
179 def commit(self, commit_message):181 def commit(self, commit_message, txn=None):
180 """Commit to branch."""182 """Commit to branch.
183
184 :param commit_message: Message for branch's commit log.
185 :param txn: Transaction to commit. Can be helpful in avoiding
186 long idle times in database transactions. May be committed
187 more than once.
188 """
181 assert self.is_open, "Committing closed DirectBranchCommit."189 assert self.is_open, "Committing closed DirectBranchCommit."
182 assert self.is_locked, "Not locked at commit time."190 assert self.is_locked, "Not locked at commit time."
183191
@@ -185,6 +193,9 @@
185 try:193 try:
186 self._checkForRace()194 self._checkForRace()
187195
196 if txn:
197 txn.commit()
198
188 rev_id = self.revision_tree.get_revision_id()199 rev_id = self.revision_tree.get_revision_id()
189 if rev_id == NULL_REVISION:200 if rev_id == NULL_REVISION:
190 if list(self.transform_preview.iter_changes()) == []:201 if list(self.transform_preview.iter_changes()) == []:
@@ -193,6 +204,9 @@
193 self.bzrbranch, commit_message)204 self.bzrbranch, commit_message)
194 IMasterObject(self.db_branch).requestMirror()205 IMasterObject(self.db_branch).requestMirror()
195206
207 if txn:
208 txn.commit()
209
196 finally:210 finally:
197 self.unlock()211 self.unlock()
198 self.is_open = False212 self.is_open = False
199213
=== modified file 'lib/lp/code/stories/branches/xx-branch-merge-proposals.txt'
--- lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2009-10-28 19:31:47 +0000
+++ lib/lp/code/stories/branches/xx-branch-merge-proposals.txt 2009-11-18 15:05:39 +0000
@@ -29,6 +29,8 @@
29 >>> _unused = branch.subscribe(subscriber,29 >>> _unused = branch.subscribe(subscriber,
30 ... BranchSubscriptionNotificationLevel.NOEMAIL, None,30 ... BranchSubscriptionNotificationLevel.NOEMAIL, None,
31 ... CodeReviewNotificationLevel.FULL)31 ... CodeReviewNotificationLevel.FULL)
32 >>> from lp.code.tests.helpers import make_erics_fooix_project
33 >>> locals().update(make_erics_fooix_project(factory))
32 >>> logout()34 >>> logout()
3335
34Any logged in user can register a merge proposal. Registering36Any logged in user can register a merge proposal. Registering
@@ -68,7 +70,8 @@
6870
69Registering the merge proposal takes the user to the new merge proposal.71Registering the merge proposal takes the user to the new merge proposal.
7072
71 >>> print nopriv_browser.url73 >>> klingon_proposal = nopriv_browser.url
74 >>> print klingon_proposal
72 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/...75 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/...
7376
74The summary reflects the selected target and prerequisite.77The summary reflects the selected target and prerequisite.
@@ -97,9 +100,6 @@
97 ... 'Add more <b>mojo</b>')100 ... 'Add more <b>mojo</b>')
98 >>> nopriv_browser.getControl('Update').click()101 >>> nopriv_browser.getControl('Update').click()
99102
100 >>> print nopriv_browser.url
101 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1
102
103 >>> print_tag_with_id(nopriv_browser.contents, 'edit-description')103 >>> print_tag_with_id(nopriv_browser.contents, 'edit-description')
104 Commit Message104 Commit Message
105 Add more &lt;b&gt;mojo&lt;/b&gt;105 Add more &lt;b&gt;mojo&lt;/b&gt;
@@ -113,7 +113,6 @@
113for the source_branch.113for the source_branch.
114114
115 >>> login('foo.bar@canonical.com')115 >>> login('foo.bar@canonical.com')
116 >>> eric = factory.makePerson(email="eric@example.com", password="test")
117 >>> bmp = factory.makeBranchMergeProposal(registrant=eric)116 >>> bmp = factory.makeBranchMergeProposal(registrant=eric)
118 >>> bmp_url = canonical_url(bmp)117 >>> bmp_url = canonical_url(bmp)
119 >>> branch_url = canonical_url(bmp.source_branch)118 >>> branch_url = canonical_url(bmp.source_branch)
@@ -128,14 +127,13 @@
128127
129 >>> sample_browser = setupBrowser(auth="Basic test@canonical.com:test")128 >>> sample_browser = setupBrowser(auth="Basic test@canonical.com:test")
130129
130
131Requesting reviews131Requesting reviews
132------------------132------------------
133133
134You can request a review of a merge proposal.134You can request a review of a merge proposal.
135135
136 >>> sample_browser.open(136 >>> sample_browser.open(klingon_proposal)
137 ... 'http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1'
138 ... )
139 >>> sample_browser.getLink('Request a review').click()137 >>> sample_browser.getLink('Request a review').click()
140 >>> sample_browser.getControl('Reviewer').value = 'mark'138 >>> sample_browser.getControl('Reviewer').value = 'mark'
141 >>> sample_browser.getControl('Review type').value = 'first'139 >>> sample_browser.getControl('Review type').value = 'first'
@@ -169,7 +167,7 @@
169 Reviewer Review Type Date Requested Status167 Reviewer Review Type Date Requested Status
170 Sample Person Pending [Review]168 Sample Person Pending [Review]
171 Mark Shuttleworth second ... ago Pending169 Mark Shuttleworth second ... ago Pending
172 Review via email: mp+1@code.launchpad.dev170 Review via email: mp+...@code.launchpad.dev
173 Request another review171 Request another review
174172
175173
@@ -178,16 +176,14 @@
178176
179People not logged in cannot perform reviews.177People not logged in cannot perform reviews.
180178
181 >>> anon_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'179 >>> anon_browser.open(klingon_proposal)
182 ... '/klingon/+merge/1')
183 >>> link = anon_browser.getLink('[Review]')180 >>> link = anon_browser.getLink('[Review]')
184 Traceback (most recent call last):181 Traceback (most recent call last):
185 LinkNotFoundError182 LinkNotFoundError
186183
187People who are logged in can perform reviews.184People who are logged in can perform reviews.
188185
189 >>> nopriv_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'186 >>> nopriv_browser.open(klingon_proposal)
190 ... '/klingon/+merge/1')
191 >>> nopriv_browser.getLink('Add a review or comment').click()187 >>> nopriv_browser.getLink('Add a review or comment').click()
192 >>> nopriv_browser.getControl(name='field.comment').value = "Don't like it"188 >>> nopriv_browser.getControl(name='field.comment').value = "Don't like it"
193 >>> nopriv_browser.getControl(name='field.vote').getControl(189 >>> nopriv_browser.getControl(name='field.vote').getControl(
@@ -201,8 +197,7 @@
201197
202People can claim reviews for teams of which they are a member.198People can claim reviews for teams of which they are a member.
203199
204 >>> sample_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'200 >>> sample_browser.open(klingon_proposal)
205 ... '/klingon/+merge/1')
206 >>> sample_browser.getLink('Request another review').click()201 >>> sample_browser.getLink('Request another review').click()
207 >>> sample_browser.getControl('Reviewer').value = 'hwdb-team'202 >>> sample_browser.getControl('Reviewer').value = 'hwdb-team'
208 >>> sample_browser.getControl('Review type').value = 'claimable'203 >>> sample_browser.getControl('Review type').value = 'claimable'
@@ -213,8 +208,7 @@
213 Reviewer Review Type Date Requested Status...208 Reviewer Review Type Date Requested Status...
214 HWDB Team claimable ... ago Pending ...209 HWDB Team claimable ... ago Pending ...
215 >>> foobar_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test")210 >>> foobar_browser = setupBrowser(auth="Basic foo.bar@canonical.com:test")
216 >>> foobar_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'211 >>> foobar_browser.open(klingon_proposal)
217 ... '/klingon/+merge/1')
218 >>> foobar_browser.getControl('Claim review').click()212 >>> foobar_browser.getControl('Claim review').click()
219 >>> pending = find_tag_by_id(213 >>> pending = find_tag_by_id(
220 ... foobar_browser.contents, 'code-review-votes')214 ... foobar_browser.contents, 'code-review-votes')
@@ -230,8 +224,7 @@
230 >>> foobar_browser.getLink('Reassign').click()224 >>> foobar_browser.getLink('Reassign').click()
231 >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team'225 >>> foobar_browser.getControl('Reviewer').value = 'hwdb-team'
232 >>> foobar_browser.getControl('Reassign').click()226 >>> foobar_browser.getControl('Reassign').click()
233 >>> foobar_browser.open('http://code.launchpad.dev/~name12/gnome-terminal'227 >>> foobar_browser.open(klingon_proposal)
234 ... '/klingon/+merge/1')
235 >>> pending = find_tag_by_id(228 >>> pending = find_tag_by_id(
236 ... foobar_browser.contents, 'code-review-votes')229 ... foobar_browser.contents, 'code-review-votes')
237230
@@ -248,8 +241,7 @@
248When a branch has been merged into the target branch, the proposal should241When a branch has been merged into the target branch, the proposal should
249be marked as merged.242be marked as merged.
250243
251 >>> nopriv_browser.open(244 >>> nopriv_browser.open(klingon_proposal)
252 ... 'http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1')
253245
254The edit icon at the end of the status allows the user to edit the status.246The edit icon at the end of the status allows the user to edit the status.
255247
@@ -264,8 +256,6 @@
264 >>> nopriv_browser.getControl(name='field.queue_status').displayValue = (256 >>> nopriv_browser.getControl(name='field.queue_status').displayValue = (
265 ... ['Merged'])257 ... ['Merged'])
266 >>> nopriv_browser.getControl('Change Status').click()258 >>> nopriv_browser.getControl('Change Status').click()
267 >>> print nopriv_browser.url
268 http://code.launchpad.dev/~name12/gnome-terminal/klingon/+merge/1
269259
270The proposal is now shown as have being merged.260The proposal is now shown as have being merged.
271261
@@ -282,6 +272,7 @@
282 >>> print extract_text(find_tag_by_id(272 >>> print extract_text(find_tag_by_id(
283 ... sample_browser.contents, 'landing-targets'))273 ... sample_browser.contents, 'landing-targets'))
284 Merged into lp://dev/~name12/gnome-terminal/main274 Merged into lp://dev/~name12/gnome-terminal/main
275 ...
285 >>> sample_browser.getLink('Merged').click()276 >>> sample_browser.getLink('Merged').click()
286 >>> print_summary(sample_browser)277 >>> print_summary(sample_browser)
287 Status: Merged278 Status: Merged
@@ -303,6 +294,7 @@
303 >>> print extract_text(find_tag_by_id(294 >>> print extract_text(find_tag_by_id(
304 ... sample_browser.contents, 'landing-targets'))295 ... sample_browser.contents, 'landing-targets'))
305 Merged into lp://dev/~name12/gnome-terminal/main at revision 42296 Merged into lp://dev/~name12/gnome-terminal/main at revision 42
297 ...
306298
307299
308Resubmitting proposals300Resubmitting proposals
@@ -315,9 +307,7 @@
315branches but with the state set to work-in-progress.307branches but with the state set to work-in-progress.
316308
317 >>> login('foo.bar@canonical.com')309 >>> login('foo.bar@canonical.com')
318 >>> fooix = factory.makeProduct(name='fooix')310 >>> bmp = factory.makeBranchMergeProposal(target_branch=trunk)
319 >>> target = factory.makeProductBranch(product=fooix, owner=eric)
320 >>> bmp = factory.makeBranchMergeProposal(target_branch=target)
321 >>> bmp_url = canonical_url(bmp)311 >>> bmp_url = canonical_url(bmp)
322 >>> logout()312 >>> logout()
323 >>> eric_browser.open(bmp_url)313 >>> eric_browser.open(bmp_url)
@@ -538,7 +528,6 @@
538 Bug #...: Bug for linking (Undecided &ndash; New)528 Bug #...: Bug for linking (Undecided &ndash; New)
539529
540530
541
542Target branch edge cases531Target branch edge cases
543------------------------532------------------------
544533
@@ -626,3 +615,30 @@
626 >>> print extract_text(get_review_diff())615 >>> print extract_text(get_review_diff())
627 Preview Diff616 Preview Diff
628 Empty617 Empty
618
619
620Merge proposal details shown on the branch page
621-----------------------------------------------
622
623A branch that has a merge proposal, but no requested reviews shows this on the
624branch page.
625
626 >>> nopriv_browser.open('http://code.launchpad.dev/~fred/fooix/feature')
627 >>> nopriv_browser.getLink('Propose for merging').click()
628 >>> nopriv_browser.getControl('Reviewer').value = ''
629 >>> nopriv_browser.getControl('Propose Merge').click()
630 >>> nopriv_browser.getLink('lp://dev/~fred/fooix/feature').click()
631 >>> print_tag_with_id(nopriv_browser.contents, 'landing-targets')
632 Ready for review for merging into lp://dev/fooix
633 No reviews requested
634
635If there are reviews either pending or completed these are also shown.
636
637The api tag is a hidden anchor that holds the URL for the merge proposal api
638access.
639
640 >>> nopriv_browser.open('http://code.launchpad.dev/~fred/fooix/proposed')
641 >>> print_tag_with_id(nopriv_browser.contents, 'landing-targets')
642 Ready for review for merging into lp://dev/fooix
643 Eric the Viking: Pending (code) requested ... ago
644 Diff: 47 lines (+7/-0) 2 files modified api
629645
=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt 2009-10-11 02:33:30 +0000
+++ lib/lp/code/templates/branch-index.pt 2009-11-18 15:05:39 +0000
@@ -25,6 +25,20 @@
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 {
26 font-weight: normal;26 font-weight: normal;
27 }27 }
28 dl.reviews dd {
29 padding-left: 2em;
30 margin-bottom: 0;
31 }
32 .yui-diff-overlay-hidden {
33 display: none;
34 }
35 .yui-diff-overlay {
36 width: 80%;
37 }
38 .yui-diff-overlay #yui-pretty-overlay-modal {
39 width: auto;
40 color: black;
41 }
2842
29 </style>43 </style>
30 <script type="text/javascript"44 <script type="text/javascript"
@@ -43,12 +57,20 @@
43 tal:attributes="src string:${lp_js}/code/branchstatus.js">57 tal:attributes="src string:${lp_js}/code/branchstatus.js">
44 </script>58 </script>
45 <script type="text/javascript"59 <script type="text/javascript"
60 tal:condition="devmode"
61 tal:define="lp_js string:${icingroot}/build"
62 tal:attributes="src string:${lp_js}/code/popupdiff.js">
63 </script>
64 <script type="text/javascript"
46 tal:content="string:65 tal:content="string:
47 YUI().use('node', 'event', 'widget', 'plugin', 'overlay',66 YUI().use('node', 'event', 'widget', 'plugin', 'overlay',
48 'lazr.choiceedit', 'code.branchstatus', function(Y) {67 'lazr.choiceedit', 'code.branchstatus',
68 'code.branchmergeproposal.popupdiff',
69 function(Y) {
49 Y.on('load',70 Y.on('load',
50 function(e) {71 function(e) {
51 Y.branchstatus.connect_status(${view/status_config});72 Y.branchstatus.connect_status(${view/status_config});
73 Y.code.branchmergeproposal.popupdiff.connect_diff_links();
52 },74 },
53 window);75 window);
54 });76 });
5577
=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
--- lib/lp/code/templates/branch-pending-merges.pt 2009-09-18 21:20:42 +0000
+++ lib/lp/code/templates/branch-pending-merges.pt 2009-11-18 15:05:38 +0000
@@ -36,6 +36,7 @@
36 <div>36 <div>
37 <tal:merge-fragment tal:replace="structure mergeproposal/@@+link-summary" />37 <tal:merge-fragment tal:replace="structure mergeproposal/@@+link-summary" />
38 </div>38 </div>
39 <tal:merge-fragment tal:replace="structure mergeproposal/@@+vote-summary" />
39 </tal:landing-candidates>40 </tal:landing-candidates>
40 </div>41 </div>
4142
4243
=== added file 'lib/lp/code/templates/branchmergeproposal-diff.pt'
--- lib/lp/code/templates/branchmergeproposal-diff.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/branchmergeproposal-diff.pt 2009-11-18 15:05:39 +0000
@@ -0,0 +1,33 @@
1<tal:root
2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3>
4
5 <div class="boardComment attachment diff"
6 tal:define="attachment view/preview_diff/diff_text">
7 <tal:real-diff condition="attachment">
8 <div class="boardCommentDetails filename">
9 <ul class="horizontal">
10 <li>
11 <a tal:attributes="href attachment/getURL">
12 <img src="/@@/download"/>
13 Download diff
14 </a>
15 </li>
16 </ul>
17 </div>
18 <div class="boardCommentBody attachmentBody">
19 <tal:diff replace="structure view/preview_diff_text/fmt:diff" />
20 </div>
21 <div class="boardCommentFooter"
22 tal:condition="view/diff_oversized">
23 The diff has been truncated for viewing.
24 </div>
25 </tal:real-diff>
26 <tal:empty-diff condition="not: attachment">
27 <div class="boardCommentBody attachmentBody">
28 <em>Empty</em>
29 </div>
30 </tal:empty-diff>
31 </div>
32
33</tal:root>
0\ No newline at end of file34\ No newline at end of file
135
=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt 2009-11-10 21:49:38 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt 2009-11-18 15:05:38 +0000
@@ -128,34 +128,8 @@
128 </div>128 </div>
129129
130 <div id="review-diff" tal:condition="view/preview_diff">130 <div id="review-diff" tal:condition="view/preview_diff">
131 <h2>Preview Diff</h2>131 <h2>Preview Diff</h2>
132 <div class="boardComment attachment"132 <div tal:replace="structure context/@@++diff"/>
133 tal:define="attachment view/preview_diff/diff_text">
134 <tal:real-diff condition="attachment">
135 <div class="boardCommentDetails filename">
136 <ul class="horizontal">
137 <li>
138 <a tal:attributes="href attachment/getURL">
139 <img src="/@@/download"/>
140 Download diff
141 </a>
142 </li>
143 </ul>
144 </div>
145 <div class="boardCommentBody attachmentBody">
146 <tal:diff replace="structure view/preview_diff_text/fmt:diff" />
147 </div>
148 <div class="boardCommentFooter"
149 tal:condition="view/review_diff_oversized">
150 The diff has been truncated for viewing.
151 </div>
152 </tal:real-diff>
153 <tal:empty-diff condition="not: attachment">
154 <div class="boardCommentBody attachmentBody">
155 <em>Empty</em>
156 </div>
157 </tal:empty-diff>
158 </div>
159 </div>133 </div>
160134
161<tal:script135<tal:script
162136
=== renamed file 'lib/lp/code/templates/branch-merge-link-summary.pt' => 'lib/lp/code/templates/branchmergeproposal-link-summary.pt'
--- lib/lp/code/templates/branch-merge-link-summary.pt 2009-09-22 16:31:42 +0000
+++ lib/lp/code/templates/branchmergeproposal-link-summary.pt 2009-11-18 15:05:39 +0000
@@ -4,7 +4,8 @@
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n">4 xmlns:i18n="http://xml.zope.org/namespaces/i18n">
55
6 <img src="/@@/merge-proposal-icon" />6 <img src="/@@/merge-proposal-icon" />
7 <a tal:attributes="href context/fmt:url"7 <a tal:attributes="href context/fmt:url;
8 title view/status_title"
8 tal:content="view/friendly_text">Approved</a>9 tal:content="view/friendly_text">Approved</a>
9 <tal:for-merging condition="not: context/queue_status/enumvalue:MERGED">10 <tal:for-merging condition="not: context/queue_status/enumvalue:MERGED">
10 for merging11 for merging
1112
=== added file 'lib/lp/code/templates/branchmergeproposal-vote-summary.pt'
--- lib/lp/code/templates/branchmergeproposal-vote-summary.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/branchmergeproposal-vote-summary.pt 2009-11-18 15:05:38 +0000
@@ -0,0 +1,52 @@
1<tal:root
2 xmlns:tal="http://xml.zope.org/namespaces/tal"
3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n">
5
6 <tal:comment condition="nothing">
7 <!--
8 Yet again we are bitten by white space issues, so some tags in this
9 template have closing brackets on following lines, and not breaks
10 between some tags.
11 -->
12 </tal:comment>
13
14<dl class="reviews">
15 <dd tal:condition="not: view/reviews">
16 No reviews requested
17 </dd>
18 <dd tal:repeat="review view/current_reviews">
19 <tal:reviewer replace="structure review/reviewer/fmt:link:mainsite">
20 Eric the Reviewer</tal:reviewer
21 ><tal:community condition="not: review/trusted">
22 (community)</tal:community>:
23 <span tal:attributes="class string:vote${review/comment/vote/name}"
24 tal:content="review/status_text">
25 Approved
26 </span>
27 <tal:vote-tags condition="review/review_type_str">
28 (<tal:tag replace="review/review_type_str"/>)
29 </tal:vote-tags>
30 <tal:date replace="review/date_of_comment/fmt:displaydate" />
31 </dd>
32 <dd tal:repeat="review view/requested_reviews"
33 tal:attributes="id string:review-${review/reviewer/name}">
34 <tal:reviewer
35 tal:replace="structure review/reviewer/fmt:link:mainsite" />:
36
37 <span class="votePENDING">Pending</span>
38 <tal:vote-tags condition="review/review_type_str">
39 (<tal:tag replace="review/review_type_str"/>)
40 </tal:vote-tags>
41 requested
42 <tal:date replace="review/date_requested/fmt:approximatedate"/>
43 </dd>
44 <dd tal:condition="context/preview_diff"
45 tal:attributes="class string:popup-diff mp-${context/id}">
46 Diff: <tal:diff replace="structure context/preview_diff/fmt:link"/>
47 <a class="api-ref" style="display:none"
48 tal:attributes="href context/preview_diff/fmt:api_url">api</a>
49 </dd>
50
51</dl>
52</tal:root>
053
=== modified file 'lib/lp/code/tests/helpers.py'
--- lib/lp/code/tests/helpers.py 2009-10-26 18:40:04 +0000
+++ lib/lp/code/tests/helpers.py 2009-11-18 15:05:38 +0000
@@ -6,10 +6,12 @@
6__metaclass__ = type6__metaclass__ = type
7__all__ = [7__all__ = [
8 'make_linked_package_branch',8 'make_linked_package_branch',
9 'make_erics_fooix_project',
9 ]10 ]
1011
1112
12from datetime import timedelta13from datetime import timedelta
14from difflib import unified_diff
13from itertools import count15from itertools import count
1416
15from zope.component import getUtility17from zope.component import getUtility
@@ -23,6 +25,46 @@
23from lp.testing import time_counter25from lp.testing import time_counter
2426
2527
28def make_erics_fooix_project(factory):
29 """Make Eric, the Fooix project, and some branches.
30
31 :return: a dict of objects to put into local scope.
32 """
33 result = {}
34 eric = factory.makePerson(
35 name='eric', displayname='Eric the Viking',
36 email='eric@example.com', password='test')
37 fooix = factory.makeProduct(
38 name='fooix', displayname='Fooix', owner=eric)
39 trunk = factory.makeProductBranch(
40 owner=eric, product=fooix, name='trunk')
41 removeSecurityProxy(fooix.development_focus).branch = trunk
42 # Development is done by Fred.
43 fred = factory.makePerson(
44 name='fred', displayname='Fred Flintstone',
45 email='fred@example.com', password='test')
46 feature = factory.makeProductBranch(
47 owner=fred, product=fooix, name='feature')
48 proposed = factory.makeProductBranch(
49 owner=fred, product=fooix, name='proposed')
50 bmp = proposed.addLandingTarget(
51 registrant=fred, target_branch=trunk, needs_review=True,
52 review_requests=[(eric, 'code')])
53 # And fake a diff.
54 naked_bmp = removeSecurityProxy(bmp)
55 preview = removeSecurityProxy(naked_bmp.updatePreviewDiff(
56 ''.join(unified_diff('', 'random content')), u'rev-a', u'rev-b'))
57 naked_bmp.source_branch.last_scanned_id = preview.source_revision_id
58 naked_bmp.target_branch.last_scanned_id = preview.target_revision_id
59 preview.diff_lines_count = 47
60 preview.added_lines_count = 7
61 preview.remvoed_lines_count = 13
62 preview.diffstat = {'file1': (3, 8), 'file2': (4, 5)}
63 return {
64 'eric': eric, 'fooix': fooix, 'trunk':trunk, 'feature': feature,
65 'proposed': proposed, 'fred': fred}
66
67
26def make_linked_package_branch(factory, distribution=None,68def make_linked_package_branch(factory, distribution=None,
27 sourcepackagename=None):69 sourcepackagename=None):
28 """Make a new package branch and make it official."""70 """Make a new package branch and make it official."""
2971
=== modified file 'lib/lp/code/tests/test_directbranchcommit.py'
--- lib/lp/code/tests/test_directbranchcommit.py 2009-10-02 09:48:30 +0000
+++ lib/lp/code/tests/test_directbranchcommit.py 2009-11-18 15:05:39 +0000
@@ -37,7 +37,7 @@
3737
38 self.committer = DirectBranchCommit(self.db_branch)38 self.committer = DirectBranchCommit(self.db_branch)
39 if update_last_scanned_id:39 if update_last_scanned_id:
40 self.db_branch.last_scanned_id = (40 self.committer.last_scanned_id = (
41 self.committer.bzrbranch.last_revision())41 self.committer.bzrbranch.last_revision())
4242
43 def _tearDownCommitter(self):43 def _tearDownCommitter(self):
4444
=== added file 'lib/lp/code/windmill/tests/test_popup_diff.py'
--- lib/lp/code/windmill/tests/test_popup_diff.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/windmill/tests/test_popup_diff.py 2009-11-18 15:05:39 +0000
@@ -0,0 +1,64 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Test for the popup diff."""
5
6__metaclass__ = type
7__all__ = []
8
9import transaction
10import unittest
11
12import windmill
13from windmill.authoring import WindmillTestClient
14
15from canonical.launchpad.windmill.testing.constants import PAGE_LOAD
16from lp.code.tests.helpers import make_erics_fooix_project
17from lp.code.windmill.testing import CodeWindmillLayer
18from lp.testing import TestCaseWithFactory
19
20
21POPUP_DIFF = (
22 u'//dd[contains(@class, "popup-diff")]'
23 '/a[contains(@class, "js-action")]')
24VISIBLE_DIFF = (
25 u'//table[contains(@class, "yui-diff-overlay") and '
26 'not(contains(@class, "yui-diff-overlay-hidden"))]')
27CLOSE_VISIBLE_DIFF = (
28 u'//table[contains(@class, "yui-diff-overlay")]'
29 '//a[@class="close-button"]')
30JS_ONLOAD_EXECUTE_DELAY = 1000
31
32
33class TestPopupOnBranchPage(TestCaseWithFactory):
34 """Test the popup diff."""
35
36 layer = CodeWindmillLayer
37
38 def test_branch_popup_diff(self):
39 """Test branch diff popups."""
40 client = WindmillTestClient("Branch popup diffs")
41 make_erics_fooix_project(self.factory)
42 transaction.commit()
43
44 start_url = (
45 windmill.settings['TEST_URL'] + '~fred/fooix/proposed')
46 client.open(url=start_url)
47 client.waits.forPageLoad(timeout=PAGE_LOAD)
48 # Sleep for a bit to make sure that the JS onload has had time to execute.
49 client.waits.sleep(milliseconds=JS_ONLOAD_EXECUTE_DELAY)
50
51 # Make sure that the link anchor has the js-action class.
52 client.asserts.assertNode(xpath=POPUP_DIFF)
53 client.click(xpath=POPUP_DIFF)
54
55 # Wait for the diff to show.
56 client.waits.forElement(xpath=VISIBLE_DIFF)
57 # Click on the close button.
58 client.click(xpath=CLOSE_VISIBLE_DIFF)
59 # Make sure that the diff has gone.
60 client.asserts.assertNotNode(xpath=VISIBLE_DIFF)
61
62
63def test_suite():
64 return unittest.TestLoader().loadTestsFromName(__name__)
065
=== modified file 'lib/lp/services/browser_helpers.py'
--- lib/lp/services/browser_helpers.py 2009-09-07 14:54:34 +0000
+++ lib/lp/services/browser_helpers.py 2009-11-18 15:05:39 +0000
@@ -6,6 +6,7 @@
6__metaclass__ = type6__metaclass__ = type
7__all__ = [7__all__ = [
8 'get_user_agent_distroseries',8 'get_user_agent_distroseries',
9 'get_plural_text',
9 ]10 ]
1011
11import re12import re
@@ -28,3 +29,10 @@
28 else:29 else:
29 return None30 return None
3031
32
33def get_plural_text(count, singular, plural):
34 """Return 'singular' if 'count' is 1, 'plural' otherwise."""
35 if count == 1:
36 return singular
37 else:
38 return plural
3139
=== modified file 'lib/lp/translations/doc/translations-export-to-branch.txt'
--- lib/lp/translations/doc/translations-export-to-branch.txt 2009-10-20 06:03:46 +0000
+++ lib/lp/translations/doc/translations-export-to-branch.txt 2009-11-18 15:05:39 +0000
@@ -45,7 +45,7 @@
45 ... if self.simulate_race:45 ... if self.simulate_race:
46 ... raise ConcurrentUpdateError("Simulated race condition.")46 ... raise ConcurrentUpdateError("Simulated race condition.")
47 ...47 ...
48 ... def commit(self, message=None):48 ... def commit(self, message=None, txn=None):
49 ... self._checkForRace()49 ... self._checkForRace()
50 ... self.logger.info("Committed %d file(s)." % self.written_files)50 ... self.logger.info("Committed %d file(s)." % self.written_files)
51 ...51 ...
@@ -129,11 +129,11 @@
129129
130 >>> transaction.commit()130 >>> transaction.commit()
131 >>> script = MockExportTranslationsToBranch(131 >>> script = MockExportTranslationsToBranch(
132 ... 'export-to-branch', test_args=['-vv'])132 ... 'export-to-branch', test_args=[])
133 >>> script.main()133 >>> script.main()
134 INFO Exporting to translations branches.134 INFO Exporting to translations branches.
135 INFO Exporting Gazblachko trunk series.135 INFO Exporting Gazblachko trunk series.
136 DEBUG No previous translations commit found.136 DEBUG ...
137 INFO Writing file 'po/main/nl.po':137 INFO Writing file 'po/main/nl.po':
138 INFO # ...138 INFO # ...
139 msgid ""139 msgid ""
@@ -143,6 +143,7 @@
143 msgid "maingazpot msgid"143 msgid "maingazpot msgid"
144 msgstr "maingazpot msgstr"144 msgstr "maingazpot msgstr"
145 <BLANKLINE>145 <BLANKLINE>
146 DEBUG ...
146 INFO Writing file 'po/module/nl.po':147 INFO Writing file 'po/module/nl.po':
147 INFO # ...148 INFO # ...
148 msgid ""149 msgid ""
@@ -152,6 +153,8 @@
152 <BLANKLINE>153 <BLANKLINE>
153 msgid "gazmod msgid"154 msgid "gazmod msgid"
154 msgstr "gazmod msgstr"155 msgstr "gazmod msgstr"
156 <BLANKLINE>
157 DEBUG ...
155 INFO Committed 2 file(s).158 INFO Committed 2 file(s).
156 INFO Unlock.159 INFO Unlock.
157 INFO Processed 1 item(s); 0 failure(s).160 INFO Processed 1 item(s); 0 failure(s).
@@ -186,6 +189,7 @@
186 >>> script.main()189 >>> script.main()
187 INFO Exporting to translations branches.190 INFO Exporting to translations branches.
188 INFO Exporting Gazblachko trunk series.191 INFO Exporting Gazblachko trunk series.
192 DEBUG ....
189 DEBUG Last commit was at ....193 DEBUG Last commit was at ....
190 INFO Unlock.194 INFO Unlock.
191 INFO Processed 1 item(s); 0 failure(s).195 INFO Processed 1 item(s); 0 failure(s).
@@ -197,6 +201,7 @@
197 >>> script.main()201 >>> script.main()
198 INFO Exporting to translations branches.202 INFO Exporting to translations branches.
199 INFO Exporting Gazblachko trunk series.203 INFO Exporting Gazblachko trunk series.
204 DEBUG ....
200 DEBUG Last commit was at ...205 DEBUG Last commit was at ...
201 INFO Writing file 'po/main/nl.po':206 INFO Writing file 'po/main/nl.po':
202 INFO ...207 INFO ...
@@ -228,11 +233,14 @@
228 >>> script.main()233 >>> script.main()
229 INFO Exporting to translations branches.234 INFO Exporting to translations branches.
230 INFO Exporting Gazblachko trunk series.235 INFO Exporting Gazblachko trunk series.
236 DEBUG ....
231 DEBUG No previous translations commit found.237 DEBUG No previous translations commit found.
238 DEBUG ....
232 INFO Writing file 'po/main/nl.po':239 INFO Writing file 'po/main/nl.po':
233 ...240 ...
234 msgstr "gazmod msgstr"241 msgstr "gazmod msgstr"
235 <BLANKLINE>242 <BLANKLINE>
243 DEBUG ...
236 INFO Unlock.244 INFO Unlock.
237 ERROR Failure: Simulated race condition.245 ERROR Failure: Simulated race condition.
238 INFO Processed 1 item(s); 1 failure(s).246 INFO Processed 1 item(s); 1 failure(s).
239247
=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
--- lib/lp/translations/scripts/translations_to_branch.py 2009-10-20 06:03:46 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py 2009-11-18 15:05:39 +0000
@@ -119,7 +119,7 @@
119 def _commit(self, source, committer):119 def _commit(self, source, committer):
120 """Commit changes to branch. Check for race conditions."""120 """Commit changes to branch. Check for race conditions."""
121 self._checkForObjections(source)121 self._checkForObjections(source)
122 committer.commit(self.commit_message)122 committer.commit(self.commit_message, txn=self.txn)
123123
124 def _isTranslationsCommit(self, revision):124 def _isTranslationsCommit(self, revision):
125 """Is `revision` an automatic translations commit?"""125 """Is `revision` an automatic translations commit?"""
@@ -159,6 +159,9 @@
159 self._checkForObjections(source)159 self._checkForObjections(source)
160160
161 committer = self._prepareBranchCommit(source.translations_branch)161 committer = self._prepareBranchCommit(source.translations_branch)
162 self.logger.debug("Created DirectBranchCommit.")
163 if self.txn:
164 self.txn.commit()
162165
163 bzr_branch = committer.bzrbranch166 bzr_branch = committer.bzrbranch
164167
@@ -185,15 +188,17 @@
185 base_path = os.path.dirname(template.path)188 base_path = os.path.dirname(template.path)
186189
187 for pofile in template.pofiles:190 for pofile in template.pofiles:
188
189 has_changed = (191 has_changed = (
190 changed_since is None or192 changed_since is None or
191 pofile.date_changed > changed_since)193 pofile.date_changed > changed_since)
192 if not has_changed:194 if not has_changed:
193 continue195 continue
194196
197 language_code = pofile.getFullLanguageCode()
198 self.logger.debug("Exporting %s." % language_code)
199
195 pofile_path = os.path.join(200 pofile_path = os.path.join(
196 base_path, pofile.getFullLanguageCode() + '.po')201 base_path, language_code + '.po')
197 pofile_contents = pofile.export()202 pofile_contents = pofile.export()
198203
199 committer.writeFile(pofile_path, pofile_contents)204 committer.writeFile(pofile_path, pofile_contents)
@@ -210,6 +215,7 @@
210 template.clearPOFileCache()215 template.clearPOFileCache()
211216
212 if change_count > 0:217 if change_count > 0:
218 self.logger.debug("Writing to branch.")
213 self._commit(source, committer)219 self._commit(source, committer)
214 finally:220 finally:
215 committer.unlock()221 committer.unlock()
@@ -231,13 +237,14 @@
231 raise237 raise
232 except Exception, e:238 except Exception, e:
233 items_failed += 1239 items_failed += 1
234 self.logger.error("Failure: %s" % e)240 message = unicode(e)
241 if message == u'':
242 message = e.__class__.__name__
243 self.logger.error("Failure: %s" % message)
235 if self.txn:244 if self.txn:
236 self.txn.abort()245 self.txn.abort()
237246
238 items_done += 1247 items_done += 1
239 if self.txn:
240 self.txn.begin()
241248
242 self.logger.info("Processed %d item(s); %d failure(s)." % (249 self.logger.info("Processed %d item(s); %d failure(s)." % (
243 items_done, items_failed))250 items_done, items_failed))
244251
=== modified file 'lib/lp/translations/utilities/gettext_po_exporter.py'
--- lib/lp/translations/utilities/gettext_po_exporter.py 2009-11-02 00:10:05 +0000
+++ lib/lp/translations/utilities/gettext_po_exporter.py 2009-11-18 15:05:39 +0000
@@ -14,6 +14,7 @@
14 'GettextPOExporter',14 'GettextPOExporter',
15 ]15 ]
1616
17import logging
17import os18import os
1819
19from zope.interface import implements20from zope.interface import implements
@@ -344,8 +345,20 @@
344 raise UnicodeEncodeError(345 raise UnicodeEncodeError(
345 '%s:\n%s' % (file_path, str(error)))346 '%s:\n%s' % (file_path, str(error)))
346347
347 # This message cannot be represented in current encoding,348 # This message cannot be represented in the current
348 # change to UTF-8 and try again.349 # encoding.
350 if translation_file.path:
351 file_description = translation_file.path
352 elif translation_file.language_code:
353 file_description = (
354 "%s translation" % translation_file.language_code)
355 else:
356 file_description = "template"
357 logging.info(
358 "Can't represent %s as %s; using UTF-8 instead." % (
359 file_description,
360 translation_file.header.charset.upper()))
361
349 old_charset = translation_file.header.charset362 old_charset = translation_file.header.charset
350 translation_file.header.charset = 'UTF-8'363 translation_file.header.charset = 'UTF-8'
351 # We need to update the header too.364 # We need to update the header too.

Subscribers

People subscribed via source and target branches

to status/vote changes: