From 206453c83689ad0dc6ceec3686153ab79bf88660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 27 Feb 2017 19:39:41 +0100 Subject: [PATCH] [FIX] web: various fixes with one2many/formviewdialogs We share the basic model record between views, and we have to make sure that everything is properly coordinated. The main issue solved by this commit is the fact that we can now edit a one2many record, then close the dialog, then edit it again and finally cancel the changes, which bring us to the previous step. Also, we had to slightly improve the AbstractView API --- .../web/static/src/js/chrome/view_manager.js | 2 +- .../static/src/js/fields/relational_fields.js | 17 +++----- .../web/static/src/js/views/abstract_view.js | 35 +++++++++++----- .../static/src/js/views/basic/basic_model.js | 27 +++++++++++++ .../static/src/js/views/basic/basic_view.js | 14 +++++++ .../src/js/views/form/form_controller.js | 11 +++-- .../src/js/views/kanban/kanban_column.js | 5 +-- .../web/static/src/js/views/view_dialogs.js | 34 ++++++++++++++-- .../test/fields/relational_fields_tests.js | 40 ++++++++++++++++--- addons/web/static/test/helpers/test_utils.js | 2 +- addons/web/static/test/views/kanban_tests.js | 2 +- 11 files changed, 149 insertions(+), 40 deletions(-) diff --git a/addons/web/static/src/js/chrome/view_manager.js b/addons/web/static/src/js/chrome/view_manager.js index 0566b3042be..d2700854fe3 100644 --- a/addons/web/static/src/js/chrome/view_manager.js +++ b/addons/web/static/src/js/chrome/view_manager.js @@ -349,7 +349,7 @@ var ViewManager = Widget.extend(ControlPanelMixin, { } view = new View(arch, view.fields_view.fields, params); - return view.createController(this).then(function(controller) { + return view.getController(this).then(function(controller) { controller.on('history_back', this, function() { if (self.action_manager) self.action_manager.trigger('history_back'); }); diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index 1915fa6c9ff..d8abf196722 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -853,20 +853,13 @@ var FieldOne2Many = FieldX2Many.extend({ */ _onOpenRecord: function (event) { var self = this; - this._super.apply(this, arguments); _.extend(event.data, { - on_save: function (record) { - // update in a form view opened from the o2m (kanban or non editable list) - var relRecord = _.find(self.value.data, function (d) { - return d.res_id === record.res_id; - }); - self._setValue({ - operation: 'UPDATE', - id: relRecord.id, - data: record.data, - }); + shouldSaveLocally: true, + on_saved: function () { + self._setValue({ operation: 'NOOP' }); }, }); + this._super.apply(this, arguments); }, }); @@ -926,10 +919,10 @@ var FieldMany2Many = FieldX2Many.extend({ * @param {OdooEvent} */ _onOpenRecord: function (event) { - this._super.apply(this, arguments); _.extend(event.data, { on_saved: this.trigger_up.bind(this, 'reload', {db_id: event.data.id}), }); + this._super.apply(this, arguments); }, }); diff --git a/addons/web/static/src/js/views/abstract_view.js b/addons/web/static/src/js/views/abstract_view.js index 22514f3ba75..98c6aaac22e 100644 --- a/addons/web/static/src/js/views/abstract_view.js +++ b/addons/web/static/src/js/views/abstract_view.js @@ -14,7 +14,7 @@ odoo.define('web.AbstractView', function (require) { * * JS views are supposed to be used like this: * 1. instantiate a view with some arch, fields and params - * 2. call the createController method on the view instance. This returns a + * 2. call the getController method on the view instance. This returns a * controller (with a model and a renderer as sub widgets) * 3. append the controller somewhere * @@ -113,12 +113,12 @@ var AbstractView = Class.extend({ * likely a view manager) * @returns {Deferred} The deferred resolves to a controller */ - createController: function (parent) { + getController: function (parent) { var self = this; - var model = this.model || this.createModel(parent); - return $.when(model.load(this.loadParams), this._loadLibs()).then(function () { + return $.when(this._loadData(parent), this._loadLibs()).then(function () { + var model = self.getModel(); var state = model.get(arguments[0]); - var renderer = self.createRenderer(parent, state); + var renderer = self.getRenderer(parent, state); var Controller = self.Controller || self.config.Controller; var controllerParams = _.extend({ initialState: state, @@ -134,16 +134,19 @@ var AbstractView = Class.extend({ return controller; }); }, - createModel: function (parent) { - var Model = this.config.Model; - return new Model(parent); + getModel: function (parent) { + if (!this.model) { + var Model = this.config.Model; + this.model = new Model(parent); + } + return this.model; }, - createRenderer: function (parent, state) { + getRenderer: function (parent, state) { var Renderer = this.config.Renderer; return new Renderer(parent, state, this.rendererParams); }, /** - * this is usedful to customize the actual class to use before calling + * this is useful to customize the actual class to use before calling * createView. * * @param {Controller} Controller @@ -156,6 +159,18 @@ var AbstractView = Class.extend({ // Private //-------------------------------------------------------------------------- + /** + * Load initial data from the model + * + * @private + * @param {Widget} parent the parent of the model, if it has to be created + * @returns {Deferred<*>} a deferred that resolves to whatever the model + * decide to return + */ + _loadData: function (parent) { + var model = this.getModel(parent); + return model.load(this.loadParams); + }, /** * Makes sure that the js_libs and css_libs are properly loaded. Note that * the ajax loadJS and loadCSS methods don't do anything if the given file diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index 785cf79775b..bc36f454914 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -403,6 +403,7 @@ var BasicModel = AbstractModel.extend({ * of all fields by default * @param {Object} params.fields contains the description of each field * @param {string} [params.type] 'record' or 'list' + * @param {string} [params.recordID] an ID for an existing resource. * @returns {Deferred -> string} resolves to a local id, or handle */ load: function (params) { @@ -733,11 +734,22 @@ var BasicModel = AbstractModel.extend({ * @param {string} record_id local resource * @param {Object} [options] * @param {boolean} [options.reload=true] if true, data will be reloaded + * @param {boolean} [options.localSave=false] if true, the record will only + * be 'locally' saved: its changes will move from the _changes key to the + * data key * @returns {Deferred} */ save: function (record_id, options) { var self = this; options = options || {}; + if (options.localSave) { + var record = self.localData[record_id]; + if (record._changes) { + _.extend(record.data, record._changes); + record._changes = null; + } + return $.when(); + } var shouldReload = 'reload' in options ? options.reload : true; return this.mutex.exec(function () { var record = self.localData[record_id]; @@ -799,6 +811,21 @@ var BasicModel = AbstractModel.extend({ } }); }, + /** + * Set and return the fieldattrs on a record element. + * It is mostly useful for the cases where a record element is shared + * between various views, such as a one2many with a tree and a form view. + * + * @param {string} recordID a valid element ID + * @param {Object} attrs the new field attrs + * @return {Object} the previous field attrs + */ + setFieldAttrs: function (recordID, attrs) { + var record = this.localData[recordID]; + var result = record.fieldAttrs; + record.fieldAttrs = attrs; + return result; + }, /** * For list resources, this changes the orderedBy key, then performs the * sort directly, in javascript. This is used for sorting static datasets, diff --git a/addons/web/static/src/js/views/basic/basic_view.js b/addons/web/static/src/js/views/basic/basic_view.js index 947a3ad26b6..aea0b88dcf3 100644 --- a/addons/web/static/src/js/views/basic/basic_view.js +++ b/addons/web/static/src/js/views/basic/basic_view.js @@ -99,12 +99,26 @@ var BasicView = AbstractView.extend({ this.loadParams.fieldNames = fieldNames; this.loadParams.fields = fields; this.loadParams.limit = parseInt(arch.attrs.limit, 10) || params.limit; + this.recordID = params.recordID; }, //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- + /** + * In some cases, we already have a preloaded record + * + * @override + * @private + * @returns {Deferred} + */ + _loadData: function () { + if (this.recordID) { + return $.when(this.recordID); + } + return this._super.apply(this, arguments); + }, /** * Visit all nodes in the arch field and process each fields * diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js index 1493297ae97..df27ef0135b 100644 --- a/addons/web/static/src/js/views/form/form_controller.js +++ b/addons/web/static/src/js/views/form/form_controller.js @@ -183,6 +183,9 @@ var FormController = BasicController.extend({ * readonly mode after saving the record * @param {boolean} [options.reload=true] if true, reload the record after * saving + * @param {boolean} [options.localSave=false] if true, the record will only + * be 'locally' saved: its changes will move from the _changes key to the + * data key * @returns {Deferred} */ saveRecord: function (options) { @@ -199,12 +202,11 @@ var FormController = BasicController.extend({ if (this.checkInvalidFields()) { return $.Deferred().reject(); } else { - return this.model.save(this.handle, {reload: shouldReload}) - .then(function (element) { + return this.model.save(this.handle, {reload: shouldReload, localSave: options.localSave}) + .then(function () { if (!stayInEdit) { self._toReadOnlyMode(); } - self.trigger_up('saved', _.clone(element.data)); }); } } @@ -411,6 +413,7 @@ var FormController = BasicController.extend({ var field = event.data.field; new dialogs.FormViewDialog(this, { res_model: field.relation, + shouldSaveLocally: true, domain: event.data.domain, context: event.data.context, on_save: event.data.on_save, @@ -539,6 +542,8 @@ var FormController = BasicController.extend({ on_saved: event.data.on_saved, model: self.model, parentID: self.handle, + recordID: record.id, + shouldSaveLocally: event.data.shouldSaveLocally, }).open(); }); }, diff --git a/addons/web/static/src/js/views/kanban/kanban_column.js b/addons/web/static/src/js/views/kanban/kanban_column.js index 5875ac21e31..b93f75cd9df 100644 --- a/addons/web/static/src/js/views/kanban/kanban_column.js +++ b/addons/web/static/src/js/views/kanban/kanban_column.js @@ -220,13 +220,12 @@ var KanbanColumn = Widget.extend({ edit_column: function (event) { event.preventDefault(); - var dialog = new view_dialogs.FormViewDialog(this, { + new view_dialogs.FormViewDialog(this, { res_model: this.relation, res_id: this.id, title: _t("Edit Column"), + on_saved: this.trigger_up.bind(this, 'reload'), }).open(); - - dialog.on('saved', null, this.trigger_up.bind(this, 'reload')); }, delete_record: function (event) { diff --git a/addons/web/static/src/js/views/view_dialogs.js b/addons/web/static/src/js/views/view_dialogs.js index f5982e2ef65..ab9588dc740 100644 --- a/addons/web/static/src/js/views/view_dialogs.js +++ b/addons/web/static/src/js/views/view_dialogs.js @@ -82,6 +82,10 @@ var FormViewDialog = ViewDialog.extend({ * @param {function} [options.on_saved] callback executed after on_save * @param {BasicModel} [options.model] if given, it will be used instead of * a new form view model + * @param {string} [options.recordID] if given, the model has to be given as + * well, and in that case, it will be used without loading anything. + * @param {boolean} [options.shouldSaveLocally] if true, the view dialog + * will save locally instead of actually saving (useful for one2manys) */ init: function (parent, options) { var self = this; @@ -91,6 +95,8 @@ var FormViewDialog = ViewDialog.extend({ this.context = options.context; this.model = options.model; this.parentID = options.parentID; + this.recordID = options.recordID; + this.shouldSaveLocally = options.shouldSaveLocally; var multi_select = !_.isNumber(options.res_id) && !options.disable_multiple_selection; var readonly = _.isNumber(options.res_id) && options.readonly; @@ -130,7 +136,15 @@ var FormViewDialog = ViewDialog.extend({ } this._super(parent, options); }, - + /** + * If we changed some fieldAttrs, we need to restore them now. + */ + destroy: function () { + if (this.recordID && this.fieldAttrs) { + this.model.setFieldAttrs(this.recordID, this.fieldAttrs); + } + this._super.apply(this, arguments); + }, //-------------------------------------------------------------------------- // Public //-------------------------------------------------------------------------- @@ -153,6 +167,10 @@ var FormViewDialog = ViewDialog.extend({ } fields_view_def.then(function (fields_view) { + if (self.recordID) { + self.fieldAttrs = + self.model.setFieldAttrs(self.recordID, fields_view.fieldAttrs); + } var formview = new FormView(fields_view.arch, fields_view.fields, { modelName: self.res_model, context: self.context, @@ -164,11 +182,15 @@ var FormViewDialog = ViewDialog.extend({ default_buttons: false, model: self.model, parentID: self.parentID, + recordID: self.recordID, }); - return formview.createController(self); + return formview.getController(self); }).then(function (formView) { self.form_view = formView; var fragment = document.createDocumentFragment(); + if (self.recordID && self.shouldSaveLocally) { + self.model.save(self.recordID, {localSave: true}); + } self.form_view.appendTo(fragment) .then(function () { var $buttons = $('
'); @@ -197,7 +219,11 @@ var FormViewDialog = ViewDialog.extend({ } def = this.options.on_save(this.form_view.model.get(this.form_view.handle)); } else { - def = this.form_view.saveRecord({stayInEdit: true, reload: false}); + def = this.form_view.saveRecord({ + stayInEdit: true, + reload: false, + localSave: this.shouldSaveLocally, + }); } return $.when(def).then(function () { // record might have been changed by the save (e.g. if this was a new record, it has an @@ -316,7 +342,7 @@ var SelectCreateDialog = ViewDialog.extend({ hasSelectors: !self.options.disable_multiple_selection, }, self.options.list_view_options)); listView.setController(SelectCreateListController); - return listView.createController(self); + return listView.getController(self); }).then(function (controller) { self.list_controller = controller; // Set the dialog's buttons diff --git a/addons/web/static/test/fields/relational_fields_tests.js b/addons/web/static/test/fields/relational_fields_tests.js index 583f9adfdd1..b306c52d0f8 100644 --- a/addons/web/static/test/fields/relational_fields_tests.js +++ b/addons/web/static/test/fields/relational_fields_tests.js @@ -1326,7 +1326,7 @@ QUnit.module('relational_fields', { }); QUnit.test('one2many list with inline form view', function (assert) { - assert.expect(2); + assert.expect(5); this.data.partner.records[0].p = []; @@ -1367,16 +1367,46 @@ QUnit.module('relational_fields', { form.$('tbody td.o_form_field_x2many_list_row_add a').click(); - // write in the many2one field + // write in the many2one field, value = 37 (xphone) $('.modal .o_form_field_many2one input').click(); var $dropdown = $('.modal .o_form_field_many2one input').autocomplete('widget'); - $dropdown.find('li:eq(1) a').mouseenter(); - $dropdown.find('li:eq(1) a').click(); + $dropdown.find('li:eq(0) a').mouseenter(); + $dropdown.find('li:eq(0) a').click(); // write in the integer field $('.modal .modal-body input.o_field_widget').val('123').trigger('input'); - $('.modal .modal-footer button:eq(0)').click(); // save and close + // save and close + $('.modal .modal-footer button:eq(0)').click(); + + assert.strictEqual(form.$('tbody td:contains(xphone)').length, 1, + "should display 'xphone' in a td"); + + // reopen the record in form view + form.$('tbody td:contains(xphone)').click(); + + assert.strictEqual($('.modal .modal-body input').val(), "xphone", + "should display 'xphone' in an input"); + + $('.modal .modal-body input.o_field_widget').val('456').trigger('input'); + + // discard + $('.modal .modal-footer span:contains(Discard)').click(); + + // reopen the record in form view + form.$('tbody td:contains(xphone)').click(); + + assert.strictEqual($('.modal .modal-body input.o_field_widget').val(), "123", + "should display 123 (previous change has been discarded)"); + + // write in the many2one field, value = 41 (xpad) + $('.modal .o_form_field_many2one input').click(); + $dropdown = $('.modal .o_form_field_many2one input').autocomplete('widget'); + $dropdown.find('li:eq(1) a').mouseenter(); + $dropdown.find('li:eq(1) a').click(); + + // save and close + $('.modal .modal-footer button:eq(0)').click(); assert.strictEqual(form.$('tbody td:contains(xpad)').length, 1, "should display 'xpad' in a td"); diff --git a/addons/web/static/test/helpers/test_utils.js b/addons/web/static/test/helpers/test_utils.js index 3828c2d8b8c..f78b0151706 100644 --- a/addons/web/static/test/helpers/test_utils.js +++ b/addons/web/static/test/helpers/test_utils.js @@ -150,7 +150,7 @@ function createAsyncView(params) { }); } - return view.createController(widget).then(function (view) { + return view.getController(widget).then(function (view) { // override the view's 'destroy' so that it calls 'destroy' on the widget // instead, as the widget is the parent of the view and the mockServer. view.destroy = function () { diff --git a/addons/web/static/test/views/kanban_tests.js b/addons/web/static/test/views/kanban_tests.js index 5356fe42ffe..2d837d8e541 100644 --- a/addons/web/static/test/views/kanban_tests.js +++ b/addons/web/static/test/views/kanban_tests.js @@ -661,7 +661,7 @@ QUnit.module('Views', { mockRPC: function (route, args) { nbRPCs++; return this._super(route, args); - } + }, }); assert.strictEqual(kanban.$('.o_kanban_group[data-id=5] .o_column_title').text(), 'xmo', 'title of the column should be "xmo"');