[IMP] util.remove_view : remove redundant t-calls #116

kmod-odoo · 2024-07-19T12:13:22Z

While removing the view, removing the content having t-call in other views which are calling to have the content of it.

As there are many specific fixes available for this [16776,15322,14413,13404,13325,12205,12335 etc..], it's better to handle it in remove_view.

Before fix:

t-call with the same xml_id will remain in other views that are using it. So during access of that view, the system is raising an error "view not found."

<t name="Payment" t-name="website_sale.payment">
    <t t-call="website_sale.cart_summary"/>
</t>

After fix:

t-call will be removed, so no error will be raised.

<t name="Payment" t-name="website_sale.payment">
</t>

Traceback:

Error while render the template
ValueError: View 'website_sale.cart_summary' in website 1 not found
Template: website_sale.payment
Path: /t/t/div/div[1]/div/div[4]/div[1]/t
Node: <t t-call="website_sale.address_on_payment"/>

robodoo · 2024-07-19T12:14:09Z

diagnoza

Please address pre-commit

src/util/records.py

diagnoza · 2024-07-24T14:03:31Z

src/util/records.py

+            if module and module not in standard_modules or not module:
+                _logger.info(
+                    "The view %s with ID: %s has been updated, removed block with t-call for depricated view %s",
+                    ('"' + module + '.' + name + '"' if module else ""),
+                    vid,
+                    xml_id,
+                )


I'm not sure about this. We generally don't touch custom code, it's up to the client to adapt their views. Imho you should limit your query fetching the views to standard_modules (but including studio views).

We update all views when fields are removed, it may make sense to remove t-calls for views we are removing. But then this means that we would probably want to rename t-calls when we rename an xmlid of a view. @KangOl wdyt?

aj-fuentes · 2024-07-24T14:24:13Z

src/util/records.py

+                SELECT iv.id,imd.module,imd.name
+                  FROM ir_ui_view iv
+             LEFT JOIN ir_model_data imd
+                    ON iv.id = imd.res_id AND imd.model = 'ir.ui.view'


Suggested change

ON iv.id = imd.res_id AND imd.model = 'ir.ui.view'

ON iv.id = imd.res_id

AND imd.model = 'ir.ui.view'

aj-fuentes · 2024-07-24T14:24:47Z

src/util/records.py

+                  FROM ir_ui_view iv
+             LEFT JOIN ir_model_data imd
+                    ON iv.id = imd.res_id AND imd.model = 'ir.ui.view'
+                 WHERE {} ~ 't-call="{}"'


Suggested change

WHERE {} ~ 't-call="{}"'

WHERE {} ~ '\yt-call=(["'']){}\1"'

aj-fuentes · 2024-07-24T14:26:14Z

src/util/records.py

+        )
+        for vid, module, name in cr.fetchall():
+            with edit_view(cr, view_id=vid) as arch:
+                node = arch.find('.//t[@t-call="' + xml_id + '"]')


Suggested change

node = arch.find('.//t[@t-call="' + xml_id + '"]')

node = arch.find(".//t[@t-call='{}']".format(xml_id))

aj-fuentes · 2024-07-24T14:27:03Z

src/util/records.py

+            standard_modules = set(modules.get_modules()) - {"studio_customization"}
+            if module and module not in standard_modules or not module:
+                _logger.info(
+                    "The view %s with ID: %s has been updated, removed block with t-call for depricated view %s",


Suggested change

"The view %s with ID: %s has been updated, removed block with t-call for depricated view %s",

"The view %s with ID: %s has been updated, removed block with t-call for deprecated view %s",

KangOl · 2024-07-24T14:38:03Z

src/util/records.py

+                if node is not None:
+                    node.getparent().remove(node)
+            standard_modules = set(modules.get_modules()) - {"studio_customization"}
+            if module and module not in standard_modules or not module:


Can be simplified as

Suggested change

if module and module not in standard_modules or not module:

if not module or module not in standard_modules:

Hello @KangOl, suggested changes done could you please have a look? thanks :)

