Merge lp:~jtv/launchpad/cp-bug-403992 into lp:launchpad/db-devel

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= Bug 403992 =

I'm hoping to get this branch cherry-picked.

Message-sharing migration is failing. This is the command that broke on
us:

    ./scripts/rosetta/message-sharing-merge.py -vvv -P elisa

The problem is with some caching the script does. It keeps a cache of
TranslationMessages for the representative POTMsgSet it's merging
subordinate POTMsgSets into. It uses this cache to detect when moving
a current or imported TranslationMessage from a subordinate POTMsgSet to
the representative POTMsgSet would conflict with an existing current or
imported TranslationMessage.

Unfortunately the script fails to update this cache with the additions
to the representative POTMsgSet as it moves TranslationMessages over.
Thus a conflict could go undetected and violate a unique constraint on
the database.

This branch fixes the problem minimally by using the non-caching method
to detect clashes, rather than the broken cache. A separate branch,
lp:~jtv/launchpad/bug-403992 also culls the dead code and its unit
tests.

There are no test changes here. Basic functionality is retained, and I
see no easy way for this exact problem to come back now that the cache
is no longer in use.

Jeroen

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2009-07-29 09:46:46 +0000
+++ lib/canonical/config/schema-lazr.conf	2009-08-05 02:18:13 +0000
@@ -584,6 +584,16 @@
 # in refcounts and memory.
 references_leak_log: /tmp/references-leak.log
 
+[diff]
+# The maximum size in bytes to read from the librarian to make available in
+# the web UI.  512k == 524288 bytes.
+# datatype: integer
+max_read_size: 524288
+
+# The maximum number of lines to format using the format_diff tal formatter.
+max_format_lines: 5000
+
+
 [distributionmirrorprober]
 # The database user which will be used by this process.
 # datatype: string
 
=== modified file 'lib/canonical/launchpad/emailtemplates/branch-merge-proposal-created.txt'
--- lib/canonical/launchpad/emailtemplates/branch-merge-proposal-created.txt	2008-11-26 07:40:34 +0000
+++ lib/canonical/launchpad/emailtemplates/branch-merge-proposal-created.txt	2009-08-04 09:15:32 +0000
@@ -1,6 +1,6 @@
 %(proposal_registrant)s has proposed merging %(source_branch)s into %(target_branch)s.
 
-%(reviews)s%(gap)s%(comment)s
+%(reviews)s%(gap)s%(comment)s%(diff_cutoff_warning)s
 -- 
 %(proposal_url)s
 %(reason)s%(edit_subscription)s
 
=== modified file 'lib/canonical/launchpad/emailtemplates/review-requested.txt'
--- lib/canonical/launchpad/emailtemplates/review-requested.txt	2009-02-15 23:44:45 +0000
+++ lib/canonical/launchpad/emailtemplates/review-requested.txt	2009-08-04 09:15:32 +0000
@@ -1,7 +1,7 @@
 You have been requested to review the proposed merge of %(source_branch)s into %(target_branch)s.
 
-%(comment)s
+%(comment)s%(diff_cutoff_warning)s
 
---
+-- 
 %(proposal_url)s
 %(reason)s%(edit_subscription)s
 
=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2009-08-04 05:02:41 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2009-08-05 07:35:57 +0000
@@ -75,6 +75,11 @@
 IBranch['linkSpecification'].queryTaggedValue(
     LAZR_WEBSERVICE_EXPORTED)['params']['spec'].schema= ISpecification
 IBranch['product'].schema = IProduct
+IBranch['setTarget'].queryTaggedValue(
+    LAZR_WEBSERVICE_EXPORTED)['params']['project'].schema= IProduct
+IBranch['setTarget'].queryTaggedValue(
+    LAZR_WEBSERVICE_EXPORTED)['params']['source_package'].schema= \
+        ISourcePackage
 IBranch['spec_links'].value_type.schema = ISpecificationBranch
 IBranch['subscribe'].queryTaggedValue(
     LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = IBranchSubscription
 
=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js	2009-08-04 12:26:40 +0000
+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js	2009-08-05 09:33:08 +0000
@@ -1319,6 +1319,155 @@
     }
 };
 
+/**
+ * Set up the "me too" selection.
+ *
+ * Called once, on load, to initialize the page. Call this function if
+ * the "me too" information is displayed on a bug page and the user is
+ * logged in.
+ *
+ * @method setup_me_too
+ */
+bugs.setup_me_too = function(user_is_affected) {
+    var me_too_content = Y.get('#affectsmetoo');
+    var me_too_edit = new MeTooChoiceSource({
+        contentBox: me_too_content, value: user_is_affected,
+        elementToFlash: me_too_content
+    });
+    me_too_edit.render();
+};
+
+/**
+ * This class is a derivative of ChoiceSource that handles the
+ * specifics of editing "me too" option.
+ *
+ * @class MeTooChoiceSource
+ * @extends ChoiceSource
+ * @constructor
+ */
+function MeTooChoiceSource() {
+    MeTooChoiceSource.superclass.constructor.apply(this, arguments);
+}
+
+MeTooChoiceSource.NAME = 'metoocs';
+MeTooChoiceSource.NS = 'metoocs';
+
+MeTooChoiceSource.HTML_PARSER = {
+    flame_icon: ".dynamic img[src$=/@@/flame-icon]"
+};
+
+MeTooChoiceSource.ATTRS = {
+    /**
+     * The title is always the same, so bake it in here.
+     *
+     * @attribute title
+     * @type String
+     */
+    title: {
+        value: 'Does this bug affect you?'
+    },
+
+    /**
+     * The items are always the same, so bake them in here.
+     *
+     * @attribute items
+     * @type Array
+     */
+    items: {
+        value: [
+            { name: 'Yes, it affects me', value: true,
+              source_name: 'This bug affects me too',
+              disabled: false },
+            { name: "No, it doesn't affect me", value: false,
+              source_name: "This bug doesn't affect me",
+              disabled: false }
+        ]
+    },
+
+    /**
+     * Y.Node containing a flame icon, displayed when the user is
+     * affected by the current bug. Should be automatically calculated
+     * by HTML_PARSER.
+     *
+     * Setter function returns Y.get(parameter) so that you can pass
+     * either a Node (as expected) or a selector.
+     *
+     * @attribute value_location
+     * @type Node
+     */
+    flame_icon: {
+        value: null,
+        set: function(v) {
+            return Y.get(v);
+        }
+    }
+};
+
+// Put this in the bugs namespace so it can be accessed for testing.
+bugs._MeTooChoiceSource = MeTooChoiceSource;
+
+Y.extend(MeTooChoiceSource, Y.ChoiceSource, {
+    initializer: function() {
+        var widget = this;
+        this.error_handler = new LP.client.ErrorHandler();
+        this.error_handler.clearProgressUI = function() {
+            widget._uiClearWaiting();
+        };
+        this.error_handler.showError = function(error_msg) {
+            widget.showError(error_msg);
+        };
+    },
+
+    showError: function(err) {
+        display_error(null, err);
+    },
+
+    syncUI: function() {
+        MeTooChoiceSource.superclass.syncUI.apply(this, arguments);
+        // Show the flame icon if the user is affected by this bug.
+        if (this.get('value')) {
+            this.get('flame_icon').removeClass('unseen');
+        } else {
+            this.get('flame_icon').addClass('unseen');
+        }
+    },
+
+    render: function() {
+        MeTooChoiceSource.superclass.render.apply(this, arguments);
+        // Force the ChoiceSource to be rendered inline.
+        this.get('boundingBox').setStyle('display', 'inline');
+        // Hide the static content and show the dynamic content.
+        this.get('contentBox').query('.static').addClass('unseen');
+        this.get('contentBox').query('.dynamic').removeClass('unseen');
+    },
+
+    _saveData: function() {
+        // Set the widget to the 'waiting' state.
+        this._uiSetWaiting();
+
+        var value = this.getInput();
+        var client =  new LP.client.Launchpad();
+        var widget = this;
+
+        var config = {
+            on: {
+                success: function(entry) {
+                    widget._uiClearWaiting();
+                    MeTooChoiceSource.superclass._saveData.call(
+                        widget, value);
+                },
+                failure: this.error_handler.getFailureHandler()
+            },
+            parameters: {
+                affected: value
+            }
+        };
+
+        client.named_post(
+            LP.client.cache.bug.self_link, 'markUserAffected', config);
+    }
+});
+
 /*
  * Check if the current user can unsubscribe the person
  * being subscribed.
@@ -1503,6 +1652,6 @@
 }
 
 }, '0.1', {requires: ['base', 'oop', 'node', 'event', 'io-base', 'substitute',
-                      'widget-position-ext', 'lazr.formoverlay', 'lazr.anim', 
+                      'widget-position-ext', 'lazr.formoverlay', 'lazr.anim',
                       'lazr.base', 'lazr.overlay', 'lazr.choiceedit',
                       'lp.picker', 'lp.client.plugins', 'lp.subscriber']});
 
=== added file 'lib/canonical/launchpad/javascript/bugs/tests/test_me_too.html'
--- lib/canonical/launchpad/javascript/bugs/tests/test_me_too.html	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/bugs/tests/test_me_too.html	2009-08-04 16:48:41 +0000
@@ -0,0 +1,34 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+  <head>
+  <title>Status Editor</title>
+
+  <!-- YUI 3.0 Setup -->
+  <script type="text/javascript" src="../../../icing/yui/3.0.0pr2/build/yui/yui.js"></script>
+  <link rel="stylesheet" href="../../../icing/yui/3.0.0pr2/build/cssreset/reset.css"/>
+  <link rel="stylesheet" href="../../../icing/yui/3.0.0pr2/build/cssfonts/fonts.css"/>
+  <link rel="stylesheet" href="../../../icing/yui/3.0.0pr2/build/cssbase/base.css"/>
+
+  <!-- Dependency -->
+  <script type="text/javascript" src="../../../icing/lazr/build/lazr.js"></script>
+  <script type="text/javascript" src="../../../icing/lazr/build/overlay/overlay.js"></script>
+  <script type="text/javascript" src="../../../icing/lazr/build/choiceedit/choiceedit.js"></script>
+  <script type="text/javascript" src="../../../javascript/client/client.js"></script>
+
+  <!-- The module under test -->
+  <script type="text/javascript" src="../bugtask-index.js"></script>
+
+  <!-- The test suite -->
+  <script type="text/javascript" src="test_me_too.js"></script>
+
+  <!-- Test layout -->
+  <link rel="stylesheet" href="../../test.css" />
+  <style type="text/css">
+    /* CSS classes specific to this test */
+    .unseen { display: none; }
+  </style>
+</head>
+<body class="yui-skin-sam">
+  <div id="log"></div>
+</body>
+</html>
 
