From 03c4c855b96c0d5d1e79d9cc3ac215abbd55cfb1 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Thu, 20 Apr 2017 13:53:41 +0200 Subject: [REF] web: restore editable list views behaviors Since the new views, the editable list view lost some behaviors. These behaviors were indeed implemented in the form controller/renderer but as the new editable list view does not use an inline form view, these behaviors had to be implemented in the basic controller/renderer. For this to work, the list editable renderer had to be changed as it was doing work that should be done by the controller. The problem is even more complex because the x2m fields are using a list renderer but not a list controller. So moving code from the renderer to the controller obviously broke the x2m fields. Right now, the problem is solved by catching renderer events and forwarding new ones to the form controller (which handles the x2m specifically). The initial goal of this commit was to share the validation of records on save. Indeed, the fields were marked as invalid in the form view but not in the list view. Also, before the new views, the editable list view had different behaviors if they were used for a x2m field or not. As these behaviors are making sense, this commit tries to restore them. Basically, what we want is: - When a record is saved (form view save or list view line leaving), the invalid fields are marked (in red), the names of the related fields are notified to the user and the record is not left. - When a record is discarded (form view discard or list view discard), the user is asked to confirm if the record is dirty before making the record readonly. - When a record is discarded, if the record is a new one, then the record should be abandoned (removed as if never existed). In the form view this induces to go back in the history and in the list view, to remove a row. - For x2m fields, the notification of invalid fields is not triggered but the user is instead asked if he wants to discard the changes made to the row (indeed, this replaces the list "Discard" button, as non existent for x2m lists). - ... Saving, discarding, marking the fields as invalid and other behaviors are thus now shared behavior of basic views. The management of the dirty flag has also been moved to the model as it was handle by the controller for the form view but by the renderer for the list view. Now this flag is directly managed in the basic model (the model can have changes thanks to the `_changes` property but not be dirty (this is the case for creations)). This change however created a problem. The view manager is currently keeping asking if there are changes to discard at each action which might lead to leave a dirty record (appswitcher / url change / ...). It however did not discard anything as leaving if the user is ok with it will lead to an implicit discard. However, as the view manager might ask for this discard multiple times by second, the controller was marking the record as not dirty the first time but without discarding the changes. This is more complex to do now, as the dirty state is part of the model and that the renderer should match the model data. To solve this problem, the view manager now actually discard changes explicitely when asked to. Even though this had been optimized to not cause any rerender in some cases where it is not needed, this could cause some performance decreasing. However, this makes some cases more logical (opening the app switcher on a dirty form view then going back to the form view by hitting the "go back" button left the form view untouched although the user asked to discard it). This solution will be improved with the view manager refactoring. This commit is also making use of the `commitChanges` system which had been implemented for HTML fields. Indeed, these fields cannot know about all of their changes, so when hitting the save button, we asked those fields to commit their value. Using this system is a great way to make the `isValid` method of x2m fields synchronous. Indeed, before this commit, the method was sometimes asking the user if he wants to discard an invalid line before save. That case can be handled by the x2m `commitChanges` method: we consider that saving the lines of x2m is an operation that has to be done before considering the save, so we ask all the x2m fields to do so at that time. Also, the system was broken since a recent commit: we indeed protected the changes - save order with a mutex but unfortunately, the `commitChanges` method was part of the save and the changes it triggered were not able to be considered because of this mutex. This had not been detected by tests as there is not current way to test html fields. --- .../web/static/src/js/views/basic/basic_model.js | 46 ++++++++++++++-------- 1 file changed, 30 insertions(+), 16 deletions(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 ea8986f9da8..f3e69a6b453 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -219,7 +219,8 @@ var BasicModel = AbstractModel.extend({ discardChanges: function (id, options) { options = options || {}; var element = this.localData[id]; - var rollback = options.rollback || this.isNew(id); + var isNew = this.isNew(id); + var rollback = options.rollback || isNew; this._visitChildren(element, function (elem) { if (rollback && elem._savePoint) { if (elem._savePoint instanceof Array) { @@ -227,8 +228,10 @@ var BasicModel = AbstractModel.extend({ } else { elem._changes = _.extend({}, elem._savePoint); } + elem._isDirty = !isNew; } else { elem._changes = null; + elem._isDirty = false; } }); }, @@ -383,26 +386,26 @@ var BasicModel = AbstractModel.extend({ return element; }, /** - * return true if a record is dirty. A record is considered dirty if it has - * some unsaved changes. A list is considered dirty if its _changes key is - * set to an array of its new datapoints (possibly empty) + * Returns true if a record is dirty. A record is considered dirty if it has + * some unsaved changes, marked by the _isDirty property on the record or + * one of its subrecords. * - * @param {string} id id for a local resource + * @param {string} id - the local resource id * @returns {boolean} */ isDirty: function (id) { var isDirty = false; - var record = this.localData[id]; - this._visitChildren(record, function (r) { - if (r.type === "record" ? !_.isEmpty(r._changes) : r._changes) { + this._visitChildren(this.localData[id], function (r) { + if (r._isDirty) { isDirty = true; } }); return isDirty; }, /** - * Check if a record is new, meaning if it is in the process of being created - * and no actual record exists in db. + * Check if a localData is new, meaning if it is in the process of being + * created and no actual record exists in db. Note: if the localData is not + * of the "record" type, then it is always considered as not new. * * Note: A virtual id is a character string composed of an integer and has * a dash and other information. @@ -413,7 +416,11 @@ var BasicModel = AbstractModel.extend({ * @returns {boolean} */ isNew: function (id) { - var res_id = this.localData[id].res_id; + var data = this.localData[id]; + if (data.type !== "record") { + return false; + } + var res_id = data.res_id; if (typeof res_id === 'number') { return false; } else if (typeof res_id === 'string' && /^[0-9]+-/.test(res_id)) { @@ -707,15 +714,20 @@ var BasicModel = AbstractModel.extend({ record.res_ids.push(id); record.count++; } + + // Update the data directly or reload them + var def; if (shouldReload) { - // erase changes as they have been applied - record._changes = {}; - return self._fetchRecord(record); + def = self._fetchRecord(record); } else { _.extend(record.data, record._changes); - record._changes = {}; - return false; } + + // Erase changes as they have been applied + record._changes = {}; + record._isDirty = false; + + return def; }); } else { return $.when(record_id); @@ -837,6 +849,7 @@ var BasicModel = AbstractModel.extend({ var field; var defs = []; record._changes = record._changes || {}; + record._isDirty = true; // apply changes to local data for (var fieldName in changes) { @@ -874,6 +887,7 @@ var BasicModel = AbstractModel.extend({ _.each(fieldNames, function (name) { if (record._changes && record._changes[name] === record.data[name]) { delete record._changes[name]; + record._isDirty = !_.isEmpty(record._changes); } }); return self._fetchSpecialData(record).then(function (fieldNames2) { -- cgit v1.2.3