kmod-odoo · 2024-07-29T13:02:03Z

Thank you for your valuable input @diagnoza, @aj-fuentes and @KangOl.
Changes have been done. Could you please confirm the changes?

diagnoza · 2024-07-29T14:32:02Z

src/util/records.py

+            standard_modules = set(modules.get_modules()) - {"studio_customization"}
+            if not module or module not in standard_modules:
+                _logger.info(
+                    "The view %s with ID: %s has been updated, removed block with t-call for deprecated view %s",


Suggested change

"The view %s with ID: %s has been updated, removed block with t-call for deprecated view %s",

"The view %s with ID: %s has been updated, removed a t-call to a deprecated view %s",

Hi @diagnoza , Changes have been made, Could you please review it back @aj-fuentes @KangOl

aj-fuentes

In principle I'm OK with this. I will check with @KangOl once he's back if we should also do something similar for xmlid renames.

aj-fuentes · 2024-08-02T14:58:18Z

src/util/records.py

+        standard_modules = set(modules.get_modules()) - {"studio_customization"}
+        for vid, module, name in cr.fetchall():
+            with edit_view(cr, view_id=vid) as arch:
+                node = arch.find(".//t[@t-call='{}']".format(xml_id))


Can we have more than one t-call?

KangOl · 2024-08-02T16:52:06Z

src/util/records.py

+            r"""
+                SELECT iv.id, imd.module, imd.name
+                  FROM ir_ui_view iv
+             LEFT JOIN ir_model_data imd
+                    ON iv.id = imd.res_id
+                   AND imd.model = 'ir.ui.view'
+                 WHERE {} ~ '\yt-call=(["'']){}\1'
+            """.format(arch_col, xml_id)


Suggested change

r"""

SELECT iv.id, imd.module, imd.name

FROM ir_ui_view iv

LEFT JOIN ir_model_data imd

ON iv.id = imd.res_id

AND imd.model = 'ir.ui.view'

WHERE {} ~ '\yt-call=(["'']){}\1'

""".format(arch_col, xml_id)

format_query(cr, """

SELECT iv.id, imd.module, imd.name

FROM ir_ui_view iv

LEFT JOIN ir_model_data imd

ON iv.id = imd.res_id

AND imd.model = 'ir.ui.view'

WHERE {} ~ %s""", arch_col

),

[r"""\yt-call=(["']){}\1""".format(xml_id)]

Hello @aj-fuentes and @KangOl

Changes have been made. Could you please confirm them?

KangOl · 2024-08-05T12:35:36Z

Note that t-call will also search for views whose key match.
So we should also search for the key of the delete view (even if it hasn't any xmlid).

kmod-odoo · 2024-08-08T09:58:44Z

Note that t-call will also search for views whose key match. So we should also search for the key of the delete view (even if it hasn't any xmlid).

Hello @KangOl,
Changes were made. Could you please confirm them?

KangOl · 2024-08-08T14:30:58Z

upgradeci retry with always hr

kmod-odoo · 2024-08-09T09:10:57Z

upgradeci retry with always hr

Hello @KangOl,
matt is still failing due to accounting and other issues unrelated to this patch. Should I take any further action?

KangOl · 2024-08-12T11:27:48Z

Can you please add a test?

kmod-odoo · 2024-08-14T10:59:11Z

Hello @KangOl,

I have added unit test for remove_view. Could you please review it?

kmod-odoo · 2024-08-20T05:59:53Z

Hello @KangOl,
Just a gentle reminder to review this PR

KangOl · 2024-08-21T13:23:38Z

The key can be different from the xmlid. You need to handle the both.

kmod-odoo · 2024-09-04T12:47:27Z

Hello @KangOl,
Changes were made. Could you please confirm them?

kmod-odoo · 2024-09-10T05:10:59Z

Hello @KangOl,
Just a gentle reminder to review this PR

kmod-odoo · 2024-09-20T13:10:22Z

Gentle reminder @KangOl

kmod-odoo · 2024-10-10T05:32:37Z

Hello @KangOl
Just a friendly reminder to review this PR
cc: @aj-fuentes

aj-fuentes · 2024-10-10T08:28:39Z

src/util/records.py

@@ -1622,3 +1635,46 @@ def remove_act_window_view_mode(cr, model, view_mode):
        """,
        [view_mode, model, view_mode, view_mode],
    )
+
+
+def remove_redudant_tcalls(cr, match=None):


There is a typo in the name, also make this method private, is not something we

Suggested change

def remove_redudant_tcalls(cr, match=None):

def _remove_redundant_tcalls(cr, match):

Why having a default value for match?

aj-fuentes · 2024-10-10T08:29:41Z

src/util/records.py

+    if xml_id != "?" or key:
+        if xml_id != key:
+            remove_redudant_tcalls(cr, xml_id)
+            remove_redudant_tcalls(cr, key)
+        else:
+            remove_redudant_tcalls(cr, xml_id)


Can be made more direct. Also if xml_id is ? you don't need to remove by xml_id.

Suggested change

if xml_id != "?" or key:

if xml_id != key:

remove_redudant_tcalls(cr, xml_id)

remove_redudant_tcalls(cr, key)

else:

remove_redudant_tcalls(cr, xml_id)

if xml_id != "?":

remove_redundant_tcalls(cr, xml_id)

if key and key != xml_id:

remove_redundant_tcalls(cr, key)

aj-fuentes · 2024-10-10T08:31:25Z

src/util/records.py

+                "The view %s with ID: %s has been updated, removed a t-call to a deprecated view %s",
+                ('"{}.{}"'.format(module, name) if module else ""),


Suggested change

"The view %s with ID: %s has been updated, removed a t-call to a deprecated view %s",

('"{}.{}"'.format(module, name) if module else ""),

"The view %swith ID: %s has been updated, removed t-calls to deprecated %r",

("`{}.{}` ".format(module, name) if module else ""),

aj-fuentes · 2024-10-10T08:33:27Z

src/util/records.py

+            SELECT iv.id, imd.module, imd.name
+              FROM ir_ui_view iv


Suggested change

SELECT iv.id, imd.module, imd.name

FROM ir_ui_view iv

SELECT iv.id,

imd.module,

imd.name

FROM ir_ui_view iv

kmod-odoo · 2024-10-14T12:59:18Z

Hello @aj-fuentes, suggested changes done could you please have a look? thanks :)

aj-fuentes

Some small comments

aj-fuentes · 2024-10-14T13:52:47Z

src/util/records.py

+        """,
+            sql.SQL(arch_col),
+        ),
+        [r"""\yt-call=(["'']){}\1""".format(match)],


Since you are passing the regexp as parameter you don't need to duplicate the single quote.
You need to regexp escape match otherwise . will match any character. Since you anyway do a xmlnode.findall later this won't at least cause wrong upates. But you can decrease the output of the query by escaping the .match

Suggested change

[r"""\yt-call=(["'']){}\1""".format(match)],

[r"""\yt-call=(["']){}\1""".format(re.escape(match))],

aj-fuentes · 2024-10-14T13:53:31Z

src/util/records.py

+    This function removes the t-call of the removed view from all types of views
+    based on xml_id and key.


Suggested change

This function removes the t-call of the removed view from all types of views

based on xml_id and key.

This function removes the t-calls to `match`.

aj-fuentes · 2024-10-14T13:54:57Z

src/util/records.py

+    This function removes the t-call of the removed view from all types of views
+    based on xml_id and key.
+
+    :param str match: contains xml_id or key based on argument


Suggested change

:param str match: contains xml_id or key based on argument

:param str match: t-calls value to remove, typically it would be a view's xml_id or key

kmod-odoo · 2024-10-15T12:07:47Z

Thank you for your valuable input @aj-fuentes.
Changes have been done. Could you please confirm the changes?

aj-fuentes

LGTM

cc: @KangOl

kmod-odoo · 2024-10-22T12:09:36Z

Hello @KangOl,
Just a gentle reminder to review this PR

kmod-odoo · 2024-10-28T10:54:42Z

@KangOl Waiting for your final review as @aj-fuentes has approved this PR. Thanks!

KangOl · 2024-10-28T12:11:30Z

I would also adapt util.rename_xmlid to alter the t-call (we already change the key of the views).

while removing the view, removing the content having t-call in other views which are calling to have the content of it. As there are many specific fixes available for this, it's better to handle it in remove_view. Before fix: t-call with the same xml_id will remain in other views that are using it. So during access of that view, the system is raising an error "view not found." ``` <t name="Payment" t-name="website_sale.payment"> <t t-call="website_sale.cart_summary"/> </t> ``` After fix: t-call will be removed, so no error will be raised. ``` <t name="Payment" t-name="website_sale.payment"> </t> ``` Traceback: ``` Error while render the template ValueError: View 'website_sale.cart_summary' in website 1 not found Template: website_sale.payment Path: /t/t/div/div[1]/div/div[4]/div[1]/t Node: <t t-call="website_sale.address_on_payment"/> ```

Testcases for remove_view and rename_xmlid

kmod-odoo · 2024-12-02T12:36:35Z

Hello @KangOl @aj-fuentes
I have adapted the rename_xmlid method to handle t-call, t-value and t-name updates and added tests for it; could you please have a look?
Thanks :)

KangOl · 2024-12-02T15:18:02Z

src/util/records.py

+                        UPDATE ir_ui_view
+                           SET {arch} = regexp_replace({arch}, %s, %s, 'g')
+                         WHERE {arch} ~ %s
+                    """.format(arch=arch_col),


Use format_query instead.

Hello @KangOl suggested changes done could you please have a look? thanks :)

When renaming a view, we should also update its references in other views where it is used in t-call, t-value and t-name attributes. This ensures consistency and prevents broken references in dependent views.

kmod-odoo · 2024-12-10T10:15:28Z

Hello @KangOl
Just a gentle reminder to review this PR

kmod-odoo · 2024-12-19T05:20:29Z

Hello @KangOl
Just a friendly reminder to review this PR

cc: @aj-fuentes

kmod-odoo · 2025-01-02T11:38:40Z

Hello @KangOl
A gentle reminder, Could you please review this PR?

cc: @aj-fuentes

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from f3943de to 13fa9cb Compare July 19, 2024 12:47

kmod-odoo requested review from a team and jjmaksoud July 19, 2024 13:20

kmod-odoo force-pushed the master-improve_remove_view-kmod branch 2 times, most recently from 761da84 to 67feba3 Compare July 24, 2024 13:38

diagnoza reviewed Jul 24, 2024

View reviewed changes

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from 67feba3 to 3e4f381 Compare July 24, 2024 14:08

aj-fuentes reviewed Jul 24, 2024

View reviewed changes

KangOl reviewed Jul 24, 2024

View reviewed changes

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from 3e4f381 to 77b0f22 Compare July 25, 2024 12:15

diagnoza reviewed Jul 29, 2024

View reviewed changes

kmod-odoo force-pushed the master-improve_remove_view-kmod branch 2 times, most recently from eebe33a to d746716 Compare July 30, 2024 07:26

aj-fuentes reviewed Jul 30, 2024

View reviewed changes

aj-fuentes reviewed Aug 2, 2024

View reviewed changes

KangOl reviewed Aug 2, 2024

View reviewed changes

kmod-odoo force-pushed the master-improve_remove_view-kmod branch 3 times, most recently from 6036e6b to 20793ea Compare August 5, 2024 11:37

kmod-odoo force-pushed the master-improve_remove_view-kmod branch 2 times, most recently from 5d985e6 to 0f26f72 Compare August 8, 2024 09:32

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from e9489d4 to 624e52c Compare August 14, 2024 10:18

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from 624e52c to eb9b3de Compare September 4, 2024 12:13

aj-fuentes reviewed Oct 10, 2024

View reviewed changes

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from eb9b3de to 6afb5a5 Compare October 14, 2024 12:20

aj-fuentes reviewed Oct 14, 2024

View reviewed changes

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from 6afb5a5 to b9f0a16 Compare October 15, 2024 11:17

aj-fuentes approved these changes Oct 16, 2024

View reviewed changes

kmod-odoo added 2 commits November 28, 2024 18:14

[ADD] util: unittest

23df9e5

Testcases for remove_view and rename_xmlid

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from b9f0a16 to 4daaa9a Compare December 2, 2024 11:12

KangOl reviewed Dec 2, 2024

View reviewed changes

[IMP] util.rename_xmlid : Adapt t-call, t-value and t-name References

9b1dbb4

When renaming a view, we should also update its references in other views where it is used in t-call, t-value and t-name attributes. This ensures consistency and prevents broken references in dependent views.

kmod-odoo force-pushed the master-improve_remove_view-kmod branch from 4daaa9a to 9b1dbb4 Compare December 3, 2024 06:02

	ON iv.id = imd.res_id AND imd.model = 'ir.ui.view'
	ON iv.id = imd.res_id
	AND imd.model = 'ir.ui.view'

	node = arch.find('.//t[@t-call="' + xml_id + '"]')
	node = arch.find(".//t[@t-call='{}']".format(xml_id))

	"The view %s with ID: %s has been updated, removed block with t-call for depricated view %s",
	"The view %s with ID: %s has been updated, removed block with t-call for deprecated view %s",

	if module and module not in standard_modules or not module:
	if not module or module not in standard_modules:

	def remove_redudant_tcalls(cr, match=None):
	def _remove_redundant_tcalls(cr, match):

		"The view %s with ID: %s has been updated, removed a t-call to a deprecated view %s",
		('"{}.{}"'.format(module, name) if module else ""),

	[r"""\yt-call=(["'']){}\1""".format(match)],
	[r"""\yt-call=(["']){}\1""".format(re.escape(match))],

		This function removes the t-call of the removed view from all types of views
		based on xml_id and key.

	This function removes the t-call of the removed view from all types of views
	based on xml_id and key.
	This function removes the t-calls to `match`.

	:param str match: contains xml_id or key based on argument
	:param str match: t-calls value to remove, typically it would be a view's xml_id or key

[IMP] util.remove_view : remove redundant t-calls #116

Are you sure you want to change the base?

[IMP] util.remove_view : remove redundant t-calls #116

Conversation

kmod-odoo commented Jul 19, 2024

robodoo commented Jul 19, 2024

diagnoza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmod-odoo commented Jul 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aj-fuentes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KangOl commented Aug 5, 2024

kmod-odoo commented Aug 8, 2024

KangOl commented Aug 8, 2024

kmod-odoo commented Aug 9, 2024

KangOl commented Aug 12, 2024

kmod-odoo commented Aug 14, 2024

kmod-odoo commented Aug 20, 2024

KangOl commented Aug 21, 2024

kmod-odoo commented Sep 4, 2024

kmod-odoo commented Sep 10, 2024

kmod-odoo commented Sep 20, 2024

kmod-odoo commented Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmod-odoo commented Oct 14, 2024

aj-fuentes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmod-odoo commented Oct 15, 2024

aj-fuentes left a comment

Choose a reason for hiding this comment

kmod-odoo commented Oct 22, 2024

kmod-odoo commented Oct 28, 2024

KangOl commented Oct 28, 2024

kmod-odoo commented Dec 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmod-odoo commented Dec 10, 2024

kmod-odoo commented Dec 19, 2024

kmod-odoo commented Jan 2, 2025

kmod-odoo commented Oct 10, 2024 •

edited

Loading