=== added file 'lib/canonical/launchpad/javascript/bugs/tests/test_me_too.js'
--- lib/canonical/launchpad/javascript/bugs/tests/test_me_too.js	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/bugs/tests/test_me_too.js	2009-08-04 16:19:27 +0000
@@ -0,0 +1,223 @@
+/* Copyright (c) 2008, Canonical Ltd. All rights reserved. */
+
+YUI({
+    base: '../../../icing/yui/current/build/',
+    filter: 'raw',
+    combine: false
+    }).use('event', 'bugs.bugtask_index', 'node', 'yuitest', 'widget-stack', 'console',
+        function(Y) {
+
+// Local aliases
+var Assert = Y.Assert,
+    ArrayAssert = Y.ArrayAssert;
+
+/*
+ * A wrapper for the Y.Event.simulate() function.  The wrapper accepts
+ * CSS selectors and Node instances instead of raw nodes.
+ */
+function simulate(widget, selector, evtype, options) {
+    var rawnode = Y.Node.getDOMNode(widget.query(selector));
+    Y.Event.simulate(rawnode, evtype, options);
+}
+
+/* Helper function to clean up a dynamically added widget instance. */
+function cleanup_widget(widget) {
+    // Nuke the boundingBox, but only if we've touched the DOM.
+    if (widget.get('rendered')) {
+        var bb = widget.get('boundingBox');
+        if (bb.get('parentNode')) {
+            bb.get('parentNode').removeChild(bb);
+        }
+    }
+    // Kill the widget itself.
+    widget.destroy();
+}
+
+var suite = new Y.Test.Suite("Bugtask Me-Too Choice Edit Tests");
+
+suite.add(new Y.Test.Case({
+
+    name: 'me_too_choice_edit_basics',
+
+    setUp: function() {
+        // Monkeypatch LP.client to avoid network traffic and to make
+        // some things work as expected.
+        LP.client.Launchpad.prototype.named_post =
+            function(url, func, config) {
+                config.on.success();
+            };
+        LP.client.cache.bug = {
+            self_link: "http://bugs.example.com/bugs/1234"
+        };
+        // add the in-page HTML
+        var inpage = Y.Node.create([
+            '<span id="affectsmetoo">',
+            '  <span class="static">',
+            '    <img src="https://bugs.edge.launchpad.net/@@/flame-icon" alt="" />',
+            '    This bug affects me too',
+            '    <a href="+affectsmetoo">',
+            '      <img class="editicon" alt="Edit"',
+            '           src="https://bugs.edge.launchpad.net/@@/edit" />',
+            '    </a>',
+            '  </span>',
+            '  <span class="dynamic unseen">',
+            '    <img class="editicon" alt="Edit"',
+            '         src="https://bugs.edge.launchpad.net/@@/edit" />',
+            '    <a href="+affectsmetoo" class="js-action"',
+            '       ><span class="value">Does this bug affect you?</span></a>',
+            '    <img src="https://bugs.edge.launchpad.net/@@/flame-icon" alt=""/>',
+            '  </span>',
+            '</span>'].join(''));
+        Y.get("body").appendChild(inpage);
+        var me_too_content = Y.get('#affectsmetoo');
+        this.config = {
+            contentBox: me_too_content, value: null,
+            elementToFlash: me_too_content
+        };
+        this.choice_edit = new Y.bugs._MeTooChoiceSource(this.config);
+        this.choice_edit.render();
+    },
+
+    tearDown: function() {
+        if (this.choice_edit._choice_list) {
+            cleanup_widget(this.choice_edit._choice_list);
+        }
+        var status = Y.get("document").query("#affectsmetoo");
+        if (status) {
+            status.get("parentNode").removeChild(status);
+        }
+    },
+
+    /**
+     * The choice edit should be displayed inline.
+     */
+    test_is_inline: function() {
+        var display = this.choice_edit.get('boundingBox').getStyle('display');
+        Assert.areEqual(
+            display, 'inline', "Not displayed inline, display is: " + display);
+    },
+
+    /**
+     * The .static area should be hidden by adding the "unseen" class.
+     */
+    test_hide_static: function() {
+        var static_area = this.choice_edit.get('contentBox').query('.static');
+        Assert.isTrue(
+            static_area.hasClass('unseen'), "Static area is not hidden.");
+    },
+
+    /**
+     * The .dynamic area should be shown by removing the "unseen" class.
+     */
+    test_hide_dynamic: function() {
+        var dynamic_area = this.choice_edit.get('contentBox').query('.dynamic');
+        Assert.isFalse(
+            dynamic_area.hasClass('unseen'), "Dynamic area is hidden.");
+    },
+
+    /**
+     * The flame icon should be hidden initially.
+     */
+    test_flame_hidden_initially: function() {
+        var flame_icon = this.choice_edit.get('flame_icon');
+        Assert.isTrue(flame_icon.hasClass('unseen'), "Flame is not hidden.");
+    },
+
+    /**
+     * The flame icon should be hidden when the user has made a
+     * negative choice (i.e. "Does not affect me").
+     */
+    test_flame_hidden_with_negative_choice: function() {
+        simulate(this.choice_edit.get('boundingBox'), '.value', 'mousedown');
+        simulate(this.choice_edit._choice_list.get('boundingBox'),
+                 'li a[href$=false]', 'click');
+        var flame_icon = this.choice_edit.get('flame_icon');
+        Assert.isTrue(flame_icon.hasClass('unseen'), "Flame is not hidden.");
+    },
+
+    /**
+     * The flame icon should be shown when the user has made a
+     * positive choice (i.e. "Affects me too").
+     */
+    test_flame_hidden_with_positive_choice: function() {
+        simulate(this.choice_edit.get('boundingBox'), '.value', 'mousedown');
+        simulate(this.choice_edit._choice_list.get('boundingBox'),
+                 'li a[href$=true]', 'click');
+        var flame_icon = this.choice_edit.get('flame_icon');
+        Assert.isFalse(flame_icon.hasClass('unseen'), "Flame is hidden.");
+    },
+
+    /**
+     * The UI should be in a waiting state while the save process is
+     * executing and return to a non-waiting state once it has
+     * finished.
+     */
+    test_ui_waiting_for_success: function() {
+        this.do_test_ui_waiting('success');
+    },
+
+    /**
+     * The UI should be in a waiting state while the save process is
+     * executing and return to a non-waiting state even if the process
+     * fails.
+     */
+    test_ui_waiting_for_failure: function() {
+        this.do_test_ui_waiting('failure');
+    },
+
+    /**
+     * Helper function that does the leg work for the
+     * test_ui_waiting_* methods.
+     */
+    do_test_ui_waiting: function(callback) {
+        var edit_icon = this.choice_edit.get('editicon');
+        // The spinner should not be displayed at first.
+        Assert.isNull(
+            edit_icon.get('src').match(/\/spinner$/),
+            "The edit icon is displaying a spinner at rest.");
+        // The spinner should not be displayed after opening the
+        // choice list.
+        simulate(this.choice_edit.get('boundingBox'), '.value', 'mousedown');
+        Assert.isNull(
+            edit_icon.get('src').match(/\/spinner$/),
+            "The edit icon is displaying a spinner after opening the choice list.");
+        // The spinner should be visible during the interval between a
+        // choice being made and a response coming back from Launchpad
+        // that the choice has been saved.
+        var edit_icon_src_during_save;
+        // Patch the named_post method to simulate success or failure,
+        // as determined by the callback argument. We cannot make
+        // assertions in this method because exceptions are swallowed
+        // somewhere. Instead, we save something testable to a local
+        // var.
+        LP.client.Launchpad.prototype.named_post =
+            function(url, func, config) {
+                edit_icon_src_during_save = edit_icon.get('src');
+                config.on[callback]();
+            };
+        simulate(this.choice_edit._choice_list.get('boundingBox'),
+                 'li a[href$=true]', 'click');
+        Assert.isNotNull(
+            edit_icon_src_during_save.match(/\/spinner$/),
+            "The edit icon is not displaying a spinner during save.");
+        // The spinner should not be displayed once a choice has been
+        // saved.
+        Assert.isNull(
+            edit_icon.get('src').match(/\/spinner$/),
+            "The edit icon is displaying a spinner once the choice has been made.");
+    }
+
+}));
+
+Y.Test.Runner.add(suite);
+
+var yconsole = new Y.Console({
+    newestOnTop: false
+});
+yconsole.render('#log');
+
+Y.on('domready', function() {
+    Y.Test.Runner.run();
+});
+
+});
 
=== modified file 'lib/canonical/launchpad/webapp/adapter.py'
--- lib/canonical/launchpad/webapp/adapter.py	2009-07-28 22:35:01 +0000
+++ lib/canonical/launchpad/webapp/adapter.py	2009-07-29 11:37:51 +0000
@@ -134,7 +134,7 @@
 def get_request_statements():
     """Get the list of executed statements in the request.
 
-    The list is composed of (starttime, endtime, statement) tuples.
+    The list is composed of (starttime, endtime, db_id, statement) tuples.
     Times are given in milliseconds since the start of the request.
     """
     return getattr(_local, 'request_statements', [])
@@ -165,7 +165,11 @@
     # convert times to integer millisecond values
     starttime = int((starttime - request_starttime) * 1000)
     endtime = int((endtime - request_starttime) * 1000)
-    _local.request_statements.append((starttime, endtime, statement))
+    # A string containing no whitespace that lets us identify which Store
+    # is being used.
+    database_identifier = connection_wrapper._database.name
+    _local.request_statements.append(
+        (starttime, endtime, database_identifier, statement))
 
     # store the last executed statement as an attribute on the current
     # thread
@@ -274,6 +278,8 @@
         # opinion on what uri is.
         # pylint: disable-msg=W0231
         self._uri = uri
+        # A unique name for this database connection.
+        self.name = uri.database
 
     def raw_connect(self):
         # Prevent database connections from the main thread if
@@ -348,6 +354,9 @@
 
 class LaunchpadSessionDatabase(Postgres):
 
+    # A unique name for this database connection.
+    name = 'session'
+
     def raw_connect(self):
         self._dsn = 'dbname=%s user=%s' % (config.launchpad_session.dbname,
                                            config.launchpad_session.dbuser)
 
=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py	2009-07-23 05:02:47 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py	2009-08-05 07:34:55 +0000
@@ -167,9 +167,9 @@
             fp.write('%s=%s\n' % (urllib.quote(key, safe_chars),
                                   urllib.quote(value, safe_chars)))
         fp.write('\n')
-        for (start, end, statement) in self.db_statements:
-            fp.write('%05d-%05d %s\n' % (start, end,
-                                         _normalise_whitespace(statement)))
+        for (start, end, database_id, statement) in self.db_statements:
+            fp.write('%05d-%05d@%s %s\n' % (
+                start, end, database_id, _normalise_whitespace(statement)))
         fp.write('\n')
         fp.write(self.tb_text)
 
@@ -206,9 +206,12 @@
             line = line.strip()
             if line == '':
                 break
-            startend, statement = line.split(' ', 1)
-            start, end = startend.split('-')
-            statements.append((int(start), int(end), statement))
+            start, end, db_id, statement = re.match(
+                r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line).groups()
+            if db_id is not None:
+                db_id = intern(db_id) # This string is repeated lots.
+            statements.append(
+                (int(start), int(end), db_id, statement))
 
         # The rest is traceback.
         tb_text = ''.join(lines)
@@ -471,9 +474,10 @@
 
             duration = get_request_duration()
 
-            statements = sorted((start, end, _safestr(statement))
-                                for (start, end, statement)
-                                    in get_request_statements())
+            statements = sorted(
+                (start, end, _safestr(database_id), _safestr(statement))
+                for (start, end, database_id, statement)
+                    in get_request_statements())
 
             oopsid, filename = self.newOopsId(now)
 
 
=== modified file 'lib/canonical/launchpad/webapp/ftests/test_adapter.txt'
--- lib/canonical/launchpad/webapp/ftests/test_adapter.txt	2009-04-17 10:32:16 +0000
+++ lib/canonical/launchpad/webapp/ftests/test_adapter.txt	2009-08-04 12:14:57 +0000
@@ -292,7 +292,7 @@
     >>> set_request_started()
     >>> store.execute('SELECT 1', noresult=True)
     >>> store.execute('SELECT 2', noresult=True)
-    >>> for starttime, endtime, statement in get_request_statements():
+    >>> for starttime, endtime, db_id, statement in get_request_statements():
     ...     print statement
     SELECT 1
     SELECT 2
 
=== modified file 'lib/canonical/launchpad/webapp/launchpadform.py'
--- lib/canonical/launchpad/webapp/launchpadform.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/webapp/launchpadform.py	2009-08-04 00:41:49 +0000
@@ -372,7 +372,7 @@
 
     render_context = True
 
-    def updateContextFromData(self, data, context=None):
+    def updateContextFromData(self, data, context=None, notify_modified=True):
         """Update the context object based on form data.
 
         If no context is given, the view's context is used.
@@ -386,12 +386,13 @@
         """
         if context is None:
             context = self.context
