CartShift 1.3.0 — selective migration - #127
Conversation
Distinguishes "no clause" (none()) from "the empty set" (matchesNothing()) via an explicit matchesNoRows() flag rather than a string comparison against '1 = 0', so a raw() fragment that happens to spell that string is never mistaken for the sentinel. any() drops empty and matches-nothing parts so one unpicked ID set does not widen or kill the rest of the disjunction; all() keeps matches-nothing parts since AND with an empty set is genuinely empty.
Resolves a chosen MigrationScope into the predicates each migrator needs, walking the one-round dependency closure: a picked customer pulls their orders, an order pulls its buyer and every product on it, and there it stops. Both any()-based predicates (seedOrderPredicate, seedSubscriptionPredicate) carry the matchesNothing() guard against ScopePredicate::any() collapsing an all-empty OR into none() (no restriction). Without it, an explicit scope selecting only products with the upward offer declined would hand every order or subscription in the shop to a scope that picked nobody.
start() now accepts an optional MigrationScope and stores it alongside entity_types; getScope()/setScope() read it back, defaulting to MigrationScope::everything() for runs (or scope-less options) that predate selective migration.
Declared on the contract, not only on AbstractMigrator, because ScopePreview (Task 9) holds a list<MigratorInterface> and calls it on each entry. AbstractMigrator is the only production implementor; patches the six test doubles that implement the interface directly with a no-op so the interface change does not fatal class loading.
AbstractMigrator implements useScope() and exposes protected scope()/ scopeResolver() to subclasses; scope() prefers an explicit override over MigrationState, and scopeResolver() memoises a ScopeResolver built from it per request. MigrationOrchestrator::startMigration() gains an optional MigrationScope parameter and passes it through to MigrationState::start(). startRetry() captures the current scope before start() mints fresh state, so a retry keeps the scope on record instead of reading back as 'everything'.
Both phases now carry the scope predicate as a conjunct of the same WHERE the keyset range lives in, rather than filtering afterwards: registered users take registeredCustomerPredicate(), guests take guestCustomerPredicate(), and the two count queries take the matching one so the totals and the batches cannot disagree. The counts move to prepare() because they now carry values. Placeholder order in every query is keyset, status scope, selection, LIMIT — and so is the value order, since prepare() binds positionally and a wrong order binds the right number of values in the wrong slots with no error. The phase transition is untouched on purpose: a scoped registered page returning zero rows is not "registered finished early", it still falls through and tops the batch up from the start of the guests.
countRegisteredCustomers() memoises, and the memo is now scope-dependent. /preview asks one migrator instance for counts under more than one scope, so an unscoped total surviving useScope() would report a migration size nobody asked for — and report it silently, since nothing else contradicts the number.
Lets wc_get_orders() stub answer per call via an optional callback global, and adds ScopedKeysetTest covering the order ID page's scope predicate, including a multi-batch walk under a date scope that proves no record is skipped or repeated at a batch boundary.
countTotal() and fetchOrderIdPage() now fold ScopeResolver::orderPredicate() into the same prepare() call as the status/type scope and the keyset cursor, so a since/explicit migration scope actually restricts which orders are counted and fetched. countTotal() moves from WooStorage::orderScopeSql() to orderScopeParts() so the predicate's values bind through one prepare() rather than nesting a prepared string inside another.
ProductMigrator's countTotal() and fetchProductIdPage() now splice
productPredicate('p.ID') into their WHERE clauses, keeping the numerator
and denominator on the same row set. Explicit-mode scope narrows the
catalogue; since/everything modes leave it untouched — a product is
cheap to migrate and an order referencing a missing one is worse than
an unused product sitting in the catalogue.
CouponMigrator::fetchBatch() now calls couponPredicate() and splices it
in as a documented no-op: coupons always travel whole under every mode.
countTotal() is left on wp_count_posts() with a comment explaining why
it has no predicate to splice into.
Adds ScopedKeysetTest coverage for both migrators, following the
existing single-shot recordingWpdb() idiom.
fetchBatch() filters after fetching, so a page can filter to nothing while wcs_get_subscriptions() still has rows. An empty batch is the orchestrator's only end-of-entity signal, so it now loops until either the source runs dry or something survives the filter, advancing the offset by rows fetched rather than rows kept. countTotal() gains the scope predicate, and the tests get a wcs_get_subscriptions() stub they never had.
seedSubscriptionPredicate() ORs customer_id and billing_email with no disjointness guard, and owner-supplied guest emails are never checked against registered accounts — so an email typed for a registered buyer counted that buyer's subscription while the PHP filter, reading the email only when customer_id was 0, never fetched it. Total above processed, for ever. The filter now mirrors the predicate's disjunction. The wpdb stub also records prepare() calls, so the one branch no assertion could see — a matches-nothing count must not reach prepare() with no values — is pinned by a test rather than by a comment.
Add ScopeConsequences, computing the four scope-driven consequence counts the receipt panel needs (product_link_missing, customer_rebuilt_from_order, subscription_paused_missing_product, coupon_disabled_missing_restrictions), each with a one-click add_products remedy where one applies. Add MigrationErrorCode::ScopeClosureTooLarge for a scope the owner picked that resolves past ScopeResolver::MAX_CLOSURE_IDS. Promote PreflightCheck::countOrdersAffectedByTypes() and countMigratableOrders() to public static so ScopeConsequences can reuse the same order-counting query the preflight screen already quotes, rather than duplicating it.
Adds the read-only preview endpoint the selection screen calls on every change: counts per requested entity type, scope consequences, the added closure counts, and the too_large flag — never a record, never a state write. Migrators are handed the candidate scope directly via useScope() rather than through MigrationState, so repeated calls (a debounce on every keystroke) touch nothing persistent.
… risky test I1: PreflightCheck::checkProductTypes() and ScopeConsequences both defined their own "supported product type" list and query. Extract PreflightCheck::SUPPORTED_PRODUCT_TYPES, productTypeCounts() and the new public unsupportedProductTypeCounts() so there is one definition both consumers read. Add a regression test pinning that checkProductTypes()'s reported unsupported set and unsupportedProductTypeCounts() agree. I2: ScopeConsequencesTest::testARemedyNamesTheProductsThatWouldCloseTheGap ran only under MigrationScope::everything(), where every remedy is structurally always null, so it asserted nothing on every run (PHPUnit's Risky flag was correct — this was flagged in review as landing under commit c200e65). Rewrite it against an explicit scope that actually produces a non-null remedy. Also broadens the customer_rebuilt_from_order docblock, which undersold its trigger to "the WP user row is gone" — a picked guest email matching a registered order's billing_email reaches it too.
/migrate now accepts an optional `scope` object (a new args array on the route — it had none before), passes it through to MigrationOrchestrator::startMigration(), and echoes the normalised scope back in the response. The CLI gains --since, --products and --customers, building the same MigrationScope. --since combined with --products/--customers is refused as a contradiction rather than silently resolved. Both paths refuse with the closure-too-large outcome before a migration id exists when ScopeResolver::exceedsClosureLimit() trips: REST returns 422 with code scope_closure_too_large, the CLI calls WP_CLI::error() and returns. Neither leaves a half-started run — MigrationState stays idle. This is the refusal ScopeResolver's docblock implied but never enforced; without it, an oversized closure would have migrated a truncated subset silently.
Task 10 review, round 1 — I1 (Important): MigrationScope::fromArray() fails open on a date it cannot parse and falls back to "everything", which is correct on the REST path (a preview and confirmation sit between the value and a running migration) but was silently reachable from `wp cartshift migrate --since=<bad-date>`, with no preview and no warning that the scope was discarded. The CLI now validates --since against MigrationScope's own normalisation before building the scope and refuses with WP_CLI::error() + return when it fails, matching the shape of the other pre-flight guards added in Task 10. I2 (Minor): ScopeResolver's docblock said the class "refuses to answer" above MAX_CLOSURE_IDS. It doesn't — it only sets a flag; the refusal lives in MigrationController::migrate() and MigrateCommand::migrate(). Corrected to name both call sites. Also covers `data.scope` in POST /migrate's response, per the brief's contract.
Feeds the "let me choose" picker with matching products or customers as the owner types. Products search post_title and the SKU lookup; customer search runs two queries against wc_orders under the shared status scope so a registered customer's kind is distinguished from a guest's, since MigrationScope stores them in different fields. Results are capped at 50 and report truncation so the picker can say "keep typing" rather than implying it showed everything. Read-only, same as /preview.
The picker was showing products of a type ProductMigrator does not source (a LearnDash course, for instance). A pick like that travels unfiltered into MigrationScope::productIds() and only evaporates later as counts['product'] === 0, with nothing telling the owner why. Search now excludes them at the query itself via a NOT IN on the product_type term join, reusing PreflightCheck::unsupportedProductTypeCounts() — the single source for "which types are unsupported" this project already centralised once — rather than a second copy of the list. Also adds a byte-identical esc_like() to the shared test wpdb stub (tests/stubs/test-bootstrap.php) so callers guarded with method_exists($wpdb, 'esc_like') exercise the real WordPress algorithm under test instead of only ever hitting their own fallback.
Adds state.scope, state.preview, state.previewLoading and state.previewSupport to useMigration(), plus the setScopeMode(), refreshPreview() and applyRemedy() actions, and the exported serializeScope() helper that maps the UI scope shape onto the wire shape MigrationScope::toArray() expects. startMigration() now sends the serialised scope on POST /migrate and sends the owner back to the select screen on a 422 scope_closure_too_large refusal instead of stranding them on the progress screen for a run that never started. refreshPreview() degrades quietly (previewSupport = 'no') when POST /preview is not installed, leaving autoIncludeDependencies() as the fallback. -- pathspec: plugins/cartshift/src/composables/useMigration.js, plugins/cartshift/tests/js/scopeSerialisation.test.js, plugins/cartshift/resources/admin/dist/**
Renders POST /preview's counts and consequence descriptors: a table of what will migrate, non-zero consequences with their remedies as one-click buttons, and "Nothing left behind." when there are none. Falls back to the old whole-shop counts when previewSupport is 'no'. product_link_missing is rendered as "At least N" rather than a bare figure — it reuses PreflightCheck::countOrdersAffectedByTypes(), which excludes anything outside publish/draft/private, so the true count can be higher. Unrecognised consequence codes still render with whatever label and hint the server sent, so a new backend code does not need a front-end release to be visible. A too_large scope gets its own plain block above the counts instead of a table nobody should trust.
Fix round 1 on MigrationReceipt.vue (Task 13): - ScopeConsequences::describe() now takes an $isMinimum flag and puts it on every descriptor as `is_minimum`. Only product_link_missing's call site passes true, matching its docblock (structurally narrower than the truth — see productLinkMissingCount()). Every other consequence defaults to false. - MigrationReceipt.vue reads `row.is_minimum` instead of a hardcoded code set, so "At least N" is a property of what the server sends, not a front-end guess tied to one code string. A future floor- producing consequence (e.g. widening product_link_missing to trashed products) is safe the day it ships the flag, no front-end release needed. - Tests assert the property, not the case: a descriptor flagged is_minimum renders "at least" whatever its code, and product_link_missing without the flag does not — plus the existing PHP descriptor-shape test now pins is_minimum's presence and that exactly one known consequence carries it today. - aria-live=\"polite\" now wraps the counts table and closure note, and the consequences block (both the "Nothing left behind." notice and the populated list), not just the list. A screen-reader user narrowing their selection to nothing now hears the all-clear. The too_large block keeps role=\"alert\" untouched.
Rebuilds the select screen around the three-door scope model (everything /
since a date / let me choose), wiring it to the Task 12 useMigration scope
state and the Task 13 MigrationReceipt (now actually mounted). Adds
ScopePicker.vue: a debounced GET scope/search box with result buttons and
removable chips, kept generic over product/customer kind.
Scope changes (mode, date, picked items, the upward offer) are debounced at
300ms before calling actions.refreshPreview(), matching the picker's own
debounce. The upward-offer sentence uses only the verified closure numbers
from state.preview.closure — the spec mock-up's illustrative "N orders
contain them" clause is dropped since the backend has no such field to back
it with an exact count.
Also makes PageHeader.vue's theme injection optional (inject('theme', null)
+ v-if guard around the switcher) — it previously threw on any screen
mounted standalone without a theme provider, which the new component tests
exposed as a pre-existing gap.
…face 422, prime the receipt
Review findings addressed:
- D1: reverted PageHeader.vue's optional theme injection — nothing in
production mounts it without a theme provider (App.vue:25 always supplies
one), so the loosening only traded a loud failure in shared code for a
silent one. Fixed the test to provide a fake theme instead.
- C1 (critical): the upward-offer sentence quoted state.preview.closure
numbers that are structurally 0/0 at the moment the offer is shown —
ScopeResolver only computes the products-containing-orders closure once
includeOrdersForProducts is already true. Reworded qualitatively ("the
summary will show exactly how many"); the real numbers now surface only
after the tick, via the receipt's own closure note, once they are real.
- C2 (critical): the 422 scope-too-large refusal and the empty-selection
guard both set state.error, but SelectScreen rendered it nowhere. Added a
role="alert" notice at the top of the scope column.
- I1 (important): the receipt was blank on the default "Everything" door —
nothing called refreshPreview() until the owner touched a control. Added
one refreshPreview() call onMounted.
- I2 (important): the debounce watch only tracked state.scope, so unticking
an entity left the receipt showing its old counts. Now watches
state.selectedEntities too.
- I3 (important): applyRemedy() pushes bare {id} products with no label,
which rendered as a blank chip with only an x. ScopePicker now falls back
to "#id" for both the chip label and matching/removal.
Deferred to the final whole-branch review, as instructed: search request
sequencing, focus loss on chip removal, unread previewSupport, redundant
order 1/2 in the CSS.
R1 (critical): I1's unconditional refreshPreview() on mount collided with
C2's unconditional error banner — a transient failure (500, timeout, dropped
connection) on the very first preview, before the owner touched anything,
now surfaced as a role="alert" box on arrival.
useMigration.js's refreshPreview() takes an options argument, {silent: true},
that skips the state.error assignment on failure while leaving the 404/501
previewSupport feature-detection handling untouched. SelectScreen.vue's
onMounted() call is the only caller that passes it; the debounced
owner-initiated refresh (on every scope/entity edit) still calls it with no
arguments, so a real failure there is reported exactly as before.
Covering tests:
- scopeSerialisation.test.js: {silent: true} swallows a 500 but not a 404
(previewSupport still flips to 'no'); an unsilenced call still sets
state.error.
- selectScreen.previewSilence.test.js (new): wires the real useMigration()
composable through SelectScreen (not the stubbed actions the rest of
selectScreen.test.js uses, which cannot exercise this) — a 500 on mount
shows no banner and leaves the receipt unprimed; a 404 on mount still
flips previewSupport; a subsequent owner-initiated refresh that fails
does show the banner, guarding against over-correcting into permanent
silence.
- selectScreen.test.js: tightened the existing mount/debounce assertions to
check the {silent: true} vs. no-argument call shape, not just call counts.
useApi.js unwraps one data level before building the error, so err.payload
is already {code, message, scope}. The refusal branch read
err.payload.data.code and never fired: an owner over the closure cap was
left on a progress screen, with a batch-retry control, for a run the server
had refused to start.
The test that covered it built a payload shape useApi cannot produce, so it
passed against the bug and would have passed against the fix. Corrected, and
backed by a second test that drives the real useApi over a real fetch
response carrying MigrationController::migrate()'s exact envelope.
Three defects, one premise: that a scope which narrows nothing loses nothing. It does. ProductMigrator sources only the supported product types, so the catalogue never travels whole. subscription_paused_missing_product returned 0 for every non-explicit mode, and in explicit mode weighed products against closedProductIds(), which is not filtered by type. A subscription selling a LearnDash course migrated paused under a plain Everything run while the receipt said nothing would be left behind. It is now counted in every mode, with the type test OR'd into the closure test in SQL rather than by fetching every subscription in the shop. coupon_disabled_missing_restrictions kept its own idea of which lost restriction disables a coupon, and disagreed with CouponMapper, which is the code that decides. It counted _exclude_product_ids, deliberately absent from WIDENING_ON_TOTAL_LOSS, and ignored both category keys, which are on it — it over-reported and under-reported at once. It now reads that list. WIDENING_ON_TOTAL_LOSS is public for exactly this reason. And consequences are filtered by the entities being migrated: untick Orders and 'order items link to no product' is not a smaller number, it is not a fact.
Four ways the panel spoke for a run nobody was going to get. The preview asked with the raw ticks, so an empty selection widened to all five entities server-side: arriving with nothing ticked showed the whole shop's figures under 'What will come across', beside a Start button that refuses for want of a selection. Nothing ticked now asks nothing, and the panel says so. Ticking Orders alone migrates products and customers too — startMigration() resolves dependencies after the last preview. The preview now resolves them first, so the counts include what the run will actually touch. A null preview fell through to the whole-shop counts under the same heading, with a date or an explicit selection active. There is no fallback now: the panel names which silence this is — nothing chosen, still working, endpoint absent, lookup failed — and prints no number for any of them. previewSupport was written and never read; it is read here. Plus the chips list: aria-live, and focus back to the search field when a chip is removed. It is the one control that decides what migrates and a screen-reader user got no word that anything had happened.
Filtering consequences by entity type removed a fact that was mislabelled as an order's. It left nothing in its place: tick only Products, in a shop with LearnDash courses in it, and the receipt rendered 'Nothing left behind.' over a run that drops every one of them. A claim of zero the backend can disprove is the same defect as a count it cannot produce. Products are the one entity that can lose something without an order or a coupon to lose it through, so they get their own consequence, on the existing UnsupportedProductType code and the same PreflightCheck slug list everything else reads. Scope-aware via productPredicate(), so an explicit pick holding no unmigratable product still reports zero. No remedy: nothing the owner can add to the scope makes a course migratable, and a button that pretends otherwise is worse than none. Also corrects the SelectScreen comment that still described the whole-shop fallback the previous commit deleted.
ScopeConsequences' own tests prove it narrows by entity type when asked. They prove nothing about whether anybody asks — reverting ScopePreview::build() to all() left the whole suite green, and ScopePreview had no test file at all. That is the exact shape of the defects this branch has already had to take back: a correct mechanism nobody was calling. No migrators are passed, so counts stay empty and these tests are about one thing: the payload the receipt reads describing the run about to start.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55d8dfce30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ($mode === self::MODE_SINCE && $since === null) { | ||
| return self::everything(); |
There was a problem hiding this comment.
Reject an unusable date instead of widening the run
When the user selects “Everything from a date” but leaves the date input empty, the UI sends {mode: 'since', since: null} and this branch silently converts it to everything. The preview response is not copied back into the UI's scope, so the radio still says date-limited and pressing Start migrates the entire store. Reject the REST migration request or prevent Start until a valid date is present rather than changing the requested mode.
Useful? React with 👍 / 👎.
| if (state.scope.mode !== 'explicit') { | ||
| state.scope.mode = 'explicit'; | ||
| } |
There was a problem hiding this comment.
Do not turn whole-store remedies into explicit scopes
When an everything or since preview reports an unsupported product used by a subscription or coupon, the backend attaches an add_products remedy; clicking it enters this branch and replaces the requested broad mode with an explicit scope containing only those product IDs. That can drop all intended orders, customers, and subscriptions, and the unsupported products still cannot migrate. Remedies should only change an already-explicit scope, or the backend should omit remedies that cannot close the gap in the current mode.
Useful? React with 👍 / 👎.
| 'included_products' => '_product_ids', | ||
| 'excluded_products' => '_exclude_product_ids', | ||
| 'included_categories' => '_product_categories', | ||
| 'excluded_categories' => '_exclude_product_categories', |
There was a problem hiding this comment.
Query the actual WooCommerce coupon meta keys
WooCommerce stores these core coupon fields as product_ids, exclude_product_ids, product_categories, and exclude_product_categories, without leading underscores. Consequently the later pm.meta_key IN (...) query normally returns no restriction rows, so the preview reports no affected coupons even though CouponMapper, which reads through the WC coupon getters, can subsequently disable them when their restrictions disappear.
Useful? React with 👍 / 👎.
| return 0; | ||
| } | ||
|
|
||
| return PreflightCheck::countOrdersAffectedByTypes($unsupported); |
There was a problem hiding this comment.
Restrict missing-product counts to the selected orders
For since and explicit scopes, this calls the preflight helper that counts affected orders across the entire migratable store and never applies ScopeResolver::orderPredicate(). A selection containing no order with an unsupported product can therefore still be shown the whole-store loss count, while a small selection may be told that thousands of its orders will lose links. Count only rows matching the current order predicate.
Useful? React with 👍 / 👎.
| scope: serializeScope(state.scope), | ||
| }); | ||
|
|
||
| state.preview = data; |
There was a problem hiding this comment.
Ignore preview responses for superseded scopes
Rapid scope or entity edits can leave multiple preview requests in flight, but every response assigns state.preview unconditionally. If an older request finishes after the newer one, its counts and consequences remain displayed for the current selection, so the user can confirm a run using a stale receipt. Track a request generation or abort superseded requests before assigning the response.
Useful? React with 👍 / 👎.
Sixteen tasks, thirty-three commits, already tagged and released as
cartshift/v1.3.0— but never merged, somainstill reads 1.2.2 in both the plugin header andversions.json. This closes that gap.It is also the first time CI has seen any of it: the branch was never opened as a PR, and
ci.ymltriggers onpull_requestonly.What it does
A shop owner can migrate a subset rather than everything — chosen products, chosen customers, a date cutoff, or the lot. The scope engine resolves dependencies upward, so an order brings its products and its buyer, because history migrates complete; live instructions never migrate broken. A receipt panel states what will come across and what will not, updating as the selection changes, and the migration endpoint refuses a selection it cannot honour rather than starting a run it cannot finish.
Verified on a real store
Dry-run only against lapka, no writes outside CartShift's own two tables. Every baseline held: 25 products, 363 customers, 699 orders, 50 coupons, 30 subscriptions; coupon collisions steady at 8 (genuine duplicate
WRACAM-codes);cartshift_id_mapat 0 rows after all five dry runs.The "41 orders contain unmigratable products" figure was confirmed by an independent SQL join replicating the HPOS
_product_idpattern — closing the one question the Task 14 review left open.--sincewas checked properly rather than assumed: a 2024-01-01 cutoff returned counts identical to unscoped, which looked like a bug until the store's earliest order turned out to be 2024-10-25. Re-running at 2026-06-01 dropped orders to 44. It works.Gates
233 Vitest, 706 PHPUnit / 2,266 assertions, zero Risky.
What the reviews caught, because it says something about the code
Every late defect on this branch came from a contract verified on one side only. A 422 refusal built carefully and rendered nowhere. A receipt fully tested and mounted nowhere. Test doubles orphaned when an interface changed. An
err.payload?.data?.coderead one level too deep, whose test hand-built a payload shapeuseApicannot produce — so the test pinned the bug and passed either way.The other recurring shape was a number that is structurally zero at the moment it is displayed: an upward offer quoting closure counts before the tick that makes them non-zero, a subscription consequence compared against a type-unfiltered product set, a coupon rule that disagreed with the mapper that actually decides. All fixed, each with a test that fails without the fix.
Merging
Not a fast-forward — diverged at
f6b012b,mainis 35 ahead. A merge commit is correct here rather than a rebase: the released tag points at55d8dfcon this branch, and rewriting those commits would leave the published ZIP with no reachable commit behind it.merge-treereports the merge clean.Carried forward, in the ledger
Four deferred minors, plus three follow-ups worth their own change: unify the supported-product-type predicate positively across picker, preflight, consequences and
ProductMigrator::getProductTypes()(the last is environment-gated and calls a subscription-typed product migratable on a store without WooCommerce Subscriptions); reject an emptyentity_typesserver-side rather than widening it; and move dependency resolution server-side, sincePOST /migratestill runs whatever a non-browser client hands it.