← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rvb/launchpad/bug-869221-archindepwidget into lp:launchpad

 

Raphaël Badin has proposed merging lp:~rvb/launchpad/bug-869221-archindepwidget into lp:launchpad with lp:~rvb/launchpad/bug-869221-forcederivedarchinep as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #869221 in Launchpad itself: "Initializing a distroseries that doesn't use the parent's archindep arch will fail"
  https://bugs.launchpad.net/launchpad/+bug/869221

For more details, see:
https://code.launchpad.net/~rvb/launchpad/bug-869221-archindepwidget/+merge/79561

This branch is the last one to fix bug 869221.  It adds a widget to the +initseries page to let the user force the arch tag for the architecture independent builder.

Here is how the new widget looks like before any parent is added:
http://people.canonical.com/~rvb/new_widget.png
and after parents are added:
http://people.canonical.com/~rvb/new_widget_populated.png

Two errors can happen:
- if the option selected on the new widget is left to the default (Auto select) and that none of the parents' arch indep arch tags are selected, then the backend won't be able to "auto select" a arch indep arch tag for the derived series so we display an error: http://people.canonical.com/~rvb/new_widget_no_arch_indep.png
- if the arch indep arch tag is not among the selected architectures this is obviously wrong: http://people.canonical.com/~rvb/new_widget_arch_indep_not_among.png

= Tests =

lib/lp/registry/javascript/distroseries/tests/test_initseries.html
lib/lp/app/javascript/formwidgets/tests/test_formwidgets.html
lib/lp/registry/javascript/distroseries/tests/test_initseries.html

= QA =

This branch (and all the others from which it depends) should let one initialize a series while specifying the arch indep builder arch tag.
-- 
https://code.launchpad.net/~rvb/launchpad/bug-869221-archindepwidget/+merge/79561
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/bug-869221-archindepwidget into lp:launchpad.
=== modified file 'lib/lp/app/javascript/formwidgets/formwidgets.js'
--- lib/lp/app/javascript/formwidgets/formwidgets.js	2011-10-17 14:22:26 +0000
+++ lib/lp/app/javascript/formwidgets/formwidgets.js	2011-10-17 14:22:26 +0000
@@ -183,7 +183,7 @@
      * It is also responsible for setting any error on the widget using
      * this.showError.
      *
-     * @method hideError
+     * @method validate
      */
     validate: function() {
         return true;
@@ -382,6 +382,20 @@
     },
 
     /**
+     * Utility method to validate that the list of selected choices is a
+     * subset of the given list.
+     *
+     * @method validate_choice_among
+     */
+    validate_choice_among: function(superset) {
+        var choices = this.get(
+            "multiple") ? this.get('choice') : [this.get('choice')];
+        return !Y.Array.some(choices, function(choice) {
+            return (Y.Array.indexOf(superset, choice.value) === -1);
+        });
+     },
+
+    /**
      * Remove a list of choices from the possible widget's choices.
      *
      * @method remove_choices
@@ -395,12 +409,13 @@
             }
         );
         // Filter out the choices mentioned.
-        choices = this.get("choices").filter(function(choice) {
+        var newchoices = this.get("choices").filter(function(choice) {
             return !owns(choicemap, choice.value);
         });
         // Set back again.
-        this.set("choices", choices);
+        this.set("choices", newchoices);
         // Tell everyone!
+        this.fire("removed_choices", this, choices);
         Y.lp.anim.green_flash({node: this.fieldNode}).run();
     },
 
@@ -418,7 +433,7 @@
             }
         );
         // Allow existing choices to be redefined.
-        choices = this.get("choices").map(function(choice) {
+        var newchoices = this.get("choices").map(function(choice) {
             if (owns(choicemap, choice.value)) {
                 choice = choicemap[choice.value];
                 delete choicemap[choice.value];
@@ -426,10 +441,11 @@
             return choice;
         });
         // Add the new choices (i.e. what remains in choicemap).
-        choices = choices.concat(values(choicemap));
+        newchoices = newchoices.concat(values(choicemap));
         // Set back again.
-        this.set("choices", choices);
+        this.set("choices", newchoices);
         // Tell everyone!
+        this.fire("added_choices", this, choices);
         Y.lp.anim.green_flash({node: this.fieldNode}).run();
     }
 
@@ -499,6 +515,18 @@
     },
 
     /**
+     * Validate the coherence of the contained widgets.
+     * Subclasses can override this method. It is meant to be used when
+     * there is validation involving more than one widget that needs to
+     * happen.
+     *
+     * @method validate_all
+     */
+    validate_all: function() {
+        return true;
+    },
+
+    /**
      * Validate all the registered widgets. Display a global error
      * displaying the number of errors if any of the encapsulated widgets
      * fails to validate.
@@ -506,27 +534,32 @@
      * @method validate
      */
     validate: function() {
-        this.hideErrors();
-        var i = 0;
+        var i;
         var error_number = 0;
-        for (i; i<this._widgets.length; i++) {
+        for (i = 0; i < this._widgets.length; i++) {
             if (this._widgets[i].validate() === false) {
                 error_number = error_number + 1;
             }
         }
         if (error_number !== 0) {
+            this.showNErrors(error_number);
+            return false;
+        }
+        return this.validate_all();
+    },
+
+    showNErrors: function(error_number) {
+        this.hideError();
+        if (error_number !== 0) {
             if (error_number === 1) {
                 this.showError('There is 1 error.');
             }
             else {
                 this.showError('There are ' + error_number + ' errors.');
             }
-            return false;
         }
-        return true;
     },
 
-
     /**
      * Show the spinner, and hide the submit button.
      *
@@ -551,19 +584,33 @@
      * @method showError
      */
     showError: function(error) {
-        this.hideErrors();
+        this.hideError();
         Y.Node.create('<p class="error message" />')
             .appendTo(this.get("contentBox"))
             .set("text", error);
     },
 
     /**
-     * Remove all errors that have been previously displayed by showError.
+     * Remove the error that has been previously displayed by showError.
+     *
+     * @method hideError
+     */
+    hideError: function(error) {
+        this.get("contentBox").all("p.error.message").remove();
+    },
+
+    /**
+     * Remove all the errors (including these displayed on the contained
+     * widgets).
      *
      * @method hideErrors
      */
     hideErrors: function(error) {
-        this.get("contentBox").all("p.error.message").remove();
+        this.hideError();
+        var i;
+        for (i = 0; i < this._widgets.length; i++) {
+            this._widgets[i].hideError();
+        }
     }
 
 });