-        context_before_modification = Snapshot(
-            context, providing=providedBy(context))
+        if notify_modified:
+            context_before_modification = Snapshot(
+                context, providing=providedBy(context))
 
         was_changed = form.applyChanges(context, self.form_fields,
                                         data, self.adapters)
-        if was_changed:
+        if was_changed and notify_modified:
             field_names = [form_field.__name__
                            for form_field in self.form_fields]
             notify(ObjectModifiedEvent(
 
=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py	2009-08-03 15:21:35 +0000
+++ lib/canonical/launchpad/webapp/tales.py	2009-08-05 02:18:13 +0000
@@ -2757,7 +2757,8 @@
             return text
         result = ['<table class="diff">']
 
-        for row, line in enumerate(text.split('\n')):
+        max_format_lines = config.diff.max_format_lines
+        for row, line in enumerate(text.splitlines()[:max_format_lines]):
             result.append('<tr>')
             result.append('<td class="line-no">%s</td>' % (row+1))
             if line.startswith('==='):
 
=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
--- lib/canonical/launchpad/webapp/tests/test_errorlog.py	2009-07-17 00:26:05 +0000
+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py	2009-07-29 14:28:18 +0000
@@ -57,8 +57,8 @@
                             'pageid', 'traceback-text', 'username', 'url', 42,
                             [('name1', 'value1'), ('name2', 'value2'),
                              ('name1', 'value3')],
-                            [(1, 5, 'SELECT 1'),
-                             (5, 10, 'SELECT 2')])
+                            [(1, 5, 'store_a', 'SELECT 1'),
+                             (5, 10, 'store_b', 'SELECT 2')])
         self.assertEqual(entry.id, 'id')
         self.assertEqual(entry.type, 'exc-type')
         self.assertEqual(entry.value, 'exc-value')
@@ -74,8 +74,12 @@
         self.assertEqual(entry.req_vars[1], ('name2', 'value2'))
         self.assertEqual(entry.req_vars[2], ('name1', 'value3'))
         self.assertEqual(len(entry.db_statements), 2)
-        self.assertEqual(entry.db_statements[0], (1, 5, 'SELECT 1'))
-        self.assertEqual(entry.db_statements[1], (5, 10, 'SELECT 2'))
+        self.assertEqual(
+            entry.db_statements[0],
+            (1, 5, 'store_a', 'SELECT 1'))
+        self.assertEqual(
+            entry.db_statements[1],
+            (5, 10, 'store_b', 'SELECT 2'))
 
     def test_write(self):
         """Test ErrorReport.write()"""
@@ -88,8 +92,8 @@
                             [('HTTP_USER_AGENT', 'Mozilla/5.0'),
                              ('HTTP_REFERER', 'http://localhost:9000/'),
                              ('name=foo', 'hello\nworld')],
-                            [(1, 5, 'SELECT 1'),
-                             (5, 10, 'SELECT\n2')])
+                            [(1, 5, 'store_a', 'SELECT 1'),
+                             (5, 10,'store_b', 'SELECT\n2')])
         fp = StringIO.StringIO()
         entry.write(fp)
         self.assertEqual(fp.getvalue(), dedent("""\
@@ -108,13 +112,58 @@
             HTTP_REFERER=http://localhost:9000/
             name%%3Dfoo=hello%%0Aworld
 
-            00001-00005 SELECT 1
-            00005-00010 SELECT 2
+            00001-00005@store_a SELECT 1
+            00005-00010@store_b SELECT 2
 
             traceback-text""" % (versioninfo.branch_nick, versioninfo.revno)))
 
     def test_read(self):
