Skip to content

[IMP] snippets.convert_html_content#399

Closed
duau-odoo wants to merge 3 commits intoodoo:masterfrom
odoo-dev:master-perf-snippets-utils-duau
Closed

[IMP] snippets.convert_html_content#399
duau-odoo wants to merge 3 commits intoodoo:masterfrom
odoo-dev:master-perf-snippets-utils-duau

Conversation

@duau-odoo
Copy link
Copy Markdown
Contributor

@duau-odoo duau-odoo commented Mar 26, 2026

No description provided.

@robodoo
Copy link
Copy Markdown
Contributor

robodoo commented Mar 26, 2026

Pull request status dashboard

@duau-odoo duau-odoo force-pushed the master-perf-snippets-utils-duau branch from 5e4462a to 0883fe9 Compare March 27, 2026 09:20
Comment thread src/util/snippets.py Outdated
Comment on lines +121 to +123
modules = _website_dependent_modules(cr)
if not modules:
return
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change this html_fiels function has it will impact existing scripts.
You need a new function snippet_fields that return only the field that can contain snippets.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it will impact existing scripts, but I think, if the list is done properly, it would be better to also improve existing scripts and not only futures one? Or is that too big of a risk ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change existing scripts to use this new function when needed.

@duau-odoo duau-odoo force-pushed the master-perf-snippets-utils-duau branch from 0883fe9 to f73efd4 Compare March 27, 2026 11:46
@duau-odoo
Copy link
Copy Markdown
Contributor Author

@KangOl what do you think about this solution ?

Comment thread src/util/snippets.py Outdated
Comment on lines +122 to +138
def _website_dependent_modules(cr):
cr.execute(
"""
WITH RECURSIVE website_dependent_modules AS (
SELECT 'website'::varchar AS name
UNION
SELECT m.name
FROM ir_module_module m
JOIN ir_module_module_dependency d ON d.module_id = m.id
JOIN website_dependent_modules w ON w.name = d.name
WHERE m.state = 'installed'
)
SELECT name
FROM website_dependent_modules
"""
)
return [name for (name,) in cr.fetchall()]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have a separated function. The CTE can be directly included in the below query.

Other issues:

  • you can't always consider website to be installed.
  • and installed is not the correct state to check. Use util.INSTALLED_MODULE_STATES
  • the starting modules are version dependent. From Odoo 19, it's html_builder. Before, it was website and mass_mailing.

Comment thread src/util/snippets.py Outdated
cr,
converter_callback,
where_column="IS NOT NULL",
only_website_snippets=False,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can be future-proof and use an enum instead of a boolean (this file is python3 only)

FieldScope(Enum):
    ALL = 1
    SNIPPETS = 2
Suggested change
only_website_snippets=False,
scope=FieldScope.ALL,

Copy link
Copy Markdown
Contributor

@nseinlet nseinlet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@KangOl KangOl requested a review from aj-fuentes March 31, 2026 14:27
Comment thread src/util/snippets.py Outdated
Comment on lines +163 to +165
SELECT model, array_agg(name)
FROM _fields
GROUP BY model
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for consistent order across runs

Suggested change
SELECT model, array_agg(name)
FROM _fields
GROUP BY model
SELECT model, array_agg(name ORDER BY name)
FROM _fields
GROUP BY model
ORDER BY model

Comment thread src/util/snippets.py Outdated
:param str where_column: filtering such as
- "like '%abc%xyz%'"
- "~* '\yabc.*xyz\y'"
:param bool only_website_snippets: when True, only process stored HTML fields
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring is misaligned with the scope parameter.

KangOl added 2 commits March 31, 2026 17:30
Ignore `mail.mail` fields. Outgoing email content doesn't need to be
changed.
Sort fields to have a consistent execution across upgrades.
@KangOl KangOl force-pushed the master-perf-snippets-utils-duau branch from 8a46a2c to 3cbbb13 Compare March 31, 2026 15:38
Copy link
Copy Markdown
Contributor

@aj-fuentes aj-fuentes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Minimal comment.

Comment thread src/util/snippets.py Outdated
Comment thread src/util/snippets.py Outdated
Add parameter to targetting only snippets fields.
This accelerate the update by not searching on fields that cannot
contain snippets, like tasks or leads.

Co-authored-by: Augustin (duau) <duau@odoo.com>
@KangOl KangOl force-pushed the master-perf-snippets-utils-duau branch from 3cbbb13 to 2b72f80 Compare April 1, 2026 10:32
@KangOl KangOl changed the title [IMP] WIP only target website_* fields for snippets upgrade [IMP] snippets.convert_html_content Apr 1, 2026
@KangOl KangOl marked this pull request as ready for review April 1, 2026 10:39
Copy link
Copy Markdown
Contributor

@KangOl KangOl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robodoo rebase-ff r+

@robodoo
Copy link
Copy Markdown
Contributor

robodoo commented Apr 1, 2026

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request Apr 1, 2026
Ignore `mail.mail` fields. Outgoing email content doesn't need to be
changed.

Part-of: #399
Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 1, 2026
Sort fields to have a consistent execution across upgrades.

Part-of: #399
Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 1, 2026
Add parameter to targetting only snippets fields.
This accelerate the update by not searching on fields that cannot
contain snippets, like tasks or leads.

closes #399

Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
Co-authored-by: Augustin (duau) <duau@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 1, 2026
Ignore `mail.mail` fields. Outgoing email content doesn't need to be
changed.

Part-of: #399
Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 1, 2026
Sort fields to have a consistent execution across upgrades.

Part-of: #399
Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 1, 2026
Add parameter to targetting only snippets fields.
This accelerate the update by not searching on fields that cannot
contain snippets, like tasks or leads.

closes #399

Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
Co-authored-by: Augustin (duau) <duau@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 1, 2026
Ignore `mail.mail` fields. Outgoing email content doesn't need to be
changed.

Part-of: #399
Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 1, 2026
Sort fields to have a consistent execution across upgrades.

Part-of: #399
Related: odoo/upgrade#9792
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
@robodoo robodoo closed this in f455c09 Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants