Skip to content

[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.

Sign up for GitHub

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

Open
Ricardoalso wants to merge 2 commits into
base: 18.0
Choose a base branch
from
Open

[FIX][18.0] connector_importer: take full control of what are the required_keys #162

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abba0fd
connector_importer: take full control of what are the required_keys
Ricardoalso Sep 11, 2024
3e9b0c0
connector_importer: refactor required keys handling
Ricardoalso Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion connector_importer/components/mapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,18 @@ def required_keys(self, create=False):
The recordset will use this to show required fields to users.
"""
req = dict(self.required)
req.update(self.work.options.mapper.get("required_keys", {}))

if self.required_keys_ignore_mapper:
Copy link
Contributor

@simahawk simahawk Aug 4, 2025

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

req = self.work.options.mapper.get("required_keys", {})
else:
req.update(self.work.options.mapper.get("required_keys", {}))

return req

@property
def required_keys_ignore_mapper(self):
return self.work.options.mapper.get("required_keys_ignore_mapper", False)
Copy link
Contributor

@simahawk simahawk Aug 4, 2025

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?

Copy link
Contributor Author

@Ricardoalso Ricardoalso Aug 7, 2025 •
edited
Loading

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


translatable = []

def translatable_keys(self, create=False):
Expand Down