From b4fdc6c01de4fd7987dde96d335e9f35d1119952 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Thu, 12 Jan 2023 14:25:27 +0000 Subject: [PATCH] [FIX] website: allow to reinstall website after deleted user-websites Before this commit, this flow was broken: - Install website - Create a new website of your own (not using the one created automatically from XML data) - Choose another color palette for that website - Uninstall the website app - Reinstall the website app - Try to choose another color palette for any website => It does not work Indeed, after the uninstallation, the DB is left in an invalid state: the SCSS customizations attachments of the website that was created by the user are not removed, they just have their website_id field emptied. Some code made at [1] was already there to remove those attachments. The problem is that it only worked for websites which were created by XML data (at website installation), not by the user. Indeed, the `unlink` method is not called during uninstallation to remove records that were created by the user, thus the `unlink` override was not called either. See [2] for some details. This fixes the issues by moving this attachment cleaning code in a dedicated method, called in `unlink` but also in the `uninstall_hook` of the website app. This also takes the opportunity to refactor the code involved, in particular to not even consider customized attachments which do not have a website_id. [1]: https://github.com/odoo/odoo/commit/2f361bec36dff09181b96d140d62c477cdf013a1 [2]: https://github.com/odoo/odoo/pull/97852#pullrequestreview-1067851656 opw-3127531 closes odoo/odoo#110338 X-original-commit: 988eafa03b57be3b3a7f110650c61ce8fda88e31 Signed-off-by: Romain Derie (rde) --- addons/web_editor/models/assets.py | 17 +++++++++++++--- addons/website/__init__.py | 8 ++++++++ addons/website/models/assets.py | 31 ++++++++++++++++++++++-------- addons/website/models/website.py | 12 ++++++++---- 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/addons/web_editor/models/assets.py b/addons/web_editor/models/assets.py index 4c304416dd8..4fcc5974893 100644 --- a/addons/web_editor/models/assets.py +++ b/addons/web_editor/models/assets.py @@ -71,8 +71,8 @@ class Assets(models.AbstractModel): 'mimetype': (file_type == 'js' and 'text/javascript' or 'text/scss'), 'datas': datas, 'url': custom_url, + **self._save_asset_attachment_hook(), } - new_attach.update(self._save_asset_hook()) self.env["ir.attachment"].create(new_attach) # Create an asset with the new attachment @@ -216,10 +216,21 @@ class Assets(models.AbstractModel): return self.env['ir.asset'].search([('path', 'like', url)]) @api.model - def _save_asset_hook(self): + def _save_asset_attachment_hook(self): """ Returns the additional values to use to write the DB on customized - attachment and asset creation. + ir.attachment creation. + + Returns: + dict + """ + return {} + + @api.model + def _save_asset_hook(self): + """ + Returns the additional values to use to write the DB on customized + ir.asset creation. Returns: dict diff --git a/addons/website/__init__.py b/addons/website/__init__.py index ab49f7f88cf..d0db9014124 100644 --- a/addons/website/__init__.py +++ b/addons/website/__init__.py @@ -19,6 +19,14 @@ def uninstall_hook(cr, registry): env['ir.asset'].search(website_domain).unlink() env['ir.ui.view'].search(website_domain).with_context(active_test=False, _force_unlink=True).unlink() + # Cleanup records which are related to websites and will not be autocleaned + # by the uninstall operation. This must be done here in the uninstall_hook + # as during an uninstallation, `unlink` is not called for records which were + # created by the user (not XML data). Same goes for @api.ondelete available + # from 15.0 and above. + env['website'].search([])._remove_attachments_on_website_unlink() + + # Properly unlink website_id from ir.model.fields def rem_website_id_null(dbname): db_registry = odoo.modules.registry.Registry.new(dbname) with db_registry.cursor() as cr: diff --git a/addons/website/models/assets.py b/addons/website/models/assets.py index de62ddcb83b..c4793ae1b7a 100644 --- a/addons/website/models/assets.py +++ b/addons/website/models/assets.py @@ -143,7 +143,13 @@ class Assets(models.AbstractModel): self = self.sudo() website = self.env['website'].get_current_website() res = super()._get_custom_attachment(custom_url, op=op) - return res.with_context(website_id=website.id).filtered(lambda x: not x.website_id or x.website_id == website) + # See _save_asset_attachment_hook -> it is guaranteed that the + # attachment we are looking for has a website_id. When we serve an + # attachment we normally serve the ones which have the right website_id + # or no website_id at all (which means "available to all websites", of + # course if they are marked "public"). But this does not apply in this + # case of customized asset files. + return res.with_context(website_id=website.id).filtered(lambda x: x.website_id == website) @api.model def _get_custom_asset(self, custom_url): @@ -159,15 +165,24 @@ class Assets(models.AbstractModel): res = super()._get_custom_asset(custom_url) return res.with_context(website_id=website.id).filter_duplicate() + @api.model + def _add_website_id(self, values): + website = self.env['website'].get_current_website() + values['website_id'] = website.id + return values + + @api.model + def _save_asset_attachment_hook(self): + """ + See web_editor.Assets._save_asset_attachment_hook + Extend to add website ID at ir.attachment creation. + """ + return self._add_website_id(super()._save_asset_attachment_hook()) + @api.model def _save_asset_hook(self): """ See web_editor.Assets._save_asset_hook - Extend to add website ID at attachment creation. + Extend to add website ID at ir.asset creation. """ - res = super()._save_asset_hook() - - website = self.env['website'].get_current_website() - if website: - res['website_id'] = website.id - return res + return self._add_website_id(super()._save_asset_hook()) diff --git a/addons/website/models/website.py b/addons/website/models/website.py index d7c95485282..48214aa0814 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -288,6 +288,14 @@ class Website(models.Model): raise UserError(_('You must keep at least one website.')) def unlink(self): + self._remove_attachments_on_website_unlink() + + companies = self.company_id + res = super().unlink() + companies._compute_website_id() + return res + + def _remove_attachments_on_website_unlink(self): # Do not delete invoices, delete what's strictly necessary attachments_to_unlink = self.env['ir.attachment'].search([ ('website_id', 'in', self.ids), @@ -297,10 +305,6 @@ class Website(models.Model): ('url', 'ilike', '.assets\\_'), ]) attachments_to_unlink.unlink() - companies = self.company_id - res = super(Website, self).unlink() - companies._compute_website_id() - return res def create_and_redirect_configurator(self): self._force()