feat(delete-handling): mirror source deletes on the destination#11
Open
mageaustralia wants to merge 4 commits into
Open
feat(delete-handling): mirror source deletes on the destination#11mageaustralia wants to merge 4 commits into
mageaustralia wants to merge 4 commits into
Conversation
The source tracker has logged action=delete since 1.0.0 but the destination
treated those rows as a no-op (return true; mark synced; leave the entity in
place). Over time the destination accumulates a ghost row for every entity
ever deleted on the source, surfacing as duplicate-SKU url_keys, suffixed
slugs from collisions, and a destination catalog that drifts further from
source on every sync cycle.
This change resolves the destination's matching entity by a portable
identifier captured at delete time (SKU for products, email for customers,
url_key for categories) and either hard-deletes or soft-disables it per
config.
Source side (Maho_DataSyncTracker 1.2.0):
- new source_identifier column on datasync_change_tracker + composite
index for the (entity_type, source_identifier) lookup
- upgrade-1.1.0-1.2.0.php adds the column idempotently
- product/customer/category delete observers capture SKU/email/url_key
into source_identifier so the destination still has a handle after
the source row is gone
Destination side (Maho_DataSync):
- new datasync/delete_handling config tree:
enabled (default 1) master switch; 0 keeps legacy no-op
mode (default 'hard') 'hard' = ->delete(), 'soft' = disable
soft_for_customer (1) customers always soft (order history)
skip_entity_types ('') CSV of types to leave untouched
- DatasyncIncremental SELECT now pulls source_identifier through
- mirrorDeletes() iterates collected delete tracker rows and dispatches
to mirrorOneDelete() per entity type; errors per row are caught and
logged so a single failure doesn't abort the whole sync
- resolveLiveIdentifier() best-effort fallback for pre-1.2.0 tracker
rows that don't carry source_identifier yet
- USER_GUIDE.md gains a Delete Handling section
Why customers default to soft:
Customers carry sales_flat_order.customer_id and refund references.
Hard-deleting orphans those rows. soft (is_active=0) preserves history
and prevents login; opt out by setting soft_for_customer=0.
Tested against a dev environment where the destination had accumulated
3,031 ghost SKUs over months of source deletes. After bumping schema,
config'ing hard mode, and running datasync:incremental, the destination
converged with the source.
DataSync stamps every imported product with datasync_source_id, the entity_id it held in the source catalogue, but it never rewrites the product_id columns of the tables copied across alongside it. Order history keeps pointing at source ids, and since the catalogue is re-imported with fresh ids, those numbers now name different products. Sales are credited to whichever product occupies the old id. Everything joining history to the catalogue by product_id inherits the error: ordered-quantity search ranking, bestseller reports, recommendations, cross-sells. The command rebuilds the map from datasync_source_id and rewrites sales_flat_order_item, sales_flat_quote_item, wishlist_item and the two report indexes. report_event.object_id is opt-in, since object_id only means a product for product-scoped event types. It previews by default and refuses a second --apply once a run is recorded, because a source id can equal another product's local id. Rewrites happen in two phases: park each new value above an offset, then subtract it. A single pass would otherwise cascade one mapping onto rows an earlier mapping had already moved. No UPDATE ... JOIN, so the statements stay portable.
The first cut of datasync:remap-product-ids rewrote every table holding a product_id: order items, quote items, wishlist items and the viewed-product report. That was wrong. A product_id only means "source entity_id" on a row DataSync imported; on a row this installation wrote itself it already means the local product, and rewriting it moves a correct reference onto a different product. Scope the rewrite through sales_flat_order.datasync_source_id, and drop the other three tables entirely -- they are populated by ordinary browsing and checkout, so no row in them is ever a source id. The class docblock says so, to stop them being added back. Rewriting by item_id instead of by product_id value also removes the need for the two-phase OFFSET parking trick: the stale ids are snapshotted before any write, so one mapping cannot cascade onto a row an earlier mapping already moved.
On a configurable order the parent row stores the CHILD's sku while pointing at the PARENT product: parent_item_id NULL product_type configurable product_id 30347 sku ABC-4 3/8 parent_item_id 566539 product_type simple product_id 30355 sku ABC-4 3/8 _resolveLocalProductId() looked the product up by sku, so both rows landed on the child. The configurable parent, which is the product customers search and browse, was credited with none of its own sales. Ordered-quantity ranking, bestseller reports, recommendations and cross-sells all read these rows. Measured against the source catalogue, 11% of imported order lines were affected. Resolve through datasync_source_id first, which is the only key that identifies a product unambiguously across catalogues, and keep sku as a fallback. Where a source id maps to more than one local product the choice is arbitrary, so fall through to sku rather than pick one. Two related corrections: - Invoice and shipment items passed the ORDER ITEM's already-translated product_id in as a source id when the payload omitted one. Use it directly instead. - When neither key resolves, return null rather than the raw source id. The source id is meaningless in this catalogue and silently points the row at whichever product happens to occupy it. Replaces datasync:remap-product-ids with datasync:fix-configurable-order-items, which backfills orders imported before this fix. The old command inferred the correct product from the source-id map; on this data 73% of the rows it targeted were already correct, so applying it would have moved correct references onto different products. The new command decides from catalog_product_super_link, only rewrites a parent row whose product_id is a simple with exactly one configurable parent, and reports rather than guesses on the rest.
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.
Summary
The source tracker has logged
action=deleterows since 1.0.0, but the destination treated them as a no-op (return true; mark synced; leave the entity in place). Over time the destination accumulates a ghost row for every entity ever deleted on the source. This surfaces as duplicate-SKUurl_keys, suffixed slug collisions, and a destination catalog that drifts further from source on every sync cycle.This PR makes destination delete handling act on the tracker rows by resolving the matching destination entity via a portable identifier captured at delete time (SKU for products, email for customers,
url_keyfor categories), then hard-deleting or soft-disabling per config.Real-world impact that motivated this
On the dev environment this PR was authored against, the destination had accumulated 3,031 ghost SKUs (28% bigger catalog than source). After bumping the source tracker schema, configuring
mode=hard, and runningdatasync:incremental, the destination converged with the source.Changes
Source tracker (
Maho_DataSyncTracker1.1.0 → 1.2.0)source_identifierVARCHAR(255) column ondatasync_change_tracker+ composite index for the(entity_type, source_identifier)lookupupgrade-1.1.0-1.2.0.phpadds the column idempotentlySKU(product),email(customer),url_key(category) intosource_identifierso the destination still has a stable handle after the source row is physically goneDestination (
Maho_DataSync)New config tree (
<default><datasync><delete_handling>):enabled10= legacy no-opmodehardhard=->delete(),soft= disablesoft_for_customer1modeskip_entity_typesgetPendingChanges()SELECT now pullssource_identifierthroughmirrorDeletes()iterates collected delete tracker rows and dispatches tomirrorOneDelete()per entity typeresolveLiveIdentifier()best-effort fallback for pre-1.2.0 tracker rows that don't carrysource_identifieryetUSER_GUIDE.mdgains aDelete HandlingsectionWhy customers default to soft
Customers carry
sales_flat_order.customer_idand refund references. Hard-deleting orphans those rows. Soft (is_active=0) preserves history and prevents login. Opt out by settingsoft_for_customer=0.Migration notes
After deploying this PR:
upgrade-1.1.0-1.2.0.phpautomatically and addssource_identifier. New deletes start capturing it immediately.enabled=1, mode=hard. If you want the old behaviour (ignore deletes), setdatasync/delete_handling/enabled = 0before the next sync.resolveLiveIdentifier()— works if the source row hasn't been physically deleted yet, skips otherwise. For a clean catch-up, the operator can run a one-shot reconciliation that compares destination SKUs to source SKUs by hand.Test plan
source_identifiercolumn exists and is indexed.action=deleteANDsource_identifier=<SKU>.source_identifier=<email>.source_identifier=<url_key>.mode=hard(default) →datasync:incrementalremoves the matching destination row by SKU/email/url_key.mode=soft→ product disabled + invisible; categoryis_active=0; customeris_active=0.soft_for_customer=1(default) wins overmode=hardfor customers only.enabled=0→ tracker rows marked synced but destination untouched (legacy behaviour preserved).source_identifier=NULL(pre-1.2.0) → best-effort lookup via live entity_id; skip+log if source row is gone.🤖 Generated with Claude Code