From ca95467befbd2ad6d940f8d9dfc4f9e2e14fcd5d Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 21 Nov 2011 13:04:13 +0100 Subject: [PATCH 1/6] [IMP] extract parsing of onchange specs into a separate method bzr revid: xmo@openerp.com-20111121120413-xw2aanwbte79y8cr --- addons/web/static/src/js/view_form.js | 130 ++++++++++++++------------ 1 file changed, 72 insertions(+), 58 deletions(-) diff --git a/addons/web/static/src/js/view_form.js b/addons/web/static/src/js/view_form.js index d833b7471c6..26dc2c03572 100644 --- a/addons/web/static/src/js/view_form.js +++ b/addons/web/static/src/js/view_form.js @@ -242,69 +242,83 @@ openerp.web.FormView = openerp.web.View.extend( /** @lends openerp.web.FormView# $pager.find('span.oe_pager_index').html(index); $pager.find('span.oe_pager_count').html(this.dataset.ids.length); }, + parse_on_change: function (on_change, widget) { + var self = this; + var onchange = _.str.trim(on_change); + var call = onchange.match(/^\s?(.*?)\((.*?)\)\s?$/); + if (!call) { + return null; + } + + var method = call[1], args = []; + var context_index = null; + var argument_replacement = { + 'False': function () {return false;}, + 'True': function () {return true;}, + 'None': function () {return null;}, + 'context': function (i) { + context_index = i; + var ctx = widget.build_context ? widget.build_context() : {}; + return ctx; + } + }; + var parent_fields = null; + _.each(call[2].split(','), function (a, i) { + var field = _.str.trim(a); + if (field in argument_replacement) { + args.push(argument_replacement[field](i)); + return; + } else if (self.fields[field]) { + var value = self.fields[field].get_on_change_value(); + args.push(value == null ? false : value); + return; + } else { + var splitted = field.split('.'); + if (splitted.length > 1 && _.str.trim(splitted[0]) === "parent" && self.dataset.parent_view) { + if (parent_fields === null) { + parent_fields = self.dataset.parent_view.get_fields_values(); + } + var p_val = parent_fields[_.str.trim(splitted[1])]; + if (p_val !== undefined) { + args.push(p_val == null ? false : p_val); + return; + } + } + } + throw "Could not get field with name '" + field + "' for onchange '" + onchange + "'"; + }); + + return { + method: method, + args: args, + context_index: context_index + }; + }, do_onchange: function(widget, processed) { var self = this; var act = function() { try { - processed = processed || []; - if (widget.node.attrs.on_change) { - var onchange = _.str.trim(widget.node.attrs.on_change); - var call = onchange.match(/^\s?(.*?)\((.*?)\)\s?$/); - if (call) { - var method = call[1], args = []; - var context_index = null; - var argument_replacement = { - 'False' : function() {return false;}, - 'True' : function() {return true;}, - 'None' : function() {return null;}, - 'context': function(i) { - context_index = i; - var ctx = widget.build_context ? widget.build_context() : {}; - return ctx; - } - }; - var parent_fields = null; - _.each(call[2].split(','), function(a, i) { - var field = _.str.trim(a); - if (field in argument_replacement) { - args.push(argument_replacement[field](i)); - return; - } else if (self.fields[field]) { - var value = self.fields[field].get_on_change_value(); - args.push(value == null ? false : value); - return; - } else { - var splitted = field.split('.'); - if (splitted.length > 1 && _.str.trim(splitted[0]) === "parent" && self.dataset.parent_view) { - if (parent_fields === null) { - parent_fields = self.dataset.parent_view.get_fields_values(); - } - var p_val = parent_fields[_.str.trim(splitted[1])]; - if (p_val !== undefined) { - args.push(p_val == null ? false : p_val); - return; - } - } - } - throw "Could not get field with name '" + field + - "' for onchange '" + onchange + "'"; - }); - var ajax = { - url: '/web/dataset/call', - async: false - }; - return self.rpc(ajax, { - model: self.dataset.model, - method: method, - args: [(self.datarecord.id == null ? [] : [self.datarecord.id])].concat(args), - context_id: context_index === null ? null : context_index + 1 - }).pipe(function(response) { - return self.on_processed_onchange(response, processed); - }); - } else { - console.warn("Wrong on_change format", on_change); + processed = processed || []; + var on_change = widget.node.attrs.on_change; + if (on_change) { + var change_spec = self.parse_on_change(on_change, widget); + if (change_spec) { + var ajax = { + url: '/web/dataset/call', + async: false + }; + return self.rpc(ajax, { + model: self.dataset.model, + method: change_spec.method, + args: [(self.datarecord.id == null ? [] : [self.datarecord.id])].concat(change_spec.args), + context_id: change_spec.context_index == undefined ? null : change_spec.context_index + 1 + }).pipe(function(response) { + return self.on_processed_onchange(response, processed); + }); + } else { + console.warn("Wrong on_change format", on_change); + } } - } } catch(e) { console.error(e); return $.Deferred().reject(); From e3a84ed7b6c08c79c94e96446a4e2497c4e15daa Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 21 Nov 2011 13:05:09 +0100 Subject: [PATCH 2/6] [IMP] throw actual error objects to improve debugging experience bzr revid: xmo@openerp.com-20111121120509-fa0rqj50w3zombs1 --- addons/web/static/src/js/view_form.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/addons/web/static/src/js/view_form.js b/addons/web/static/src/js/view_form.js index 26dc2c03572..1bb43a0e4b8 100644 --- a/addons/web/static/src/js/view_form.js +++ b/addons/web/static/src/js/view_form.js @@ -285,7 +285,8 @@ openerp.web.FormView = openerp.web.View.extend( /** @lends openerp.web.FormView# } } } - throw "Could not get field with name '" + field + "' for onchange '" + onchange + "'"; + throw new Error("Could not get field with name '" + field + + "' for onchange '" + onchange + "'"); }); return { From 95e2fcf8845f39a15b366135b2a12c42580aa9f2 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 21 Nov 2011 13:26:22 +0100 Subject: [PATCH 3/6] [ADD] sanity tests for basic checking of onchange parsing bzr revid: xmo@openerp.com-20111121122622-g6jzo74ctgc6r1hk --- addons/web/static/test/onchange.js | 58 ++++++++++++++++++++++++++++++ addons/web/static/test/test.html | 1 + 2 files changed, 59 insertions(+) create mode 100644 addons/web/static/test/onchange.js diff --git a/addons/web/static/test/onchange.js b/addons/web/static/test/onchange.js new file mode 100644 index 00000000000..07eb6c1bc2c --- /dev/null +++ b/addons/web/static/test/onchange.js @@ -0,0 +1,58 @@ +$(document).ready(function () { + var openerp, + make_form = function (default_values) { + var fields = {}; + _(default_values).each(function (value, name) { + fields[name] = value instanceof Function ? value : { + get_on_change_value: function () { return value; } + }; + }); + return _.extend(new openerp.web.FormView(null, {}), + {fields: fields}); + }; + module("form.onchange", { + setup: function () { + openerp = window.openerp.init(true); + window.openerp.web.core(openerp); + window.openerp.web.chrome(openerp); + // views loader stuff + window.openerp.web.data(openerp); + window.openerp.web.views(openerp); + window.openerp.web.list(openerp); + window.openerp.web.form(openerp); + } + }); +// test('Parse args-less onchange', function () { +// var f = new openerp.web.FormView(null, {}); +// var result = f.parse_on_change('on_change_foo()', {}); +// equal(result.method, 'on_change_foo'); +// deepEqual(result.args, []); +// }); + test('Parse 1-arg onchange', function () { + var f = make_form({foo: 3}); + var result = f.parse_on_change('on_change_foo(foo)', {}); + equal(result.method, 'on_change_foo'); + deepEqual(result.args, [3]); + }); + test('Parse 2-args onchange', function () { + var f = make_form({foo: 3, bar: 'qux'}); + var result = f.parse_on_change('on_change_foo(bar, foo)', {}); + equal(result.method, 'on_change_foo'); + deepEqual(result.args, ['qux', 3]); + }); + test('Literal null', function () { + var f = make_form(); + var result = f.parse_on_change('on_null(None)', {}); + deepEqual(result.args, [null]); + }); + test('Literal true', function () { + var f = make_form(); + var result = f.parse_on_change('on_null(True)', {}); + deepEqual(result.args, [true]); + }); + test('Literal false', function () { + var f = make_form(); + var result = f.parse_on_change('on_null(False)', {}); + deepEqual(result.args, [false]); + }); +}); diff --git a/addons/web/static/test/test.html b/addons/web/static/test/test.html index d182ce18707..9bbfff168fd 100644 --- a/addons/web/static/test/test.html +++ b/addons/web/static/test/test.html @@ -49,4 +49,5 @@ + From 9d09c570aefc8c96a026a4316937c7a42ff6cf3d Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 21 Nov 2011 13:28:45 +0100 Subject: [PATCH 4/6] [ADD] support for arguments-less onchange (why the hell not) bzr revid: xmo@openerp.com-20111121122845-98lctm470ncttz74 --- addons/web/static/src/js/view_form.js | 7 +++++-- addons/web/static/test/onchange.js | 12 ++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/addons/web/static/src/js/view_form.js b/addons/web/static/src/js/view_form.js index 1bb43a0e4b8..1f35c0e1dc3 100644 --- a/addons/web/static/src/js/view_form.js +++ b/addons/web/static/src/js/view_form.js @@ -250,8 +250,11 @@ openerp.web.FormView = openerp.web.View.extend( /** @lends openerp.web.FormView# return null; } - var method = call[1], args = []; - var context_index = null; + var method = call[1], args = [], context_index = null; + if (!_.str.trim(call[2])) { + return {method: method, args: args, context_index: context_index} + } + var argument_replacement = { 'False': function () {return false;}, 'True': function () {return true;}, diff --git a/addons/web/static/test/onchange.js b/addons/web/static/test/onchange.js index 07eb6c1bc2c..c606915f31a 100644 --- a/addons/web/static/test/onchange.js +++ b/addons/web/static/test/onchange.js @@ -22,12 +22,12 @@ $(document).ready(function () { window.openerp.web.form(openerp); } }); -// test('Parse args-less onchange', function () { -// var f = new openerp.web.FormView(null, {}); -// var result = f.parse_on_change('on_change_foo()', {}); -// equal(result.method, 'on_change_foo'); -// deepEqual(result.args, []); -// }); + test('Parse args-less onchange', function () { + var f = new openerp.web.FormView(null, {}); + var result = f.parse_on_change('on_change_foo()', {}); + equal(result.method, 'on_change_foo'); + deepEqual(result.args, []); + }); test('Parse 1-arg onchange', function () { var f = make_form({foo: 3}); var result = f.parse_on_change('on_change_foo(foo)', {}); From 8f67c5c367a2653918f5e0473c3d0f8e031f3aaf Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 21 Nov 2011 13:41:45 +0100 Subject: [PATCH 5/6] [ADD] support for string literals in form @on_change lp bug: https://launchpad.net/bugs/889741 fixed bzr revid: xmo@openerp.com-20111121124145-6i2kvesyc689g3ez --- addons/web/static/src/js/view_form.js | 36 ++++++++++++++++++--------- addons/web/static/test/onchange.js | 7 ++++++ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/addons/web/static/src/js/view_form.js b/addons/web/static/src/js/view_form.js index 1f35c0e1dc3..bd9cf27e0ed 100644 --- a/addons/web/static/src/js/view_form.js +++ b/addons/web/static/src/js/view_form.js @@ -268,26 +268,38 @@ openerp.web.FormView = openerp.web.View.extend( /** @lends openerp.web.FormView# var parent_fields = null; _.each(call[2].split(','), function (a, i) { var field = _.str.trim(a); + + // literal constant or context if (field in argument_replacement) { args.push(argument_replacement[field](i)); return; - } else if (self.fields[field]) { + } + // form field + if (self.fields[field]) { var value = self.fields[field].get_on_change_value(); args.push(value == null ? false : value); return; - } else { - var splitted = field.split('.'); - if (splitted.length > 1 && _.str.trim(splitted[0]) === "parent" && self.dataset.parent_view) { - if (parent_fields === null) { - parent_fields = self.dataset.parent_view.get_fields_values(); - } - var p_val = parent_fields[_.str.trim(splitted[1])]; - if (p_val !== undefined) { - args.push(p_val == null ? false : p_val); - return; - } + } + // parent field + var splitted = field.split('.'); + if (splitted.length > 1 && _.str.trim(splitted[0]) === "parent" && self.dataset.parent_view) { + if (parent_fields === null) { + parent_fields = self.dataset.parent_view.get_fields_values(); + } + var p_val = parent_fields[_.str.trim(splitted[1])]; + if (p_val !== undefined) { + args.push(p_val == null ? false : p_val); + return; } } + // string literal + var first_char = field[0], last_char = field[field.length-1]; + if ((first_char === '"' && last_char === '"') + || (first_char === "'" && last_char === "'")) { + args.push(field.slice(1, -1)); + return; + } + throw new Error("Could not get field with name '" + field + "' for onchange '" + onchange + "'"); }); diff --git a/addons/web/static/test/onchange.js b/addons/web/static/test/onchange.js index c606915f31a..9f047f33d93 100644 --- a/addons/web/static/test/onchange.js +++ b/addons/web/static/test/onchange.js @@ -55,4 +55,11 @@ $(document).ready(function () { var result = f.parse_on_change('on_null(False)', {}); deepEqual(result.args, [false]); }); + test('Literal string', function () { + var f = make_form(); + var result = f.parse_on_change('on_str("foo")', {}); + deepEqual(result.args, ['foo']); + var result2 = f.parse_on_change("on_str('foo')", {}); + deepEqual(result2.args, ['foo']); + }); }); From b40b60e27828c105deb3a68bc7b912b5400c50ce Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 21 Nov 2011 13:45:32 +0100 Subject: [PATCH 6/6] [IMP] use _.map to extract values from onchange parameter names, instead of doing it indirectly via _.each bzr revid: xmo@openerp.com-20111121124532-ndo2me1ehos7mu0m --- addons/web/static/src/js/view_form.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/addons/web/static/src/js/view_form.js b/addons/web/static/src/js/view_form.js index bd9cf27e0ed..69106434799 100644 --- a/addons/web/static/src/js/view_form.js +++ b/addons/web/static/src/js/view_form.js @@ -250,9 +250,9 @@ openerp.web.FormView = openerp.web.View.extend( /** @lends openerp.web.FormView# return null; } - var method = call[1], args = [], context_index = null; + var method = call[1]; if (!_.str.trim(call[2])) { - return {method: method, args: args, context_index: context_index} + return {method: method, args: [], context_index: null} } var argument_replacement = { @@ -265,20 +265,18 @@ openerp.web.FormView = openerp.web.View.extend( /** @lends openerp.web.FormView# return ctx; } }; - var parent_fields = null; - _.each(call[2].split(','), function (a, i) { + var parent_fields = null, context_index = null; + var args = _.map(call[2].split(','), function (a, i) { var field = _.str.trim(a); // literal constant or context if (field in argument_replacement) { - args.push(argument_replacement[field](i)); - return; + return argument_replacement[field](i); } // form field if (self.fields[field]) { var value = self.fields[field].get_on_change_value(); - args.push(value == null ? false : value); - return; + return value == null ? false : value; } // parent field var splitted = field.split('.'); @@ -288,16 +286,14 @@ openerp.web.FormView = openerp.web.View.extend( /** @lends openerp.web.FormView# } var p_val = parent_fields[_.str.trim(splitted[1])]; if (p_val !== undefined) { - args.push(p_val == null ? false : p_val); - return; + return p_val == null ? false : p_val; } } // string literal var first_char = field[0], last_char = field[field.length-1]; if ((first_char === '"' && last_char === '"') || (first_char === "'" && last_char === "'")) { - args.push(field.slice(1, -1)); - return; + return field.slice(1, -1); } throw new Error("Could not get field with name '" + field +