fix(products): honest position compare for matched variants in diff
A Variant Position column previously produced a spurious update with a
false before:None (CatalogVariant keeps position as an attribute, not in
fields{}). Compare against the attribute, like images already do.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -9,7 +9,8 @@ Deliberately DB-free: the catalog snapshot dataclasses are defined here and the
|
||||
repo layer builds them. Only fields present in the file's canonical fields{}
|
||||
participate in a comparison — an absent column is untouched, never a change
|
||||
(§6.5.1); a present-but-empty cell resolves to the field's CLEAR_DEFAULTS entry.
|
||||
Catalog variants/images absent from the file are likewise untouched (INV-10).
|
||||
Catalog variants/images absent from the file are likewise untouched (INV-10);
|
||||
file variants match catalog variants by their option-value combination (INV-13).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -89,7 +90,7 @@ def compute_diff(catalog: dict[str, CatalogProduct], products: list[CanonicalPro
|
||||
)
|
||||
summary[_SUMMARY_KEY[kind]] += 1
|
||||
fingerprint = hashlib.sha256(
|
||||
json.dumps(records, sort_keys=True, separators=(",", ":"), default=str).encode()
|
||||
json.dumps(records, sort_keys=True, separators=(",", ":")).encode()
|
||||
).hexdigest()
|
||||
return DiffResult(records=records, summary=summary, fingerprint=fingerprint)
|
||||
|
||||
@@ -140,11 +141,13 @@ def _update_detail(product: CanonicalProduct, current: CatalogProduct) -> dict:
|
||||
{"options": list(variant.options), "kind": "add", "set": _resolved_variant_fields(variant)}
|
||||
)
|
||||
continue
|
||||
variant_changes = [
|
||||
_change(f, match.fields.get(f), resolved)
|
||||
for f, resolved in _resolved_fields(variant.fields).items()
|
||||
if resolved != match.fields.get(f)
|
||||
]
|
||||
variant_changes = []
|
||||
for f, resolved in _resolved_fields(variant.fields).items():
|
||||
# position lives on the catalog variant as an attribute, not in
|
||||
# fields{} — compare it explicitly (as images do for theirs).
|
||||
before = match.position if f == "position" else match.fields.get(f)
|
||||
if resolved != before:
|
||||
variant_changes.append(_change(f, before, resolved))
|
||||
if variant_changes:
|
||||
variant_entries.append(
|
||||
{"options": list(variant.options), "kind": "update", "changes": variant_changes}
|
||||
|
||||
@@ -95,6 +95,24 @@ def test_new_option_combo_is_variant_add_existing_untouched():
|
||||
assert "add" in kinds
|
||||
|
||||
|
||||
POSITION_HEADER = HEADER + ",Variant Position"
|
||||
|
||||
|
||||
def test_matching_variant_position_column_classifies_unchanged():
|
||||
# position is an attribute on CatalogVariant (not in fields{}); the compare
|
||||
# must read it from there, not invent a before:None.
|
||||
diff = compute_diff(_catalog_mug(), _canon(POSITION_HEADER, MUG_ROW + ",1"))
|
||||
assert diff.records[0]["kind"] == "unchanged"
|
||||
|
||||
|
||||
def test_changed_variant_position_reports_honest_before():
|
||||
diff = compute_diff(_catalog_mug(), _canon(POSITION_HEADER, MUG_ROW + ",2"))
|
||||
[rec] = diff.records
|
||||
assert rec["kind"] == "update"
|
||||
[ventry] = rec["detail"]["variants"]
|
||||
assert {"field": "position", "before": 1, "after": 2} in ventry["changes"]
|
||||
|
||||
|
||||
def test_error_product_classifies_error():
|
||||
diff = compute_diff({}, _canon("Handle,Title,Variant Price", "mug,Mug,nope"))
|
||||
[rec] = diff.records
|
||||
|
||||
Reference in New Issue
Block a user