[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 |
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 |
kmod-odoo
force-pushed
the
master-improve_remove_view-kmod
branch
from
b9f0a16 to
4daaa9a
Compare
|
Hello @KangOl @aj-fuentes |
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 |
src/util/records.py
Outdated
| @@ -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.
There was a problem hiding this comment.
Let's go for t-name and t-call only for the moment.
There was a problem hiding this comment.
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
9b1dbb4 to
6afbfb4
Compare
|
Hello @aj-fuentes, |
|
The commits are out of sync. There is no point of having a commit only for the tests. Split it and move the corresponding test to each of the other two commits. I'm still hesitant on whether all commits can be squashed... IMO if we need to revert this we would probably want to revert it all. @KangOl wdyt? |
|
Hello @KangOl cc: @aj-fuentes |
|
Hello @KangOl cc: @aj-fuentes |
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"/>
```
When renaming a view, we should also update its references in other views where it is used in t-call 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
6afbfb4 to
cf6f3db
Compare
|
Hello @KangOl @aj-fuentes I have moved the corresponding tests to each of the other two commits. I'm now waiting for your input on whether I should squash all commits or keep them separate. |
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: