From b2c1d5ede2254e13bea32451faefa28a6eddcd4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Tue, 12 Jun 2018 16:05:18 +0200 Subject: [PATCH] [IMP] web: fix default sequence for list with handle The sequence had a static default value, but when there is an handle, it's important that new items are added at the top or the bottom of the list. This is a rare case where the defaul_get from the server will be ignored by the view for a certain field. This will work: - both in editable mode (inline) and in dialog mode, - both for X2Many and standalone list. --- addons/account/views/account_invoice_view.xml | 9 +- addons/sale/views/sale_views.xml | 11 +- .../static/src/js/fields/relational_fields.js | 6 +- .../static/src/js/views/basic/basic_model.js | 123 +++++++++++++++- .../js/views/list/list_editable_renderer.js | 4 +- .../static/src/js/views/list/list_renderer.js | 7 +- .../tests/fields/relational_fields_tests.js | 131 ++++++++++++++++++ addons/web/static/tests/views/list_tests.js | 33 +++++ doc/reference/javascript_reference.rst | 4 +- 9 files changed, 312 insertions(+), 16 deletions(-) diff --git a/addons/account/views/account_invoice_view.xml b/addons/account/views/account_invoice_view.xml index d36e616a870..8327d889601 100644 --- a/addons/account/views/account_invoice_view.xml +++ b/addons/account/views/account_invoice_view.xml @@ -70,6 +70,13 @@ + + @@ -686,7 +693,7 @@

Create a credit note

- Note that the easiest way to create a credit note is to do it directly + Note that the easiest way to create a credit note is to do it directly from the customer invoice.

diff --git a/addons/sale/views/sale_views.xml b/addons/sale/views/sale_views.xml index 7e763917c4c..0cc04eefd4c 100644 --- a/addons/sale/views/sale_views.xml +++ b/addons/sale/views/sale_views.xml @@ -271,6 +271,13 @@
%%
+ + No orders to upsell found

- These are orders with products invoiced based on ordered quantities, + These are orders with products invoiced based on ordered quantities, in the case you have delivered more than what was ordered.

