launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03953
[Merge] lp:~rvb/launchpad/init-series-bug-789091-initseries into lp:launchpad
Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/init-series-bug-789091-initseries into lp:launchpad with lp:~rvb/launchpad/init-series-bug-789091-previous-series as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #789091 in Launchpad itself: "initialise_distroseries doesn't work with multiple parents"
https://bugs.launchpad.net/launchpad/+bug/789091
For more details, see:
https://code.launchpad.net/~rvb/launchpad/init-series-bug-789091-initseries/+merge/64567
This branch fixes the UI to support the second use-case for initializing series (+initseries).
- Use case #1 (is_first_derivation=True): if the distribution has no initialized series, the series to be initialized can be initialized from one or more parents and the +initseries page allows to choose the architectures and the packagesets to be copied from the parents
- Use case #2 (is_first_derivation=False): if the distribution already has an initialized series, the series parent should be by default populated with the previous_series's parents (but this can be changed, DistroSeriesDerivationVocabulary is the vocabulary of allowed parents), no packageset picker should be displayed (all the packagesets will be copied) and the possible architecture should be the ones from the previous_series.
Note: The Javascript file (lib/lp/registry/javascript/distroseries.js) has been rewritten a bit to ease the testing (basically I've split the widget creating and the wiring of the widgets together to be able to monkeypatch the interaction with the LP server).
= Tests =
./bin/test -cvv test_distroseries test_is_first_derivation
./bin/test -cvv test_distroseries test_not_is_first_derivation
lib/lp/registry/javascript/tests/test_distroseries.html
= Q/A =
Test the +initseries form for the 2 use cases.
In the second use case (an initialized series already exists), make sure that:
- the parent picker is pre-populated with the previous_series' parents
- the architecture list is from the previous_series
- the packageset picker is not displayed
- the derivation was made from the previous series
- the parents set (DSP) are the parent from the parent picker
--
https://code.launchpad.net/~rvb/launchpad/init-series-bug-789091-initseries/+merge/64567
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/init-series-bug-789091-initseries into lp:launchpad.
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-06-13 09:23:39 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-06-14 16:13:31 +0000
@@ -23,6 +23,7 @@
import apt_pkg
from lazr.restful.interface import copy_field
+from lazr.restful.interfaces import IJSONRequestCache
from zope.component import getUtility
from zope.event import notify
from zope.formlib import form
@@ -648,6 +649,29 @@
label = 'Initialize series'
page_title = label
+ def initialize(self):
+ super(DistroSeriesInitializeView, self).initialize()
+ cache = IJSONRequestCache(self.request).objects
+ distribution = self.context.distribution
+ is_first_derivation = not distribution.has_published_sources
+ cache['is_first_derivation'] = is_first_derivation
+ if not is_first_derivation:
+ def vocabularyValue(series):
+ # Format the series fields like the series vocabulary
+ # picker would do.
+ return {
+ 'value': series.id,
+ 'title': '%s: %s'
+ % (series.distribution.displayname, series.title),
+ 'api_uri': canonical_url(
+ series, path_only_if_possible=True)}
+
+ cache['previous_series'] = vocabularyValue(
+ self.context.previous_series)
+ previous_parents = self.context.previous_series.getParentSeries()
+ cache['previous_parents'] = [
+ vocabularyValue(series) for series in previous_parents]
+
@action(u"Initialize Series", name='initialize')
def submit(self, action, data):
"""Stub for the Javascript in the page to use."""
@@ -680,13 +704,6 @@
self.context.isInitializing())
@property
- def rebuilding_allowed(self):
- """If the distribution has got any initialized series already,
- rebuilding is not allowed.
- """
- return not self.context.distribution.has_published_sources
-
- @property
def next_url(self):
return canonical_url(self.context)
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-06-14 08:26:34 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-06-14 16:13:31 +0000
@@ -10,6 +10,7 @@
from textwrap import TextWrapper
from BeautifulSoup import BeautifulSoup
+from lazr.restful.interfaces import IJSONRequestCache
from lxml import html
import soupmatchers
from storm.zope.interfaces import IResultSet
@@ -533,23 +534,64 @@
u"javascript-disabled",
message.get("class").split())
- def test_rebuilding_allowed(self):
- # If the distro has no initialized series, rebuilding is allowed.
+ def test_is_first_derivation(self):
+ # If the distro has no initialized series, this initialization
+ # is a 'first_derivation'.
distroseries = self.factory.makeDistroSeries()
self.factory.makeDistroSeries(
distribution=distroseries.distribution)
view = create_initialized_view(distroseries, "+initseries")
- self.assertTrue(view.rebuilding_allowed)
-
- def test_rebuilding_not_allowed(self):
- # If the distro has an initialized series, no rebuilding is allowed.
- distroseries = self.factory.makeDistroSeries()
+ cache = IJSONRequestCache(view.request).objects
+
+ self.assertTrue(cache['is_first_derivation'])
+
+ def assetVocFormatedSeries(self, series, formatted):
+ # Helper to assert that the formatted dict is of the form:
+ # {'value':series_id, 'api_uri': api_uri, 'title': series_title}.
+ # (i.e. the format returned by a js vocabulary picker)
+ self.assertEqual(
+ {'value': series.id,
+ 'api_uri': '/%s/%s'
+ % (series.distribution.name,
+ series.name),
+ 'title': '%s: %s'
+ % (series.distribution.displayname,
+ series.title)},
+ formatted
+ )
+
+ def test_not_is_first_derivation(self):
+ # If the distro has an initialized series, this initialization
+ # is not a 'first_derivation'. The previous_series and the
+ # previous_series' parents are in LP.cache to be used by
+ # Javascript on the +initseries page.
+ previous_series = self.factory.makeDistroSeries()
+ previous_parent1 = self.factory.makeDistroSeriesParent(
+ derived_series=previous_series).parent_series
+ previous_parent2 = self.factory.makeDistroSeriesParent(
+ derived_series=previous_series).parent_series
+ distroseries = self.factory.makeDistroSeries(
+ previous_series=previous_series)
another_distroseries = self.factory.makeDistroSeries(
distribution=distroseries.distribution)
self.factory.makeSourcePackagePublishingHistory(
distroseries=another_distroseries)
view = create_initialized_view(distroseries, "+initseries")
- self.assertFalse(view.rebuilding_allowed)
+ cache = IJSONRequestCache(view.request).objects
+
+ self.assertFalse(cache['is_first_derivation'])
+ self.assetVocFormatedSeries(
+ previous_series,
+ cache['previous_series'])
+ self.assertEqual(
+ 2,
+ len(cache['previous_parents']))
+ self.assetVocFormatedSeries(
+ previous_parent1,
+ cache['previous_parents'][0])
+ self.assetVocFormatedSeries(
+ previous_parent2,
+ cache['previous_parents'][1])
def test_form_hidden_when_distroseries_is_initialized(self):
# The form is hidden when the feature flag is set but the series has
=== modified file 'lib/lp/registry/javascript/distroseries.js'
--- lib/lp/registry/javascript/distroseries.js 2011-06-10 15:54:48 +0000
+++ lib/lp/registry/javascript/distroseries.js 2011-06-14 16:13:31 +0000
@@ -362,7 +362,7 @@
/**
* Remove a parent series.
*
- * @method add_parent
+ * @method remove_parent
*/
remove_parent: function(parent_id) {
if (this.get('parents').length === 1) {
@@ -1126,7 +1126,7 @@
this.error_handler.clearProgressUI = Y.bind(this.hideSpinner, this);
this.error_handler.showError = Y.bind(this.showError, this);
this.clean_display();
- // _packagesets maps each distroseries' id to a collection of ids
+ // _packagesets maps each distroseries' id to a collection of ids
// of its packagesets.
// It's populated each time a new distroseries is added as a parent
// and used when a distroseries is removed to get all the
@@ -1238,6 +1238,7 @@
this.architectureChoice = config.architectureChoice;
this.packagesetChoice = config.packagesetChoice;
this.packageCopyOptions = config.packageCopyOptions;
+ this.form_container = config.form_container;
},
/**
@@ -1283,7 +1284,8 @@
distribution: this.context.distribution_link,
parents: this.deriveFromChoices.get("parents"),
architectures: this.architectureChoice.get("choice"),
- packagesets: this.packagesetChoice.get("choice"),
+ packagesets: this.packagesetChoice !== null ?
+ this.packagesetChoice.get("choice") : [],
rebuild: this.packageCopyOptions.get("choice") === (
"Copy Source and Rebuild"),
overlays: this.deriveFromChoices.get("overlays"),
@@ -1338,6 +1340,16 @@
* @function setup
*/
namespace.setup = function() {
+ var form_actions = namespace.setupWidgets();
+ namespace.setupInteraction(form_actions);
+};
+
+/**
+ * Setup the widgets objects and return the form object.
+ *
+ * @function setupWidgets
+ */
+namespace.setupWidgets = function() {
var form_container = Y.one("#initseries-form-container");
var form_table = form_container.one("table.form");
var form_table_body = form_table.append(Y.Node.create('<tbody />'));
@@ -1366,20 +1378,23 @@
"if you want to use all the available " +
"architectures)."))
.render(form_table_body);
- var packageset_choice =
- new PackagesetPickerWidget()
- .set("name", "field.packagesets")
- .set("size", 5)
- .set("help", {link: '/+help/init-series-packageset-help.html',
- text: 'Packagesets help'})
- .set("multiple", true)
- .set("label", "Package sets to copy from parent:")
- .set("description", (
- "The package sets that will be imported " +
- "into the derived distroseries (select none " +
- "if you want to import all the available " +
- "package sets)."))
- .render(form_table_body);
+ var packageset_choice = null;
+ if (LP.cache['is_first_derivation']) {
+ packageset_choice =
+ new PackagesetPickerWidget()
+ .set("name", "field.packagesets")
+ .set("size", 5)
+ .set("help", {link: '/+help/init-series-packageset-help.html',
+ text: 'Packagesets help'})
+ .set("multiple", true)
+ .set("label", "Package sets to copy from parent:")
+ .set("description", (
+ "The package sets that will be imported " +
+ "into the derived distroseries (select none " +
+ "if you want to import all the available " +
+ "package sets)."))
+ .render(form_table_body);
+ }
var package_copy_options =
new ChoiceListWidget()
.set("name", "field.package_copy_options")
@@ -1392,15 +1407,6 @@
"Copy Source and Binaries"])
.set("choice", "Copy Source and Binaries")
.render(form_table_body);
- // Disable rebuilding if LP.cache['rebuilding_allowed'] is false.
- if (!LP.cache['rebuilding_allowed']) {
- package_copy_options.fieldNode
- .one('input[value="Copy Source and Rebuild"]')
- .set('disabled', 'disabled');
- package_copy_options.set("description", (
- "Note that you cannot rebuild sources because the " +
- "distribution already has an initialized series."));
- }
var form_actions =
new DeriveDistroSeriesActionsWidget({
context: LP.cache.context,
@@ -1408,9 +1414,21 @@
deriveFromChoices: parents_selection,
architectureChoice: architecture_choice,
packagesetChoice: packageset_choice,
- packageCopyOptions: package_copy_options
+ packageCopyOptions: package_copy_options,
+ form_container: form_container
});
+ return form_actions;
+};
+
+/**
+ * Setup the interaction between the widgets.
+ *
+ * @function setupInteraction
+ * @param {DeriveDistroSeriesActionsWidget} The form widget containing all
+ * the other widgets.
+ */
+namespace.setupInteraction = function(form_actions) {
// Wire up the add parent series link.
var link = Y.one('#add-parent-series');
if (Y.Lang.isValue(link)) {
@@ -1419,29 +1437,56 @@
}
Y.on('add_parent', function(parent) {
- var added = parents_selection.add_parent(parent);
+ var added = form_actions.deriveFromChoices.add_parent(parent);
if (added) {
Y.fire("parent_added", parent);
}
});
- Y.on('parent_added', function(parent) {
- architecture_choice.add_distroseries(parent);
- packageset_choice.add_distroseries(parent);
- });
+ if (LP.cache['is_first_derivation']) {
+ // Wire the architecture and packageset pickers to the parent picker.
+ Y.on('parent_added', function(parent) {
+ form_actions.architectureChoice.add_distroseries(parent);
+ form_actions.packagesetChoice.add_distroseries(parent);
+ });
- Y.on('parent_removed', function(parent_id) {
- architecture_choice.remove_distroseries(parent_id);
- packageset_choice.remove_distroseries(parent_id);
- });
+ Y.on('parent_removed', function(parent_id) {
+ form_actions.architectureChoice.remove_distroseries(parent_id);
+ form_actions.packagesetChoice.remove_distroseries(parent_id);
+ });
+ }
+ else {
+ // Disable rebuilding if LP.cache['is_first_derivation'] is false.
+ form_actions.packageCopyOptions.fieldNode
+ .one('input[value="Copy Source and Rebuild"]')
+ .set('disabled', 'disabled');
+ form_actions.packageCopyOptions.set("description", (
+ "Note that you cannot rebuild sources because the " +
+ "distribution already has an initialized series."));
+ // The architectures are those from the previous_series.
+ form_actions.architectureChoice.set("description", (
+ "Choose the architectures you want to " +
+ "use from the previous series (or select none " +
+ "if you want to use all the available " +
+ "architectures)."));
+ form_actions.architectureChoice.add_distroseries(
+ LP.cache['previous_series']);
+ // Setup the pre-selected parents (parents from the previous series).
+ var index;
+ for (index in LP.cache['previous_parents']) {
+ form_actions.deriveFromChoices.add_parent(
+ LP.cache['previous_parents'][index]);
+ }
+ }
// Wire up the form submission.
- form_container.one("form").on(
+ form_actions.form_container.one("form").on(
"submit", function(event) {
event.halt(); form_actions.submit(); });
// Show the form.
- form_container.removeClass("unseen");
+ form_actions.form_container.removeClass("unseen");
+
};
=== modified file 'lib/lp/registry/javascript/tests/test_distroseries.js'
--- lib/lp/registry/javascript/tests/test_distroseries.js 2011-06-09 09:11:19 +0000
+++ lib/lp/registry/javascript/tests/test_distroseries.js 2011-06-14 16:13:31 +0000
@@ -1134,6 +1134,97 @@
testFormActionsWidget, testDeriveDistroSeriesActionsWidget);
suite.add(new Y.Test.Case(testDeriveDistroSeriesActionsWidget));
+ 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);
+ },
+
+ setUpFirstDerivation: function() {
+ window.LP = {cache: {is_first_derivation: true}};
+ },
+
+ setUpNotFirstDerivation: function() {
+ window.LP = {cache:
+ {is_first_derivation: false,
+ previous_series: {
+ api_uri: '/ubuntu/natty',
+ value: '3',
+ title: 'Ubunty: Natty'},
+ previous_parents: [
+ {api_uri: '/debian/sid',
+ value: '4',
+ title: 'Debian: Sid'},
+ {api_uri: '/zz/aa',
+ value: '5',
+ title: 'ZZ: aa'}]}};
+ },
+
+ testIsFirstDerivation: function() {
+ this.setUpFirstDerivation();
+ var form_actions = initseries.setupWidgets();
+ initseries.setupInteraction(form_actions);
+
+ // No pre-populated parent.
+ ArrayAssert.itemsAreEqual(
+ [],
+ form_actions.deriveFromChoices.get("parents"));
+ },
+
+ testIsNotFirstDerivation: function() {
+ this.setUpNotFirstDerivation();
+ var form_actions = initseries.setupWidgets();
+ // 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);
+ }
+ };
+ form_actions.architectureChoice.client = client;
+ initseries.setupInteraction(form_actions);
+
+ // Parents are populated.
+ ArrayAssert.itemsAreEqual(
+ ['4', '5'],
+ form_actions.deriveFromChoices.get("parents"));
+ // No packageset choice widget.
+ Assert.isNull(form_actions.packagesetChoice);
+ // The architecture picker features the architectures
+ // from the previous series.
+ ArrayAssert.itemsAreEqual(
+ ['hppa', 'i386'],
+ form_actions.architectureChoice.get("choices"));
+ }
+ };
+
+ suite.add(new Y.Test.Case(testDeriveDistroSeriesSetup));
+
// Lock, stock, and two smoking barrels.
var handle_complete = function(data) {
status_node = Y.Node.create(
=== modified file 'lib/lp/registry/templates/distroseries-initialize.pt'
--- lib/lp/registry/templates/distroseries-initialize.pt 2011-06-09 13:11:57 +0000
+++ lib/lp/registry/templates/distroseries-initialize.pt 2011-06-14 16:13:31 +0000
@@ -26,9 +26,6 @@
please use the <code>initDerivedDistroSeries</code> API call via the
web service.
</p>
- <script
- tal:content="string:LP.cache['rebuilding_allowed'] = ('${view/rebuilding_allowed}' === 'True');">
- </script>
<div class="unseen" id="initseries-form-container">
<metal:form use-macro="context/@@launchpad_form/form" />
</div>
Follow ups
