From 9731e9be6d263f2b57e77b4ab37d1def3311449a Mon Sep 17 00:00:00 2001 From: Nicolas Bayet Date: Thu, 24 Nov 2022 14:38:25 +0000 Subject: [PATCH] [FIX] web_editor,project,note: ensure same history This commit add a mechanism to ensure that someone could never save changes from an history that diverge (in case there is a partition in the RTC network or a person A was disconnected while another person B saved changes that were not transmitted to person A). task-3002163 closes odoo/odoo#112099 Signed-off-by: David Monjoie (dmo) --- addons/hr/models/hr_job.py | 6 ++ addons/note/models/note.py | 7 +- addons/project/models/project.py | 3 + addons/web_editor/controllers/main.py | 42 +++++++++- .../static/src/js/backend/html_field.js | 14 +++- .../js/editor/odoo-editor/src/OdooEditor.js | 34 +++++++- .../odoo-editor/test/spec/collab.test.js | 4 +- .../static/src/js/wysiwyg/wysiwyg.js | 83 ++++++++++++------- odoo/tools/mail.py | 1 + 9 files changed, 154 insertions(+), 40 deletions(-) diff --git a/addons/hr/models/hr_job.py b/addons/hr/models/hr_job.py index 6b56c1346dd..089a16cf2b9 100644 --- a/addons/hr/models/hr_job.py +++ b/addons/hr/models/hr_job.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo import api, fields, models, _ +from odoo.addons.web_editor.controllers.main import handle_history_divergence class Job(models.Model): @@ -54,3 +55,8 @@ class Job(models.Model): if 'name' not in default: default['name'] = _("%s (copy)") % (self.name) return super(Job, self).copy(default=default) + + def write(self, vals): + if len(self) == 1: + handle_history_divergence(self, 'description', vals) + return super(Job, self).write(vals) diff --git a/addons/note/models/note.py b/addons/note/models/note.py index dd89b29c693..5d1c9d7f4ee 100644 --- a/addons/note/models/note.py +++ b/addons/note/models/note.py @@ -5,7 +5,7 @@ from random import randint from odoo import api, fields, models, _ from odoo.tools import html2plaintext - +from odoo.addons.web_editor.controllers.main import handle_history_divergence class Stage(models.Model): @@ -146,3 +146,8 @@ class Note(models.Model): def action_open(self): return self.write({'open': True}) + + def write(self, vals): + if len(self) == 1: + handle_history_divergence(self, 'memo', vals) + return super(Note, self).write(vals) diff --git a/addons/project/models/project.py b/addons/project/models/project.py index 61a20ffec75..a3af5b96464 100644 --- a/addons/project/models/project.py +++ b/addons/project/models/project.py @@ -10,6 +10,7 @@ from random import randint from odoo import api, Command, fields, models, tools, SUPERUSER_ID, _, _lt from odoo.addons.rating.models import rating_data +from odoo.addons.web_editor.controllers.main import handle_history_divergence from odoo.exceptions import UserError, ValidationError, AccessError from odoo.osv import expression from odoo.tools.misc import get_lang, format_date @@ -2075,6 +2076,8 @@ class Task(models.Model): return tasks def write(self, vals): + if len(self) == 1: + handle_history_divergence(self, 'description', vals) portal_can_write = False if self.env.user.has_group('base.group_portal') and not self.env.su: # Check if all fields in vals are in SELF_WRITABLE_FIELDS diff --git a/addons/web_editor/controllers/main.py b/addons/web_editor/controllers/main.py index 490e1039972..f96a7c49257 100644 --- a/addons/web_editor/controllers/main.py +++ b/addons/web_editor/controllers/main.py @@ -19,7 +19,7 @@ from odoo.http import request from odoo import http, tools, _, SUPERUSER_ID from odoo.addons.http_routing.models.ir_http import slug, unslug from odoo.addons.web_editor.tools import get_video_url_data -from odoo.exceptions import UserError, MissingError +from odoo.exceptions import UserError, MissingError, ValidationError from odoo.modules.module import get_resource_path from odoo.tools import file_open from odoo.tools.mimetypes import guess_mimetype @@ -31,6 +31,38 @@ from ..models.ir_attachment import SUPPORTED_IMAGE_MIMETYPES logger = logging.getLogger(__name__) DEFAULT_LIBRARY_ENDPOINT = 'https://media-api.odoo.com' +diverging_history_regex = 'data-last-history-steps="([0-9,]*?)"' + +def ensure_no_history_divergence(record, html_field_name, incoming_history_ids): + server_history_matches = re.search(diverging_history_regex, record[html_field_name]) + # Do not check old documents without data-last-history-steps. + if server_history_matches: + server_last_history_id = server_history_matches[1].split(',')[-1] + if server_last_history_id not in incoming_history_ids: + logger.error('The document was already saved from someone with a different history for model %r, field %r with id %r.', record._name, html_field_name, record.id) + raise ValidationError(_('The document was already saved from someone with a different history for model %r, field %r with id %r.', record._name, html_field_name, record.id)) + +def handle_history_divergence(record, html_field_name, vals): + # Do not handle history divergence if the field is not in the values. + if html_field_name not in vals: + return + incoming_html = vals[html_field_name] + incoming_history_matches = re.search(diverging_history_regex, incoming_html) + # When there is no incoming history id, it means that the value does not + # comes from the odoo editor or the collaboration was not activated. In + # project, it could come from the collaboration pad. In that case, we do not + # handle history divergences. + if incoming_history_matches is None: + return + incoming_history_ids = incoming_history_matches[1].split(',') + incoming_last_history_id = incoming_history_ids[-1] + + if record[html_field_name]: + ensure_no_history_divergence(record, html_field_name, incoming_history_ids) + + # Save only the latest id. + vals[html_field_name] = incoming_html[0:incoming_history_matches.start(1)] + incoming_last_history_id + incoming_html[incoming_history_matches.end(1):] + class Web_Editor(http.Controller): #------------------------------------------------------ # convert font into picture @@ -734,3 +766,11 @@ class Web_Editor(http.Controller): @http.route('/web_editor/tests', type='http', auth="user") def test_suite(self, mod=None, **kwargs): return request.render('web_editor.tests') + + @http.route("/web_editor/ensure_common_history", type="json", auth="user") + def ensure_common_history(self, model_name, field_name, res_id, history_ids): + record = request.env[model_name].browse([res_id]) + try: + ensure_no_history_divergence(record, field_name, history_ids) + except ValidationError: + return record[field_name] diff --git a/addons/web_editor/static/src/js/backend/html_field.js b/addons/web_editor/static/src/js/backend/html_field.js index d1562c85eff..ca3d5668611 100644 --- a/addons/web_editor/static/src/js/backend/html_field.js +++ b/addons/web_editor/static/src/js/backend/html_field.js @@ -59,7 +59,10 @@ export class HtmlFieldWysiwygAdapterComponent extends ComponentAdapter { const newCollaborationChannel = newProps.widgetArgs[0].collaborationChannel; if ( - (newValue !== newProps.editingValue && lastValue !== newValue) || + ( + stripHistoryIds(newValue) !== stripHistoryIds(newProps.editingValue) && + stripHistoryIds(lastValue) !== stripHistoryIds(newValue) + ) || !_.isEqual(lastRecordInfo, newRecordInfo) || !_.isEqual(lastCollaborationChannel, newCollaborationChannel)) { @@ -278,7 +281,7 @@ export class HtmlField extends Component { async updateValue() { const value = this.getEditingValue(); const lastValue = (this.props.value || "").toString(); - if (value !== null && !(!lastValue && value === "


