launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/init-multiple-parents-packagesetpicker into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #793603 in Launchpad itself: "The packageset picker on +initseries is not properly cleaned up when a parent is removed for the parent picker."
  https://bugs.launchpad.net/launchpad/+bug/793603

For more details, see:
https://code.launchpad.net/~rvb/launchpad/init-multiple-parents-packagesetpicker/+merge/63775

This branch fixes the packagesets widget on the +initseries page. The packagesets from a parent where not removed properly when the parent was removed from the parent picker widget.

= Q/A =

On DF:
- Create a new series
- Go to the initialisation page series/+initseries
- Add a parent with packagesets
- Make sure the parent's packagesets are added to the packagesets widget
- Remove the parent
- Make sure the packagesets from this parent are properly removed from the packagesets widget

= Tests =

lib/lp/registry/javascript/tests/test_distroseries.html
(testRemoveDistroSeriesUpdatesPackageSets)
-- 
https://code.launchpad.net/~rvb/launchpad/init-multiple-parents-packagesetpicker/+merge/63775
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/init-multiple-parents-packagesetpicker into lp:launchpad.

=== modified file 'lib/lp/registry/javascript/distroseries.js'
--- lib/lp/registry/javascript/distroseries.js	2011-06-01 10:31:05 +0000
+++ lib/lp/registry/javascript/distroseries.js	2011-06-07 21:48:32 +0000
@@ -1050,8 +1050,8 @@
         var select = this.init_select();
         var option = Y.Node.create("<option />");
         option.set("value", choice.value)
-                .set("text", choice.text)
-                .setData("data", choice.data);
+            .set("text", choice.text)
+            .setData("data", choice.data);
         var options = select.all('option');
         if (options.isEmpty()) {
             select.append(option);
@@ -1073,6 +1073,7 @@
      * @method add_packagesets
      */
     add_packagesets: function(packagesets, distroseries) {
+        this._packagesets[distroseries.value] = packagesets.entries;
         packagesets.entries.forEach(
             function(packageset) {
                 var value = packageset.get("id");
@@ -1095,9 +1096,12 @@
      * @method remove_distroseries
      */
     remove_distroseries: function(distroseries_id) {
-        this.fieldNode.all(
-            'option[value$="-' + distroseries_id + '"]').remove();
-        Y.lazr.anim.green_flash({node: this.fieldNode}).run();
+        this._packagesets[distroseries_id].forEach(
+            function(packageset) {
+                this.fieldNode.all(
+                    'option[value$="' + packageset.get("id") + '"]').remove();
+            }, this);
+       Y.lazr.anim.green_flash({node: this.fieldNode}).run();
         if (this.fieldNode.all('option').isEmpty()) {
             this.clean_display();
         }
@@ -1109,6 +1113,7 @@
         this.error_handler.clearProgressUI = Y.bind(this.hideSpinner, this);
         this.error_handler.showError = Y.bind(this.showError, this);
         this.clean_display();
+        this._packagesets = {};
     }
 
 });

=== modified file 'lib/lp/registry/javascript/tests/test_distroseries.js'
--- lib/lp/registry/javascript/tests/test_distroseries.js	2011-06-01 09:17:43 +0000
+++ lib/lp/registry/javascript/tests/test_distroseries.js	2011-06-07 21:48:32 +0000
@@ -854,6 +854,48 @@
                 attrselect("value")(this.widget.get("choices")));
         },
 
+        testRemoveDistroSeriesUpdatesPackageSets: function() {
+            var package_sets1 = [
+                {id: "4", name: "foo", description: "Foo"},
+                {id: "5", name: "bar", description: "Bar"}
+            ];
+            var package_sets2 = [
+                {id: "6", name: "baz", description: "Baz"}
+            ];
+            var package_sets_collection1 =
+                new Y.lp.client.Collection(
+                    null, {entries: package_sets1}, null);
+            var package_sets_collection2 =
+                new Y.lp.client.Collection(
+                    null, {entries: package_sets2}, null);
+            this.widget.client = {
+                named_get: function(path, operation, config) {
+                    var name = config.parameters.distroseries.substr('-5');
+                    if (name === 'hoary') {
+                        config.on.success(package_sets_collection1);
+                    }
+                    else {
+                        config.on.success(package_sets_collection2);
+                    }
+                }
+            };
+            var distroseries1 = {
+                id:'4', value: "4", title: "series1",
+                api_uri: "ubuntu/hoary"};
+            var distroseries2 = {
+                id:'5', value: "5", title: "series2",
+                api_uri: "ubuntu/breezy"};
+            this.widget.add_distroseries(distroseries1);
+            this.widget.add_distroseries(distroseries2);
+            ArrayAssert.itemsAreEqual(
+                ["5", "6", "4"],
+                attrselect("value")(this.widget.get("choices")));
+            this.widget.remove_distroseries("5");
+            ArrayAssert.itemsAreEqual(
+                ["5", "4"],
+                attrselect("value")(this.widget.get("choices")));
+        },
+
         testSetDistroSeriesSpinner: function() {
             var widget = this.widget;
             widget.client = {