-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
[14.0][FIX] connector_importer: skip_fields_unchanged #136
base: 14.0
Are you sure you want to change the base?
[14.0][FIX] connector_importer: skip_fields_unchanged #136
Conversation
Hi @simahawk, |
2a9d9ce
to
952d043
Compare
Shall I update the module version? |
this will be done by bot on merge |
for k, v in current_values.items(): | ||
if values[k] != v: | ||
# FIXME: New value can be "1" and existing 1. Needs field conversion |
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.
We should probably use field's convert_to_read
w/ use_name_get=False
@@ -71,3 +71,31 @@ def test_find_domain(self): | |||
self.assertEqual( | |||
domain, [("name", "=", values["name"]), ("age", "=", values["age"])] | |||
) | |||
|
|||
def test_odoo_write_purge_values(self): |
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.
thanks for adding a test! 🤝
When
skip_fields_unchanged
is set for the record handler, the behaviour comparing existing with new values raises an error becauseread
returns a list and another one, thatid
is not invalues
.And the operator is wrong. Currently it would import the values only if they are the same as the existing.
I added a FIXME for later to convert the new values. Is this ok?