-        """Test ErrorReport.read()"""
+        """Test ErrorReport.read()."""
+        fp = StringIO.StringIO(dedent("""\
+            Oops-Id: OOPS-A0001
+            Exception-Type: NotFound
+            Exception-Value: error message
+            Date: 2005-04-01T00:00:00+00:00
+            Page-Id: IFoo:+foo-template
+            User: Sample User
+            URL: http://localhost:9000/foo
+            Duration: 42
+
+            HTTP_USER_AGENT=Mozilla/5.0
+            HTTP_REFERER=http://localhost:9000/
+            name%3Dfoo=hello%0Aworld
+
+            00001-00005@store_a SELECT 1
+            00005-00010@store_b SELECT 2
+
+            traceback-text"""))
+        entry = ErrorReport.read(fp)
+        self.assertEqual(entry.id, 'OOPS-A0001')
+        self.assertEqual(entry.type, 'NotFound')
+        self.assertEqual(entry.value, 'error message')
+        self.assertEqual(entry.time, datetime.datetime(2005, 4, 1))
+        self.assertEqual(entry.pageid, 'IFoo:+foo-template')
+        self.assertEqual(entry.tb_text, 'traceback-text')
+        self.assertEqual(entry.username, 'Sample User')
+        self.assertEqual(entry.url, 'http://localhost:9000/foo')
+        self.assertEqual(entry.duration, 42)
+        self.assertEqual(len(entry.req_vars), 3)
+        self.assertEqual(entry.req_vars[0], ('HTTP_USER_AGENT',
+                                             'Mozilla/5.0'))
+        self.assertEqual(entry.req_vars[1], ('HTTP_REFERER',
+                                             'http://localhost:9000/'))
+        self.assertEqual(entry.req_vars[2], ('name=foo', 'hello\nworld'))
+        self.assertEqual(len(entry.db_statements), 2)
+        self.assertEqual(
+            entry.db_statements[0],
+            (1, 5, 'store_a', 'SELECT 1'))
+        self.assertEqual(
+            entry.db_statements[1],
+            (5, 10, 'store_b', 'SELECT 2'))
+
+
+    def test_read_no_store_id(self):
+        """Test ErrorReport.read() for old logs with no store_id."""
         fp = StringIO.StringIO(dedent("""\
             Oops-Id: OOPS-A0001
             Exception-Type: NotFound
@@ -137,8 +186,6 @@
         self.assertEqual(entry.id, 'OOPS-A0001')
         self.assertEqual(entry.type, 'NotFound')
         self.assertEqual(entry.value, 'error message')
-        # XXX jamesh 2005-11-30:
-        # this should probably convert back to a datetime
         self.assertEqual(entry.time, datetime.datetime(2005, 4, 1))
         self.assertEqual(entry.pageid, 'IFoo:+foo-template')
         self.assertEqual(entry.tb_text, 'traceback-text')
@@ -152,8 +199,8 @@
                                              'http://localhost:9000/'))
         self.assertEqual(entry.req_vars[2], ('name=foo', 'hello\nworld'))
         self.assertEqual(len(entry.db_statements), 2)
-        self.assertEqual(entry.db_statements[0], (1, 5, 'SELECT 1'))
-        self.assertEqual(entry.db_statements[1], (5, 10, 'SELECT 2'))
+        self.assertEqual(entry.db_statements[0], (1, 5, None, 'SELECT 1'))
+        self.assertEqual(entry.db_statements[1], (5, 10, None, 'SELECT 2'))
 
 
 class TestErrorReportingUtility(unittest.TestCase):
@@ -422,7 +469,7 @@
         # Test ErrorReportingUtility.raising() with an XML-RPC request.
         request = TestRequest()
         directlyProvides(request, IXMLRPCRequest)
-        request.getPositionalArguments = lambda : (1,2)
+        request.getPositionalArguments = lambda: (1, 2)
         utility = ErrorReportingUtility()
         now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC)
         try:
@@ -469,7 +516,6 @@
             utility.raising(sys.exc_info(), request, now=now)
             self.assertEqual(request.oopsid, None)
 
-
     def test_raising_for_script(self):
         """Test ErrorReportingUtility.raising with a ScriptRequest."""
         utility = ErrorReportingUtility()
@@ -753,7 +799,7 @@
         # logged will have OOPS reports generated for them.
         error_message = self.factory.getUniqueString()
         try:
-            1/0
+            ignored = 1/0
         except ZeroDivisionError:
             self.logger.exception(error_message)
         oops_report = self.error_utility.getLastOopsReport()
 
=== modified file 'lib/canonical/launchpad/webapp/tests/test_tales.py'
--- lib/canonical/launchpad/webapp/tests/test_tales.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/webapp/tests/test_tales.py	2009-08-05 01:06:49 +0000
@@ -10,12 +10,13 @@
 from zope.security.proxy import removeSecurityProxy
 from zope.testing.doctestunit import DocTestSuite
 
+from canonical.config import config
 from canonical.launchpad.ftests import test_tales
-from lp.testing import login, TestCase, TestCaseWithFactory
 from canonical.launchpad.testing.pages import find_tags_by_class
 from canonical.launchpad.webapp.tales import FormattersAPI
 from canonical.testing import (
     DatabaseFunctionalLayer, LaunchpadFunctionalLayer)
+from lp.testing import login, TestCase, TestCaseWithFactory
 
 
 def test_requestapi():
@@ -209,6 +210,13 @@
             '<td class="text"> </td></tr></table>',
             FormattersAPI(' ').format_diff())
 
+    def test_format_unicode(self):
+        # Sometimes the strings contain unicode, those should work too.
+        self.assertEqual(
+            u'<table class="diff"><tr><td class="line-no">1</td>'
+            u'<td class="text">Unicode \u1010</td></tr></table>',
+            FormattersAPI(u'Unicode \u1010').format_diff())
+
     def test_cssClasses(self):
         # Different parts of the diff have different css classes.
         diff = dedent('''\
@@ -240,6 +248,25 @@
              'diff-comment text'],
             [str(tag['class']) for tag in text])
 
+    def test_config_value_limits_line_count(self):
+        # The config.diff.max_line_format contains the maximum number of lines
+        # to format.
+        diff = dedent('''\
+            === modified file 'tales.py'
+            --- tales.py
+            +++ tales.py
+            @@ -2435,6 +2435,8 @@
+                 def format_diff(self):
+            -        removed this line
+            +        added this line
+            ########
+            # A merge directive comment.
+            ''')
+        self.pushConfig("diff", max_format_lines=3)
+        html = FormattersAPI(diff).format_diff()
+        line_count = html.count('<td class="line-no">')
+        self.assertEqual(3, line_count)
+
 
 class TestPreviewDiffFormatter(TestCaseWithFactory):
     """Test the PreviewDiffFormatterAPI class."""
 
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2009-07-24 10:11:20 +0000
+++ lib/lp/bugs/browser/bugtask.py	2009-08-04 14:57:07 +0000
@@ -2915,13 +2915,15 @@
         return self.context.isUserAffected(self.user)
 
     @property
-    def affects_form_value(self):
-        """The value to use in the inline me too form."""
-        affected = self.context.isUserAffected(self.user)
-        if affected is None or affected == False:
-            return 'YES'
+    def current_user_affected_js_status(self):
+        """A javascript literal indicating if the user is affected."""
+        affected = self.current_user_affected_status
+        if affected is None:
+            return 'null'
+        elif affected:
+            return 'true'
         else:
-            return 'NO'
+            return 'false'
 
 
 class BugTaskTableRowView(LaunchpadView):
 
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2009-06-25 00:40:31 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2009-07-23 10:34:08 +0000
@@ -1,23 +1,63 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+__metaclass__ = type
+
+
 import unittest
 
 from zope.testing.doctest import DocTestSuite
 
-from lp.bugs.browser import bugtask
+from canonical.launchpad.ftests import login
 from canonical.launchpad.testing.systemdocs import (
     LayeredDocFileSuite, setUp, tearDown)
 from canonical.testing import LaunchpadFunctionalLayer
 
+from lp.bugs.browser import bugtask
+from lp.bugs.browser.bugtask import BugTasksAndNominationsView
+from lp.testing import TestCaseWithFactory
+
+
+class TestBugTasksAndNominationsView(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestBugTasksAndNominationsView, self).setUp()
+        login('[e-mail]')
+        self.bug = self.factory.makeBug()
+        self.view = BugTasksAndNominationsView(self.bug, None)
+
+    def test_current_user_affected_status(self):
+        self.failUnlessEqual(
+            None, self.view.current_user_affected_status)
+        self.view.context.markUserAffected(self.view.user, True)
+        self.failUnlessEqual(
+            True, self.view.current_user_affected_status)
+        self.view.context.markUserAffected(self.view.user, False)
+        self.failUnlessEqual(
+            False, self.view.current_user_affected_status)
+
+    def test_current_user_affected_js_status(self):
+        self.failUnlessEqual(
+            'null', self.view.current_user_affected_js_status)
+        self.view.context.markUserAffected(self.view.user, True)
+        self.failUnlessEqual(
+            'true', self.view.current_user_affected_js_status)
+        self.view.context.markUserAffected(self.view.user, False)
+        self.failUnlessEqual(
+            'false', self.view.current_user_affected_js_status)
+
 
 def test_suite():
     suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(TestBugTasksAndNominationsView))
     suite.addTest(DocTestSuite(bugtask))
     suite.addTest(LayeredDocFileSuite(
         'bugtask-target-link-titles.txt', setUp=setUp, tearDown=tearDown,
         layer=LaunchpadFunctionalLayer))
     return suite
 
+
 if __name__ == '__main__':
     unittest.TextTestRunner().run(test_suite())
 
=== modified file 'lib/lp/bugs/doc/bugnotification-email.txt'
--- lib/lp/bugs/doc/bugnotification-email.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/doc/bugnotification-email.txt	2009-07-22 09:12:01 +0000
@@ -6,7 +6,9 @@
 themselves; for that, see bugnotifications.txt.
 
 The reference spec associated with this document is available on the
-Launchpad wiki. <https://launchpad.canonical.com/FormattingBugNotifications>
+Launchpad development wiki:
+
+    https://dev.launchpad.net/Bugs/Specs/FormattingBugNotifications
 
 You need to be logged in to edit bugs in Malone, so let's get started:
 
 
=== modified file 'lib/lp/bugs/externalbugtracker/mantis.py'
--- lib/lp/bugs/externalbugtracker/mantis.py	2009-06-25 00:40:31 +0000
+++ lib/lp/bugs/externalbugtracker/mantis.py	2009-07-22 09:12:01 +0000
@@ -80,7 +80,9 @@
     """An `ExternalBugTracker` for dealing with Mantis instances.
 
     For a list of tested Mantis instances and their behaviour when
-    exported from, see http://launchpad.canonical.com/MantisBugtrackers.
+    exported from, see:
+
+        https://dev.launchpad.net/Bugs/ExternalBugTrackers/Mantis
     """
 
     # Custom opener that automatically sends anonymous credentials to
 
=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-affects-me-too.txt	2009-07-31 13:49:53 +0000
@@ -1,7 +1,7 @@
 = Marking a bug as affecting the user =
 
-Users can mark bugs as affecting them. Let's create a sample bug to try
-this on.
+Users can mark bugs as affecting them. Let's create a sample bug to
+try this out.
 
    >>> login(ANONYMOUS)
    >>> from canonical.launchpad.webapp import canonical_url
@@ -9,15 +9,27 @@
    >>> test_bug_url = canonical_url(test_bug)
    >>> logout()
 
-The user goes to the bug's index page, and clicks the edit action link
-near 'This bug affects me too'.
+The user goes to the bug's index page, and finds a statement that the
+bug is not marked as affecting them.
 
    >>> user_browser.open(test_bug_url)
    >>> print extract_text(find_tag_by_id(
-   ...     user_browser.contents, 'affectsmetooform'))
-   This bug doesn't affect me...
-
-   >>> user_browser.getLink('change').click()
+   ...     user_browser.contents, 'affectsmetoo').find(
+   ...         None, 'static'))
+   This bug doesn't affect me
+
+Next to the statement is a link containing an edit icon.
+
+   >>> edit_link = find_tag_by_id(
+   ...     user_browser.contents, 'affectsmetoo').a
+   >>> print edit_link['href']
+   +affectsmetoo
+   >>> print edit_link.img['src']
+   /@@/edit
+
+The user is affected by this bug, so clicks the link.
+
+   >>> user_browser.getLink(url='+affectsmetoo').click()
    >>> print user_browser.url
    http://bugs.launchpad.dev/.../+bug/.../+affectsmetoo
    >>> user_browser.getControl(name='field.affects').value
@@ -28,22 +40,26 @@
    >>> user_browser.getControl('Change').click()
 
 The bug page loads again, and now the text is changed, to make it
-clear to the user that they can change the selection.
+clear to the user that they have marked this bug as affecting them.
 
-   >>> print extract_text(find_tags_by_class(
-   ...     user_browser.contents, 'menu-link-affectsmetoo')[0])
-   change
+   >>> print extract_text(find_tag_by_id(
+   ...     user_browser.contents, 'affectsmetoo').find(
+   ...         None, 'static'))
+   This bug affects me too
 
 Next to it, we also see the 'hot bug' icon, to indicate that the user
 has marked the bug as affecting them.
 
    >>> print find_tag_by_id(
-   ...     user_browser.contents, 'affectsmetooform').img['src']
+   ...     user_browser.contents, 'affectsmetoo').img['src']
    /@@/flame-icon
 
-   >>> user_browser.getLink('change').click()
-
-The user is changing his selection to 'No' and submits the form.
+On second thoughts, the user realises that this bug does not affect
+them, so they click on the edit link once more.
+
+   >>> user_browser.getLink(url='+affectsmetoo').click()
+
+The user changes his selection to 'No' and submits the form.
 
    >>> user_browser.getControl(name='field.affects').value = ['NO']
    >>> user_browser.getControl('Change').click()
@@ -51,18 +67,43 @@
 Back at the bug page, the text changes once again.
 
    >>> print extract_text(find_tag_by_id(
-   ...     user_browser.contents, 'affectsmetooform'))
-   This bug doesn't affect me...
-
-== One-click interaction ==
-
-If the user's browser provides javascript, they don't need to go to
-another page to change their selection. Instead, an in-page form is
-submitted when they click the action link.
-
-   >>> me_too_form = user_browser.getForm(id='affectsmetooform')
-   >>> user_browser.getControl(name='field.affects').value
-   'YES'
-   >>> me_too_form.submit()
-   >>> user_browser.getControl(name='field.affects').value
-   'NO'
+   ...     user_browser.contents, 'affectsmetoo').find(
+   ...         None, 'static'))
+   This bug doesn't affect me
+
+
+== Static and dynamic support ==
+
+A bug page contains markup to support both static (no Javascript) and
+dynamic (Javascript enabled) scenarios.
+
+   >>> def class_filter(css_class):
+   ...     def test(node):
+   ...         return css_class in node.get('class', '').split()
+   ...     return test
+
+   >>> static_content = find_tag_by_id(
+   ...     user_browser.contents, 'affectsmetoo').find(
+   ...         class_filter('static'))
+
+   >>> static_content is not None
+   True
+
+   >>> dynamic_content = find_tag_by_id(
+   ...     user_browser.contents, 'affectsmetoo').find(
+   ...         class_filter('dynamic'))
+
+   >>> dynamic_content is not None
+   True
+
+The dynamic content is hidden by the presence of the "unseen" CSS
+class.
+
+   >>> print static_content.get('class')
+   static
+
+   >>> print dynamic_content.get('class')
+   dynamic unseen
+
+It is the responsibilty of Javascript running in the page to unhide
+the dynamic content and hide the static content.
 
=== modified file 'lib/lp/bugs/templates/bugtasks-and-nominations-table.pt'
--- lib/lp/bugs/templates/bugtasks-and-nominations-table.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/bugs/templates/bugtasks-and-nominations-table.pt	2009-08-04 14:51:23 +0000
@@ -38,11 +38,9 @@
 
 </table>
 
-<div
-  class="actions"
-  tal:define="current_bugtask view/current_bugtask"
-
-        tal:condition="view/displayAlsoAffectsLinks">
+<div class="actions"
+     tal:define="current_bugtask view/current_bugtask"
+     tal:condition="view/displayAlsoAffectsLinks">
     <tal:also-affects-links define="context_menu context/menu:context">
       <tal:addupstream
         define="link context_menu/addupstream"
@@ -56,56 +54,50 @@
         define="link context_menu/nominate"
         condition="link/enabled"
         replace="structure link/render" />
-      <form
-        id="affectsmetooform"
-        name="affectsmetooform"
-        method="post"
-        enctype="multipart/form-data"
-        accept-charset="UTF-8"
-        tal:define="link context_menu/affectsmetoo"
-        tal:condition="link/enabled"
-        tal:attributes="action link/url"
-        style="display: inline">
-        <input
-          name="field.affects"
-          type="hidden"
-          tal:attributes="value view/affects_form_value" />
-        <input
-          type="hidden"
-          name="field.actions.change"
-          value="" />
-        <tal:affected condition="view/current_user_affected_status">
-          <img width="14" height="14" src="/@@/flame-icon" alt="" />
-          This bug affects me too
-        </tal:affected>
-        <tal:affected condition="not: view/current_user_affected_status">
-          This bug doesn't affect me
-        </tal:affected>
-        (<tal:affectsmetoo
-        define="link context_menu/affectsmetoo"
-        condition="link/enabled"
-        replace="structure link/render" />)
-        <tal:nothing condition="nothing">
-          The following is a trick to allow users to mark
-          themselves as affected by a bug with only one click.
-          If Javascript is available, we submit the form and
-          immediately go back to the same page, saving them the
-          need to go to a new page.
-        </tal:nothing>
-        <script type="text/javascript">
-            function sendMeTooForm(e) {
-                $('affectsmetooform').submit();
-                e.preventDefault();
-            }
-            function connectMeTooLink() {
-                var me_too_link = getFirstElementByTagAndClassName(
-                'a', 'menu-link-affectsmetoo');
-                connect(me_too_link, 'onclick', sendMeTooForm);
-            }
-            registerLaunchpadFunction(connectMeTooLink);
+      <span id="affectsmetoo" style="display: inline"
+            tal:condition="link/enabled"
+            tal:define="link context_menu/affectsmetoo;
+                        affected view/current_user_affected_status">
+
+        <tal:comment condition="nothing">
+          This .static section is shown in browsers with javascript
+          enabled, and before setup_me_too is run.
+        </tal:comment>
+        <span class="static">
+          <tal:affected condition="affected">
+            <img width="14" height="14" src="/@@/flame-icon" alt="" />
+            This bug affects me too
+          </tal:affected>
+          <tal:not-affected condition="not:affected">
+            This bug doesn't affect me
+          </tal:not-affected>
+          <a href="+affectsmetoo">
+            <img class="editicon" src="/@@/edit" alt="Edit" />
+          </a>
+        </span>
+
+        <tal:comment condition="nothing">
+          This .dynamic section is used by setup_me_too to display
+          controls and information in the correct places.
+        </tal:comment>
+        <span class="dynamic unseen">
+          <img src="/@@/flame-icon" alt=""/>
+          <a href="+affectsmetoo" class="js-action"
+             ><span class="value">Does this bug affect you?</span></a>
+          <img class="editicon" src="/@@/edit" alt="Edit" />
+        </span>
+
+        <script type="text/javascript" tal:content="string:
+          YUI().use('event', 'bugs.bugtask_index', function(Y) {
+              Y.on('load', function(e) {
+                  Y.bugs.setup_me_too(${view/current_user_affected_js_status});
+              }, window);
+          });
+          ">
         </script>
-      </form>
+
+      </span>
     </tal:also-affects-links>
-
 </div>
+
 </tal:root>
 
=== added file 'lib/lp/bugs/windmill/tests/test_bugs/test_bug_me_too.py'
--- lib/lp/bugs/windmill/tests/test_bugs/test_bug_me_too.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/windmill/tests/test_bugs/test_bug_me_too.py	2009-07-31 15:27:05 +0000
@@ -0,0 +1,65 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from canonical.launchpad.windmill.testing import lpuser
+
+from windmill.authoring import WindmillTestClient
+
+
+def test_me_too():
+    """Test the "this bug affects me too" options on bug pages.
+
+    This test ensures that, with Javascript enabled, the "me too"
+    status can be edited in-page.
+    """
+    client = WindmillTestClient('Bug "me too" test')
+    lpuser.SAMPLE_PERSON.ensure_login(client)
+
+    # Open bug 11 and wait for it to finish loading.
+    client.open(url=u'http://bugs.launchpad.dev:8085/jokosher/+bug/11/+index')
+    client.waits.forPageLoad(timeout=u'20000')
+
+    # Wait for setup_me_too to sort out the "me too" elements.
+    client.waits.forElement(
+        xpath=(u"//span[@id='affectsmetoo' and "
+               u"@class='yui-metoocs-content']"))
+
+    # Currently this bug does not affect the logged-in user.
+    client.asserts.assertText(
+        xpath=u"//span[@id='affectsmetoo']/span[@class='value']",
+        validator=u"This bug doesn't affect me")
+
+    # There is an edit icon next to the text which can be clicked to
+    # edit the "me too" status. However, we won't click it with
+    # Windmill because the widget actually responds to mouse-down, and
+    # Windmill seems to do something funny instead.
+    client.mouseDown(
+        xpath=u"//span[@id='affectsmetoo']//img[@class='editicon']")
+    client.mouseUp(
+        xpath=u"//span[@id='affectsmetoo']//img[@class='editicon']")
+
+    # Wait for the modal dialog to appear.
+    client.waits.forElement(id=u'yui-pretty-overlay-modal')
+
+    # There's a close button if we change our mind.
+    client.click(
+        xpath=(u"//div[@id='yui-pretty-overlay-modal']//"
+               u"a[@class='close-button']"))
+
+    # Wait for the modal dialog to disappear. Unfortunately the test
+    # below doesn't work, nor does testing clientWidth, or anything I
+    # could think of, so it's commented out for now because chasing
+    # this is not a good use of time.
+
+    # client.asserts.assertElemJS(
+    #     id=u'yui-pretty-overlay-modal',
+    #     js=(u'getComputedStyle(element, '
+    #         u'"visibility").visibility == "hidden"'))
+
+    # However, we want to mark this bug as affecting the logged-in
+    # user. We can also click on the content box of the "me too"
+    # widget; we are not forced to use the edit icon.
+    client.click(xpath=u"//span[@id='affectsmetoo']")
+    client.waits.forElement(id=u'yui-pretty-overlay-modal')
+
+    # XXX: Gavin Panella bug=361097 2009-07-31: Finish this.
 
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2009-07-19 04:41:14 +0000
+++ lib/lp/code/browser/branch.py	2009-08-04 00:41:49 +0000
@@ -36,12 +36,15 @@
 from zope.app.form.browser import TextAreaWidget
 from zope.traversing.interfaces import IPathAdapter
 from zope.component import getUtility, queryAdapter
+from zope.event import notify
 from zope.formlib import form
-from zope.interface import Interface, implements
+from zope.interface import Interface, implements, providedBy
 from zope.publisher.interfaces import NotFound
 from zope.schema import Choice, Text
 from lazr.delegates import delegates
 from lazr.enum import EnumeratedType, Item
+from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.snapshot import Snapshot
 from lazr.uri import URI
 
 from canonical.cachedproperty import cachedproperty
@@ -574,7 +577,6 @@
     the user to be able to edit it.
     """
     use_template(IBranch, include=[
-        'owner',
         'name',
         'url',
         'description',
@@ -582,6 +584,7 @@
         'whiteboard',
         ])
     private = copy_field(IBranch['private'], readonly=False)
+    owner = copy_field(IBranch['owner'], readonly=False)
 
 
 class BranchEditFormView(LaunchpadEditFormView):
@@ -598,9 +601,16 @@
     @action('Change Branch', name='change')
     def change_action(self, action, data):
         # If the owner or product has changed, add an explicit notification.
+        # We take our own snapshot here to make sure that the snapshot records
+        # changes to the owner and private, and we notify the listeners
+        # explicitly below rather than the notification that would normally be
+        # sent in updateContextFromData.
+        branch_before_modification = Snapshot(
+            self.context, providing=providedBy(self.context))
         if 'owner' in data:
-            new_owner = data['owner']
+            new_owner = data.pop('owner')
             if new_owner != self.context.owner:
+                self.context.setOwner(new_owner, self.user)
                 self.request.response.addNotification(
                     "The branch owner has been changed to %s (%s)"
                     % (new_owner.displayname, new_owner.name))
@@ -616,7 +626,13 @@
                 else:
                     self.request.response.addNotification(
                         "The branch is now publicly accessible.")
-        if self.updateContextFromData(data):
+        if self.updateContextFromData(data, notify_modified=False):
+            # Notify the object has changed with the snapshot that was taken
+            # earler.
+            field_names = [
+                form_field.__name__ for form_field in self.form_fields]
+            notify(ObjectModifiedEvent(
+                self.context, branch_before_modification, field_names))
             # Only specify that the context was modified if there
             # was in fact a change.
             self.context.date_last_modified = UTC_NOW
 
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2009-07-24 03:52:27 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2009-08-05 02:18:13 +0000
@@ -42,6 +42,7 @@
 from lazr.lifecycle.event import ObjectModifiedEvent
 
 from canonical.cachedproperty import cachedproperty
+from canonical.config import config
 
 from canonical.launchpad import _
 from lp.code.adapters.branch import BranchMergeProposalDelta
@@ -372,7 +373,7 @@
             result.append(dict(style=style, comment=comment))
         return result
 
-    @property
+    @cachedproperty
     def review_diff(self):
         """Return a (hopefully) intelligently encoded review diff."""
         if self.context.review_diff is None:
@@ -382,7 +383,25 @@
         except UnicodeDecodeError:
             diff = self.context.review_diff.diff.text.decode('windows-1252',
                                                              'replace')
-        return diff
+        # Strip off the trailing carriage returns.
+        return diff.rstrip('\n')
+
+    @cachedproperty
+    def review_diff_oversized(self):
+        """Return True if the review_diff is over the configured size limit.
+
+        The diff can be over the limit in two ways.  If the diff is oversized
+        in bytes it will be cut off at the Diff.text method.  If the number of
+        lines is over the max_format_lines, then it is cut off at the fmt:diff
+        processing.
+        """
+        review_diff = self.context.review_diff
+        if review_diff is None:
+            return False
+        if review_diff.diff.oversized:
+            return True
+        diff_text = self.review_diff
+        return diff_text.count('\n') >= config.diff.max_format_lines
 
     @property
     def has_bug_or_spec(self):
 
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2009-08-02 23:25:37 +0000
@@ -8,14 +8,15 @@
 from unittest import TestLoader
 
 import transaction
+from zope.security.proxy import removeSecurityProxy
 
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from canonical.testing import DatabaseFunctionalLayer
 from lp.code.browser.branchmergeproposallisting import (
     ActiveReviewsView, BranchMergeProposalListingView, PersonActiveReviewsView,
     ProductActiveReviewsView)
 from lp.code.enums import CodeReviewVote
 from lp.testing import ANONYMOUS, login, login_person, TestCaseWithFactory
-from canonical.launchpad.webapp.servers import LaunchpadTestRequest
-from canonical.testing import DatabaseFunctionalLayer
 
 _default = object()
 
@@ -201,8 +202,7 @@
         self.assertReviewGroupForUser(
             self.bmp.registrant, self._view.OTHER)
         team = self.factory.makeTeam(self.bmp.registrant)
-        login_person(self.bmp.source_branch.owner)
-        self.bmp.source_branch.owner = team
+        removeSecurityProxy(self.bmp.source_branch).owner = team
         self.assertReviewGroupForUser(
             self.bmp.registrant, ActiveReviewsView.MINE)
 
 
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2009-07-23 02:06:55 +0000
+++ lib/lp/code/configure.zcml	2009-07-30 00:30:02 +0000
@@ -430,9 +430,9 @@
                     "/>
     <require
         permission="launchpad.Edit"
-        attributes="destroySelf setPrivate"
+        attributes="destroySelf setPrivate setOwner setTarget"
         set_attributes="name url mirror_status_message
-                        owner author description product lifecycle_status
+                        description lifecycle_status
                         last_mirrored last_mirrored_id last_mirror_attempt
                         mirror_failures pull_disabled next_mirror_time
                         last_scanned last_scanned_id revision_count branch_type
 
=== modified file 'lib/lp/code/doc/branch.txt'
--- lib/lp/code/doc/branch.txt	2009-06-16 03:31:05 +0000
+++ lib/lp/code/doc/branch.txt	2009-08-02 23:59:31 +0000
@@ -116,9 +116,9 @@
     >>> print new_branch.owner.name
     registrant
 
-A user can create a branch where the owner is either themselves,
-or a team that they are a member of.  The registrant is not writable,
-whereas the owner is.
+A user can create a branch where the owner is either themselves, or a team
+that they are a member of.  Neither the owner nor the registrant are writable,
+but the owner can be set using the `setOwner` method.
 
     >>> login('[e-mail]')
     >>> new_branch.registrant = factory.makePerson()
@@ -126,7 +126,8 @@
       ...
     ForbiddenAttribute: ('registrant', <Branch ...>)
 
-    >>> new_branch.owner = factory.makePerson(name='new-owner')
+    >>> team = factory.makeTeam(name='new-owner', owner=new_branch.owner)
+    >>> new_branch.setOwner(new_owner=team, user=new_branch.owner)
     >>> print new_branch.registrant.name
     registrant
     >>> print new_branch.owner.name
 
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2009-07-29 03:44:05 +0000
+++ lib/lp/code/interfaces/branch.py	2009-08-05 04:02:58 +0000
@@ -19,6 +19,7 @@
     'BranchCreatorNotMemberOfOwnerTeam',
     'BranchCreatorNotOwner',
     'BranchExists',
+    'BranchTargetError',
     'BranchTypeError',
     'CannotDeleteBranch',
     'DEFAULT_BRANCH_STATUS_IN_LISTING',
@@ -90,7 +91,7 @@
     """Raised when creating a branch that already exists."""
 
     def __init__(self, existing_branch):
-        # XXX: JonathanLange 2008-12-04 spec=package-branches: This error
+        # XXX: TimPenhey 2009-07-12 bug=405214: This error
         # message logic is incorrect, but the exact text is being tested
         # in branch-xmlrpc.txt.
         params = {'name': existing_branch.name}
@@ -108,6 +109,10 @@
         BranchCreationException.__init__(self, message)
 
 
+class BranchTargetError(Exception):
+    """Raised when there is an error determining a branch target."""
+
+
 class CannotDeleteBranch(Exception):
     """The branch cannot be deleted at this time."""
 
@@ -402,14 +407,45 @@
             title=_("The user that registered the branch."),
             required=True, readonly=True,
             vocabulary='ValidPersonOrTeam'))
+
     owner = exported(
         ParticipatingPersonChoice(
             title=_('Owner'),
-            required=True,
+            required=True, readonly=True,
             vocabulary='UserTeamsParticipationPlusSelf',
             description=_("Either yourself or a team you are a member of. "
                           "This controls who can modify the branch.")))
 
+    @call_with(user=REQUEST_USER)
+    @operation_parameters(
+        new_owner=Reference(
+            title=_("The new owner of the branch."),
+            schema=IPerson))
+    @export_write_operation()
+    def setOwner(new_owner, user):
+        """Set the owner of the branch to be `new_owner`."""
+
+    @call_with(user=REQUEST_USER)
+    @operation_parameters(
+        project=Reference(
+            title=_("The project the branch belongs to."),
+            schema=Interface, required=False), # Really IProduct
+        source_package=Reference(
+            title=_("The source package the branch belongs to."),
+            schema=Interface, required=False)) # Really ISourcePackage
+    @export_write_operation()
+    def setTarget(user, project=None, source_package=None):
+        """Set the target of the branch to be `project` or `source_package`.
+
+        Only one of `project` or `source_package` can be set, and if neither
+        is set, the branch gets moved into the junk namespace of the branch
+        owner.
+
+        :raise: `BranchTargetError` if both project and source_package are set,
+          or if either the project or source_package fail to be adapted to an
+          IBranchTarget.
+        """
+
     reviewer = exported(
         PublicPersonChoice(
             title=_('Default Review Team'),
@@ -456,7 +492,7 @@
     product = exported(
         ReferenceChoice(
             title=_('Project'),
-            required=False,
+            required=False, readonly=True,
             vocabulary='Product',
             schema=Interface,
             description=_("The project this branch belongs to.")),
 
=== modified file 'lib/lp/code/interfaces/branchnamespace.py'
--- lib/lp/code/interfaces/branchnamespace.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/branchnamespace.py	2009-07-27 10:19:21 +0000
@@ -70,6 +70,24 @@
     def isNameUsed(name):
         """Is 'name' already used in this namespace?"""
 
+    def moveBranch(branch, mover, new_name=None, rename_if_necessary=False):
+        """Move the branch into this namespace.
+
+        :param branch: The `IBranch` to move.
+        :param mover: The `IPerson` doing the moving.
+        :param new_name: A new name for the branch.
+        :param rename_if_necessary: Rename the branch if the branch name
+            exists already in this namespace.
+        :raises BranchCreatorNotMemberOfOwnerTeam: if the namespace owner is
+            a team, and 'mover' is not in that team.
+        :raises BranchCreatorNotOwner: if the namespace owner is an individual
+            and 'mover' is not the owner.
+        :raises BranchCreationForbidden: if 'mover' is not allowed to create
+            a branch in this namespace due to privacy rules.
+        :raises BranchExists: if a branch with the 'name' exists already in
+            the namespace, and 'rename_if_necessary' is False.
+        """
+
 
 class IBranchNamespacePolicy(Interface):
     """Methods relating to branch creation and validation."""
@@ -133,11 +151,13 @@
             validation constraints on IBranch.name.
         """
 
-    def validateMove(branch, mover):
+    def validateMove(branch, mover, name=None):
         """Check that 'mover' can move 'branch' into this namespace.
 
         :param branch: An `IBranch` that might be moved.
         :param mover: The `IPerson` who would move it.
+        :param name: A new name for the branch.  If None, the branch name is
+            used.
         :raises BranchCreatorNotMemberOfOwnerTeam: if the namespace owner is
             a team, and 'mover' is not in that team.
         :raises BranchCreatorNotOwner: if the namespace owner is an individual
 
=== modified file 'lib/lp/code/interfaces/diff.py'
--- lib/lp/code/interfaces/diff.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/interfaces/diff.py	2009-08-04 23:07:33 +0000
@@ -27,7 +27,15 @@
 class IDiff(Interface):
     """A diff that is stored in the Library."""
 
-    text = Text(title=_('Textual contents of a diff.'), readonly=True)
+    text = Text(
+        title=_('Textual contents of a diff.'), readonly=True,
+        description=_("The text may be cut off at a defined maximum size."))
+
+    oversized = Bool(
+        readonly=True,
+        description=_(
+            "True if the size of the content is over the defined maximum "
+            "size."))
 
     diff_text = exported(
         Bytes(title=_('Content of this diff'), required=True, readonly=True))
 
=== modified file 'lib/lp/code/mail/branchmergeproposal.py'
--- lib/lp/code/mail/branchmergeproposal.py	2009-07-22 18:37:32 +0000
+++ lib/lp/code/mail/branchmergeproposal.py	2009-08-05 02:18:13 +0000
@@ -193,6 +193,7 @@
             'gap': '',
             'reviews': '',
             'whiteboard': '', # No more whiteboard.
+            'diff_cutoff_warning': '',
             }
 
         requested_reviews = []
@@ -212,6 +213,12 @@
             params['comment'] = (self.comment.message.text_contents)
             if len(requested_reviews) > 0:
                 params['gap'] = '\n\n'
+
+        if (self.review_diff is not None and
+            self.review_diff.diff.oversized):
+            params['diff_cutoff_warning'] = (
+                "The attached diff has been truncated due to its size.")
+
         return params
 
     def _getTemplateParams(self, email):
 
=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
--- lib/lp/code/mail/tests/test_branchmergeproposal.py	2009-07-22 18:37:32 +0000
+++ lib/lp/code/mail/tests/test_branchmergeproposal.py	2009-08-05 02:25:16 +0000
@@ -3,7 +3,7 @@
 
 """Tests for BranchMergeProposal mailings"""
 
-from unittest import TestLoader, TestCase
+from unittest import TestLoader
 import transaction
 
 from zope.security.proxy import removeSecurityProxy
@@ -22,20 +22,17 @@
 from lp.code.model.branch import update_trigger_modified_fields
 from lp.code.model.codereviewvote import CodeReviewVoteReference
 from canonical.launchpad.webapp import canonical_url
-from lp.testing import login, login_person, TestCaseWithFactory
-from lp.testing.factory import LaunchpadObjectFactory
+from lp.testing import login_person, TestCaseWithFactory
 from lp.testing.mail_helpers import pop_notifications
 
 
-class TestMergeProposalMailing(TestCase):
+class TestMergeProposalMailing(TestCaseWithFactory):
     """Test that reasonable mailings are generated"""
 
     layer = LaunchpadFunctionalLayer
 
     def setUp(self):
-        TestCase.setUp(self)
-        login('[e-mail]')
-        self.factory = LaunchpadObjectFactory()
+        super(TestMergeProposalMailing, self).setUp('[e-mail]')
 
     def makeProposalWithSubscriber(self, diff_text=None,
                                    initial_comment=None):
@@ -182,6 +179,22 @@
                          attachment['Content-Disposition'])
         self.assertEqual('Fake diff', attachment.get_payload(decode=True))
 
+    def test_generateEmail_attaches_diff_oversize_truncated(self):
+        """An oversized diff will be truncated, and the receiver informed."""
+        self.pushConfig("diff", max_read_size=25)
+        content = "1234567890" * 10
+        bmp, subscriber = self.makeProposalWithSubscriber(
+            diff_text=content)
+        mailer = BMPMailer.forCreation(bmp, bmp.registrant)
+        ctrl = mailer.generateEmail('[e-mail]', subscriber)
+        (attachment,) = ctrl.attachments
+        self.assertEqual('text/x-diff', attachment['Content-Type'])
+        self.assertEqual('inline; filename="review-diff.txt"',
+                         attachment['Content-Disposition'])
+        self.assertEqual(content[:25], attachment.get_payload(decode=True))
+        warning_text = "The attached diff has been truncated due to its size."
+        self.assertTrue(warning_text in ctrl.body)
+
     def test_forModificationNoModification(self):
         """Ensure None is returned if no change has been made."""
         merge_proposal, person = self.makeProposalWithSubscriber()
 
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2009-07-19 04:41:14 +0000
+++ lib/lp/code/model/branch.py	2009-07-31 01:07:04 +0000
@@ -57,7 +57,7 @@
 from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
 from lp.code.interfaces.branch import (
     bazaar_identity, BranchCannotBePrivate, BranchCannotBePublic,
-    BranchTypeError, CannotDeleteBranch,
+    BranchTargetError, BranchTypeError, CannotDeleteBranch,
     DEFAULT_BRANCH_STATUS_IN_LISTING, IBranch,
     IBranchNavigationMenu, IBranchSet)
 from lp.code.interfaces.branchcollection import IAllBranches
@@ -71,7 +71,7 @@
 from lp.code.interfaces.seriessourcepackagebranch import (
     IFindOfficialBranchLinks)
 from lp.registry.interfaces.person import (
-    validate_person_not_private_membership, validate_public_person)
+    IPerson, validate_person_not_private_membership, validate_public_person)
 
 
 class Branch(SQLBase):
