From fd21e0b6885c485a1af0509fe8dbe9a239b35f8b Mon Sep 17 00:00:00 2001 From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me> Date: Thu, 11 Jun 2026 09:49:39 +0200 Subject: [PATCH 1/4] feat(#188): warn-by-default guardrail for templates without variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After every product.template import, automatically check the just-imported templates and warn if any have no variant (Odoo's load() doesn't auto-create them) — so the problem surfaces at import time, not in production. The check is scoped to the import's records and skipped for other models. - variant_manager.check_missing_variants_after_import(config, model, id_map, fix). - run_import: new fix_missing_variants param; calls the guardrail before returning. - import CLI: --fix-missing-variants flag (create inline instead of only warning). - 6 unit tests + docs. Follow-up to #188 / PR #201. --- .gitignore | 3 ++ docs/guides/odoo_workflows.md | 23 +++++++++ pydoclint-baseline.txt | 2 +- src/fluvo/__main__.py | 8 ++++ src/fluvo/importer.py | 11 +++++ src/fluvo/lib/actions/variant_manager.py | 58 ++++++++++++++++++++++ tests/test_variant_manager.py | 61 +++++++++++++++++++++++- 7 files changed, 164 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index b22a68fc..902635a3 100644 --- a/.gitignore +++ b/.gitignore @@ -72,3 +72,6 @@ htmlcov/ # agent tooling state .crush/ + +# stray test artifact (import fail file written to cwd) +/fail.csv diff --git a/docs/guides/odoo_workflows.md b/docs/guides/odoo_workflows.md index c5690691..00698f2b 100644 --- a/docs/guides/odoo_workflows.md +++ b/docs/guides/odoo_workflows.md @@ -271,6 +271,29 @@ fluvo workflow create-missing-variants --connection-file conf/connection.conf \ | `--batch-size` | Number of variants to create per RPC call (default: 200). | | `--dry-run` | Only report how many templates lack a variant; create nothing. | +### Automatic guardrail during import + +You don't have to remember to run the command. After **every `product.template` +import**, fluvo automatically checks the just-imported templates and **warns** if +any ended up without a variant — so you find out at import time, not in production: + +```text +WARNING 3 imported product template(s) have NO variants and are unusable in + sales/purchase orders and BoMs ... Fix with 'fluvo workflow + create-missing-variants', or re-run with --fix-missing-variants. +``` + +Pass `--fix-missing-variants` to the `import` command to create them inline instead +of only warning: + +```bash +fluvo import --connection-file conf/connection.conf --file product_template.csv \ + --model product.template --fix-missing-variants +``` + +The check is scoped to the records from that import and is skipped for every other +model, so it adds no overhead to non-product imports. + --- ## Data Processing Workflows diff --git a/pydoclint-baseline.txt b/pydoclint-baseline.txt index 9ce8f814..852ad27c 100644 --- a/pydoclint-baseline.txt +++ b/pydoclint-baseline.txt @@ -63,7 +63,7 @@ src/fluvo/importer.py DOC203: Function `_get_env_from_config` return type(s) in docstring not consistent with the return annotation. Return annotation types: ['Optional[str]']; docstring return section types: [''] DOC111: Function `_run_preflight_checks`: The option `--arg-type-hints-in-docstring` is `False` but there are type hints in the docstring arg list DOC101: Function `run_import`: Docstring contains fewer arguments than in function signature. - DOC103: Function `run_import`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [adaptive_throttle: bool, auto_clean: bool, auto_create_refs: bool, auto_defer: bool, auto_groupby: bool, batch_delay: float, batch_size: int, check_refs: str, config: Union[str, dict[str, Any]], context: Any, deferred_fields: Optional[list[str]], encoding: str, fail: bool, filename: str, groupby: Optional[list[str]], headless: bool, ignore: Optional[list[str]], max_batch_bytes: int, model: Optional[str], no_checkpoint: bool, no_preflight_checks: bool, o2m: bool, resolve_relations: Optional[list[dict[str, Any]]], resume: bool, separator: str, set_empty_on_missing: bool, skip: int, skip_existing: bool, skip_unchanged: bool, stream: bool, unique_id_field: Optional[str], worker: int]. + DOC103: Function `run_import`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [adaptive_throttle: bool, auto_clean: bool, auto_create_refs: bool, auto_defer: bool, auto_groupby: bool, batch_delay: float, batch_size: int, check_refs: str, config: Union[str, dict[str, Any]], context: Any, deferred_fields: Optional[list[str]], encoding: str, fail: bool, filename: str, fix_missing_variants: bool, groupby: Optional[list[str]], headless: bool, ignore: Optional[list[str]], max_batch_bytes: int, model: Optional[str], no_checkpoint: bool, no_preflight_checks: bool, o2m: bool, resolve_relations: Optional[list[dict[str, Any]]], resume: bool, separator: str, set_empty_on_missing: bool, skip: int, skip_existing: bool, skip_unchanged: bool, stream: bool, unique_id_field: Optional[str], worker: int]. DOC203: Function `run_import` return type(s) in docstring not consistent with the return annotation. Return annotation types: ['Optional[dict[str, int]]']; docstring return section types: ['dict[str, int]'] DOC501: Function `run_import` has raise statements, but the docstring does not have a "Raises" section DOC503: Function `run_import` exceptions in the "Raises" section in the docstring do not match those in the function body. Raised exceptions in the docstring: []. Raised exceptions in the body: ['TypeError']. diff --git a/src/fluvo/__main__.py b/src/fluvo/__main__.py index 83fff4ee..204aaeb4 100644 --- a/src/fluvo/__main__.py +++ b/src/fluvo/__main__.py @@ -1107,6 +1107,14 @@ def vat_validate_cmd( help="Apply safe, type-aware coercions before load (strip whitespace, " "normalize null tokens, canonicalize booleans). Off by default.", ) +@click.option( + "--fix-missing-variants", + is_flag=True, + default=False, + help="For product.template imports: create default variants for any imported " + "templates that end up with none (Odoo's load() does not auto-create them). " + "Without this flag, fluvo only warns about them.", +) @click.option( "--defer-parent-store", is_flag=True, diff --git a/src/fluvo/importer.py b/src/fluvo/importer.py index 8fed9b71..d38b9253 100755 --- a/src/fluvo/importer.py +++ b/src/fluvo/importer.py @@ -154,6 +154,7 @@ def run_import( # noqa: C901 max_batch_bytes: int = 5 * 1024 * 1024, resolve_relations: Optional[list[dict[str, Any]]] = None, auto_clean: bool = False, + fix_missing_variants: bool = False, ) -> Optional[dict[str, int]]: """Main entry point for the import command, handling all orchestration. @@ -537,6 +538,16 @@ def _base(field: str) -> str: ) ) + # Guardrail (#188): Odoo's load() does not auto-create default variants, so a + # product.template import can silently leave templates unusable. Warn (or fix + # with --fix-missing-variants) for the just-imported templates. + if is_truly_successful: + from .lib.actions.variant_manager import check_missing_variants_after_import + + check_missing_variants_after_import( + config, model, id_map, fix=fix_missing_variants + ) + return id_map diff --git a/src/fluvo/lib/actions/variant_manager.py b/src/fluvo/lib/actions/variant_manager.py index fb3aec15..01c27840 100644 --- a/src/fluvo/lib/actions/variant_manager.py +++ b/src/fluvo/lib/actions/variant_manager.py @@ -94,3 +94,61 @@ def run_create_missing_variants( f"--- Create Missing Variants Finished: {created} created, {failed} failed ---" ) return failed == 0 + + +def check_missing_variants_after_import( + config: Union[str, dict[str, Any]], + model: Optional[str], + id_map: dict[str, int], + fix: bool = False, +) -> int: + """Post-import guardrail: warn (or fix) when imported templates lack variants. + + Odoo's ``load()`` does not auto-create default variants, so a + ``product.template`` import can silently leave templates with no variants + (#188). This checks the *just-imported* templates and warns; with ``fix=True`` + it creates the missing default variants inline. A no-op for other models. + + Args: + config: Connection config — a ``.conf`` file path or a connection dict. + model: The model that was just imported. + id_map: Mapping of external IDs to database IDs from the import. + fix: If True, create the missing variants; otherwise only warn. + + Returns: + int: The number of imported templates found without a variant. + """ + if model != "product.template" or not id_map: + return 0 + + db_ids = list(id_map.values()) + try: + if isinstance(config, dict): + connection: Any = conf_lib.get_connection_from_dict(config) + else: + connection = conf_lib.get_connection_from_config(config_file=config) + template_obj = connection.get_model("product.template") + orphan_ids = template_obj.search( + [("id", "in", db_ids), ("product_variant_count", "=", 0)] + ) + except Exception as e: + log.warning(f"Could not check imported templates for missing variants: {e}") + return 0 + + if not orphan_ids: + return 0 + + if fix: + log.warning( + f"{len(orphan_ids)} imported product template(s) have no variants; " + "creating the missing default variants (--fix-missing-variants)." + ) + run_create_missing_variants(config, domain=[("id", "in", orphan_ids)]) + else: + log.warning( + f"{len(orphan_ids)} imported product template(s) have NO variants and " + "are unusable in sales/purchase orders and BoMs (Odoo's load() does not " + "auto-create them). Fix with 'fluvo workflow create-missing-variants', " + "or re-run the import with --fix-missing-variants." + ) + return len(orphan_ids) diff --git a/tests/test_variant_manager.py b/tests/test_variant_manager.py index 48e95eeb..fa9d9720 100644 --- a/tests/test_variant_manager.py +++ b/tests/test_variant_manager.py @@ -7,7 +7,10 @@ from click.testing import CliRunner from fluvo.__main__ import cli -from fluvo.lib.actions.variant_manager import run_create_missing_variants +from fluvo.lib.actions.variant_manager import ( + check_missing_variants_after_import, + run_create_missing_variants, +) CFG = "fluvo.lib.actions.variant_manager.conf_lib.get_connection_from_config" CFG_DICT = "fluvo.lib.actions.variant_manager.conf_lib.get_connection_from_dict" @@ -198,3 +201,59 @@ def test_cli_rejects_an_invalid_domain(tmp_path: Path) -> None: ) assert result.exit_code != 0 assert "Invalid --domain" in result.output + + +# --- post-import guardrail (#188) --- +@patch(CFG) +def test_guardrail_skips_non_product_template(mock_get: MagicMock) -> None: + """The guardrail is a no-op (no connection) for other models.""" + assert check_missing_variants_after_import("c.conf", "res.partner", {"x": 1}) == 0 + mock_get.assert_not_called() + + +def test_guardrail_skips_empty_id_map() -> None: + """The guardrail is a no-op when nothing was imported.""" + assert check_missing_variants_after_import("c.conf", "product.template", {}) == 0 + + +@patch(CFG) +def test_guardrail_returns_zero_when_no_orphans(mock_get: MagicMock) -> None: + """No imported template lacks a variant -> returns 0, creates nothing.""" + conn, _t, product = _mock_conn([]) + mock_get.return_value = conn + n = check_missing_variants_after_import("c.conf", "product.template", {"a": 1}) + assert n == 0 + product.create.assert_not_called() + + +@patch(CFG) +def test_guardrail_warns_only_by_default(mock_get: MagicMock) -> None: + """With fix=False, orphans are reported but NOT created.""" + conn, _t, product = _mock_conn([10, 11]) + mock_get.return_value = conn + n = check_missing_variants_after_import( + "c.conf", "product.template", {"a": 10, "b": 11} + ) + assert n == 2 + product.create.assert_not_called() + + +@patch(CFG) +def test_guardrail_fixes_when_requested(mock_get: MagicMock) -> None: + """With fix=True, the missing variants are created.""" + conn, _t, product = _mock_conn([10, 11]) + mock_get.return_value = conn + n = check_missing_variants_after_import( + "c.conf", "product.template", {"a": 10, "b": 11}, fix=True + ) + assert n == 2 + product.create.assert_called() + + +@patch(CFG) +def test_guardrail_swallows_connection_errors(mock_get: MagicMock) -> None: + """A connection failure during the check returns 0 (never breaks the import).""" + mock_get.side_effect = Exception("boom") + assert ( + check_missing_variants_after_import("c.conf", "product.template", {"a": 1}) == 0 + ) From f7759f56c3bca1e7aba226b78cef910620cef386 Mon Sep 17 00:00:00 2001 From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me> Date: Thu, 11 Jun 2026 11:05:18 +0200 Subject: [PATCH 2/4] fix(#188): batch guardrail search + run on partial imports (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Batch the orphan-template search in chunks of 2000 ids to avoid oversized RPC payloads / slow SQL on large imports (the issue had 22k templates). + test. - Run the guardrail whenever records were imported (if id_map), not only on a fully-successful import — partially-successful imports still write templates that may lack variants. --- src/fluvo/importer.py | 5 +++-- src/fluvo/lib/actions/variant_manager.py | 15 ++++++++++++--- tests/test_variant_manager.py | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/fluvo/importer.py b/src/fluvo/importer.py index d38b9253..fcdfcc1f 100755 --- a/src/fluvo/importer.py +++ b/src/fluvo/importer.py @@ -540,8 +540,9 @@ def _base(field: str) -> str: # Guardrail (#188): Odoo's load() does not auto-create default variants, so a # product.template import can silently leave templates unusable. Warn (or fix - # with --fix-missing-variants) for the just-imported templates. - if is_truly_successful: + # with --fix-missing-variants) for the just-imported templates. Runs whenever + # any records were imported (id_map), including partially-successful imports. + if id_map: from .lib.actions.variant_manager import check_missing_variants_after_import check_missing_variants_after_import( diff --git a/src/fluvo/lib/actions/variant_manager.py b/src/fluvo/lib/actions/variant_manager.py index 01c27840..6abe056f 100644 --- a/src/fluvo/lib/actions/variant_manager.py +++ b/src/fluvo/lib/actions/variant_manager.py @@ -128,9 +128,18 @@ def check_missing_variants_after_import( else: connection = conf_lib.get_connection_from_config(config_file=config) template_obj = connection.get_model("product.template") - orphan_ids = template_obj.search( - [("id", "in", db_ids), ("product_variant_count", "=", 0)] - ) + # Batch the search: a large import can yield tens of thousands of ids, + # and a single "in" query risks oversized RPC payloads / slow SQL. + orphan_ids: list[int] = [] + for i in range(0, len(db_ids), 2000): + orphan_ids.extend( + template_obj.search( + [ + ("id", "in", db_ids[i : i + 2000]), + ("product_variant_count", "=", 0), + ] + ) + ) except Exception as e: log.warning(f"Could not check imported templates for missing variants: {e}") return 0 diff --git a/tests/test_variant_manager.py b/tests/test_variant_manager.py index fa9d9720..5f836e90 100644 --- a/tests/test_variant_manager.py +++ b/tests/test_variant_manager.py @@ -257,3 +257,21 @@ def test_guardrail_swallows_connection_errors(mock_get: MagicMock) -> None: assert ( check_missing_variants_after_import("c.conf", "product.template", {"a": 1}) == 0 ) + + +@patch(CFG) +def test_guardrail_batches_the_search_for_large_imports( + mock_get: MagicMock, +) -> None: + """The orphan search is chunked (2000/call) to avoid huge RPC payloads.""" + conn = MagicMock() + template = MagicMock() + product = MagicMock() + template.search.return_value = [] + conn.get_model.side_effect = lambda m: ( + template if m == "product.template" else product + ) + mock_get.return_value = conn + id_map = {f"x{i}": i for i in range(4500)} # -> 3 batches (2000 + 2000 + 500) + check_missing_variants_after_import("c.conf", "product.template", id_map) + assert template.search.call_count == 3 From 31a00db32bb10bcad4fb5f8e117bf96d33982325 Mon Sep 17 00:00:00 2001 From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me> Date: Thu, 11 Jun 2026 11:31:02 +0200 Subject: [PATCH 3/4] fix(#188): chunk the --fix-missing-variants create calls (review) The fix path passed all orphan_ids to run_create_missing_variants in one domain, undoing the batched-search safeguard. Chunk the fix calls in 2000s too. --- src/fluvo/lib/actions/variant_manager.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/fluvo/lib/actions/variant_manager.py b/src/fluvo/lib/actions/variant_manager.py index 6abe056f..cc3fbd6b 100644 --- a/src/fluvo/lib/actions/variant_manager.py +++ b/src/fluvo/lib/actions/variant_manager.py @@ -152,7 +152,12 @@ def check_missing_variants_after_import( f"{len(orphan_ids)} imported product template(s) have no variants; " "creating the missing default variants (--fix-missing-variants)." ) - run_create_missing_variants(config, domain=[("id", "in", orphan_ids)]) + # Chunk the fix calls too, so the no-oversized-payload safeguard that the + # search above applies isn't undone by an unbatched create domain. + for i in range(0, len(orphan_ids), 2000): + run_create_missing_variants( + config, domain=[("id", "in", orphan_ids[i : i + 2000])] + ) else: log.warning( f"{len(orphan_ids)} imported product template(s) have NO variants and " From ba286d191632bdc68733ce477405d4f6fd77e250 Mon Sep 17 00:00:00 2001 From: bosd <5e2fd43-d292-4c90-9d1f-74ff3436329a@anonaddy.me> Date: Thu, 11 Jun 2026 16:23:49 +0200 Subject: [PATCH 4/4] refactor(#188): guardrail creates from found ids; gate on product.template (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract _create_default_variants (batched create + Odoo<14 fallback), shared by run_create_missing_variants and the guardrail. - Guardrail fix path now creates directly from the orphan_ids it already found — no redundant re-search (and no oversized id-domain), resolving the tension between the two review rounds. - Dedup id_map values (list(set(...))) so duplicate db ids aren't queried twice. - Gate the guardrail call on model == 'product.template' to skip the import/call entirely for other models. --- src/fluvo/importer.py | 2 +- src/fluvo/lib/actions/variant_manager.py | 76 ++++++++++++++---------- 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/src/fluvo/importer.py b/src/fluvo/importer.py index fcdfcc1f..27df3d35 100755 --- a/src/fluvo/importer.py +++ b/src/fluvo/importer.py @@ -542,7 +542,7 @@ def _base(field: str) -> str: # product.template import can silently leave templates unusable. Warn (or fix # with --fix-missing-variants) for the just-imported templates. Runs whenever # any records were imported (id_map), including partially-successful imports. - if id_map: + if model == "product.template" and id_map: from .lib.actions.variant_manager import check_missing_variants_after_import check_missing_variants_after_import( diff --git a/src/fluvo/lib/actions/variant_manager.py b/src/fluvo/lib/actions/variant_manager.py index cc3fbd6b..9f0fdee2 100644 --- a/src/fluvo/lib/actions/variant_manager.py +++ b/src/fluvo/lib/actions/variant_manager.py @@ -17,6 +17,45 @@ from ...logging_config import log +def _create_default_variants( + product_obj: Any, template_ids: list[int], batch_size: int = 200 +) -> tuple[int, int]: + """Create one default product.product per template id, in batches. + + Args: + product_obj: The ``product.product`` model proxy. + template_ids: Template ids to give a default variant. + batch_size: Variants to create per RPC call. + + Returns: + tuple[int, int]: ``(created, failed)`` counts. + """ + created = 0 + failed = 0 + for start in range(0, len(template_ids), batch_size): + batch = template_ids[start : start + batch_size] + try: + # Batch create works on Odoo >= 14. Older Odoo rejects a list of + # dicts, so fall back to one-by-one creation if the batch call fails. + product_obj.create([{"product_tmpl_id": tid} for tid in batch]) + created += len(batch) + except Exception as batch_err: + log.warning( + f"Batch variant creation failed ({batch_err}); " + "falling back to individual creation." + ) + for tid in batch: + try: + product_obj.create({"product_tmpl_id": tid}) + created += 1 + except Exception as item_err: + failed += 1 + log.error( + f"Failed to create variant for template {tid}: {item_err}" + ) + return created, failed + + def run_create_missing_variants( config: Union[str, dict[str, Any]], domain: Optional[list[Any]] = None, @@ -66,30 +105,7 @@ def run_create_missing_variants( log.info("Dry run: no variants will be created.") return True - created = 0 - failed = 0 - for start in range(0, len(orphan_ids), batch_size): - batch = orphan_ids[start : start + batch_size] - try: - # Batch create works on Odoo >= 14. Older Odoo rejects a list of - # dicts, so fall back to one-by-one creation if the batch call fails. - product_obj.create([{"product_tmpl_id": tid} for tid in batch]) - created += len(batch) - except Exception as batch_err: - log.warning( - f"Batch variant creation failed ({batch_err}); " - "falling back to individual creation." - ) - for tid in batch: - try: - product_obj.create({"product_tmpl_id": tid}) - created += 1 - except Exception as item_err: - failed += 1 - log.error( - f"Failed to create variant for template {tid}: {item_err}" - ) - + created, failed = _create_default_variants(product_obj, orphan_ids, batch_size) log.info( f"--- Create Missing Variants Finished: {created} created, {failed} failed ---" ) @@ -121,13 +137,14 @@ def check_missing_variants_after_import( if model != "product.template" or not id_map: return 0 - db_ids = list(id_map.values()) + db_ids = list(set(id_map.values())) try: if isinstance(config, dict): connection: Any = conf_lib.get_connection_from_dict(config) else: connection = conf_lib.get_connection_from_config(config_file=config) template_obj = connection.get_model("product.template") + product_obj = connection.get_model("product.product") # Batch the search: a large import can yield tens of thousands of ids, # and a single "in" query risks oversized RPC payloads / slow SQL. orphan_ids: list[int] = [] @@ -152,12 +169,9 @@ def check_missing_variants_after_import( f"{len(orphan_ids)} imported product template(s) have no variants; " "creating the missing default variants (--fix-missing-variants)." ) - # Chunk the fix calls too, so the no-oversized-payload safeguard that the - # search above applies isn't undone by an unbatched create domain. - for i in range(0, len(orphan_ids), 2000): - run_create_missing_variants( - config, domain=[("id", "in", orphan_ids[i : i + 2000])] - ) + # Create directly from the ids already found (no re-search), with the + # shared helper's internal batching. + _create_default_variants(product_obj, orphan_ids) else: log.warning( f"{len(orphan_ids)} imported product template(s) have NO variants and "