launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03229
[Merge] lp:~abentley/launchpad/js-usage into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/js-usage into lp:launchpad with lp:~abentley/launchpad/js-translation-2 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/js-usage/+merge/56655
= Summary =
Ajaxify setting translation usage.
== Proposed fix ==
Since the form used for updating translations is complicated, submit the form
to the appserver instead of using the web service.
== Pre-implementation notes ==
Discussed with deryck.
== Implementation details ==
Extracted update_form from replace_productseries.
Extracted form_url from replace_productseries.
Implemented usage_overlay.
Implemented submit_form to submit the form data from the usage overlay.
Implemented FormErrorHandler as a subclass of ErrorHandler to handle web page
form submission errors instead of web service errors.
Allow error handler to be specified for IOHandler.
Update tests to use the *visible* overlay, not the first overlay.
== Tests ==
bin/test --layer=WindmillLayer sharing_details
firefox lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html
== Demo and Q/A ==
Go to a +sharing-details page and click the edit icon next to "Translations
(are/are not) enabled on the upstream project."
A green checkmark should be shown when "Type of service for translations
application" is "External" or"Launchpad". The other values should be settable,
but have no impact on the main page.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/testing/windmill/widgets.py
lib/lp/app/javascript/lp.js
lib/lp/translations/templates/sourcepackage-sharing-details.pt
lib/lp/app/javascript/client.js
lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js
lib/lp/app/javascript/picker.js
lib/lp/translations/browser/tests/test_sharing_details.py
lib/lp/translations/windmill/tests/test_sourcepackage_sharing_details.py
lib/lp/translations/javascript/sourcepackage_sharing_details.js
lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html
lib/lp/translations/browser/sourcepackage.py
./lib/lp/testing/windmill/widgets.py
29: E501 line too long (80 characters)
35: E501 line too long (80 characters)
36: E501 line too long (81 characters)
95: E501 line too long (83 characters)
122: E301 expected 1 blank line, found 0
27: Line exceeds 78 characters.
29: Line exceeds 78 characters.
35: Line exceeds 78 characters.
36: Line exceeds 78 characters.
43: Line exceeds 78 characters.
95: Line exceeds 78 characters.
./lib/lp/app/javascript/lp.js
53: Expected '!==' and instead saw '!='.
117: Expected '!==' and instead saw '!='.
187: Expected '===' and instead saw '=='.
290: Expected '===' and instead saw '=='.
305: Expected '===' and instead saw '=='.
306: Expected '===' and instead saw '=='.
338: Move 'var' declarations to the top of the function.
338: Stopping. (76% scanned).
0: JSLINT had a fatal error.
./lib/lp/app/javascript/client.js
30: Expected '===' and instead saw '=='.
33: Expected '===' and instead saw '=='.
54: Move 'var' declarations to the top of the function.
54: Stopping. (5% scanned).
0: JSLINT had a fatal error.
4: Line exceeds 78 characters.
191: Line exceeds 78 characters.
570: Line exceeds 78 characters.
618: Line exceeds 78 characters.
745: Line exceeds 78 characters.
./lib/lp/app/javascript/picker.js
43: Expected ';' and instead saw 'if'.
92: Expected '!==' and instead saw '!='.
223: Expected '!==' and instead saw '!='.
224: Expected '{' and instead saw 'temp_spinner'.
257: Expected '!==' and instead saw '!='.
270: 'header' used out of scope.
271: 'step_title' used out of scope.
319: Move 'var' declarations to the top of the function.
319: Stopping. (85% scanned).
0: JSLINT had a fatal error.
194: Line exceeds 78 characters.
--
https://code.launchpad.net/~abentley/launchpad/js-usage/+merge/56655
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/js-usage into lp:launchpad.
=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js 2011-04-06 20:58:54 +0000
+++ lib/lp/app/javascript/client.js 2011-04-06 20:58:54 +0000
@@ -743,22 +743,59 @@
// If it was a server error...
} else if (o.status >= 500) {
var server_error = 'Server error, please contact an administrator.';
- if (o.getResponseHeader('X-Lazr-OopsId')) {
- server_error = server_error + ' OOPS ID:' + o.getResponseHeader('X-Lazr-OOPSid');
+ var oops_id = self.get_oops_id(o);
+ if (oops_id) {
+ server_error = server_error + ' OOPS ID:' + oops_id;
}
self.showError(server_error);
// Otherwise we send some sane text as an error
} else if (o.status == 412){
self.showError(o.status + ' ' + o.statusText);
} else {
- self.showError(o.responseText);
+ self.showError(self.get_generic_error(o));
}
};
+ },
+ get_oops_id: function(response){
+ return response.getResponseHeader('X-Lazr-OopsId');
+ },
+ get_generic_error: function(response){
+ return response.responseText;
}
};
module.ErrorHandler = ErrorHandler;
+
+var FormErrorHandler = function(){};
+
+
+FormErrorHandler.prototype = new ErrorHandler();
+
+
+FormErrorHandler.prototype.get_oops_id = function(response){
+ var oops_re = /code class\="oopsid">(OOPS-[^<]*)/;
+ var result = response.responseText.match(oops_re);
+ if (result === null) {
+ return null;
+ }
+ return result[1];
+}
+
+
+FormErrorHandler.prototype.get_generic_error = function(response){
+ if (response.status !== 403){
+ return "Sorry, you don't have permission to make this change.";
+ }
+ else {
+ return response.status + ' ' + response.statusText;
+ }
+}
+
+
+module.FormErrorHandler = FormErrorHandler;
+
+
}, "0.1" ,{"requires":["attribute", "io", "json-parse", "json-stringify"]});
=== modified file 'lib/lp/translations/javascript/sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/sourcepackage_sharing_details.js 2011-04-06 20:58:54 +0000
+++ lib/lp/translations/javascript/sourcepackage_sharing_details.js 2011-04-06 20:58:54 +0000
@@ -140,10 +140,70 @@
namespace.TranslationSharingConfig = TranslationSharingConfig;
-function IOHandler(flash_target){
+function form_url(entry, view_name){
+ entry_url = Y.lp.get_url_path(entry.get('web_link'));
+ return entry_url + '/' + view_name + '/++form++';
+}
+
+
+function update_form(overlay, entry, view_name){
+ var url = form_url(entry, view_name);
+ overlay.loadFormContentAndRender(url);
+ overlay.render();
+}
+
+
+function submit_form(config, form_data, entry, view_name, action){
+ var form_data_entries = [];
+ var key;
+ for (key in form_data){
+ if (form_data.hasOwnProperty(key)){
+ encoded_key = encodeURIComponent(key);
+ values = form_data[key];
+ for (i=0; i < values.length; i++){
+ form_entry = (
+ encoded_key + '=' + encodeURIComponent(values[i]));
+ form_data_entries.push(form_entry);
+ }
+ }
+ }
+ form_data_entries.push('field.actions.' + action + '=ignored');
+ var data = form_data_entries.join('&');
+ var url = form_url(entry, view_name);
+ config.method = 'POST';
+ config.data = data;
+ Y.io(url, config);
+}
+
+
+function add_activator(picker, selector){
+ var element = Y.one(selector);
+ element.on('click', function(e) {
+ e.halt();
+ this.show();
+ }, picker);
+ element.addClass(picker.get('picker_activator_css_class'));
+}
+
+
+function enum_title(form_data, name, map){
+ var key = form_data[name][0];
+ Y.log(key);
+ var title = map[key.toLowerCase()];
+ Y.log(title);
+ return title;
+}
+
+
+function IOHandler(flash_target, error_handler){
that = this;
this.flash_target = flash_target;
- this.error_handler = new Y.lp.client.ErrorHandler();
+ if (!Y.Lang.isValue(error_handler)){
+ this.error_handler = new Y.lp.client.ErrorHandler();
+ }
+ else {
+ this.error_handler = error_handler;
+ }
this.error_handler.showError = function(error_msg) {
Y.lp.app.errors.display_error(Y.one(that.flash_target), error_msg);
};
@@ -201,6 +261,7 @@
initializer: function(source_package){
this.set('tsconfig', new TranslationSharingConfig());
this.set('productseries', null);
+ this.set('product', null);
this.set('branch', null);
this.set('source_package', null);
this.set('branch_picker_config', null);
@@ -211,10 +272,12 @@
* @param branch_summary {Object} An object containing api_url, css,
* description, value, title
*/
- configure: function(config, branch_picker_config, import_overlay){
+ configure: function(config, branch_picker_config, import_overlay,
+ usage_overlay){
this.set('branch_picker_config', branch_picker_config);
this.set('source_package', config.context);
this.set('import_overlay', import_overlay);
+ this.set('usage_overlay', usage_overlay);
this.replace_productseries(config.productseries);
this.replace_product(config.product);
this.set_branch(config.upstream_branch);
@@ -232,20 +295,24 @@
if (!Y.Lang.isNull(productseries)){
autoimport_mode = productseries.get(
'translations_autoimport_mode');
- var import_url = productseries.get('web_link');
- import_url = Y.lp.get_url_path(import_url);
- import_url += '/+translations-settings/++form++';
var import_overlay = this.get('import_overlay');
- import_overlay.loadFormContentAndRender(import_url);
- import_overlay.render();
+ update_form(
+ import_overlay, productseries, '+translations-settings');
}
this.set_autoimport_mode(autoimport_mode);
},
+ set_product: function(product){
+ this.set('product', product);
+ this.get('branch_picker_config').context = product;
+ },
replace_product: function(product){
- this.get('branch_picker_config').context = product;
+ this.set_product(product);
var translations_usage = namespace.usage.unknown;
if (!Y.Lang.isNull(product)){
translations_usage = product.get('translations_usage');
+ var usage_overlay = this.get('usage_overlay');
+ update_form(
+ usage_overlay, product, '+configure-translations');
}
this.set_translations_usage(translations_usage);
},
@@ -366,6 +433,10 @@
picker_selector: function(check, complete){
return this.check_selector(check, complete) + '-picker a';
},
+ set_check_picker: function(check, picker){
+ add_activator(picker, this.picker_selector(check, true));
+ add_activator(picker, this.picker_selector(check, false));
+ },
/**
* Update the display of a single checklist item.
*/
@@ -415,16 +486,7 @@
};
var picker = Y.lp.app.picker.create(
'BranchRestrictedOnProduct', branch_picker_config);
- function add_activator(picker, selector){
- var element = Y.one(selector);
- /* Copy-pasted because picker can't normally be activated by two
- * different elements. */
- element.on('click', function(e) {
- e.halt();
- this.show();
- }, picker);
- element.addClass(picker.get('picker_activator_css_class'));
- }
+ /* Picker can't normally be activated by two different elements. */
add_activator(picker, '#branch-complete-picker a');
var productseries_picker_config = {
picker_activator: '#packaging-complete-picker a',
@@ -435,6 +497,7 @@
};
var productseries_picker = Y.lp.app.picker.create(
'ProductSeries', productseries_picker_config);
+ /* Picker can't normally be activated by two different elements. */
add_activator(productseries_picker, '#packaging-incomplete-picker a');
var import_overlay = new Y.lazr.FormOverlay({
headerContent: '<h2>Import settings<h2>',
@@ -443,10 +506,9 @@
});
import_overlay.set('form_submit_callback', function(form_data){
Y.log(form_data['field.translations_autoimport_mode']);
- var key = form_data['field.translations_autoimport_mode'][0];
- Y.log(key);
- var mode = namespace.autoimport_modes[key.toLowerCase()];
- Y.log(mode);
+ mode = enum_title(
+ form_data, 'field.translations_autoimport_mode',
+ namespace.autoimport_modes);
import_overlay.hide();
var product_series = sharing_controller.get('productseries');
product_series.set('translations_autoimport_mode', mode);
@@ -469,9 +531,35 @@
product_series.removeAttr('http_etag');
product_series.lp_save(handler.chain_config(update_controller));
});
- add_activator(import_overlay, '#upstream-sync-complete-picker a');
- add_activator(import_overlay, '#upstream-sync-incomplete-picker a');
- sharing_controller.configure(cache, branch_picker_config, import_overlay);
+ var autoimport = sharing_controller.get('tsconfig').get('autoimport');
+ sharing_controller.set_check_picker(autoimport, import_overlay);
+ var usage_overlay = new Y.lazr.FormOverlay({
+ headerContent: '<h2>Usage settings<h2>',
+ centered: true,
+ visible: false
+ });
+ var usage = sharing_controller.get('tsconfig').get('translations_usage');
+ usage_overlay.set('form_submit_callback', function(form_data){
+ usage_overlay.hide();
+ var product = sharing_controller.get('product');
+ function get_product(config){
+ lp_client.get(product.get('self_link'), config);
+ }
+ function replace_product(new_product){
+ sharing_controller.replace_product(new_product);
+ sharing_controller.update();
+ sharing_controller.flash_check_green(usage);
+ }
+ css_selector = sharing_controller.visible_check_selector(usage);
+ var io_handler = new IOHandler(
+ css_selector, new Y.lp.client.FormErrorHandler());
+ var config = io_handler.chain_config(get_product, replace_product);
+ submit_form(
+ config, form_data, product, '+configure-translations', 'change');
+ });
+ sharing_controller.set_check_picker(usage, usage_overlay);
+ sharing_controller.configure(
+ cache, branch_picker_config, import_overlay, usage_overlay);
sharing_controller.update();
};
}, "0.1", {"requires": [
=== modified file 'lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js 2011-04-06 20:58:54 +0000
+++ lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js 2011-04-06 20:58:54 +0000
@@ -117,7 +117,8 @@
loadFormContentAndRender: function(){},
render: function(){}
};
- ctrl.configure(cache, {}, import_overlay);
+ usage_overlay = import_overlay;
+ ctrl.configure(cache, {}, import_overlay, usage_overlay);
var tsconfig = ctrl.get('tsconfig');
Y.Assert.areEqual(
tsconfig.get('product_series').get('text'), 'title1');
=== modified file 'lib/lp/translations/templates/sourcepackage-sharing-details.pt'
--- lib/lp/translations/templates/sourcepackage-sharing-details.pt 2011-04-06 20:58:54 +0000
+++ lib/lp/translations/templates/sourcepackage-sharing-details.pt 2011-04-06 20:58:54 +0000
@@ -72,8 +72,10 @@
</li>
<li tal:attributes="class view/translations_disabled_class"
id="translation-incomplete">
- Translations are not enabled on the upstream project.
- <a tal:replace="structure view/configure_translations_link_unconfigured/escapedtext" />
+ Translations are not enabled on the upstream project.
+ <span id="translation-incomplete-picker">
+ <a tal:replace="structure view/configure_translations_link_unconfigured/escapedtext" />
+ </span>
</li>
<li tal:attributes="class view/translations_enabled_class"
id="translation-complete">
=== modified file 'lib/lp/translations/windmill/tests/test_sourcepackage_sharing_details.py'
--- lib/lp/translations/windmill/tests/test_sourcepackage_sharing_details.py 2011-04-06 20:58:54 +0000
+++ lib/lp/translations/windmill/tests/test_sourcepackage_sharing_details.py 2011-04-06 20:58:54 +0000
@@ -9,6 +9,9 @@
import transaction
+from lp.app.enums import (
+ ServiceUsage,
+ )
from lp.testing import (
feature_flags,
set_feature_flag,
@@ -21,6 +24,7 @@
FOR_ELEMENT,
)
from lp.testing.windmill.widgets import (
+ OnPageWidget,
search_and_select_picker_widget,
)
from lp.translations.interfaces.translations import (
@@ -49,19 +53,28 @@
client.waits.forElement(
id='branch-incomplete', timeout=FOR_ELEMENT)
client.click(xpath='//*[@id="packaging-incomplete-picker"]/a')
- search_and_select_picker_widget(self.client, 'my-ps-name', 1)
+ search_and_select_picker_widget(client, 'my-ps-name', 1)
client.waits.forElementProperty(
id='packaging-incomplete', option='className|sprite no unseen',
timeout=FOR_ELEMENT)
client.click(xpath='//*[@id="branch-incomplete-picker"]/a')
- search_and_select_picker_widget(self.client, 'product-branch', 1)
+ search_and_select_picker_widget(client, 'product-branch', 1)
client.waits.forElementProperty(
xpath='//*[@id="upstream-sync-incomplete-picker"]/a',
option='className|sprite edit', timeout=FOR_ELEMENT)
+ overlay = OnPageWidget(client, 'yui3-lazr-formoverlay')
+ client.click(xpath='//*[@id="translation-incomplete-picker"]/a')
+ client.click(id='field.translations_usage.1')
+ client.click(
+ xpath=overlay.visible_xpath + '//input[@value="Submit"]')
+ client.waits.forElementProperty(
+ id='translation-incomplete', option='className|sprite no unseen',
+ timeout=FOR_ELEMENT)
client.click(
xpath='//*[@id="upstream-sync-incomplete-picker"]/a')
client.click(id='field.translations_autoimport_mode.2')
- client.click(xpath='//input[@value="Submit"]')
+ client.click(
+ xpath=overlay.visible_xpath + '//input[@value="Submit"]')
client.waits.forElementProperty(
id='upstream-sync-incomplete',
option='className|sprite no unseen', timeout=FOR_ELEMENT)
@@ -69,5 +82,7 @@
self.assertEqual(sourcepackage.productseries, productseries)
self.assertEqual(branch, productseries.branch)
self.assertEqual(
+ ServiceUsage.LAUNCHPAD, productseries.product.translations_usage)
+ self.assertEqual(
TranslationsBranchImportMode.IMPORT_TRANSLATIONS,
productseries.translations_autoimport_mode)