@@ -113,6 +113,12 @@
     owner = ForeignKey(
         dbName='owner', foreignKey='Person',
         storm_validator=validate_person_not_private_membership, notNull=True)
+
+    def setOwner(self, new_owner, user):
+        """See `IBranch`."""
+        new_namespace = self.target.getNamespace(new_owner)
+        new_namespace.moveBranch(self, user, rename_if_necessary=True)
+
     reviewer = ForeignKey(
         dbName='reviewer', foreignKey='Person',
         storm_validator=validate_public_person, default=None)
@@ -162,6 +168,28 @@
             target = self.product
         return IBranchTarget(target)
 
+    def setTarget(self, user, project=None, source_package=None):
+        """See `IBranch`."""
+        if project is not None:
+            if source_package is not None:
+                raise BranchTargetError(
+                    'Cannot specify both a project and a source package.')
+            else:
+                target = IBranchTarget(project)
+                if target is None:
+                    raise BranchTargetError(
+                        '%r is not a valid project target' % project)
+        elif source_package is not None:
+            target = IBranchTarget(source_package)
+            if target is None:
+                raise BranchTargetError(
+                    '%r is not a valid source package target' % source_package)
+        else:
+            target = IBranchTarget(self.owner)
+            # Person targets are always valid.
+        namespace = target.getNamespace(self.owner)
+        namespace.moveBranch(self, user, rename_if_necessary=True)
+
     @property
     def namespace(self):
         """See `IBranch`."""
 
=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/branchnamespace.py	2009-08-04 00:41:49 +0000
@@ -16,6 +16,7 @@
 from zope.component import getUtility
 from zope.event import notify
 from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
 
 from lazr.lifecycle.event import ObjectCreatedEvent
 from storm.locals import And
@@ -160,6 +161,25 @@
         self.validateBranchName(name)
         self.validateRegistrant(mover)
 
+    def moveBranch(self, branch, mover, new_name=None,
+                   rename_if_necessary=False):
+        """See `IBranchNamespace`."""
+        # Check to see if the branch is already in this namespace.
+        old_namespace = branch.namespace
+        if self.name == old_namespace.name:
+            return
+        if new_name is None:
+            new_name = branch.name
+        if rename_if_necessary:
+            new_name = self.findUnusedName(new_name)
+        self.validateMove(branch, mover, new_name)
+        # Remove the security proxy of the branch as the owner and target
+        # attributes are readonly through the interface.
+        naked_branch = removeSecurityProxy(branch)
+        naked_branch.owner = self.owner
+        self.target._retargetBranch(naked_branch)
+        naked_branch.name = new_name
+
     def createBranchWithPrefix(self, branch_type, prefix, registrant,
                                url=None):
         """See `IBranchNamespace`."""
 
=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/model/branchtarget.py	2009-08-04 00:41:49 +0000
@@ -13,7 +13,8 @@
 
 from zope.component import getUtility
 from zope.interface import implements