") && value !== lastValue) { + if (value !== null && !(!lastValue && stripHistoryIds(value) === "


") && value !== lastValue) { this.props.setDirty(false); this.currentEditingValue = value; await this.props.update(value); @@ -636,6 +639,9 @@ HtmlField.extractProps = ({ attrs, field }) => { resizable: 'resizable' in attrs.options ? attrs.options.resizable : false, editorPlugins: [QWebPlugin], }; + if ('collaborative' in attrs.options) { + wysiwygOptions.collaborative = attrs.options.collaborative; + } if ('allowCommandImage' in attrs.options) { // Set the option only if it is explicitly set in the view so a default // can be set elsewhere otherwise. @@ -667,6 +673,10 @@ HtmlField.extractProps = ({ attrs, field }) => { registry.category("fields").add("html", HtmlField, { force: true }); +function stripHistoryIds(value) { + return value && value.replace(/\sdata-last-history-steps="[^"]*?"/, '') || value; +} + // Ensure all external links are opened in a new tab. const retargetLinks = (container) => { for (const externalLink of container.querySelectorAll(`a:not([href^="${location.origin}"]):not([href^="/"])`)) { diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js index cc32eaaf090..9769ef0728e 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js @@ -324,6 +324,9 @@ export class OdooEditor extends EventTarget { this.idSet(editable); this._historyStepsActive = true; this.historyReset(); + if (this.options.initialHistoryId) { + this.historySetInitialId(this.options.initialHistoryId); + } this._pluginCall('sanitizeElement', [editable]); @@ -964,13 +967,37 @@ export class OdooEditor extends EventTarget { this._firstStepId = firstStep.id; this._historySnapshots = [{ step: firstStep }]; this._historySteps.push(firstStep); + // The historyIds carry the ids of the steps that were dropped when + // doing a snapshot. + // Those historyIds are used to compare if the last step saved in the + // server is present in the current historySteps or historyIds to + // ensure it is the same history branch. + this._historyIds = []; + } + /** + * Set the initial document history id. + * + * To prevent a saving a document with a diverging history, we store the + * last history id in the first node of the document to the database. + * This method provide the initial document history id to the editor. + */ + historySetInitialId(id) { + this._historyIds.unshift(id); + } + /** + * Get all the history ids for the current history branch. + * + * See `_historyIds` in `historyReset`. + */ + historyGetBranchIds() { + return this._historyIds.concat(this._historySteps.map(s => s.id)); } historyGetSnapshotSteps() { // If the current snapshot has no time, it means that there is the no // other snapshot that have been made (either it is the one created upon // initialization or reseted by historyResetFromSteps). if (!this._historySnapshots[0].time) { - return this._historySteps; + return { steps: this._historySteps, historyIds: this.historyGetBranchIds() }; } const steps = []; let snapshot; @@ -989,9 +1016,10 @@ export class OdooEditor extends EventTarget { steps.push(snapshot.step); steps.reverse(); - return steps; + return { steps, historyIds: this.historyGetBranchIds() }; } - historyResetFromSteps(steps) { + historyResetFromSteps(steps, historyIds) { + this._historyIds = historyIds; this.observerUnactive(); for (const node of [...this.editable.childNodes]) { node.remove(); diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js index 1ea1347eb58..4a9f2d9952a 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/collab.test.js @@ -348,7 +348,7 @@ describe('Collaboration', () => { clientInfos.c1.editor._historyMakeSnapshot(); // Insure the snapshot is considered to be older than 30 seconds. clientInfos.c1.editor._historySnapshots[0].time = 1; - const steps = clientInfos.c1.editor.historyGetSnapshotSteps(); + const { steps } = clientInfos.c1.editor.historyGetSnapshotSteps(); clientInfos.c2.editor.historyResetFromSteps(steps); chai.expect(clientInfos.c2.editor._historySteps.map(x => x.id)).to.deep.equal([ @@ -403,7 +403,7 @@ describe('Collaboration', () => { // Fake the time of the snapshot so it is considered to be // older than 30 seconds. clientInfos.c1.editor._historySnapshots[0].time = 1; - const steps = clientInfos.c1.editor.historyGetSnapshotSteps(); + const { steps } = clientInfos.c1.editor.historyGetSnapshotSteps(); clientInfos.c3.editor.historyResetFromSteps(steps); // In the meantime client 2 send the step to client 1 diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 9172ab28753..c6c696daf73 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -125,6 +125,7 @@ const Wysiwyg = Widget.extend({ if (options.value) { this.$editable.html(options.value); } + const initialHistoryId = options.value && this._getInitialHistoryId(options.value); this.$editable.data('wysiwyg', this); this.$editable.data('oe-model', options.recordInfo.res_model); this.$editable.data('oe-id', options.recordInfo.res_id); @@ -167,6 +168,7 @@ const Wysiwyg = Widget.extend({ powerboxFilters: this.options.powerboxFilters || [], showEmptyElementHint: this.options.showEmptyElementHint, controlHistoryFromDocument: this.options.controlHistoryFromDocument, + initialHistoryId, getContentEditableAreas: this.options.getContentEditableAreas, getReadOnlyAreas: this.options.getReadOnlyAreas, getUnremovableElements: this.options.getUnremovableElements, @@ -459,23 +461,6 @@ const Wysiwyg = Widget.extend({ }; const rpcMutex = new Mutex(); - this._getCurrentRecord = async () => { - const records = await this._rpc({ - model: modelName, - method: "read", - args: [ - [resId], - [fieldName, 'write_date'] - ], - }); - records[0].body = records[0][fieldName]; - return records[0]; - } - this._getRecordWriteDate = (record) => { - const dateString = record.write_date.replace(/^(\d{4}-\d{2}-\d{2}) ((\d{2}:?){3})$/, '$1T$2Z'); - return new Date(dateString); - } - this._getNewPtp = () => { // Wether or not the history has been sent or received at least once. let historySyncAtLeastOnce = false; @@ -613,7 +598,6 @@ const Wysiwyg = Widget.extend({ }, CHECK_OFFLINE_TIME); this._peerToPeerLoading = new Promise(async (resolve) => { - this._currentRecord = await this._getCurrentRecord(); if (!ICE_SERVERS) { ICE_SERVERS = await this._rpc({route: '/web_editor/get_ice_servers'}); } @@ -692,8 +676,8 @@ const Wysiwyg = Widget.extend({ } if (firstClientId !== this._currentClientId) { - const historySteps = await this.ptp.requestClient(firstClientId, 'get_history_from_snapshot', undefined, { transport: 'rtc' }); - this.odooEditor.historyResetFromSteps(historySteps); + const { steps, historyIds } = await this.ptp.requestClient(firstClientId, 'get_history_from_snapshot', undefined, { transport: 'rtc' }); + this.odooEditor.historyResetFromSteps(steps, historyIds); const remoteSelection = await this.ptp.requestClient(firstClientId, 'get_collaborative_selection', undefined, { transport: 'rtc' }); if (remoteSelection) { this.odooEditor.onExternalMultiselectionUpdate(remoteSelection); @@ -868,6 +852,7 @@ const Wysiwyg = Widget.extend({ $editable.find('a.o_image, span.fa, i.fa').html(''); $editable.find('[aria-describedby]').removeAttr('aria-describedby').removeAttr('data-bs-original-title'); this.odooEditor && this.odooEditor.cleanForSave($editable[0]); + this._attachHistoryIds($editable[0]); return $editable.html(); }, /** @@ -1036,7 +1021,10 @@ const Wysiwyg = Widget.extend({ return closestElement(...args); }, async cleanForSave() { - this.odooEditor && this.odooEditor.cleanForSave(); + if (this.odooEditor) { + this.odooEditor.cleanForSave(); + this._attachHistoryIds(); + } if (this.snippetsMenu) { await this.snippetsMenu.cleanForSave(); @@ -2497,14 +2485,8 @@ const Wysiwyg = Widget.extend({ this.preSavePromiseReject = undefined; } try { - const fieldName = this.options.collaborationChannel.collaborationFieldName; - const currentContent = this._currentRecord[fieldName]; - const currentRecordDate = this._getRecordWriteDate(this._currentRecord); - const dbRecord = await this._getCurrentRecord(); - const dbContent = dbRecord[fieldName]; - const dbRecordDate = this._getRecordWriteDate(dbRecord); - - if (currentContent !== dbContent && dbRecordDate !== currentRecordDate) { + const serverContent = await this._ensureCommonHistory(); + if (serverContent) { const $dialogContent = $(QWeb.render('web_editor.collaboration-reset-dialog')); $dialogContent.append($(this.odooEditor.editable).clone()); const dialog = new Dialog(this, { @@ -2514,7 +2496,7 @@ const Wysiwyg = Widget.extend({ }); dialog.open({shouldFocusButtons:true}); - this.resetEditor(dbRecord.body); + this.resetEditor(serverContent); // We were in a peer to peer session before the conflict, join // it again immediately. this._joinPeerToPeer(); @@ -2526,6 +2508,32 @@ const Wysiwyg = Widget.extend({ resetPreSavePromise(); } }, + _getInitialHistoryId: function (value) { + const matchId = value.match(/data-last-history-steps="([0-9,]*?)"/); + return matchId && matchId[1]; + }, + /** + * When the collaboration is active, ensure that we do not try to save with + * a different history branch to the database. If the history is different, + * return the database html content. + * + * See `_historyIds` in `historyReset` in OdooEditor. + * + * @return {string} The database html content if the history is different. + */ + async _ensureCommonHistory() { + if (!this.ptp) return; + const historyIds = this.odooEditor.historyGetBranchIds(); + return this._rpc({ + route: '/web_editor/ensure_common_history', + params: { + history_ids: historyIds, + model_name: this.options.collaborationChannel.collaborationModelName, + field_name: this.options.collaborationChannel.collaborationFieldName, + res_id: this.options.collaborationChannel.collaborationResId, + }, + }); + }, _generateClientId: function () { // No need for secure random number. return Math.floor(Math.random() * Math.pow(2, 52)).toString(); @@ -2563,6 +2571,10 @@ const Wysiwyg = Widget.extend({ this.odooEditor.historyReset(); // Wait until editor is focused to join the peer to peer network. this.$editable[0].addEventListener('focus', this._joinPeerToPeer); + const initialHistoryId = value && this._getInitialHistoryId(value); + if (initialHistoryId) { + this.odooEditor.historySetInitialId(initialHistoryId); + } }, /** * Set contenteditable=false for all `.o_not_editable` found within node if @@ -2601,7 +2613,16 @@ const Wysiwyg = Widget.extend({ }); } } - } + }, + _attachHistoryIds(editable = this.odooEditor.editable) { + if (this.options.collaborative) { + const historyIds = this.odooEditor.historyGetBranchIds().join(','); + const firstChild = editable.children[0]; + if (firstChild) { + firstChild.setAttribute('data-last-history-steps', historyIds); + } + } + }, }); Wysiwyg.activeCollaborationChannelNames = new Set(); diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index b2f80cd7e60..53f2f43a34a 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -32,6 +32,7 @@ safe_attrs = clean.defs.safe_attrs | frozenset( ['style', 'data-o-mail-quote', # quote detection 'data-oe-model', 'data-oe-id', 'data-oe-field', 'data-oe-type', 'data-oe-expression', 'data-oe-translation-initial-sha', 'data-oe-nodeid', + 'data-last-history-steps', 'data-publish', 'data-id', 'data-res_id', 'data-interval', 'data-member_id', 'data-scroll-background-ratio', 'data-view-id', 'data-class', 'data-mimetype', 'data-original-src', 'data-original-id', 'data-gl-filter', 'data-quality', 'data-resize-width', 'data-shape', 'data-shape-colors', 'data-file-name', 'data-original-mimetype',