From 8b6063ac1630781477751fa593725f34a08430d8 Mon Sep 17 00:00:00 2001 From: Mohammed Shekha Date: Thu, 21 Jan 2021 08:45:12 +0000 Subject: [PATCH] [IMP] web, *: discard unchanged record in editable list * sale_product_matrix Before this commit, if we created a new row in an editable list view, do not "touched" it and clicked out of the list then the record tried to be saved and threw error notifications. Now, that flow will discard the record to make it consistent with the form view which discards the record when we leave the form. task-2431691 closes odoo/odoo#68382 Signed-off-by: Aaron Bohy (aab) --- .../tests/tours/sale_product_matrix_tour.js | 3 +- .../static/src/js/views/basic/basic_model.js | 3 + .../src/js/views/list/list_controller.js | 4 +- .../js/views/list/list_editable_renderer.js | 31 +++++- .../relational_fields/field_many2one_tests.js | 5 +- .../relational_fields/field_one2many_tests.js | 16 ++- .../tests/fields/relational_fields_tests.js | 4 +- addons/web/static/tests/views/list_tests.js | 104 +++++++++++++++++- 8 files changed, 155 insertions(+), 15 deletions(-) diff --git a/addons/sale_product_matrix/static/tests/tours/sale_product_matrix_tour.js b/addons/sale_product_matrix/static/tests/tours/sale_product_matrix_tour.js index 10d15c34843..e5e56c5c4e0 100644 --- a/addons/sale_product_matrix/static/tests/tours/sale_product_matrix_tour.js +++ b/addons/sale_product_matrix/static/tests/tours/sale_product_matrix_tour.js @@ -69,7 +69,8 @@ tour.register('sale_matrix_tour', { run: 'click' // apply the matrix }, { trigger: ".o_form_editable .o_field_many2one[name='partner_id'] input", - extra_trigger: ".o_sale_order", + // wait for qty to be 1 + extra_trigger: '.o_sale_order .o_field_cell.o_data_cell.o_list_number:contains("1.00")', run: 'text Agrolait' }, { trigger: ".ui-menu-item > a", 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 87a47fa1bb1..79e19818e92 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -582,6 +582,7 @@ var BasicModel = AbstractModel.extend({ getFieldNames: element.getFieldNames, id: element.id, isDirty: element.isDirty, + isNew: element.isNew, limit: element.limit, model: element.model, offset: element.offset, @@ -640,6 +641,7 @@ var BasicModel = AbstractModel.extend({ groupsOffset: element.groupsOffset, id: element.id, isDirty: element.isDirty, + isNew: element.isNew, isOpen: element.isOpen, isSample: this.isSampleModel, limit: element.limit, @@ -4129,6 +4131,7 @@ var BasicModel = AbstractModel.extend({ dataPoint.getDomain = this._getDomain.bind(this, dataPoint); dataPoint.getFieldNames = this._getFieldNames.bind(this, dataPoint); dataPoint.isDirty = this.isDirty.bind(this, dataPoint.id); + dataPoint.isNew = this.isNew.bind(this, dataPoint.id); this.localData[dataPoint.id] = dataPoint; diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js index 9313e6b6332..5263eea7cc4 100644 --- a/addons/web/static/src/js/views/list/list_controller.js +++ b/addons/web/static/src/js/views/list/list_controller.js @@ -83,7 +83,7 @@ var ListController = BasicController.extend({ */ canBeRemoved: async function () { const _super = this._super.bind(this); - await this.renderer.unselectRow(); + await this.renderer.unselectRow({ canDiscard: true }); return _super(...arguments); }, /* @@ -415,7 +415,7 @@ var ListController = BasicController.extend({ return null; } return Object.assign(this._super(...arguments), { - validate: () => this.renderer.unselectRow(), + validate: () => this.renderer.unselectRow({ canDiscard: true }), }); }, /** 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 230733448e2..7a215ed322a 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 @@ -495,11 +495,14 @@ ListRenderer.include({ * prevent subsequent editions. These edits would be lost, because the list * view only saves records when unselecting a row. * + * @param {Object} [options] + * @param {boolean} [options.canDiscard=false] when set to true, + * discard the record if it is new and not dirty otherwise save it * @returns {Promise} The promise 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 () { + unselectRow: function (options={}) { // Protect against calling this method when no row is selected if (this.currentRow === null) { return Promise.resolve(); @@ -514,7 +517,23 @@ ListRenderer.include({ } toggleWidgets(true); - return new Promise((resolve, reject) => { + return new Promise(async (resolve, reject) => { + const record = this._getRecord(recordID); + if (options.canDiscard && record.isNew()) { + await this.commitChanges(recordID); + if (!record.isDirty()) { + this.trigger_up('mutexify', { + action: () => { + return this.trigger_up('discard_changes', { + recordID: recordID, + onSuccess: resolve, + onFailure: reject, + }); + }, + }); + return; + } + } this.trigger_up('save_line', { recordID: recordID, onSuccess: resolve, @@ -1325,7 +1344,7 @@ ListRenderer.include({ var recordId = this._getRecordID(rowIndex); // To select a row, the currently selected one must be unselected first var self = this; - return this.unselectRow().then((selectNextRow = true) => { + return this.unselectRow({ canDiscard: true }).then((selectNextRow = true) => { if (!selectNextRow) { return Promise.resolve(); } @@ -1853,7 +1872,11 @@ ListRenderer.include({ return; } - this.unselectRow(); + this.unselectRow({ + // if save button is clicked then save the record forcefully even if + // it is non dirty + canDiscard: ![...event.target.classList].includes('o_list_button_save') + }); }, }); diff --git a/addons/web/static/tests/fields/relational_fields/field_many2one_tests.js b/addons/web/static/tests/fields/relational_fields/field_many2one_tests.js index bb21df5ccfb..500b61eed1f 100644 --- a/addons/web/static/tests/fields/relational_fields/field_many2one_tests.js +++ b/addons/web/static/tests/fields/relational_fields/field_many2one_tests.js @@ -2880,6 +2880,7 @@ QUnit.module('fields', {}, function () { arch: '
' + '' + '' + + '' + '' + '' + '' + @@ -2908,6 +2909,8 @@ QUnit.module('fields', {}, function () { domain = [['id', 'in', [10]]]; // domain for subrecord 1 await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); await testUtils.dom.click(form.$('.o_field_widget[name=trululu] input')); + // add some value to foo field to make record dirty + await testUtils.fields.editInput(form.$('tr.o_selected_row input[name="foo"]'), 'new value'); // add a second row with another domain for the m2o domain = [['id', 'in', [5]]]; // domain for subrecord 2 @@ -2916,7 +2919,7 @@ QUnit.module('fields', {}, function () { // check again the first row to ensure that the domain hasn't change domain = [['id', 'in', [10]]]; // domain for subrecord 1 should have been kept - await testUtils.dom.click(form.$('.o_data_row:first .o_data_cell')); + await testUtils.dom.click(form.$('.o_data_row:first .o_data_cell:eq(1)')); await testUtils.dom.click(form.$('.o_field_widget[name=trululu] input')); form.destroy(); diff --git a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js index d6fb446359f..2a09ebd1f98 100644 --- a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js +++ b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js @@ -2669,9 +2669,10 @@ QUnit.module('fields', {}, function () { res_id: 1, }); - // add a record, then click in form view to confirm it + // add a record, add value to turtle_foo then click in form view to confirm it await testUtils.form.clickEdit(form); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); + await testUtils.fields.editInput(form.$('input[name="turtle_foo"]'), 'nora'); await testUtils.dom.click(form.$el); assert.strictEqual(form.$('.o_field_widget[name=turtles] .o_pager').text().trim(), '1-4 / 5', @@ -2871,11 +2872,12 @@ QUnit.module('fields', {}, function () { res_id: 1, }); - // edit mode, then click on Add an item 2 times + // edit mode, then click on Add an item, enter value in turtle_foo and Add an item again assert.containsOnce(form, 'tr.o_data_row', "should have 1 data rows"); await testUtils.form.clickEdit(form); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); + await testUtils.fields.editInput(form.$('input[name="turtle_foo"]'), 'nora'); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); assert.containsN(form, 'tr.o_data_row', 3, "should have 3 data rows"); @@ -3006,7 +3008,8 @@ QUnit.module('fields', {}, function () { // the record currently being added should not count in the pager assert.containsNone(form, '.o_field_widget[name=turtles] .o_pager'); - // unselect the row + // enter value in turtle_foo field and click outside to unselect the row + await testUtils.fields.editInput(form.$('input[name="turtle_foo"]'), 'nora'); await testUtils.dom.click(form.$el); assert.containsNone(form, '.o_selected_row'); assert.containsNone(form, '.o_field_widget[name=turtles] .o_pager'); @@ -3147,8 +3150,11 @@ QUnit.module('fields', {}, function () { // add 4 records (to have more records than the limit) await testUtils.form.clickEdit(form); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); + await testUtils.fields.editInput(form.$('input[name="turtle_foo"]'), 'nora'); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); + await testUtils.fields.editInput(form.$('input[name="turtle_foo"]'), 'nora'); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); + await testUtils.fields.editInput(form.$('input[name="turtle_foo"]'), 'nora'); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); assert.containsN(form, 'tr.o_data_row', 5); @@ -8069,9 +8075,11 @@ QUnit.module('fields', {}, function () { assert.strictEqual($('.o_data_cell').text(), ""); // click add pizza - // save the modal + // press enter to save the record // check it's pizza await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a:eq(1)')); + const $input = form.$('.o_field_widget[name="p"] .o_selected_row .o_field_widget[name="display_name"]'); + await testUtils.fields.triggerKeydown($input, 'enter'); // click add pasta await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a:eq(2)')); await testUtils.form.clickSave(form); diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index f01f78a2912..cb9718169d4 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -539,7 +539,7 @@ QUnit.module('relational_fields', { }); QUnit.test('one2many, onchange, edition and multipage...', async function (assert) { - assert.expect(7); + assert.expect(8); this.data.partner.onchanges = { turtles: function (obj) { @@ -570,6 +570,7 @@ QUnit.module('relational_fields', { }, }); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); + await testUtils.fields.editInput(form.$('input[name="turtle_foo"]'), 'nora'); await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a')); assert.verifySteps([ @@ -577,6 +578,7 @@ QUnit.module('relational_fields', { 'read turtle', 'onchange turtle', 'onchange partner', + "onchange partner", 'onchange turtle', 'onchange partner', ]); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index d146a538b18..85a34406ccf 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -5110,7 +5110,7 @@ QUnit.module('Views', { model: 'foo', data: this.data, arch: '' + - '' + + '' + '', mockRPC: function (route, args) { if (args.method === 'create') { @@ -5121,16 +5121,19 @@ QUnit.module('Views', { }); await testUtils.dom.click(list.$buttons.find('.o_list_button_add')); + await testUtils.fields.editInput(list.$('tr.o_selected_row input[name="foo"]'), 'new value'); await testUtils.dom.click(list.$('.o_list_view')); assert.strictEqual(createCount, 1, "should have created a record"); await testUtils.dom.click(list.$buttons.find('.o_list_button_add')); + await testUtils.fields.editInput(list.$('tr.o_selected_row input[name="foo"]'), 'new value'); await testUtils.dom.click(list.$('tfoot')); assert.strictEqual(createCount, 2, "should have created a record"); await testUtils.dom.click(list.$buttons.find('.o_list_button_add')); + await testUtils.fields.editInput(list.$('tr.o_selected_row input[name="foo"]'), 'new value'); await testUtils.dom.click(list.$('tbody tr').last()); assert.strictEqual(createCount, 3, "should have created a record"); @@ -6343,6 +6346,7 @@ QUnit.module('Views', { // Press 'Tab' -> should go to next line await testUtils.fields.triggerKeydown(form.$('.o_field_widget[name=o2m] .o_selected_row input'), 'tab'); + await testUtils.nextTick(); // wait for discard assert.hasClass(form.$('.o_field_widget[name=o2m] .o_data_row:nth(1)'),'o_selected_row', "second row should be in edition"); @@ -7249,7 +7253,7 @@ QUnit.module('Views', { list.destroy(); }); - QUnit.test('list with handle widget, create, move and discard', async function (assert) { + QUnit.test('list with handle widget, create and move should keep empty lines at last', async function (assert) { // When there are less than 4 records in the table, empty lines are added // to have at least 4 rows. This test ensures that the empty line added // when a new record is discarded is correctly added on the bottom of @@ -7587,6 +7591,100 @@ QUnit.module('Views', { list.destroy(); }); + QUnit.test('editable list view: non dirty record with required fields', async function (assert) { + assert.expect(10); + + const list = await createView({ + arch: ` + + + + `, + data: this.data, + model: 'foo', + View: ListView, + }); + + await testUtils.dom.click($('.o_list_button_add')); + assert.containsOnce(list, '.o_selected_row'); + + // do not change anything and then click outside should discard record + await testUtils.dom.click('body'); + assert.containsNone(list, '.o_selected_row'); + + await testUtils.dom.click($('.o_list_button_add')); + assert.containsOnce(list, '.o_selected_row'); + // do not change anything and then click save button should not allow to discard record + await testUtils.dom.click($('.o_list_button_save')); + assert.containsOnce(list, '.o_selected_row'); + + // selecting some other row should discard non dirty record + assert.strictEqual(list.$('.o_selected_row').data('id'), 'foo_7', + "foo_7 record should be currently selected"); + await testUtils.dom.click(list.$('.o_data_row:eq(1) .o_data_cell:eq(0)')); + assert.strictEqual(list.$('.o_selected_row').data('id'), 'foo_2', + "foo_2 record should be currently selected"); + + // click somewhere else to discard currently selected row + await testUtils.dom.click('body'); + await testUtils.dom.click($('.o_list_button_add')); + assert.containsOnce(list, '.o_selected_row'); + // do not change anything and press Enter key should not allow to discard record + await testUtils.fields.triggerKeydown(list.$('tr.o_selected_row input.o_field_widget[name="foo"]'), 'enter'); + assert.containsOnce(list, '.o_selected_row'); + + // discard row and create new record and keep required field empty and click anywhere + await testUtils.dom.click(list.$buttons.find('.o_list_button_discard')); + await testUtils.dom.click($('.o_list_button_add')); + assert.containsOnce(list, '.o_selected_row', "row should be selected"); + await testUtils.fields.editInput(list.$('.o_selected_row .o_field_widget[name=int_field]'), 123); + await testUtils.dom.click('body'); + assert.containsOnce(list, '.o_selected_row', "row should still be selected"); + + list.destroy(); + }); + + QUnit.test('editable list view: do not commit changes twice', async function (assert) { + assert.expect(5); + + this.data.foo.onchanges = { + bar: function (obj) { + obj.foo = 'changed'; + }, + }; + + const list = await createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: ` + + + + `, + mockRPC(route, { method, model }) { + if (model === 'foo' && method === 'onchange') { + assert.step('onchange'); + } + return this._super(...arguments); + }, + }); + testUtils.mock.intercept(list, "field_changed", function (ev) { + assert.deepEqual(ev.data.changes, { bar: false }); + }, true); + + assert.strictEqual(list.$('.o_field_cell[name="foo"]')[0].textContent, 'yop'); + + await testUtils.dom.click(list.$('.o_field_cell[name="bar"]')[0]); + await testUtils.dom.click(list.$('.o_field_cell[name="bar"] label')[0]); + + await testUtils.dom.click(list.$('.o_list_button_save')[0]); + assert.strictEqual(list.$('.o_field_cell[name="foo"]')[0].textContent, 'changed'); + assert.verifySteps([ 'onchange' ]); + + list.destroy(); + }); + QUnit.test('editable list view: multi edition', async function (assert) { assert.expect(26); @@ -8543,6 +8641,7 @@ QUnit.module('Views', { await testUtils.dom.click(form.$('.o_field_x2many_list_row_add > a')); assert.containsOnce(form, '.o_data_row'); assert.hasClass(form.$('.o_data_row'), 'o_selected_row'); + await testUtils.fields.editInput(form.$('.o_field_x2many[name="o2m"] .o_selected_row input[name="display_name"]'), 'new value'); // click outside to unselect the row await testUtils.dom.click(document.body); @@ -10453,6 +10552,7 @@ QUnit.module('Views', { await testUtils.dom.click(list.$('.o_group_header:first')); // open group await testUtils.dom.click(list.$('.o_group_field_row_add a')); // add a new row + await testUtils.fields.editInput(list.$('input[name="foo"]'), 'xyz'); // make record dirty await testUtils.dom.click($('body')); // unselect row assert.verifySteps(['1']); assert.strictEqual(list.$('.o_data_row .o_data_cell:eq(1)').text(), 'Low',