-
Notifications
You must be signed in to change notification settings - Fork 68
[FIX] util/records:Mark records noupdate #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
upgradeci retry with always only documents |
Indeed, it should not be automatically determined. |
eca8e4e
to
a9a0894
Compare
Hello @aj-fuentes , @KangOl Thanks for your quick review. I have update patch so it will work only whenever new record will be created. your insight will be valuable and it will prevent blocking for future case also That's the main purpose of this fix |
64cc32c
to
938f3a0
Compare
src/util/records.py
Outdated
data_node = doc.find(".//data") | ||
if (data_node is not None and eval(data_node.attrib.get("noupdate", "0"))) or ( | ||
eval(node.attrib.get("noupdate", "0")) | ||
): | ||
force_noupdate_to_record = True | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to NOT do this.
data_node = doc.find(".//data") | |
if (data_node is not None and eval(data_node.attrib.get("noupdate", "0"))) or ( | |
eval(node.attrib.get("noupdate", "0")) | |
): | |
force_noupdate_to_record = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if record is noupdate False
which is created in that case ?. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if record is noupdate
False
which is created in that case ?. :(
I don't understand what you meant.
938f3a0
to
fbb5ae7
Compare
src/util/records.py
Outdated
@@ -1210,6 +1223,17 @@ def add_ref(ref): | |||
if reset_write_metadata and write_data: | |||
cr.execute("UPDATE {} SET write_uid=%s, write_date=%s WHERE id=%s".format(table), write_data) | |||
|
|||
# force noupdate newly created record and inherit records |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you meant here. But I think what you could do is to put the noupdate
flag in new_root
such that any new record created from it gets the noupdate flag.
Doing this post load may miss some newly created records (and children).
src/util/records.py
Outdated
data_node = doc.find(".//data") | ||
if (data_node is not None and eval(data_node.attrib.get("noupdate", "0"))) or ( | ||
eval(node.attrib.get("noupdate", "0")) | ||
): | ||
force_noupdate_to_record = True | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if record is noupdate
False
which is created in that case ?. :(
I don't understand what you meant.
OK, I think I finally understand the initial problem. The purpose of this function is to bypass the diff --git src/util/records.py src/util/records.py
index 0ea524d..a7360aa 100644
--- src/util/records.py
+++ src/util/records.py
@@ -1108,7 +1108,8 @@ def __update_record_from_xml(
return
else:
# The xmlid doesn't already exists, nothing to reset
- reset_write_metadata = noupdate = reset_translations = False
+ reset_write_metadata = reset_translations = False
+ noupdate = True
fields = None
write_data = None Then we have a second issue with the xmlids of the parent records in case of |
fbb5ae7
to
0b16db2
Compare
Indeed @KangOl and @aj-fuentes , I was thinkin for the case where the record with Thanks |
0b16db2
to
f96207d
Compare
f96207d
to
13eaf26
Compare
@sagu-odoo can you test with the last version? |
Yes, this is filtered by the |
for the case I am testing for that it's not working. due to this, during debugging that i found that mail_alias didn't yield here > /home/odoo/odoo18/upgrade-util/src/util/inherit.py(106)direct_inherit_parents()
-> for inh in inhs:
(Pdb) n
> /home/odoo/odoo18/upgrade-util/src/util/inherit.py(107)direct_inherit_parents()
-> if inh.model == model and cmp_(inh):
(Pdb) parent
'mail.alias'
(Pdb) cmp_(inh)
True
(Pdb) inh.model == model
False
(Pdb) model
'documents.document'
(Pdb) inh.model
'mail.alias.mixin' |
Ho! The usage of |
Don't yield grand_parent multiple times.
13eaf26
to
0d5f106
Compare
Function that yield inherit*s* parent and the delegate m2o field.
Use the new `inherits_parents` function to get the inherits models.
As the purpose of this function is to bypass the `noupdate=1` attribute in the XML declarations, when we create the record, we should flag it as noupdate.
For models with `_inherits`, an xmlid is created for the parent record. We also need to update those xmlids. As example, when called on a `product.product` record, we now also set the flag on the xmlid of the `product.template` record.
Set the record in noupdate=False using the `force_noupdate` function, taking adventage of its new behavior of also updating the parent records.
0d5f106
to
75b0d78
Compare
Hello @KangOl , I have tested with this new patch but here due to _cached decorator it not working because if model inherit is changed during upgrade those are not updated because it already in |
If particular xml don't have noupdate marked but parent
data
node have noupdate in that case from update_records_from_xml creating record as noupdatefalse
which is causing issue in future upgrade because it going for delete. For preventing such case adding noupdate flag to record and inherit created record acccoring to this commit by that//data
node or thatxml node
itself.use case:
client db don't have the
documents.document_internal_folder
xmlid which is created when migrated to 18.0 from this script with noupdate false and because data node have noupdate true but particular xml don't have noupdate that is why it created as noupdate false and other inherit records also with noupdate and after migrating from onwards version related to that mail.alias records xml creating according to thisand later on version in 18.3 if related inherit model is changed but still some how use case of those records for preventing in that case.
before upgrade
before fix
after fix
upg-2987024
opw-4874805