-
-
Notifications
You must be signed in to change notification settings - Fork 153
[FIX][18.0] connector_importer: take full control of what are the required_keys #162
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: 18.0
Are you sure you want to change the base?
Conversation
|
Hi @simahawk, |
| req = dict(self.required) | ||
| req.update(self.work.options.mapper.get("required_keys", {})) | ||
|
|
||
| if self.required_keys_ignore_mapper: |
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.
if this case you should not use update before
|
|
||
| @property | ||
| def required_keys_ignore_mapper(self): | ||
| return self.work.options.mapper.get("required_keys_ignore_mapper", False) |
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.
If I remember well, we decided to use False as default in prior versions to not alter the behavior of existing import conf. I'd say, we can safely assume that if you are setting required_keys on the mapper you want to take full control anyway.
WDYT?
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.
It has been a long time since we discussed this topic, but AFAIR
this was necessary in order to suppress the required name field in the ProductProductMapper https://github.com/OCA/connector-interfaces/blob/18.0/connector_importer_product/components/product_product/mapper.py#L32
The present commit body states:
"Not forcing required default value if required_keys_ignore_mapper and required_keys are defined in options.mapper."
The business use case for this was around updating product.product records with weight and size values provided by an external system. The external system reads the product barcode and creates unique records based on it.
With an import type looking something like:
- model: product.product
options:
importer:
odoo_unique_key: barcode
override_existing: true
mapper:
name: product.product.mapper
required_keys:
"Code à barres": barcode
required_keys_ignore_mapper: true
source_key_rename:
"Code à barres": "barcode"
So here required_keys_ignore_mapper and required_keys worked in pair. If we set the default as True, it will ignore the value in self.required 3e9b0c0#diff-1dc9a52e0c26498d8a6b7eeb06f5c031c5f862c67263f047f2cd187be6c37811L55-L56 and create some issues when we use the importer in order to create records if we didn't check all required columns. But I agree required_keys on the mapper should contain all required keys according to the DB schema if we want take full control of it
Not forcing required default value if required_keys_ignore_mapper and required_keys are defined in options.mapper. Adding the new property required_keys_ignore_mapper in order to override the result of required_keys function
59e04dc to
3e9b0c0
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
connector_importer: take full control of what are the required_keys
Not forcing required default value if required_keys_ignore_mapper and required_keys are defined in options.mapper. Adding the new property required_keys_ignore_mapper in order to override the result of required_keys function
Commit ported from the following fix https://github.com/OCA/connector-interfaces/pull/141/commits to V18