From ed10f6a814a185bb682f82dc2de0041bb96181ba Mon Sep 17 00:00:00 2001 From: "Arthur Detroux (ard)" Date: Wed, 6 Mar 2024 09:04:51 +0000 Subject: [PATCH] [FIX] website_sale, *: only reload once on options changes *: web_editor, website Commit [1] introduced new image settings for the product page. When those settings were introduced, the editor was reloaded after the RPC to properly reflect when the settings were changed. While this was done, a data-reload was also added in the XML template of the options. This was not necessary as the data-reload only works for some methods which are not used in the image settings. Therefore, the data-reload did nothing. However, with commit [2], the settings were moved and combined with page options. So the image settings automatically inherited the page options behavior, which is to save and reload the page when a method with data-reload is called. This caused the editor to ask for a reload twice. This could cause the page to reload before the RPC was finished: - Add a 10 seconds sleep at the start of /shop/config/website controller - Go to products page - Enter edit mode - Change the number of products per row => Nothing happens Indeed, the page is reloaded immediately, before the 10 seconds rpc is actually done. This is also needed by the conversion of the SnippetsMenu to OWL: It seems like the race condition introduces a traceback. This commit fixes the issue by ignoring any save request from an option that will reload anyway. It also awaits the RPC to finish before reloading the page. [1]: https://github.com/odoo/odoo/commit/54c6d36cfbea31fe60b888bbb903b0c6f22216b3 [2]: https://github.com/odoo/odoo/commit/b274cf2427951761e59eb357fb796b54be375507#diff-754f6c793d6a168d006d2a9da108142b889036031ac1a2e62060c225d060f858 closes odoo/odoo#158468 X-original-commit: 556ae457b02e9c077d09fa9c3f9f1e6c6e26b345 Signed-off-by: Quentin Smetz (qsm) --- .../static/src/js/editor/snippets.options.js | 2 ++ .../static/src/js/editor/snippets.options.js | 17 +++++++++++ .../static/src/js/website_sale.editor.js | 28 +++++++++++-------- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/snippets.options.js b/addons/web_editor/static/src/js/editor/snippets.options.js index d3fd4442efc..deb72ac4f99 100644 --- a/addons/web_editor/static/src/js/editor/snippets.options.js +++ b/addons/web_editor/static/src/js/editor/snippets.options.js @@ -4438,8 +4438,10 @@ const SnippetOptionWidget = Widget.extend({ return; } + this.__willReload = requiresReload; // Call widget option methods and update $target await this._select(previewMode, widget); + this.__willReload = false; // If it is not preview mode, the user selected the option for good // (so record the action) diff --git a/addons/website/static/src/js/editor/snippets.options.js b/addons/website/static/src/js/editor/snippets.options.js index d8f8b0246e3..c4261a0e735 100644 --- a/addons/website/static/src/js/editor/snippets.options.js +++ b/addons/website/static/src/js/editor/snippets.options.js @@ -656,6 +656,7 @@ options.userValueWidgetsRegistry['we-gpspicker'] = GPSPicker; options.Class.include({ custom_events: Object.assign({}, options.Class.prototype.custom_events || {}, { 'google_fonts_custo_request': '_onGoogleFontsCustoRequest', + 'request_save': '_onSaveRequest', }), specialCheckAndReloadMethodsNames: ['customizeWebsiteViews', 'customizeWebsiteVariable', 'customizeWebsiteColor'], @@ -1029,6 +1030,22 @@ options.Class.include({ reloadEditor: true, }); }, + /** + * This handler prevents reloading the page twice with a `request_save` + * event when a widget is already going to handle reloading the page. + * + * @param {OdooEvent} ev + */ + _onSaveRequest(ev) { + // If a widget requires a reload, any subsequent request to save is + // useless, as the reload will save the page anyway. It can cause + // a race condition where the wysiwyg attempts to reload the page twice, + // so ignore the request. + if (this.__willReload) { + ev.stopPropagation(); + return; + } + } }); function _getLastPreFilterLayerElement($el) { diff --git a/addons/website_sale/static/src/js/website_sale.editor.js b/addons/website_sale/static/src/js/website_sale.editor.js index 28d64feed79..88f35a5fa56 100644 --- a/addons/website_sale/static/src/js/website_sale.editor.js +++ b/addons/website_sale/static/src/js/website_sale.editor.js @@ -51,14 +51,14 @@ options.registry.WebsiteSaleGridLayout = options.Class.extend({ */ setPpr: function (previewMode, widgetValue, params) { this.ppr = parseInt(widgetValue); - this.rpc('/shop/config/website', { 'shop_ppr': this.ppr }); + return this.rpc('/shop/config/website', { 'shop_ppr': this.ppr }); }, /** * @see this.selectClass for params */ setDefaultSort: function (previewMode, widgetValue, params) { this.default_sort = widgetValue; - this.rpc('/shop/config/website', { 'shop_default_sort': this.default_sort }); + return this.rpc('/shop/config/website', { 'shop_default_sort': this.default_sort }); }, //-------------------------------------------------------------------------- @@ -488,7 +488,9 @@ options.registry.WebsiteSaleProductPage = options.Class.extend({ }, _updateWebsiteConfig(params) { - this.rpc('/shop/config/website', params).then(() => this.trigger_up('request_save', {reload: true, optionSelector: this.data.selector})); + // TODO: Remove the request_save in master, it's already done by the + // data-page-options set to true in the template. + return this.rpc('/shop/config/website', params).then(() => this.trigger_up('request_save', {reload: true, optionSelector: this.data.selector})); }, _getZoomOptionData() { @@ -504,11 +506,11 @@ options.registry.WebsiteSaleProductPage = options.Class.extend({ const zoomOption = this._getZoomOptionData(); const updateWidth = this._updateWebsiteConfig.bind(this, { product_page_image_width: widgetValue }); if (!zoomOption || widgetValue !== "100_pc") { - updateWidth(); + await updateWidth(); } else { const defaultZoomOption = "website_sale.product_picture_magnify_click"; await this._customizeWebsiteData(defaultZoomOption, { possibleValues: zoomOption._methodsParams.optionsPossibleValues["customizeWebsiteViews"] }, true); - updateWidth(); + await updateWidth(); } }, @@ -519,7 +521,7 @@ options.registry.WebsiteSaleProductPage = options.Class.extend({ const zoomOption = this._getZoomOptionData(); const updateLayout = this._updateWebsiteConfig.bind(this, { product_page_image_layout: widgetValue }); if (!zoomOption) { - updateLayout(); + await updateLayout(); } else { const imageWidthOption = this.productDetailMain.dataset.image_width; let defaultZoomOption = widgetValue === "grid" ? "website_sale.product_picture_magnify_click" : "website_sale.product_picture_magnify_hover"; @@ -527,7 +529,7 @@ options.registry.WebsiteSaleProductPage = options.Class.extend({ defaultZoomOption = "website_sale.product_picture_magnify_click"; } await this._customizeWebsiteData(defaultZoomOption, { possibleValues: zoomOption._methodsParams.optionsPossibleValues["customizeWebsiteViews"] }, true); - updateLayout(); + await updateLayout(); } }, @@ -672,17 +674,21 @@ options.registry.WebsiteSaleProductPage = options.Class.extend({ 2: 'medium', 3: 'big', }[widgetValue]; - this.rpc('/shop/config/website', { + this.productPageGrid.dataset.image_spacing = spacing; + // TODO: Remove the request_save in master, it's already done by the + // data-page-options set to true in the template. + return this.rpc('/shop/config/website', { 'product_page_image_spacing': spacing, }).then(() => this.trigger_up('request_save', {reload: true, optionSelector: this.data.selector})); - this.productPageGrid.dataset.image_spacing = spacing; }, setColumns(previewMode, widgetValue, params) { - this.rpc('/shop/config/website', { + this.productPageGrid.dataset.grid_columns = widgetValue; + // TODO: Remove the request_save in master, it's already done by the + // data-page-options set to true in the template. + return this.rpc('/shop/config/website', { 'product_page_grid_columns': widgetValue, }).then(() => this.trigger_up('request_save', {reload: true, optionSelector: this.data.selector})); - this.productPageGrid.dataset.grid_columns = widgetValue; }, /**