[16.0][IMP] connector_extension: run _must_skip before dependency imports - #943
Open
eantones wants to merge 4 commits into
Open
[16.0][IMP] connector_extension: run _must_skip before dependency imports#943eantones wants to merge 4 commits into
eantones wants to merge 4 commits into
Conversation
The record importer evaluated _must_skip at the end of run(), after
dependencies were imported and after alternate-key adoption. A skipped
record therefore left all its dependency imports committed (there is
no rollback on skip): connectors that refuse an update still imported
and updated every dependency of the refused record, and no hook could
prevent it.
The late position was never a design decision. The v11
components_custom rewrite that introduced alternate-key adoption sank
the whole binding-resolution block below dependencies and mapping
(adoption consumes mapped values, and mapping a record may need its
dependencies already imported), and the gate, glued since the first
frameworks to the binding lookup, sank with it. Its docstring ("right
after we read the data") kept describing the original early position,
and no implementation ever needed the late one: the only overrides in
the fleet decide on the binding alone.
Move the gate right after the primary-key binding lookup, before
dependency imports, and pass the external data to the hook so
implementations can also decide from the payload:
_must_skip(binding) -> _must_skip(binding, external_data)
Alternate-key adoption stays on the create path, after dependencies
and mapping, which it genuinely needs. The only semantic change is
that the gate now sees the primary-key binding instead of a
possibly-adopted one - the behavior every existing override was
originally written against on its own framework.
The 16.0 consumers define no _must_skip override, so this lands
behavior-neutral here; overrides on other branches must add the new
parameter when this change reaches them.
Marketplaces re-send old orders with the buyer contact erased (GDPR anonymization), sometimes together with a legitimate order update (e.g. a refund). The contact name is part of the partner identity (the address hash feeds both binder keys), so an erased name can never match the partner created at first import: the import failed the job in the partner name mapping and the real update was lost with it. Skip the buyer-partner import on those re-syncs from the partner importer gate (contact name erased + not bound + the order already in Odoo -> nothing to import: the order keeps pointing to the partners of its first import), and keep the stored pointers untouched in the order mapper when an incoming address is anonymized on an update. The order update itself (status, etc.) still lands. The first import of an order with an empty contact name keeps failing loudly in the partner mapper: there a real partner must be created and there is no data to create it with - that is a configuration or source-data problem to surface, not a case to skip. Detection is based on the erased name only: the payload anonymized flag proved unreliable at the dangerous moment (it lagged the actual data wipe on the stored payload waves) and the lengow_status values carried by anonymized re-syncs are exactly the ones whose updates must not be lost.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 16.0 #943 +/- ##
==========================================
+ Coverage 54.00% 54.49% +0.48%
==========================================
Files 337 337
Lines 6947 7008 +61
Branches 1035 1040 +5
==========================================
+ Hits 3752 3819 +67
+ Misses 3057 3053 -4
+ Partials 138 136 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…mpty names A first import can meet an already-anonymized order: the connector saw the order too late (a marketplace mapping added months later, a backfill, a long backend downtime) and the marketplace erased the buyer contact in the meantime. The empty-name error so far only gave the configuration angle (contact name source), which is misleading there: nothing is misconfigured, and re-importing cannot bring back a name that no longer exists in the payload. When the order is old enough that anonymization is the likely cause (wording heuristic only, no behavior depends on it), extend the error with the order date and age and explain that the name most likely no longer exists in any field Lengow sends, so if the order still needs importing its contact data must be recovered outside Lengow. The order date now travels stamped on both addresses like the other order identifiers, so the partner mapper can use it.
…mization
The empty-name error appended the anonymization explanation only when
the order was older than a hardcoded threshold (90 days). No real
anonymization window exists to derive such a number from (each
marketplace has its own and none publishes it), so stop guessing:
always state the order date and age and phrase the anonymization cause
conditionally ("if it is an old order, ...") - the reader applies to
the stated age the judgment the number pretended to encode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Framework change + first consumer. Supersedes #942.
Commit 1 — connector_extension: run
_must_skipbefore dependency imports_must_skipfired at the end ofrun(), after dependency imports and alternate-key adoption: a refused record left every dependency import committed (there is no rollback on skip), and no hook could prevent those side effects._import_dependencies._must_skip(binding, external_data)— inputs passed byrun()as parameters.Audited before changing (every branch + every open PR head of nuobit/odoo-addons, oxigensalud/odoo-addons and nuobit/lighting-vertical):
_must_skipis invoked only from frameworkrun()bodies, nobody forksrun()(only thin wrappers delegating tosuper()), and 16.0 has zero overrides — this commit is behavior-neutral on 16.0 by itself. The only overrides anywhere (connector_woocommerce 14.0/#874, connector_oxigesti_spms #169) decide onbindingalone and regain their original own-framework semantics with the early gate; they must add the new parameter when this reaches their branch (18.0 FWP / optional 14.0 BP).Commit 2 — connector_lengow: keep order addresses on anonymized re-syncs
Marketplaces re-send old orders with the buyer contact erased (GDPR anonymization), sometimes carrying a legitimate update (e.g. a refund). The contact name feeds the partner identity hash, so an erased name can never match the partner created at first import: the job failed in the partner name mapping and the real update was lost with it.
_must_skip: contact name erased + not bound + the order already in Odoo → skip; the order keeps the partners of its first import.anonymizedflag lags the actual wipe on the stored payload waves, and thelengow_statusvalues carried by anonymized re-syncs are exactly the ones whose updates must not be lost.Tests
connector_lengowsuite on a dedicated test DB with both modules upgraded: 10/10 green, including the two new tests (fully and partially anonymized re-sync) exercising the new pipeline end to end.Follow-ups (separate PRs)