=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js'
--- lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js	2011-10-17 14:22:26 +0000
+++ lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js	2011-10-17 14:22:26 +0000
@@ -277,6 +277,19 @@
                 this.container.all("li > input").getAttribute("type"));
         },
 
+        testAddChoicesFiresEvent: function() {
+            var event_fired = false;
+            var handleEvent = function(e, new_choices) {
+                event_fired = true;
+                ArrayAssert.itemsAreEqual(["a", "b"], new_choices);
+            };
+            this.widget.on(
+                this.widget.name + ":added_choices",
+                handleEvent, this.widget);
+            this.widget.add_choices(["a", "b"]);
+            Assert.isTrue(event_fired);
+        },
+
         testRenderRemoveChoices: function() {
             this.widget.set("choices", ["a", "b", "c", "d"]);
             this.widget.render(this.container);
@@ -289,6 +302,20 @@
                 this.container.all("li > label").get("text"));
         },
 
+        testRemoveChoicesFiresEvent: function() {
+            this.widget.add_choices(["a", "b", "c"]);
+            var event_fired = false;
+            var handleEvent = function(e, removed_choices) {
+                event_fired = true;
+                ArrayAssert.itemsAreEqual(["a", "b"], removed_choices);
+            };
+            this.widget.on(
+                this.widget.name + ":removed_choices",
+                handleEvent, this.widget);
+            this.widget.remove_choices(["a", "b"]);
+            Assert.isTrue(event_fired);
+        },
+
         testRenderChoicesChangeMultiple: function() {
             this.widget.set("choices", ["a", "b"]);
             this.widget.render(this.container);
@@ -500,8 +527,20 @@
             ArrayAssert.containsMatch(
                 function(event) { return event.attrName === "choices"; },
                 events);
+        },
+
+        testValidateChoicesAmong: function() {
+            this.widget.add_choices(
+                [{value: 'c', text: 'c', data: 'c'},
+                 {value: 'b', text: 'b', data: 'b'},
+                 {value: 'a', text: 'a', data: 'a'}
+                ]);
+            this.widget.set('choice', ['a', 'b']);
+            Assert.isTrue(
+                this.widget.validate_choice_among(['a', 'b', 'd']));
+            Assert.isFalse(
+                this.widget.validate_choice_among(['a', 'e', 'd']));
         }
-
     };
 
     testChoiceListWidget = Y.merge(
@@ -565,13 +604,22 @@
 
         testRegisterWidget: function() {
             var fake_widget = 'Fake Widget';
-            Assert.areEqual(0, this.widget._widgets.length);
+            var length = this.widget._widgets.length;
             this.widget.registerWidget(fake_widget);
-            Assert.areEqual(1, this.widget._widgets.length);
+            Assert.areEqual(
+                length + 1, this.widget._widgets.length);
             Assert.areSame(
                 "Fake Widget",
-                this.widget._widgets[0]);
-        },
+                this.widget._widgets[length]);
+        }
+
+    };
+
+    suite.add(new Y.Test.Case(testFormActionsWidget));
+
+
+    var testFormActionsWidgetValidation = {
+        name: 'TestFormActionsWidgetValidation',
 
         createMockWidget: function(return_value) {
             var mockWidget = Y.Mock();
@@ -612,7 +660,6 @@
             Y.Mock.verify(widget1);
             Y.Mock.verify(widget2);
             Assert.isFalse(result);
-            Y.log(this.widget.fieldNode);
             Assert.areEqual(
                 'There is 1 error.',
                 this.widget.get("contentBox").one('.message').get('text'));
@@ -628,7 +675,6 @@
             Y.Mock.verify(widget1);
             Y.Mock.verify(widget2);
             Assert.isFalse(result);
-            Y.log(this.widget.fieldNode);
             Assert.areEqual(
                 'There are 2 errors.',
                 this.widget.get("contentBox").one('.message').get('text'));
@@ -636,7 +682,9 @@
 
     };
 
-    suite.add(new Y.Test.Case(testFormActionsWidget));
+    testFormActionsWidgetValidation = Y.merge(
+        testFormActionsWidgetValidation, testFormActionsWidget);
+    suite.add(new Y.Test.Case(testFormActionsWidgetValidation));
 
     // Exports.
     namespace.testFormRowWidget = testFormRowWidget;

=== modified file 'lib/lp/registry/javascript/distroseries/initseries.js'
--- lib/lp/registry/javascript/distroseries/initseries.js	2011-10-17 14:22:26 +0000
+++ lib/lp/registry/javascript/distroseries/initseries.js	2011-10-17 14:22:26 +0000
@@ -45,6 +45,8 @@
         this.registerWidget(this.deriveFromChoices);
         this.architectureChoice = config.architectureChoice;
         this.registerWidget(this.architectureChoice);
+        this.architectureIndepChoice = config.architectureIndepChoice;
+        this.registerWidget(this.architectureIndepChoice);
         this.packagesetChoice = config.packagesetChoice;
         this.registerWidget(this.packagesetChoice);
         this.packageCopyOptions = config.packageCopyOptions;
@@ -74,12 +76,54 @@
     },
 
     /**
+     * Validate the coherence of the fields. Returns true if the form is
+     * fit for submission. Returns false otherwise and displays appropriate
+     * errors.
+     *
+     * @method validate
+     */
+    validate: function() {
+        this.hideErrors();
+        var arch_indep_choice = this.architectureIndepChoice.get(
+            'choice').value;
+        if (arch_indep_choice === this.architectureIndepChoice.AUTOSELECT) {
+            // If no arch indep arch tag has been explicitely selected
+            // check that one from the parents' is present among the selected
+            // architectures.
+            if (!this.architectureChoice.validate_arch_indep()) {
+                this.architectureChoice.show_error_arch_indep();
+                this.architectureIndepChoice.showError(
+                    'Alternatively, you can specify the architecture ' +
+                    'independent builder.');
+                return false;
+            }
+        }
+        else {
+            // Check that the arch indep arch tag is among the selected
+            // architectures.
+            var choices_objs = this.architectureChoice.get('choice');
+            if (choices_objs.length !== 0) {
+                var choices = Y.Array.map(
+                    choices_objs,
+                    function(choice_obj) { return choice_obj.value; });
+                if (!this.architectureIndepChoice.validate_choice_among(
+                         choices)) {
+                    this.architectureIndepChoice.showError(
+                        'The selected architecture independent builder is ' +
+                        'not among the selected architectures.');
+                    return false;
+                }
+            }
+        }
+        return true;
+    },
+
+    /**
      * Validate all the widgets and call submit as appropriate.
      *
      * @method submit
      */
     check_and_submit: function() {
-        var check = this.validate();
         if (this.validate()) {
             this.submit();
         }
@@ -93,6 +137,8 @@
     submit: function() {
         var self = this;
         var values = attrselect("value");
+        var arch_indep = values(
+            this.architectureIndepChoice.get("choice"))[0];
         var config = {
             on: {
                 start: function() {
@@ -111,6 +157,9 @@
                 parents: this.deriveFromChoices.get("parents"),
                 architectures:
                     values(this.architectureChoice.get("choice")),
+                archindep_archtag:
+                    arch_indep === this.architectureIndepChoice.AUTOSELECT ?
+                        null : arch_indep,
                 packagesets: this.packagesetChoice !== null ?
                     values(this.packagesetChoice.get("choice")) : [],
                 rebuild:
@@ -210,6 +259,14 @@
                      "if you want to use all the available " +
                      "architectures)."))
             .render(form_table_body);
+    var arch_indep_choice =
+       new widgets.ArchIndepChoiceListWidget()
+            .set("name", "field.archindep_archtag")
+            .set("label", "Architecture independent builder:")
+            .set("description", (
+                     "Choose the architecture tag that should be " +
+                     "used to build architecture independent binaries."))
+            .render(form_table_body);
     var packageset_choice = null;
     if (cache.is_first_derivation) {
         packageset_choice =
@@ -246,6 +303,7 @@
             srcNode: form_container.one("div.actions"),
             deriveFromChoices: parents_selection,
             architectureChoice: architecture_choice,
+            architectureIndepChoice: arch_indep_choice,
             packagesetChoice: packageset_choice,
             packageCopyOptions: package_copy_options,
             form_container: form_container
@@ -280,6 +338,20 @@
         }
     });
 
+    form_actions.architectureChoice.on(
+        form_actions.architectureChoice.name + ":added_choices",
+        function(e, arch_list) {
+            this.add_choices(arch_list);
+        },
+        form_actions.architectureIndepChoice);
+
+    form_actions.architectureChoice.on(
+        form_actions.architectureChoice.name + ":removed_choices",
+        function(e, arch_list) {
+            this.remove_choices(arch_list);
+        },
+        form_actions.architectureIndepChoice);
+
     if (cache.is_first_derivation) {
         // Wire the architecture and packageset pickers to the parent picker.
         Y.on('parent_added', function(parent) {

=== modified file 'lib/lp/registry/javascript/distroseries/tests/test_initseries.html'
--- lib/lp/registry/javascript/distroseries/tests/test_initseries.html	2011-08-09 15:51:53 +0000
+++ lib/lp/registry/javascript/distroseries/tests/test_initseries.html	2011-10-17 14:22:26 +0000
@@ -57,5 +57,23 @@
     <ul id="suites">
       <li>lp.registry.distroseries.initseries.test</li>
     </ul>
+
+    <script type="text/x-template" id="init-form">
+      <div style="display:none" class="unseen"
+           id="initseries-form-container">
+        <div>
+          <form action="">
+            <table class="form" id="launchpad-form-widgets">
+            </table>
+            <div class="actions" id="launchpad-form-actions">
+              <input type="submit" id="field.actions.initialize"
+                     name="field.actions.initialize"
+                     value="Initialize Series" class="button" />
+            </div>
+          </form>
+        </div>
+      </div>
+    </script>
+
   </body>
 </html>

=== modified file 'lib/lp/registry/javascript/distroseries/tests/test_initseries.js'
--- lib/lp/registry/javascript/distroseries/tests/test_initseries.js	2011-08-22 14:03:02 +0000
+++ lib/lp/registry/javascript/distroseries/tests/test_initseries.js	2011-10-17 14:22:26 +0000
@@ -18,6 +18,7 @@
 
     var suite = new Y.Test.Suite("distroseries.initseries Tests");
     var initseries = Y.lp.registry.distroseries.initseries;
+    var widgets = Y.lp.registry.distroseries.widgets;
 
     var testDeriveDistroSeriesActionsWidget = {
         name: 'TestDeriveDistroSeriesActionsWidget',
@@ -61,6 +62,13 @@
                         ];
                     }
                 },
+                architectureIndepChoice: {
+                    AUTOSELECT: '-',
+                    get: function(name) {
+                        Assert.areEqual("choice", name);
+                        return {value: "sparc", text: "sparc"};
+                    }
+                },
                 packagesetChoice: {
                     get: function(name) {
                         Assert.areEqual("choice", name);
@@ -132,6 +140,9 @@
                         ["i386", "sparc"],
                         config.parameters.architectures);
                     ArrayAssert.itemsAreEqual(
+                        "sparc",
+                        config.parameters.archindep_archtag);
+                    ArrayAssert.itemsAreEqual(
                         ["4", "5"],
                         config.parameters.packagesets);
                     Assert.isTrue(config.parameters.rebuild);
@@ -151,26 +162,97 @@
         testDeriveDistroSeriesActionsWidget);
     suite.add(new Y.Test.Case(testDeriveDistroSeriesActionsWidget));
 
+
+    var init_form = Y.one('#init-form').getContent();
+
     var testDeriveDistroSeriesSetup = {
         name: 'TestDeriveDistroSeriesSetup',
 
         setUp: function() {
-            var node = Y.Node.create(
-                '<div style="display:none;"' +
-                '  class="unseen" id="initseries-form-container">' +
-                '  <div>' +
-                '    <form action="">' +
-                '      <table class="form" id="launchpad-form-widgets">' +
-                '      </table>' +
-                '      <div class="actions" id="launchpad-form-actions">' +
-                '        <input type="submit" id="field.actions.initialize"' +
-                '          name="field.actions.initialize" ' +
-                '          value="Initialize Series" class="button" />' +
-                '      </div>' +
-                '    </form>' +
-                '  </div>' +
-                '</div>');
-            Y.one('body').appendChild(node);
+           var node = Y.Node.create(init_form);
+           Y.one('body').appendChild(node);
+        },
+
+        setUpArches: function(arch_choices, archindep_tags) {
+            var cache = {is_first_derivation: true};
+            this.form_actions = initseries.setupWidgets(cache);
+            this.form_actions.architectureChoice.set(
+                'choices', arch_choices);
+            this.form_actions.architectureChoice._archindep_tags =
+                archindep_tags;
+            this.form_actions.architectureIndepChoice.add_choices(
+                arch_choices);
+        },
+
+        assertShowsError: function(field, expected_error_msg) {
+            var error_msg = field.get(
+                'contentBox').one('.message').get('text');
+            Assert.areEqual(expected_error_msg, error_msg);
+        },
+
+        testValidateNoArchIndepChoiceOk: function() {
+            this.setUpArches(['i386', 'hppa'], {'3': 'i386'});
+            this.form_actions.architectureChoice.set(
+                'choice', 'i386');
+            this.form_actions.architectureIndepChoice.set(
+                'choice', 'i386');
+            Assert.isTrue(this.form_actions.validate());
+        },
+
+        testValidateNoArchIndepChoiceFailNotAmong: function() {
+            // Validation fails if the selected arch indep architecture
+            // is not among the selected architectures for the derived series.
+            this.setUpArches(['i386', 'hppa'], {'3': 'i386'});
+            this.form_actions.architectureChoice.set(
+                'choice', 'i386');
+            this.form_actions.architectureIndepChoice.set(
+                'choice', 'hppa');
+            Assert.isFalse(this.form_actions.validate());
+            this.assertShowsError(
+                this.form_actions.architectureIndepChoice,
+                'The selected architecture independent builder is not ' +
+                'among the selected architectures.');
+        },
+
+        testValidateAutoArchIndepOk: function() {
+            this.setUpArches(['i386', 'hppa'], {'3': 'i386'});
+            this.form_actions.architectureChoice.set(
+                'choice', 'i386');
+            this.form_actions.architectureIndepChoice.set(
+                'choice',
+                this.form_actions.architectureIndepChoice.AUTOSELECT);
+            Assert.isTrue(this.form_actions.validate());
+        },
+
+        testValidateAutoArchIndepOkAll: function() {
+            // If no architecture is selected, it means that all the
+            // architectures from the parents will be copied over.
+            this.setUpArches(['i386', 'hppa'], {'3': 'i386'});
+            this.form_actions.architectureIndepChoice.set(
+                'choice',
+                this.form_actions.architectureIndepChoice.AUTOSELECT);
+            Assert.isTrue(this.form_actions.validate());
+        },
+
+        testValidateAutoArchIndepChoiceFail: function() {
+            // Validation fails if the arch indep architecture is not
+            // specified and none of the parents' arch indep architectures
+            // has been selected.
+            this.setUpArches(['i386', 'hppa'], {'3': 'i386'});
+            this.form_actions.architectureChoice.set(
+                'choice', 'hppa');
+            this.form_actions.architectureIndepChoice.set(
+                'choice',
+                this.form_actions.architectureIndepChoice.AUTOSELECT);
+            Assert.isFalse(this.form_actions.validate());
+            this.assertShowsError(
+                this.form_actions.architectureChoice,
+                'The distroseries has no architectures selected to build ' +
+                'architecture independent binaries.');
+            this.assertShowsError(
+                this.form_actions.architectureIndepChoice,
+                'Alternatively, you can specify the architecture ' +
+                'independent builder.');
         },
 
         testIsFirstDerivation: function() {
@@ -192,6 +274,23 @@
                 form_actions.packageCopyOptions.get('choice').value);
         },
 
+        getFakeClient: function(res_sequence, return_obj) {
+            var count = 0;
+            var client = {
+                get: function(path, config) {
+                    Assert.areEqual(path, res_sequence[count][0]);
+                    var return_obj = res_sequence[count][1];
+                    count = count + 1;
+                    if (return_obj instanceof Array) {
+                        return_obj = new Y.lp.client.Collection(
+                            null, {entries: return_obj}, null);
+                    }
+                    config.on.success(return_obj);
+                }
+            };
+            return client;
+        },
+
         testIsNotFirstDerivation: function() {
             var cache = {
                 is_first_derivation: false,
@@ -209,21 +308,19 @@
             };
             var form_actions = initseries.setupWidgets(cache);
             // Monkeypatch LP client.
-            var client = {
-                get: function(path, config) {
-                    Assert.areEqual(
-                        "/ubuntu/natty/architectures",
-                        path);
-                    // Return previous_series' architectures.
-                    var arches = new Y.lp.client.Collection(
-                        null,
-                        {entries: [
-                            {'architecture_tag': 'hppa'},
-                            {'architecture_tag': 'i386'}]},
-                        null);
-                    config.on.success(arches);
-                }
-            };
+            var mockNominatedArchIndep = Y.Mock();
+            Y.Mock.expect(mockNominatedArchIndep, {
+                method: "get",
+                args: ["architecture_tag"],
+                returns: "i386"
+            });
+            var client = this.getFakeClient(
+                [["/ubuntu/natty/architectures",
+                  [{'architecture_tag': 'hppa'},
+                   {'architecture_tag': 'i386'}]],
+                 ["/api/devel/ubuntu/natty/nominatedarchindep",
+                  mockNominatedArchIndep]]
+                );
             form_actions.architectureChoice.client = client;
             initseries.setupInteraction(form_actions, cache);
 
@@ -236,8 +333,10 @@
             // The architecture picker features the architectures
             // from the previous series.
             ArrayAssert.itemsAreEqual(
-                ['hppa', 'i386'], attrselect("value")(
+                ['hppa', 'i386'],
+                attrselect("value")(
                     form_actions.architectureChoice.get("choices")));
+            Y.Mock.verify(mockNominatedArchIndep);
          }
     };
 

=== modified file 'lib/lp/registry/javascript/distroseries/tests/test_widgets.js'
--- lib/lp/registry/javascript/distroseries/tests/test_widgets.js	2011-10-17 14:22:26 +0000
+++ lib/lp/registry/javascript/distroseries/tests/test_widgets.js	2011-10-17 14:22:26 +0000
@@ -173,23 +173,42 @@
             this.container.remove(true);
         },
 
+        setupDistroArchSerieses: function(arches) {
+            var distro_arch_serieses = [];
+            var i;
+            for (i=0; i<arches.length; i++) {
+                distro_arch_serieses.push({architecture_tag: arches[i]});
+            }
+            var res = new Y.lp.client.Collection(
+                null, {entries: distro_arch_serieses}, null);
+            return res;
+         },
+
         testAddDistroArchSerieses: function() {
-            var distro_arch_serieses = [
-                {architecture_tag: "i386"},
-                {architecture_tag: "amd64"},
-                {architecture_tag: "i386"}
-            ];
-            var distro_arch_serieses_collection =
-                new Y.lp.client.Collection(
-                    null, {entries: distro_arch_serieses}, null);
-            this.widget.add_distroarchseries(
-                3,
-                distro_arch_serieses_collection);
+           var collection = this.setupDistroArchSerieses(
+               ["i386", "amd64", "i386"]);
+           this.widget.add_distroarchseries(3, collection);
             ArrayAssert.itemsAreEqual(
                 ["amd64", "i386"],
                 attrselect("value")(this.widget.get("choices")));
         },
 
+        testAddDistroArchSeriesesFiresEvent: function() {
+            var collection = this.setupDistroArchSerieses(
+                ["i386", "amd64", "i386"]);
+            var event_fired = false;
+            var handleEvent = function(e, arch_list) {
+                event_fired = true;
+                ArrayAssert.itemsAreEqual(
+                    ["i386", "amd64", "i386"], arch_list);
+            };
+            this.widget.on(
+                this.widget.name + ":added_choices",
+                handleEvent, this.widget);
+            this.widget.add_distroarchseries(3, collection);
+            Assert.isTrue(event_fired);
+        },
+
         test_populate_archindep_tags_InitiatesIO: function() {
             var distroseries = {api_uri: "ubuntu/hoary", value: 3};
             var io = false;
@@ -208,8 +227,7 @@
             Assert.isTrue(io, "No IO initiated.");
         },
 
-        test_populate_archindep_tags: function() {
-            var distroseries = {api_uri: "ubuntu/hoary", value: "3"};
+        setup_widget_client: function() {
             this.widget.client = {
                 get: function(path, config) {
                     var nominatedarchindep = {
@@ -221,35 +239,42 @@
                     config.on.success(nominatedarchindep);
                 }
             };
+         },
+
+        test_populate_archindep_tags: function() {
+            this.setup_widget_client();
+            distroseries = {api_uri: "ubuntu/hoary", value: "3"};
             this.widget._populate_archindep_tags(distroseries);
             Assert.areEqual("i386", this.widget._archindep_tags["3"]);
         },
 
-        testValidateOk: function() {
-            this.widget.render(this.container);
+        test_validate_arch_indep_Ok: function() {
             this.widget.add_choices(['i386', 'hppa']);
             this.widget.set('choice', ['i386']);
             this.widget._archindep_tags['3'] = 'i386';
-            var res = this.widget.validate();
+            var res = this.widget.validate_arch_indep();
             Assert.isTrue(res);
         },
 
-        testValidateOkEmpty: function() {
-            this.widget.render(this.container);
+        test_validate_arch_indep_Ok_empty: function() {
             this.widget.add_choices(['i386', 'hppa']);
             this.widget.set('choice', []);
             this.widget._archindep_tags['3'] = 'i386';
-            var res = this.widget.validate();
+            var res = this.widget.validate_arch_indep();
             Assert.isTrue(res);
         },
 
-        testValidateFails: function() {
-            this.widget.render(this.container);
+        test_validate_arch_indep_Fails: function() {
             this.widget.add_choices(['i386', 'hppa']);
             this.widget.set('choice', ['i386']);
             this.widget._archindep_tags['3'] = 'hppa';
-            var res = this.widget.validate();
+            var res = this.widget.validate_arch_indep();
             Assert.isFalse(res);
+        },
+
+        test_show_error_arch_indep: function() {
+            this.widget.render(this.container);
+            this.widget.show_error_arch_indep();
             Assert.areEqual(
                 "The distroseries has no architectures selected to "
                 + "build architecture independent binaries.",
@@ -309,6 +334,25 @@
             Assert.isUndefined(this.widget._archindep_tags["4"]);
         },
 
+        testRemoveDistroSeriesFiresEvent: function() {
+            var collection = this.setupDistroArchSerieses(
+                ["i386", "amd64", "i386"]);
+            this.widget.add_distroarchseries(3, collection);
+            var collection2 = this.setupDistroArchSerieses(
+                ["i386", "amd64", "hppa"]);
+            this.widget.add_distroarchseries(4, collection2);
+            var event_fired = false;
+            var handleEvent = function(e, arch_list) {
+                event_fired = true;
+                ArrayAssert.itemsAreEqual(["hppa"], arch_list);
+            };
+            this.widget.on(
+                this.widget.name + ":removed_choices",
+                handleEvent, this.widget);
+            this.widget.remove_distroseries("4");
+            Assert.isTrue(event_fired);
+        },
+
         testSetDistroSeriesError: function() {
             var widget = this.widget;
             widget.client = {
@@ -331,6 +375,47 @@
         testArchitecturesChoiceListWidget);
     suite.add(new Y.Test.Case(testArchitecturesChoiceListWidget));
 
+
+    var testArchIndepChoiceListWidget = {
+        name: 'TestArchIndepChoiceListWidget',
+
+        setUp: function() {
+            this.container = Y.Node.create("<div />");
+            this.widget = new widgets.ArchIndepChoiceListWidget();
+        },
+
+        tearDown: function() {
+            this.container.remove(true);
+        },
+
+        testInitialized: function() {
+            Assert.isFalse(this.widget.get('multiple'));
+            ArrayAssert.itemsAreEqual(
+                ["Auto select"],
+                attrselect("text")(this.widget.get("choices")));
+            ArrayAssert.itemsAreEqual(
+                [this.widget.AUTOSELECT],
+                attrselect("value")(this.widget.get("choices")));
+            Assert.areEqual(
+                this.widget.AUTOSELECT,
+                attrselect("value")(this.widget.get('choice')));
+        },
+
+        test_default_select_called: function() {
+            this.widget.add_choices(['4', '5']);
+            this.widget.set('choice', '5');
+            this.widget.remove_choices(['5']);
+                Y.log(this.widget.get('choice'));
+            Assert.areEqual(
+                this.widget.AUTOSELECT,
+                attrselect("value")(this.widget.get('choice')));
+        }
+
+    };
+
+    suite.add(new Y.Test.Case(testArchIndepChoiceListWidget));
+
+
     var testPackagesetPickerWidget = {
         name: 'TestPackagesetPickerWidget',
 

=== modified file 'lib/lp/registry/javascript/distroseries/widgets.js'
--- lib/lp/registry/javascript/distroseries/widgets.js	2011-10-17 14:22:26 +0000
+++ lib/lp/registry/javascript/distroseries/widgets.js	2011-10-17 14:22:26 +0000
@@ -387,7 +387,7 @@
 
     /**
      * Given a newly added distroseries object, populate the
-     * _archindep_tags field with it's architecture independent arch_tag.
+     * _archindep_tags field with its architecture independent arch_tag.
      *
      * @method _populate_archindep_tags
      * @param {Object} distroseries The distroseries object
@@ -400,21 +400,31 @@
         var on = {
             success: function (nominatedarchindep) {
                 var arch_tag = nominatedarchindep.get('architecture_tag');
-                self._archindep_tags[distroseries.value] = arch_tag;
+                self._add_archindep_archtag_for_series(
+                    distroseries.value, arch_tag);
             },
             failure: this.error_handler.getFailureHandler()
         };
         this.client.get(path, {on: on});
     },
 
+    _add_archindep_archtag_for_series: function(distroseries_id, arch_tag) {
+        this._archindep_tags[distroseries_id] = arch_tag;
+    },
+
+    _remove_archindep_archtag_for_series: function(distroseries_id) {
+        delete this._archindep_tags[distroseries_id];
+    },
+
     /**
      * Validate the architectures choice: make sure that at least one
      * architecture is suitable to build architecture independent packages.
+     * This will be called only if no architecture independent architecture
+     * arch tag is selected on another widget.
      *
-     * @method validate
+     * @method validate_arch_indep
      */
-    validate: function() {
-        this.hideError();
+    validate_arch_indep: function() {
         var choice_objects = this.get('choice');
         var choices = choice_objects.map(
             function(choice_object) { return choice_object.value; });
@@ -434,10 +444,14 @@
                 }
             }
         }
+       return false;
+    },
+
+    show_error_arch_indep: function() {
+        this.hideError();
         this.showError(
             ["The distroseries has no architectures selected to ",
              "build architecture independent binaries."].join(''));
-        return false;
     },
 
     /**
@@ -448,7 +462,7 @@
      */
     remove_distroseries: function(distroseries_id) {
         // Cleanup the corresponding _archindep_tags' entry.
-        delete this._archindep_tags[distroseries_id];
+        this._remove_archindep_archtag_for_series(distroseries_id);
         // Compute which das is only in the distroseries to be removed.
         var arch_to_remove = [];
         var das = this._distroseries[distroseries_id];
@@ -503,6 +517,67 @@
 
 
 /**
+ * A special form of ChoiceListWidget for choosing the architecture
+ * independent architecture tag.  It is initially only populated with
+ * the 'Auto select' option which means that the architecture independent
+ * tag will be selected among the parents'.
+ *
+ * @class ArchIndepChoiceListWidget
+ */
+var ArchIndepChoiceListWidget;
+
+ArchIndepChoiceListWidget = function() {
+    ArchIndepChoiceListWidget
+        .superclass.constructor.apply(this, arguments);
+};
+
+Y.mix(ArchIndepChoiceListWidget, {
+
+    NAME: 'archIndepChoiceListWidget',
+
+    ATTRS: {
+    }
+
+});
+
+Y.extend(ArchIndepChoiceListWidget, formwidgets.ChoiceListWidget, {
+
+    AUTOSELECT: '-',
+
+    initializer: function(config) {
+        this.client = new Y.lp.client.Launchpad();
+        this.error_handler = new Y.lp.client.ErrorHandler();
+        this.error_handler.clearProgressUI = Y.bind(this.hideSpinner, this);
+        this.error_handler.showError = Y.bind(this.showError, this);
+        this._distroseries = {};
+        this.set('multiple', false);
+        this.add_choices([{value:this.AUTOSELECT, text:'Auto select'}]);
+        this.default_select();
+
+        this.on(
+            this.name + ":removed_choices",
+            Y.rbind(this.default_select, this));
+    },
+
+
+    /**
+     * Make sure that one choice is selected.  If none is selected, then
+     * select the default choice ('Auto select').
+     *
+     * @method default_select
+     */
+    default_select: function() {
+        if (!Y.Lang.isValue(this.get('choice'))) {
+            this.set('choice', this.AUTOSELECT);
+        }
+    }
+
+});
+
+namespace.ArchIndepChoiceListWidget = ArchIndepChoiceListWidget;
+
+
+/**
  * A special form of ChoiceListWidget for choosing packagesets.
  *
  * @class PackagesetPickerWidget