-from zope.security.proxy import isinstance as zope_isinstance
+from zope.security.proxy import (
+    removeSecurityProxy, isinstance as zope_isinstance)
 
 from lp.code.interfaces.branchcollection import IAllBranches
 from lp.code.interfaces.branchtarget import (
@@ -128,6 +129,16 @@
         # those cases.
         return bug.default_bugtask
 
+    def _retargetBranch(self, branch):
+        """Set the branch target to refer to this target.
+
+        This only updates the target related attributes of the branch, and
+        expects a branch without a security proxy as a parameter.
+        """
+        branch.product = None
+        branch.distroseries = self.sourcepackage.distroseries
+        branch.sourcepackagename = self.sourcepackage.sourcepackagename
+
 
 class PersonBranchTarget(_BaseBranchTarget):
     implements(IBranchTarget)
@@ -182,6 +193,16 @@
         """See `IBranchTarget`."""
         return bug.default_bugtask
 
+    def _retargetBranch(self, branch):
+        """Set the branch target to refer to this target.
+
+        This only updates the target related attributes of the branch, and
+        expects a branch without a security proxy as a parameter.
+        """
+        branch.product = None
+        branch.distroseries = None
+        branch.sourcepackagename = None
+
 
 class ProductBranchTarget(_BaseBranchTarget):
     implements(IBranchTarget)
@@ -265,6 +286,16 @@
             task = bug.bugtasks[0]
         return task
 
+    def _retargetBranch(self, branch):
+        """Set the branch target to refer to this target.
+
+        This only updates the target related attributes of the branch, and
+        expects a branch without a security proxy as a parameter.
+        """
+        branch.product = self.product
+        branch.distroseries = None
+        branch.sourcepackagename = None
+
 
 def get_canonical_url_data_for_target(branch_target):
     """Return the `ICanonicalUrlData` for an `IBranchTarget`."""
 
=== modified file 'lib/lp/code/model/diff.py'
--- lib/lp/code/model/diff.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/diff.py	2009-08-04 23:07:33 +0000
@@ -15,8 +15,9 @@
 from zope.component import getUtility
 from zope.interface import classProvides, implements
 
+from canonical.config import config
+from canonical.database.sqlbase import SQLBase
 from canonical.uuid import generate_uuid
-from canonical.database.sqlbase import SQLBase
 
 from lp.code.interfaces.diff import (
     IDiff, IPreviewDiff, IStaticDiff, IStaticDiffSource)
@@ -45,10 +46,19 @@
         else:
             self.diff_text.open()
             try:
-                return self.diff_text.read()
+                return self.diff_text.read(config.diff.max_read_size)
             finally:
                 self.diff_text.close()
 
+    @property
+    def oversized(self):
+        # If the size of the content of the librarian file is over the
+        # config.diff.max_read_size, then we have an oversized diff.
+        if self.diff_text is None:
+            return False
+        diff_size = self.diff_text.content.filesize
+        return diff_size > config.diff.max_read_size
+
     @classmethod
     def fromTrees(klass, from_tree, to_tree, filename=None):
         """Create a Diff from two Bazaar trees.
 
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2009-07-23 02:06:55 +0000
+++ lib/lp/code/model/tests/test_branch.py	2009-08-05 02:04:06 +0000
@@ -39,7 +39,8 @@
     BranchVisibilityRule, CodeReviewNotificationLevel)
 from lp.code.interfaces.branch import (
     BranchCannotBePrivate, BranchCannotBePublic,
-    CannotDeleteBranch, DEFAULT_BRANCH_STATUS_IN_LISTING)
+    BranchCreatorNotMemberOfOwnerTeam, BranchCreatorNotOwner,
+    BranchTargetError, CannotDeleteBranch, DEFAULT_BRANCH_STATUS_IN_LISTING)
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
 from lp.code.interfaces.branchmergeproposal import InvalidBranchMergeProposal
@@ -57,7 +58,6 @@
 from lp.code.model.codeimport import CodeImport, CodeImportSet
 from lp.code.model.codereviewcomment import CodeReviewComment
 from lp.registry.interfaces.person import IPersonSet
-from lp.registry.interfaces.product import IProductSet
 from lp.registry.model.product import ProductSet
 from lp.registry.model.sourcepackage import SourcePackage
 from lp.soyuz.interfaces.publishing import PackagePublishingPocket
@@ -213,8 +213,7 @@
         # attribute is updated too.
         branch = self.factory.makeAnyBranch()
         new_owner = self.factory.makePerson()
-        login('[e-mail]')
-        branch.owner = new_owner
+        removeSecurityProxy(branch).owner = new_owner
         # Call the function that is normally called through the event system
         # to auto reload the fields updated by the db triggers.
         update_trigger_modified_fields(branch)
@@ -1054,9 +1053,9 @@
 
     def setUp(self):
         TestCaseWithFactory.setUp(self, '[e-mail]')
-        self.product = getUtility(IProductSet).getByName('firefox')
+        self.product = self.factory.makeProduct()
 
-        self.user = getUtility(IPersonSet).getByName('no-priv')
+        self.user = self.factory.makePerson()
         self.source = self.factory.makeProductBranch(
             name='source-branch', owner=self.user, product=self.product)
         self.target = self.factory.makeProductBranch(
@@ -1069,7 +1068,7 @@
 
     def test_junkSource(self):
         """Junk branches cannot be used as a source for merge proposals."""
-        self.source.product = None
+        self.source.setTarget(user=self.source.owner)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target)
@@ -1078,12 +1077,13 @@
         """The product of the target branch must match the product of the
         source branch.
         """
-        self.target.product = None
+        self.target.setTarget(user=self.target.owner)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target)
 
-        self.target.product = getUtility(IProductSet).getByName('bzr')
+        project = self.factory.makeProduct()
+        self.target.setTarget(user=self.target.owner, project=project)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target)
@@ -1097,12 +1097,13 @@
     def test_dependentBranchSameProduct(self):
         """The dependent branch, if it is there, must be for the same product.
         """
-        self.dependent.product = None
+        self.dependent.setTarget(user=self.dependent.owner)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target, self.dependent)
 
-        self.dependent.product = getUtility(IProductSet).getByName('bzr')
+        project = self.factory.makeProduct()
+        self.dependent.setTarget(user=self.dependent.owner, project=project)
         self.assertRaises(
             InvalidBranchMergeProposal, self.source.addLandingTarget,
             self.user, self.target, self.dependent)
@@ -1650,5 +1651,149 @@
         self.assertEqual(branch.spec_links.count(), 0)
 
 
+class TestBranchSetOwner(TestCaseWithFactory):
+    """Tests for IBranch.setOwner."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_owner_sets_team(self):
+        # The owner of the branch can set the owner of the branch to be a team
+        # they are a member of.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam(owner=branch.owner)
+        login_person(branch.owner)
+        branch.setOwner(team, branch.owner)
+        self.assertEqual(team, branch.owner)
+
+    def test_owner_cannot_set_nonmember_team(self):
+        # The owner of the branch cannot set the owner to be a team they are
+        # not a member of.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam()
+        login_person(branch.owner)
+        self.assertRaises(
+            BranchCreatorNotMemberOfOwnerTeam,
+            branch.setOwner,
+            team, branch.owner)
+
+    def test_owner_cannot_set_other_user(self):
+        # The owner of the branch cannot set the new owner to be another
+        # person.
+        branch = self.factory.makeAnyBranch()
+        person = self.factory.makePerson()
+        login_person(branch.owner)
+        self.assertRaises(
+            BranchCreatorNotOwner,
+            branch.setOwner,
+            person, branch.owner)
+
+    def test_admin_can_set_any_team_or_person(self):
+        # A Launchpad admin can set the branch to be owned by any team or
+        # person.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam()
+        # To get a random administrator, choose the admin team owner.
+        admin = getUtility(ILaunchpadCelebrities).admin.teamowner
+        login_person(admin)
+        branch.setOwner(team, admin)
+        self.assertEqual(team, branch.owner)
+        person = self.factory.makePerson()
+        branch.setOwner(person, admin)
+        self.assertEqual(person, branch.owner)
+
+    def test_bazaar_experts_can_set_any_team_or_person(self):
+        # A bazaar expert can set the branch to be owned by any team or
+        # person.
+        branch = self.factory.makeAnyBranch()
+        team = self.factory.makeTeam()
+        # To get a random administrator, choose the admin team owner.
+        experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner
+        login_person(experts)
+        branch.setOwner(team, experts)
+        self.assertEqual(team, branch.owner)
+        person = self.factory.makePerson()
+        branch.setOwner(person, experts)
+        self.assertEqual(person, branch.owner)
+
+
+class TestBranchSetTarget(TestCaseWithFactory):
+    """Tests for IBranch.setTarget."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_not_both_project_and_source_package(self):
+        # Only one of project or source_package can be passed in, not both.
+        branch = self.factory.makePersonalBranch()
+        project = self.factory.makeProduct()
+        source_package = self.factory.makeSourcePackage()
+        login_person(branch.owner)
+        self.assertRaises(
+            BranchTargetError,
+            branch.setTarget,
+            user=branch.owner, project=project, source_package=source_package)
+
+    def test_junk_branch_to_project_branch(self):
+        # A junk branch can be moved to a project.
+        branch = self.factory.makePersonalBranch()
+        project = self.factory.makeProduct()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, project=project)
+        self.assertEqual(project, branch.target.context)
+
+    def test_junk_branch_to_package_branch(self):
+        # A junk branch can be moved to a source package.
+        branch = self.factory.makePersonalBranch()
+        source_package = self.factory.makeSourcePackage()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, source_package=source_package)
+        self.assertEqual(source_package, branch.target.context)
+
+    def test_project_branch_to_other_project_branch(self):
+        # Move a branch from one project to another.
+        branch = self.factory.makeProductBranch()
+        project = self.factory.makeProduct()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, project=project)
+        self.assertEqual(project, branch.target.context)
+
+    def test_project_branch_to_package_branch(self):
+        # Move a branch from a project to a package.
+        branch = self.factory.makeProductBranch()
+        source_package = self.factory.makeSourcePackage()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, source_package=source_package)
+        self.assertEqual(source_package, branch.target.context)
+
+    def test_project_branch_to_junk_branch(self):
+        # Move a branch from a project to junk.
+        branch = self.factory.makeProductBranch()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner)
+        self.assertEqual(branch.owner, branch.target.context)
+
+    def test_package_branch_to_other_package_branch(self):
+        # Move a branch from one package to another.
+        branch = self.factory.makePackageBranch()
+        source_package = self.factory.makeSourcePackage()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, source_package=source_package)
+        self.assertEqual(source_package, branch.target.context)
+
+    def test_package_branch_to_project_branch(self):
+        # Move a branch from a package to a project.
+        branch = self.factory.makePackageBranch()
+        project = self.factory.makeProduct()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner, project=project)
+        self.assertEqual(project, branch.target.context)
+
+    def test_package_branch_to_junk_branch(self):
+        # Move a branch from a package to junk.
+        branch = self.factory.makePackageBranch()
+        login_person(branch.owner)
+        branch.setTarget(user=branch.owner)
+        self.assertEqual(branch.owner, branch.target.context)
+
+
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)
 
=== renamed file 'lib/lp/code/tests/test_branchnamespace.py' => 'lib/lp/code/model/tests/test_branchnamespace.py'
--- lib/lp/code/tests/test_branchnamespace.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_branchnamespace.py	2009-07-29 02:16:43 +0000
@@ -1871,5 +1871,70 @@
             BranchCreatorNotOwner, self.albert, self.doug)
 
 
+class TestBranchNamespaceMoveBranch(TestCaseWithFactory):
+    """Test the IBranchNamespace.moveBranch method.
+
+    The edge cases of the validateMove are tested in the NamespaceMixin for
+    each of the namespaces.
+    """
+
+    layer = DatabaseFunctionalLayer
+
+    def assertNamespacesEqual(self, expected, result):
+        """Assert that the namespaces refer to the same thing.
+
+        The name of the namespace contains the user name and the context
+        parts, so is the easiest thing to check.
+        """
+        self.assertEqual(expected.name, result.name)
+
+    def test_move_to_same_namespace(self):
+        # Moving to the same namespace is effectively a no-op.  No exceptions
+        # about matching branch names should be raised.
+        branch = self.factory.makeAnyBranch()
+        namespace = branch.namespace
+        namespace.moveBranch(branch, branch.owner)
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+    def test_name_clash_raises(self):
+        # A name clash will raise an exception.
+        branch = self.factory.makeAnyBranch(name="test")
+        another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
+        namespace = another.namespace
+        self.assertRaises(
+            BranchExists, namespace.moveBranch, branch, branch.owner)
+
+    def test_move_with_rename(self):
+        # A name clash with 'rename_if_necessary' set to True will cause the
+        # branch to be renamed instead of raising an error.
+        branch = self.factory.makeAnyBranch(name="test")
+        another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
+        namespace = another.namespace
+        namespace.moveBranch(branch, branch.owner, rename_if_necessary=True)
+        self.assertEqual("test-1", branch.name)
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+    def test_move_with_new_name(self):
+        # A new name for the branch can be specified as part of the move.
+        branch = self.factory.makeAnyBranch(name="test")
+        another = self.factory.makeAnyBranch(owner=branch.owner, name="test")
+        namespace = another.namespace
+        namespace.moveBranch(branch, branch.owner, new_name="foo")
+        self.assertEqual("foo", branch.name)
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+    def test_sets_branch_owner(self):
+        # Moving to a new namespace may change the owner of the branch if the
+        # owner of the namespace is different.
+        branch = self.factory.makeAnyBranch(name="test")
+        team = self.factory.makeTeam(branch.owner)
+        product = self.factory.makeProduct()
+        namespace = ProductNamespace(team, product)
+        namespace.moveBranch(branch, branch.owner)
+        self.assertEqual(team, branch.owner)
+        # And for paranoia.
+        self.assertNamespacesEqual(namespace, branch.namespace)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py	2009-08-04 00:41:49 +0000
@@ -46,6 +46,24 @@
         branches = self.target.collection.getBranches()
         self.assertEqual([branch], list(branches))
 
+    def test_retargetBranch_packageBranch(self):
+        # Retarget an existing package branch to this target.
+        branch = self.factory.makePackageBranch()
+        self.target._retargetBranch(removeSecurityProxy(branch))
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_productBranch(self):
+        # Retarget an existing product branch to this target.
+        branch = self.factory.makeProductBranch()
+        self.target._retargetBranch(removeSecurityProxy(branch))
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_personalBranch(self):
+        # Retarget an existing personal branch to this target.
+        branch = self.factory.makePersonalBranch()
+        self.target._retargetBranch(removeSecurityProxy(branch))
+        self.assertEqual(self.target, branch.target)
+
 
 class TestPackageBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
 
@@ -212,6 +230,33 @@
         # The default merge target is always None.
         self.assertIs(None, self.target.default_merge_target)
 
+    def test_retargetBranch_packageBranch(self):
+        # Retarget an existing package branch to this target.  Override the
+        # mixin tests, and specify the owner of the branch.  This is needed to
+        # match the target as the target is the branch owner for a personal
+        # branch.
+        branch = self.factory.makePackageBranch(owner=self.original)
+        self.target._retargetBranch(removeSecurityProxy(branch))
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_productBranch(self):
+        # Retarget an existing product branch to this target.  Override the
+        # mixin tests, and specify the owner of the branch.  This is needed to
+        # match the target as the target is the branch owner for a personal
+        # branch.
+        branch = self.factory.makeProductBranch(owner=self.original)
+        self.target._retargetBranch(removeSecurityProxy(branch))
+        self.assertEqual(self.target, branch.target)
+
+    def test_retargetBranch_personalBranch(self):
+        # Retarget an existing personal branch to this target.  Override the
+        # mixin tests, and specify the owner of the branch.  This is needed to
+        # match the target as the target is the branch owner for a personal
+        # branch.
+        branch = self.factory.makePersonalBranch(owner=self.original)
+        self.target._retargetBranch(removeSecurityProxy(branch))
+        self.assertEqual(self.target, branch.target)
+
 
 class TestProductBranchTarget(TestCaseWithFactory, BaseBranchTargetTests):
 
 
=== modified file 'lib/lp/code/model/tests/test_diff.py'
--- lib/lp/code/model/tests/test_diff.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_diff.py	2009-08-04 23:07:33 +0000
@@ -6,27 +6,66 @@
 __metaclass__ = type
 
 
+from cStringIO import StringIO
 from unittest import TestLoader
 
-from canonical.testing import (
-    DatabaseFunctionalLayer, LaunchpadFunctionalLayer, LaunchpadZopelessLayer)
 import transaction
 
+from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.testing import verifyObject
+from canonical.testing import LaunchpadFunctionalLayer, LaunchpadZopelessLayer
 from lp.code.model.diff import Diff, StaticDiff
 from lp.code.interfaces.diff import (
     IDiff, IPreviewDiff, IStaticDiff, IStaticDiffSource)
 from lp.testing import login, login_person, TestCaseWithFactory
-from canonical.launchpad.webapp import canonical_url
-from canonical.launchpad.webapp.testing import verifyObject
 
 
 class TestDiff(TestCaseWithFactory):
 
-    layer = DatabaseFunctionalLayer
+    layer = LaunchpadFunctionalLayer
 
     def test_providesInterface(self):
         verifyObject(IDiff, Diff())
 
+    def _create_diff(self, content):
+        # Create a Diff object with the content specified.
+        sio = StringIO()
+        sio.write(content)
+        size = sio.tell()
+        sio.seek(0)
+        diff = Diff.fromFile(sio, size)
+        # Commit to make the alias available for reading.
+        transaction.commit()
+        return diff
+
+    def test_text_reads_librarian_content(self):
+        # IDiff.text will read at most config.diff.max_read_size bytes from
+        # the librarian.
+        content = "1234567890" * 10
+        diff = self._create_diff(content)
+        self.assertEqual(content, diff.text)
+
+    def test_oversized_normal(self):
+        # A diff smaller than config.diff.max_read_size is not oversized.
+        content = "1234567890" * 10
+        diff = self._create_diff(content)
+        self.assertFalse(diff.oversized)
+
+    def test_text_read_limited_by_config(self):
+        # IDiff.text will read at most config.diff.max_read_size bytes from
+        # the librarian.
+        self.pushConfig("diff", max_read_size=25)
+        content = "1234567890" * 10
+        diff = self._create_diff(content)
+        self.assertEqual(content[:25], diff.text)
+
+    def test_oversized_for_big_diff(self):
+        # A diff larger than config.diff.max_read_size is oversized.
+        self.pushConfig("diff", max_read_size=25)
+        content = "1234567890" * 10
+        diff = self._create_diff(content)
+        self.assertTrue(diff.oversized)
+
 
 class TestStaticDiff(TestCaseWithFactory):
     """Test that StaticDiff objects work."""
 
=== modified file 'lib/lp/code/model/tests/test_revision.py'
--- lib/lp/code/model/tests/test_revision.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/model/tests/test_revision.py	2009-08-02 23:17:10 +0000
@@ -146,7 +146,8 @@
         branch.createBranchRevision(1, rev)
         # Once the branch is connected to the revision, we now specify
         # a product for the branch.
-        branch.product = self.factory.makeProduct()
+        project = self.factory.makeProduct()
+        branch.setTarget(user=branch.owner, project=project)
         # The revision is now identified as needing karma allocated.
         self.assertEqual(
             [rev], list(RevisionSet.getRevisionsNeedingKarmaAllocated()))
 
=== modified file 'lib/lp/code/scripts/tests/test_revisionkarma.py'
--- lib/lp/code/scripts/tests/test_revisionkarma.py	2009-06-25 04:06:00 +0000
+++ lib/lp/code/scripts/tests/test_revisionkarma.py	2009-08-04 00:41:49 +0000
@@ -49,7 +49,8 @@
         branch.createBranchRevision(1, rev)
         # Once the branch is connected to the revision, we now specify
         # a product for the branch.
-        branch.product = self.factory.makeProduct()
+        project = self.factory.makeProduct()
+        branch.setTarget(user=branch.owner, project=project)
         # Commit and switch to the script db user.
         transaction.commit()
         LaunchpadZopelessLayer.switchDbUser(config.revisionkarma.dbuser)
 
=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt	2009-08-04 05:02:41 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt	2009-08-05 02:18:13 +0000
@@ -139,6 +139,10 @@
       <div class="boardCommentBody attachmentBody">
         <tal:diff replace="structure view/review_diff/fmt:diff" />
       </div>
+      <div class="boardCommentFooter"
+           tal:condition="view/review_diff_oversized">
+        The diff has been truncated for viewing.
+      </div>
     </tal:real-diff>
     <tal:empty-diff condition="not: attachment">
       <div class="boardCommentBody attachmentBody">
 
=== modified file 'lib/lp/codehosting/tests/test_acceptance.py'
--- lib/lp/codehosting/tests/test_acceptance.py	2009-07-23 18:10:26 +0000
+++ lib/lp/codehosting/tests/test_acceptance.py	2009-08-04 00:41:49 +0000
@@ -382,7 +382,7 @@
         # rename as far as bzr is concerned: the URL changes.
         LaunchpadZopelessTestSetup().txn.begin()
         branch = self.getDatabaseBranch('testuser', None, 'test-branch')
-        branch.product = Product.byName('firefox')
+        branch.setTarget(user=branch.owner, project=Product.byName('firefox'))
         LaunchpadZopelessTestSetup().txn.commit()
 
         self.assertNotBranch(
 
=== modified file 'lib/lp/registry/browser/tests/private-team-creation-views.txt'
--- lib/lp/registry/browser/tests/private-team-creation-views.txt	2009-07-31 02:45:01 +0000
+++ lib/lp/registry/browser/tests/private-team-creation-views.txt	2009-08-03 21:46:09 +0000
@@ -400,8 +400,7 @@
     ...     bugtask = bug.default_bugtask
     ...     bugtask.transitionToAssignee(team)
     ...     # A branch.
-    ...     branch = factory.makeBranch()
-    ...     branch.owner = team
+    ...     branch = factory.makeBranch(owner=team, registrant=team_owner)
     ...     # A branch subscription.
     ...     from lp.code.enums import (
     ...         BranchSubscriptionDiffSize,
 
=== modified file 'lib/lp/registry/doc/private-team-roles.txt'
--- lib/lp/registry/doc/private-team-roles.txt	2009-07-31 02:45:01 +0000
+++ lib/lp/registry/doc/private-team-roles.txt	2009-08-03 01:41:07 +0000
@@ -23,9 +23,10 @@
 -----------------
 
     >>> # Create the necessary teams.
-    >>> from lp.registry.interfaces.person import PersonVisibility
     >>> team_owner = factory.makePerson(name='team-owner')
-    >>> login('[e-mail]')
+    >>> from lp.registry.interfaces.person import IPersonSet, PersonVisibility
+    >>> admin_user = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> login_person(admin_user)
     >>> priv_team = factory.makeTeam(name='private-team',
     ...     owner=team_owner,
     ...     visibility=PersonVisibility.PRIVATE)
@@ -102,12 +103,12 @@
 Private teams can be assigned as the owner of a branch
 
     >>> branch = factory.makeBranch()
-    >>> branch.owner = priv_team
+    >>> branch.setOwner(priv_team, user=admin_user)
 
 But private membership teams cannot own a branch.
 
     >>> branch = factory.makeBranch()
-    >>> branch.owner = pm_team
+    >>> branch.setOwner(pm_team, user=admin_user)
     Traceback (most recent call last):
     ...
     PrivatePersonLinkageError: Cannot link person
 
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2009-07-23 17:47:50 +0000
+++ lib/lp/testing/factory.py	2009-08-03 21:46:09 +0000
@@ -600,7 +600,10 @@
             distroseries = sourcepackage.distroseries
 
         if registrant is None:
-            registrant = owner
+            if owner.is_team:
+                registrant = owner.teamowner
+            else:
+                registrant = owner
 
         if branch_type in (BranchType.HOSTED, BranchType.IMPORTED):
             url = None
 
=== modified file 'lib/lp/translations/scripts/message_sharing_migration.py'
--- lib/lp/translations/scripts/message_sharing_migration.py	2009-07-19 04:41:14 +0000
+++ lib/lp/translations/scripts/message_sharing_migration.py	2009-08-05 10:45:31 +0000
@@ -309,8 +309,8 @@
                     message = removeSecurityProxy(message)
 
                     clashing_current, clashing_imported, twin = (
-                        self._findClashesFromDicts(
-                            existing_tms, current_tms, imported_tms, message))
+                        self._findClashes(
+                            message, representative, message.potemplate))
 
                     if clashing_current or clashing_imported:
                         saved = self._saveByDiverging(