- A typical example is the pre-paid hours of service, + A typical example is the pre-paid hours of service, where you want to sell extra hours to the customer because the initial hours have already been used.

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index 83ce59f1eb6..bf2c303a2bb 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -1293,12 +1293,14 @@ var FieldOne2Many = FieldX2Many.extend({ */ _onAddRecord: function (ev) { var self = this; + var data = ev.data || {}; + // we don't want interference with the components upstream. ev.stopPropagation(); if (this.editable) { if (!this.activeActions.create) { - if (ev.data.onFail) { - ev.data.onFail(); + if (data.onFail) { + data.onFail(); } } else if (!this.creatingRecord) { this.creatingRecord = true; 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 ae9f0611abf..822302630fa 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -183,6 +183,7 @@ var BasicModel = AbstractModel.extend({ fields: list.fields, fieldsInfo: list.fieldsInfo, parentID: list.id, + position: position, viewType: list.viewType, }; return this._makeDefaultRecord(list.model, params).then(function (id) { @@ -304,6 +305,104 @@ var BasicModel = AbstractModel.extend({ return record.id; }); }, + /** + * Compute the default value that the handle field should take. + * We need to compute this in order for new lines to be added at the correct position. + * + * @private + * @param {Object} listID + * @param {string} position + * @return {Object} empty object if no overrie has to be done, or: + * field: the name of the field to override, + * value: the value to use for that field + */ + _computeOverrideDefaultFields: function (listID, position) { + var list = this.localData[listID]; + var handleField; + + // Here listID is actually just parentID, it's not yet confirmed + // to be a list. + // If we are not in the case that interests us, + // listID will be undefined and this check will work. + if (!list) { + return {}; + } + + position = position || 'bottom'; + + // Let's find if there is a field with handle. + if (!list.fieldsInfo) { + return {}; + } + for (var field in list.fieldsInfo.list) { + if (list.fieldsInfo.list[field].widget === 'handle') { + handleField = field; + break; + // If there are 2 handle fields on the same list, + // we take the first one we find. + // And that will be alphabetically on the field name... + } + } + + if (!handleField) { + return {}; + } + + // We don't want to override the default value + // if the list is not ordered by the handle field. + var isOrderedByHandle = list.orderedBy + && list.orderedBy.length + && list.orderedBy[0].asc === true + && list.orderedBy[0].name === handleField; + + if (!isOrderedByHandle) { + return {}; + } + + // We compute the list (get) to apply the pending changes before doing our work, + // otherwise new lines might not be taken into account. + // We use raw: true because we only need to load the first level of relation. + var computedList = this.get(list.id, {raw: true}); + + // We don't need to worry about the position of a new line if the list is empty. + if (!computedList || !computedList.data || !computedList.data.length) { + return {}; + } + + // If there are less elements in the list than the limit of + // the page then take the index of the last existing line. + + // If the button is at the top, we want the new element on + // the first line of the page. + + // If the button is at the bottom, we want the new element + // after the last line of the page + // (= theorically it will be the first element of the next page). + + // We ignore list.offset because computedList.data + // will only have the current page elements. + + var index = Math.min( + computedList.data.length - 1, + position !== 'top' ? list.limit - 1 : 0 + ); + + // This positioning will almost be correct. There might just be + // an issue if several other lines have the same handleFieldValue. + + // TODO ideally: if there is an element with the same handleFieldValue, + // that one and all the following elements must be incremented + // by 1 (at least until there is a gap in the numbering). + + // We don't do it now because it's not an important case. + var handleFieldValue = computedList.data[index].data[handleField]; + handleFieldValue = position !== 'top' ? handleFieldValue : handleFieldValue - 1; + + return { + field: handleField, + value: handleFieldValue, + }; + }, /** * Delete a list of records, then, if the records have a parent, reload it. * @@ -1251,15 +1350,16 @@ var BasicModel = AbstractModel.extend({ */ _addX2ManyDefaultRecord: function (list, options) { var self = this; + var position = options && options.position || 'top'; var params = { context: this._getContext(list), fields: list.fields, fieldsInfo: list.fieldsInfo, parentID: list.id, + position: position, viewType: list.viewType, }; return this._makeDefaultRecord(list.model, params).then(function (id) { - var position = options && options.position || 'top'; list._changes.push({operation: 'ADD', id: id, position: position, isNew: true}); var record = self.localData[id]; list._cache[record.res_id] = id; @@ -3436,7 +3536,7 @@ var BasicModel = AbstractModel.extend({ _makeDefaultRecord: function (modelName, params) { var self = this; - var determineExtraFields = function() { + var determineExtraFields = function () { // Fields that are present in the originating view, that need to be initialized // Hence preventing their value to crash when getting back to the originating view var parentRecord = self.localData[params.parentID]; @@ -3446,12 +3546,12 @@ var BasicModel = AbstractModel.extend({ return []; var fieldsFromOrigin = _.filter(Object.keys(originView[parentRecord.viewType]), - function(fieldname) { + function (fieldname) { return params.fields[fieldname] !== undefined; }); return fieldsFromOrigin; - } + }; var fieldNames = Object.keys(params.fieldsInfo[params.viewType]); var fields_key = _.without(fieldNames, '__last_update'); @@ -3465,6 +3565,7 @@ var BasicModel = AbstractModel.extend({ context: params.context, }) .then(function (result) { + var record = self._makeDataPoint({ modelName: modelName, fields: params.fields, @@ -3475,6 +3576,20 @@ var BasicModel = AbstractModel.extend({ viewType: params.viewType, }); + // We want to overwrite the default value of the handle field (if any), + // in order for new lines to be added at the correct position. + // -> This is a rare case where the defaul_get from the server + // will be ignored by the view for a certain field (usually "sequence"). + + var overrideDefaultFields = self._computeOverrideDefaultFields( + params.parentID, + params.position + ); + + if (overrideDefaultFields) { + result[overrideDefaultFields.field] = overrideDefaultFields.value; + } + return self.applyDefaultValues(record.id, result, {fieldNames: _.union(fieldNames, extraFields)}) .then(function () { var def = $.Deferred(); 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 9ec82878e70..26b4f68d303 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 @@ -477,7 +477,7 @@ ListRenderer.include({ }, /** * The renderer needs to support reordering lines. This is only active in - * edit mode. The hasHandle attribute is used when there is a sequence + * edit mode. The handleField attribute is set when there is a sequence * widget. * * @override @@ -485,7 +485,7 @@ ListRenderer.include({ */ _renderBody: function () { var $body = this._super(); - if (this.hasHandle) { + if (this.handleField) { $body.sortable({ axis: 'y', items: '> tr.o_data_row', diff --git a/addons/web/static/src/js/views/list/list_renderer.js b/addons/web/static/src/js/views/list/list_renderer.js index e84d6687cc9..79304309eb6 100644 --- a/addons/web/static/src/js/views/list/list_renderer.js +++ b/addons/web/static/src/js/views/list/list_renderer.js @@ -50,8 +50,9 @@ var ListRenderer = BasicRenderer.extend({ */ init: function (parent, state, params) { this._super.apply(this, arguments); - this.hasHandle = false; - this.handleField = 'sequence'; + // This attribute lets us know if there is a handle widget on a field, + // and on which field it is set. + this.handleField = null; this._processColumns(params.columnInvisibleFields || {}); this.rowDecorations = _.chain(this.arch.attrs) .pick(function (value, key) { @@ -181,7 +182,6 @@ var ListRenderer = BasicRenderer.extend({ */ _processColumns: function (columnInvisibleFields) { var self = this; - self.hasHandle = false; self.handleField = null; this.columns = _.reject(this.arch.children, function (c) { var reject = c.attrs.modifiers.column_invisible; @@ -191,7 +191,6 @@ var ListRenderer = BasicRenderer.extend({ reject = columnInvisibleFields[c.attrs.name]; } if (!reject && c.attrs.widget === 'handle') { - self.hasHandle = true; self.handleField = c.attrs.name; } return reject; diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index b96e21c2855..c4267c159d3 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -9074,6 +9074,137 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('one2many with sequence field, override default_get, bottom when inline', function (assert) { + assert.expect(2); + + this.data.partner.records[0].turtles = [3, 2, 1]; + + this.data.turtle.fields.turtle_int.default = 10; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch:'
' + + '' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + viewOptions: { + mode: 'edit', + }, + }); + + // starting condition + assert.strictEqual($('.o_data_cell').text(), "blipyopkawa"); + + // click add a new line + // save the record + // check line is at the correct place + + var inputText = 'ninja'; + form.$('.o_field_x2many_list_row_add a').click(); + form.$('.o_input[name="turtle_foo"]').val(inputText).trigger('input'); + form.$buttons.find('.o_form_button_save').click(); + + assert.strictEqual($('.o_data_cell').text(), "blipyopkawa" + inputText); + form.destroy(); + }); + + QUnit.test('one2many with sequence field, override default_get, top when inline', function (assert) { + assert.expect(2); + + this.data.partner.records[0].turtles = [3, 2, 1]; + + this.data.turtle.fields.turtle_int.default = 10; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch:'
' + + '' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + viewOptions: { + mode: 'edit', + }, + }); + + // starting condition + assert.strictEqual($('.o_data_cell').text(), "blipyopkawa"); + + // click add a new line + // save the record + // check line is at the correct place + + var inputText = 'ninja'; + form.$('.o_field_x2many_list_row_add a').click(); + form.$('.o_input[name="turtle_foo"]').val(inputText).trigger('input'); + form.$buttons.find('.o_form_button_save').click(); + + assert.strictEqual($('.o_data_cell').text(), inputText + "blipyopkawa"); + form.destroy(); + }); + + QUnit.test('one2many with sequence field, override default_get, bottom when popup', function (assert) { + assert.expect(3); + + this.data.partner.records[0].turtles = [3, 2, 1]; + + this.data.turtle.fields.turtle_int.default = 10; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch:'
' + + '' + + '' + + '' + + '' + + '' + + '' + + // NOTE: at some point we want to fix this in the framework so that an invisible field is not required. + '' + + '' + + '' + + '' + + '', + res_id: 1, + viewOptions: { + mode: 'edit', + }, + }); + + // starting condition + assert.strictEqual($('.o_data_cell').text(), "blipyopkawa"); + + // click add a new line + // save the record + // check line is at the correct place + + var inputText = 'ninja'; + $('.o_field_x2many_list_row_add a').click(); + $('.o_input[name="turtle_foo"]').val(inputText).trigger('input'); + $('.modal .modal-footer .btn-primary:first').click(); + + assert.strictEqual($('.o_data_cell').text(), "blipyopkawa" + inputText); + + $('.o_form_button_save').click(); + assert.strictEqual($('.o_data_cell').text(), "blipyopkawa" + inputText); + form.destroy(); + }); + QUnit.test('one2many with sequence field, fetch name_get from empty list', function (assert) { // There was a bug where a RPC would fail because no route was set. // The scenario is: diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index ed57c6968ac..3aa5f6e7c70 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -3613,6 +3613,39 @@ QUnit.module('Views', { list.destroy(); }); + + QUnit.test('list with handle field, override default_get, bottom when inline', function (assert) { + assert.expect(2); + + this.data.foo.fields.int_field.default = 10; + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: + '' + + '' + + '' + +'', + }); + + // starting condition + assert.strictEqual($('.o_data_cell').text(), "blipblipyopgnap"); + + // click add a new line + // save the record + // check line is at the correct place + + var inputText = 'ninja'; + $('.o_list_button_add').click(); + list.$('.o_input[name="foo"]').val(inputText).trigger('input'); + $('.o_list_button_add').click(); + + assert.strictEqual($('.o_data_cell').text(), "blipblipyopgnap" + inputText); + + list.destroy(); + }); }); }); diff --git a/doc/reference/javascript_reference.rst b/doc/reference/javascript_reference.rst index 720157fef37..6cbfd3563dd 100644 --- a/doc/reference/javascript_reference.rst +++ b/doc/reference/javascript_reference.rst @@ -1478,9 +1478,11 @@ order. - handle (HandleWidget) - This field's job is to be displayed as a *handle* in a list view, and allow + This field's job is to be displayed as a *handle* in a list view, and allows reordering the various records by drag and dropping lines. + .. warning:: Having more than one field with a handle widget on the same list is not supported. + - Supported field types: *integer*