diff options
10 files changed, 387 insertions, 48 deletions
diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js index 47802d5d8c7..6eac12a2dba 100644 --- a/addons/web/static/src/js/fields/abstract_field.js +++ b/addons/web/static/src/js/fields/abstract_field.js @@ -153,7 +153,7 @@ var AbstractField = Widget.extend({ // the 'required' flag is basically only needed to determine if the widget // is in a valid state (not valid if empty and required) - this.required = options.required || false; + this.required = options.required || this.field.required; // the 'idForLabel' is the (html) id that should be set to the relevent // dom entity. If done correctly, clicking on the corresponding label @@ -202,7 +202,13 @@ var AbstractField = Widget.extend({ * a field widget is valid if its value is valid and if there is a value when * it is required. This is checked before saving a record, by the form view. * - * @returns {boolean} + * Note: the return value may be a boolean. It is necessary for some fields + * (x2many) to sometimes ask confirmation to the user if the current dirty + * invalid line can be discarded. This is an asynchronous operation, so we + * need the expressivity of something like a deferred. + * + * @returns {boolean|Deferred} true/false if the widget is valid, or a + * deferred which resolves to true/false if the widget is valid or not */ isValid: function () { var is_required = this.required; diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index c29f958cc06..4ae30264ff0 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -560,6 +560,28 @@ var FieldX2Many = AbstractField.extend({ isSet: function () { return true; }, + /** + * This is a tricky override. For relational fields, we don't just need to + * know if the field is valid. We also need to discard some invalid lines, + * if the user is ok with it (or automatically if the invalid line is not + * dirty). This is the reason why we need to change the signature of the + * AbstractField isValid method to optionally return a deferred. + * + * @override + * @returns {boolean|Deferred} if there is a renderer (sub list or kanban + * view), we may need to ask the user => it will return a deferred. + * Otherwise, it returns a boolean + */ + isValid: function () { + if (!this.renderer) { + return this._super.apply(this, arguments); + } + var def = $.Deferred(); + this.renderer.canBeSaved() + .then(def.resolve.bind(def, true)) + .fail(def.resolve.bind(def, false)); + return def; + }, //-------------------------------------------------------------------------- // Private 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 0b5de486bb9..1fe0e52130b 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -545,6 +545,22 @@ var BasicModel = AbstractModel.extend({ }); }, /** + * In some case, we may need to remove an element from a list, without going + * through the notifyChanges machinery. The motivation for this is when the + * user click on 'Add an item' in a field one2many with a required field, + * then clicks somewhere else. The new line need to be discarded, but we + * don't want to trigger a real notifyChanges (no need for that, and also, + * we don't want to rerender the UI). + * + * @param {string} elementID some valid element id. It is necessary that the + * corresponding element has a parent. + */ + removeLine: function (elementID) { + var record = this.localData[elementID]; + var parent = this.localData[record.parentID]; + parent._changes = _.without(parent._changes, elementID); + }, + /** * Save a local resource, if needed. This is a complicated operation, * - it needs to check all changes, * - generate commands for x2many fields, @@ -1652,6 +1668,11 @@ var BasicModel = AbstractModel.extend({ removedIds = _.difference(list.res_ids, relIds); addedIds = _.difference(relIds, list.res_ids); keptIds = _.intersection(list.res_ids, relIds); + + // the didChange variable keep track of the fact that at + // least one id was updated + var didChange = false; + for (i = 0; i < keptIds.length; i++) { relRecord = _.findWhere(relData, {res_id: keptIds[i]}); var command; @@ -1659,11 +1680,18 @@ var BasicModel = AbstractModel.extend({ var r = this.get(relRecord.id, {raw: true}); var changes = _.pick(r.data, Object.keys(relRecord._changes)); command = x2ManyCommands.update(relRecord.res_id, changes); + didChange = true; } else { command = x2ManyCommands.link_to(keptIds[i]); } commands[fieldName].push(command); } + if (!didChange && addedIds.length === 0 && removedIds.length === 0) { + // in this situation, we have no changed ids, no added + // ids and no removed ids, so we can safely ignore the + // last changes + commands[fieldName] = null; + } for (i = 0; i < addedIds.length; i++) { relRecord = _.findWhere(relData, {res_id: addedIds[i]}); relRecord = this.get(relRecord.id, {raw: true}); diff --git a/addons/web/static/src/js/views/field_manager_mixin.js b/addons/web/static/src/js/views/field_manager_mixin.js index 10d0c06ad76..9fcd34ffee2 100644 --- a/addons/web/static/src/js/views/field_manager_mixin.js +++ b/addons/web/static/src/js/views/field_manager_mixin.js @@ -12,6 +12,7 @@ var BasicModel = require('web.BasicModel'); var FieldManagerMixin = { custom_events: { + discard_line: '_onDiscardLine', field_changed: '_onFieldChanged', load: '_onLoad', }, @@ -59,6 +60,19 @@ var FieldManagerMixin = { //-------------------------------------------------------------------------- /** + * In some case, we may need to remove an element from a list, without going + * through the notifyChanges machinery. The motivation for this is when the + * user click on 'Add an item' in a field one2many with a required field, + * then clicks somewhere else. The new line need to be discarded, but we + * don't want to trigger a real notifyChanges (no need for that, and also, + * we don't want to rerender the UI). + * + * @param {OdooEvent} event + */ + _onDiscardLine: function (event) { + this.model.removeLine(event.data.id); + }, + /** * This is the main job of the FMM: deciding what to do when a controlled * field changes. Most of the time, it notifies the model that a change * just occurred, then confirm the change. 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 22f4688bc70..d41ff592687 100644 --- a/addons/web/static/src/js/views/form/form_controller.js +++ b/addons/web/static/src/js/views/form/form_controller.js @@ -75,14 +75,11 @@ var FormController = BasicController.extend({ return def; }, /** - * @returns {boolean} + * @returns {Deferred} */ - checkInvalidFields: function () { - var invalidFields = this.renderer.checkInvalidFields(); - if (invalidFields.length) { - this._notifyInvalidFields(invalidFields); - } - return !!invalidFields.length; + canBeSaved: function () { + return this.renderer.canBeSaved() + .fail(this._notifyInvalidFields.bind(this)); }, /** * This method switches the form view in edit mode, with a new record. @@ -225,17 +222,19 @@ var FormController = BasicController.extend({ } return $.Deferred().resolve(); } else { - if (this.checkInvalidFields()) { - return $.Deferred().reject(); - } else { - return this.model.save(this.handle, {reload: shouldReload, savePoint: options.savePoint}) - .then(function () { - if (!stayInEdit) { - self._toReadOnlyMode(); - } - self.isDirty = false; + return this.canBeSaved() + .then(function () { + return self.model.save(self.handle, { + reload: shouldReload, + savePoint: options.savePoint }); - } + }) + .then(function () { + if (!stayInEdit) { + self._toReadOnlyMode(); + } + self.isDirty = false; + }); } }, /** diff --git a/addons/web/static/src/js/views/form/form_renderer.js b/addons/web/static/src/js/views/form/form_renderer.js index 2cdc2f91a1d..f417b53885d 100644 --- a/addons/web/static/src/js/views/form/form_renderer.js +++ b/addons/web/static/src/js/views/form/form_renderer.js @@ -42,22 +42,39 @@ return BasicRenderer.extend({ * toggling the o_form_invalid css class. It has to be done both on the * widget, and on the label, if there is a label. * - * @returns {string[]} the list of invalid field names + * @returns {Deferred} + * if it fails, it gives the list of invalid field names */ - checkInvalidFields: function () { + canBeSaved: function () { var self = this; - var invalid_fields = []; - _.each(this.widgets, function (widget) { - var is_valid = widget.isValid(); - if (!is_valid) { - invalid_fields.push(widget.name); + var invalidFields = []; + var defs = []; + + function markWidget(widget, isValid) { + if (!isValid) { + invalidFields.push(widget.name); } - widget.$el.toggleClass('o_form_invalid', !is_valid); + widget.$el.toggleClass('o_form_invalid', !isValid); var idForLabel = self.idsForLabels[widget.name]; var $label = idForLabel ? self.$('label[for=' + idForLabel + ']') : $(); - $label.toggleClass('o_form_invalid', !is_valid); + $label.toggleClass('o_form_invalid', !isValid); + } + + _.each(this.widgets, function (widget) { + var isValid = widget.isValid(); + if (isValid instanceof $.Deferred) { + defs.push(isValid.then(function (isValid) { + markWidget(widget, isValid); + })); + } else { + markWidget(widget, isValid); + } + }); + return $.when.apply($, defs).then(function () { + if (invalidFields.length) { + return $.Deferred().reject(invalidFields); + } }); - return invalid_fields; }, /** * returns the active tab pages for each notebook diff --git a/addons/web/static/src/js/views/kanban/kanban_renderer.js b/addons/web/static/src/js/views/kanban/kanban_renderer.js index 4fe030eb168..9e4e6865c0f 100644 --- a/addons/web/static/src/js/views/kanban/kanban_renderer.js +++ b/addons/web/static/src/js/views/kanban/kanban_renderer.js @@ -112,6 +112,12 @@ var KanbanRenderer = BasicRenderer.extend({ this.widgets[0].addQuickCreate(); }, /** + * @returns {Deferred} + */ + canBeSaved: function () { + return $.when(); + }, + /** * Removes a widget (record if ungrouped, column if grouped) from the view. * * @param {Widget} widget the instance of the widget to remove diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js index 337d62b1dc1..67dcecb03ed 100644 --- a/addons/web/static/src/js/views/list/list_editable_renderer.js +++ b/addons/web/static/src/js/views/list/list_editable_renderer.js @@ -13,6 +13,7 @@ odoo.define('web.EditableListRenderer', function (require) { */ var core = require('web.core'); +var Dialog = require('web.Dialog'); var Domain = require('web.Domain'); var ListRenderer = require('web.ListRenderer'); @@ -59,6 +60,10 @@ ListRenderer.include({ this.currentRow = null; this.currentCol = null; + + // the dirtyRows object is there to keep track of lines that have been + // modified by the user + this.dirtyRows = {}; }, /** * @override @@ -77,6 +82,36 @@ ListRenderer.include({ //-------------------------------------------------------------------------- /** + * @returns {Deferred} + */ + canBeSaved: function () { + var self = this; + var invalidDirtyWidgets = []; + var invalidCleanWidgets = []; + _.each(this.widgets, function (widget) { + // Note: the isValid method may return a deferred in general, but + // not in this case. Here, this method is called by a fieldone2many + // when it need to make sure the state is valid. So, the widgets + // that we consider here are widgets that can be in a one2many, and + // we don't support a list inside a list (for now at least). + if (!widget.isValid()) { + if (self.dirtyRows[widget.dataPointID]) { + invalidDirtyWidgets.push(widget); + } else { + invalidCleanWidgets.push(widget); + } + } + }); + if (invalidDirtyWidgets.length) { + // TODO: ask for confirmation, open dialog, and depending on user + // input, resolve and reject deferred + } + if (invalidCleanWidgets.length) { + this._unselectRow(); + } + return $.when(); + }, + /** * This method is called by the controller, when a change has been confirmed * by the model. In that situation, we need to properly update the cell * widgets. (it is necessary, because the value could have been changed by @@ -452,30 +487,84 @@ ListRenderer.include({ * This method is called whenever we click/move outside of a row that was * in edit mode. This is the moment we save all accumulated changes on that * row, if needed. + * + * If the current row is valid, then it can be unselected. If it is not + * valid, we need to check if it is dirty. If it is not dirty, it can be + * unselected (and will be removed). If it is dirty, we need to ask the user + * for confirmation. If the user agrees, the row will be removed, otherwise + * nothing happens (it is still selected) + * + * @returns {Deferred} The deferred resolves if the row was unselected (and + * possibly removed). If may be rejected, when the row is dirty and the + * user refuses to discard its changes. */ _unselectRow: function () { var self = this; + + // We need to protect against calling this method when no row is + // selected. if (this.currentRow === null) { - return; + return $.when(); } - // trigger the save (if the record isn't dirty, the datamodel won't save - // anything) - this.trigger_up('field_changed', { - dataPointID: this.state.data[this.currentRow].id, - changes: {}, - force_save: true, - }); - this.trigger_up('change_mode', {mode: 'readonly'}); - var widgets = _.where(this.widgets, {__rowIndex: this.currentRow}); - _.each(widgets, function (widget) { - self._setCellValue(self.currentRow, widget.__colIndex, true); - widget.destroy(); + + var discarded = false; + + function discardLine() { + self.trigger_up('discard_line', {id: currentLineID}); + discarded = true; + def.resolve(); + } + + var rowWidgets = _.where(this.widgets, {__rowIndex: this.currentRow}); + var isRowValid = _.every(rowWidgets, function (w) { return w.isValid();}); + var $row = this.$('tbody tr.o_selected_row'); + + var def; + + if (isRowValid) { + // trigger the save (if the record isn't dirty, the datamodel won't save + // anything) + self.trigger_up('field_changed', { + dataPointID: self.state.data[self.currentRow].id, + changes: {}, + force_save: true, + }); + self.trigger_up('change_mode', {mode: 'readonly'}); + $row.removeClass('o_selected_row'); + $row.find('td').removeClass('o_edit_mode'); + def = $.when(); + } else { + def = $.Deferred(); + var currentLineID = this.state.data[this.currentRow].id; + var isDirty = this.dirtyRows[currentLineID]; + if (isDirty) { + // if the currently selected record is dirty, we want to ask for + // confirmation before discarding it + var message = _t("The line has been modified, your changes will be discarded. Are you sure you want to discard the changes ?"); + var options = { + title: _t("Warning"), + confirm_callback: discardLine, + cancel_callback: def.reject.bind(def) + }; + var dialog = Dialog.confirm(this, message, options); + dialog.$modal.on('hidden.bs.modal', def.reject.bind(def)); + } else { + discardLine(); + } + } + + return def.then(function finalizeUnselect() { + var widgets = _.where(self.widgets, {__rowIndex: self.currentRow}); + _.each(widgets, function (widget) { + self._setCellValue(self.currentRow, widget.__colIndex, true); + widget.destroy(); + }); + if (discarded) { + $row.remove(); + } + self.widgets = _.without(self.widgets, widgets); + self.currentRow = null; }); - self.widgets = _.without(self.widgets, widgets); - var $row = this.$('tbody tr').eq(this.currentRow); - $row.removeClass('o_selected_row'); - $row.find('td').removeClass('o_edit_mode'); - this.currentRow = null; }, //-------------------------------------------------------------------------- @@ -547,6 +636,7 @@ ListRenderer.include({ * @param {OdooEvent} event */ _onFieldChanged: function (event) { + this.dirtyRows[event.data.dataPointID] = true; var $td = event.target.$el.parent(); $td.addClass('o_field_dirty'); }, diff --git a/addons/web/static/src/js/views/view_dialogs.js b/addons/web/static/src/js/views/view_dialogs.js index fe7ddddaf10..6f0fe8328c3 100644 --- a/addons/web/static/src/js/views/view_dialogs.js +++ b/addons/web/static/src/js/views/view_dialogs.js @@ -207,7 +207,7 @@ var FormViewDialog = ViewDialog.extend({ var self = this; var def; if (this.options.on_save) { - if (this.form_view.checkInvalidFields()) { + if (this.form_view.canBeSaved()) { return $.Deferred().reject(); } def = this.options.on_save(this.form_view.model.get(this.form_view.handle)); diff --git a/addons/web/static/test/fields/relational_fields_tests.js b/addons/web/static/test/fields/relational_fields_tests.js index 0cc697e96e5..68a50ee30e9 100644 --- a/addons/web/static/test/fields/relational_fields_tests.js +++ b/addons/web/static/test/fields/relational_fields_tests.js @@ -1312,6 +1312,163 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('one2many list (editable): discarding required empty data', function (assert) { + assert.expect(7); + + this.data.turtle.fields.turtle_foo.required = true; + delete this.data.turtle.fields.turtle_foo.default; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '<form string="Partners">' + + '<group>' + + '<field name="turtles">' + + '<tree editable="top">' + + '<field name="turtle_foo"/>' + + '</tree>' + + '</field>' + + '</group>' + + '</form>', + res_id: 2, + mockRPC: function (route, args) { + if (args.method) { + assert.step(args.method); + } + return this._super.apply(this, arguments); + }, + }); + + // edit mode, then click on Add an item, then click elsewhere + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "should have 0 data rows"); + form.$buttons.find('.o_form_button_edit').click(); + form.$('.o_form_field_x2many_list_row_add a').click(); + form.$('label.o_form_label').first().click(); + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "should still have 0 data rows"); + + // click on Add an item again, then click on save + form.$('.o_form_field_x2many_list_row_add a').click(); + form.$buttons.find('.o_form_button_save').click(); + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "should still have 0 data rows"); + + assert.verifySteps(['read', 'default_get', 'default_get']); + form.destroy(); + }); + + QUnit.test('unselecting a line with missing required data', function (assert) { + assert.expect(5); + + this.data.turtle.fields.turtle_foo.required = true; + delete this.data.turtle.fields.turtle_foo.default; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '<form string="Partners">' + + '<group>' + + '<field name="turtles">' + + '<tree editable="top">' + + '<field name="turtle_foo"/>' + + '<field name="turtle_int"/>' + + '</tree>' + + '</field>' + + '</group>' + + '</form>', + res_id: 2, + }); + + // edit mode, then click on Add an item, then click elsewhere + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "should have 0 data rows"); + form.$buttons.find('.o_form_button_edit').click(); + form.$('.o_form_field_x2many_list_row_add a').click(); + assert.strictEqual(form.$('tr.o_data_row').length, 1, + "should have 1 data rows"); + + // adding a value in the non required field, so it is dirty, but with + // a missing required field + form.$('input[name="turtle_int"]').val('12345').trigger('input'); + + // click elsewhere, + form.$('label.o_form_label').click(); + assert.strictEqual($('.modal').length, 1, + 'a confirmation model should be opened'); + + // click on cancel, the line should still be selected + $('.modal .modal-footer button.btn-default').click(); + assert.strictEqual(form.$('tr.o_data_row.o_selected_row').length, 1, + "should still have 1 selected data row"); + + // click elsewhere, and click on ok (on the confirmation dialog) + form.$('label.o_form_label').click(); + $('.modal .modal-footer button.btn-primary').click(); + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "should have 0 data rows (invalid line has been discarded"); + + form.destroy(); + }); + + QUnit.test('editing a o2m, with required field and onchange', function (assert) { + assert.expect(12); + + this.data.turtle.fields.turtle_foo.required = true; + delete this.data.turtle.fields.turtle_foo.default; + this.data.turtle.onchanges = { + turtle_foo: function (obj) { + obj.turtle_int = obj.turtle_foo.length; + }, + }; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '<form string="Partners">' + + '<group>' + + '<field name="turtles">' + + '<tree editable="top">' + + '<field name="turtle_foo"/>' + + '<field name="turtle_int"/>' + + '</tree>' + + '</field>' + + '</group>' + + '</form>', + res_id: 2, + mockRPC: function (route, args) { + if (args.method) { + assert.step(args.method); + } + return this._super.apply(this, arguments); + }, + }); + + // edit mode, then click on Add an item + assert.strictEqual(form.$('tr.o_data_row').length, 0, + "should have 0 data rows"); + form.$buttons.find('.o_form_button_edit').click(); + form.$('.o_form_field_x2many_list_row_add a').click(); + + // input some text in required turtle_foo field + form.$('input[name="turtle_foo"]').val('aubergine').trigger('input'); + assert.strictEqual(form.$('input[name="turtle_int"]').val(), "9", + "onchange should have been triggered"); + + // save and check everything is fine + form.$buttons.find('.o_form_button_save').click(); + assert.strictEqual(form.$('.o_data_row td:contains(aubergine)').length, 1, + "should have one row with turtle_foo value"); + assert.strictEqual(form.$('.o_data_row td:contains(9)').length, 1, + "should have one row with turtle_int value"); + + assert.verifySteps(['read', 'default_get', 'onchange', 'onchange', 'write', 'read', 'read']); + form.destroy(); + }); + QUnit.test('onchange in a one2many', function (assert) { assert.expect(1); |