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..27df3d35 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,17 @@ 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. Runs whenever + # any records were imported (id_map), including partially-successful imports. + if model == "product.template" and id_map: + 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..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,31 +105,78 @@ 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 ---" ) 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(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] = [] + 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 + + 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)." + ) + # 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 " + "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..5f836e90 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,77 @@ 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 + ) + + +@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