[IMP] util.remove_view : remove redundant t-calls #116
Conversation
|
|
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
from
f3943de to
13fa9cb
Compare
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
2 times, most recently
from
761da84 to
67feba3
Compare
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
from
67feba3 to
3e4f381
Compare
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
from
3e4f381 to
77b0f22
Compare
|
Thank you for your valuable input @diagnoza, @aj-fuentes and @KangOl. |
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
2 times, most recently
from
eebe33a to
d746716
Compare
There was a problem hiding this comment.
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.
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
3 times, most recently
from
6036e6b to
20793ea
Compare
|
Note that |
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
2 times, most recently
from
5d985e6 to
0f26f72
Compare
Hello @KangOl, |
|
upgradeci retry with always hr |
Hello @KangOl, |
|
Hello @KangOl, |
|
The |
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
from
624e52c to
eb9b3de
Compare
|
Hello @KangOl, |
|
Hello @KangOl, |
|
Gentle reminder @KangOl |
|
Hello @KangOl |
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
from
eb9b3de to
6afb5a5
Compare
|
Hello @aj-fuentes, suggested changes done could you please have a look? thanks :) |
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
from
6afb5a5 to
b9f0a16
Compare
|
Thank you for your valuable input @aj-fuentes. |
|
Hello @KangOl, |
|
@KangOl Waiting for your final review as @aj-fuentes has approved this PR. Thanks! |
|
I would also adapt |
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
force-pushed
the
master-improve_remove_view-kmod
branch
from
b9f0a16 to
4daaa9a
Compare
|
Hello @KangOl @aj-fuentes |
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
|
Hello @KangOl |
|
Hello @KangOl cc: @aj-fuentes |
|
Hello @KangOl cc: @aj-fuentes |
|
Hello @KangOl, cc: @aj-fuentes |
|
Hello @KangOl cc: @aj-fuentes |
| @@ -786,6 +796,34 @@ def rename_xmlid(cr, old, new, noupdate=None, on_collision="fail"): | |||
| # Don't change the view keys inconditionally to avoid changing unrelated views. | |||
| cr.execute("UPDATE ir_ui_view SET key = %s WHERE key = %s", [new, old]) | |||
|
|
|||
| # Adapting t-call, t-value and t-name references in views | |||
| search_pattern = r"""\yt-(call|name|value)=(["']){}\2""".format(old) | |||
There was a problem hiding this comment.
I'm not sure why would we update t-name or t-value. Do we have examples?
There was a problem hiding this comment.
For t-name If a cowed view of a renamed view exists, it will have the old t-name, so we can replace it with the new one. For t-value, I noticed some code using is_view_active(view_name), so I initially thought it would be beneficial to include both.
However, since there aren't many examples of t-value usage, it can be removed. We can just proceed with t-name and t-call change.
There was a problem hiding this comment.
The usage of is_view_active can be used in other context (t-if, t-elif) and should be handled explicitly with another search/replace.
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:
After fix:
Traceback: