fix(woocommerce): pagination + after-filter replaces 800-id include hack (#262)#265
Open
Avatarsia wants to merge 18 commits intoOpenXE-org:masterfrom
Open
fix(woocommerce): pagination + after-filter replaces 800-id include hack (#262)#265Avatarsia wants to merge 18 commits intoOpenXE-org:masterfrom
Avatarsia wants to merge 18 commits intoOpenXE-org:masterfrom
Conversation
Avatarsia
pushed a commit
to Avatarsia/OpenXE
that referenced
this pull request
Apr 21, 2026
Adds the three WooCommerce PRs that are currently open against openxe-org/openxe to the nightly production manifest: - fix/woocommerce-order-import-pagination (PR OpenXE-org#265) — foundation - feature/woocommerce-batch-stock-sync (PR OpenXE-org#266) — independent - refactor/woocommerce-composer-sdk (PR OpenXE-org#267) — stacked on OpenXE-org#265, listed after it so the sequential merge in Pass 2 picks the dependency up first and the composer refactor applies on top cleanly. Grouped under a dedicated header block so the intent stays visible once the PRs merge upstream and can be removed from the manifest.
Adds getHeaders() / getHeader() accessors to the inline WCResponse class and captures HTTP response headers case-insensitively via CURLOPT_HEADERFUNCTION. Required foundation for pagination handling (Issue OpenXE-org#262).
Exposes the underlying WCResponse of the most recent request so callers can read response headers (X-WP-Total, X-WP-TotalPages) without changing the existing JSON-body return contract. Follow-up to 291197d, required by the pagination work in issue OpenXE-org#262.
Reads felder.letzter_import_timestamp from shopexport.einstellungen_json with a 30-day fallback for first runs, and adds a persistLastImportTimestamp() helper that does a read-modify-write via DatabaseService named params. Infrastructure for the pagination loop in issue OpenXE-org#262; not yet called here.
Replaces the fake greater-than-id filter (800 hardcoded IDs) with the WC v3 after=<iso-8601> parameter and walks X-WP-TotalPages up to MAX_PAGES_PER_RUN=5 pages per run (500 orders). Persists a progress timestamp via persistLastImportTimestamp() after each processed order so aborted runs resume cleanly. Adds a one-shot ab_nummer->timestamp translation for existing shops transitioning from the legacy cursor. Fixes silent data loss when more than 20 orders arrived between runs. Issue OpenXE-org#262.
Captures lastImportTimestamp into a local variable before the pagination
loop so progress persistence inside the loop does not mutate the GET
filter. Without this, after=\$lastTs moves forward each iteration while
page advances too, causing 100 orders per extra page to be skipped.
Also fixes two smaller issues:
- resolveAbNummerToTimestamp() returns ts-1 so the strictly-after
filter does not lose the transition order.
- explode(';', \$this->statusPending) is now PHP 8.1+ safe via (string)
cast.
Follow-up to abe58aa, addresses code review findings on issue OpenXE-org#262.
Captures scope, fix parameters (MAX_PAGES_PER_RUN=5, 30-day first-run fallback, UTC timestamps), implementation steps, integration test matrix T1-T10, rollout and rollback strategy, risks and mitigations. Companion doc to issue OpenXE-org#262 and the fix commits on this branch.
$this->app->DatabaseService is only lazy-bound in the web context. When the shopimporter runs through the cron trigger the service is not available, which breaks the timestamp persistence path. Falls back to $this->app->DB with real_escape_string when DatabaseService is absent. Discovered during the WC 8.9.3 + 10.7.0 integration test matrix on the .143 test instance.
WCHttpClient::authenticate() had isQueryStringAuth() smuggled into its SSL-gating during f12b09a. That changed the auth scheme for existing HTTP-configured shops from OAuth 1.0a to basic-auth-over-query-string (since query_string_auth=true is set at client construction site). Restores the pre-f12b09a4 behaviour. The CLI-context fallback for persistLastImportTimestamp from f12b09a is kept. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The caller in shopimport.php uses $result[0] per iteration of a for loop capped by ImportGetAuftraegeAnzahl() and maxmanuell. Returning 500 orders per call therefore silently dropped 499 of them while advancing the server-side after-cursor past them. Restores the historical 1-order contract; the after-filter still replaces the legacy 800-id include hack, and per-order persist gives us resume-after-crash semantics with at most one order lost per crash (consistent with pre-OpenXE-org#262 behaviour). MAX_PAGES_PER_RUN and ORDERS_PER_PAGE constants are removed; the caller loop (bounded by maxmanuell, default 100) now owns the batch size. Follow-up to review on OpenXE-org#262. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After the external review of OpenXE-org#262 highlighted that shopimport.php expects $result[0] per RemoteGetAuftrag iteration, the internal pagination loop was dropped. Plan now reflects: single-order per call, caller loop bounded by maxmanuell, per-order progress persist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… when nothing to import shopimport.php:1308 gates on is_array($result), which happily accepts [] and then crashes trying to dereference $result[0]['id']. The pre-OpenXE-org#262 behaviour returned null on empty — restore that. Spotted by review of 7af9edb.
The defensive getKonfig($data['shopid'] ?? null, $data) call inside
ImportGetAuftrag() is actively harmful: CatchRemoteCommand('data')
returns the getauftrag-payload, which does not carry a shopid (see
class.remote.php:194/241). The re-init therefore clears $this->shopid
and rebuilds the WCClient from empty preferences. RemoteCommand()
already initialises the importer with the real shop id before dispatch
(class.remote.php:2685), so the duplicate call is both redundant and
broken. Spotted by re-review of OpenXE-org#262.
shopimport.php calls RemoteGetAuftraegeAnzahl() before RemoteGetAuftrag() in its main flow. If the stored cursor is still the 30-day fallback and all pending orders are older than 30 days, the count returns 0 and ImportGetAuftrag() never runs, so the one-shot ab_nummer -> timestamp migration never fires and the shop stays on the fallback forever. Extract the migration into a private helper and invoke it from both count and fetch paths. Idempotent via lastImportTimestampIsFallback.
The after-filter is strictly-greater-than, so orders sharing an identical date_created_gmt with the last processed order were silently dropped. Move to a tuple cursor: persist both timestamp and order id, query with after=<ts-1s> plus exclude=[last_id]. Orders with the same GMT second now reach the caller in subsequent iterations without duplicating the already-processed one. Schema is additive (new felder.letzter_import_order_id key in shopexport.einstellungen_json). Persistence helper becomes persistLastImportCursor; the single-argument persistLastImportTimestamp remains as a wrapper so the ab_nummer migration path keeps working without a second rewrite.
…ration Reflects re-review findings: tuple cursor (ts, id) for same-second order resilience, migration helper called from both count and fetch paths, scope list updated to match current single-order design.
…s offset Bei identischem date_created_gmt mehrerer Orders hielt exclude nur die zuletzt importierte ID. Nach zwei Orders im selben Bucket wurde die erste wieder sichtbar und Count- wie Fetch-Pfad liefen in eine Endlosschleife. Cursor persistiert jetzt die komplette Liste aller IDs innerhalb des aktuellen Sekunden-Buckets (felder.letzter_import_order_ids als JSON- Array). Bei Bucket-Wechsel wird die Liste zurueckgesetzt; bei gleichem Bucket wird die neue ID angehaengt. Gleichzeitig wird die -1s-Korrektur am Query gated: nur wenn mindestens eine exclude-ID bekannt ist, wird der after-Filter um 1 Sekunde nach hinten verschoben. Dadurch entfaellt die Doppel-Subtraktion nach der ab_nummer-Migration (resolveAbNummerToTimestamp schon -1s, Query war nochmal -1s -> 2s zurueck in der Vergangenheit). Der Erstlauf nach Migration liefert jetzt exakt die ab_nummer-Order als Startpunkt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Avatarsia
force-pushed
the
fix/woocommerce-order-import-pagination
branch
from
84fdb97 to
e6dd616
Compare
…ails If resolveAbNummerToTimestamp() cannot find the referenced legacy order (404, missing date_created_gmt, etc.) the migration previously left the importer on the volatile 30-day fallback, which is recomputed on every run as now()-30d. The cron cycle would then re-scan the same rolling window forever, multiplying API load and caller-dedup activity. On resolution failure we now explicitly persist the current fallback timestamp so the fallback flag flips to false and the lower bound stays stable across runs. Also emits a warning so the operator can spot the stale ab_nummer. Spotted by review of OpenXE-org#265.
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
Behebt stillen Datenverlust im WooCommerce-Bestellimport. Der frühere Code
verlor bei mehr als 20 neuen Bestellungen zwischen zwei Cron-Läufen Aufträge
durch fehlende Pagination und einen künstlichen 800-ID-
include-Filter.Fixes #262.
Changes
Nur
www/pages/shopimporter_woocommerce.phpund ein neues Plan-Dokument.after=<timestamp>-Filter auf WC-REST-v3 ersetzt den 800-ID-Hack.(letzter_import_timestamp, letzter_import_order_ids[])inshopexport.einstellungen_json. Orders mit identischemdate_created_gmtwerden über eine Bucket-akkumulierte
exclude-Liste sauber abgegrenzt.ImportGetAuftrag()liefert weiterhin max. eine Orderpro Call (wie vor Bug: WooCommerce Bestellimport verliert Aufträge bei >20 neuen Bestellungen (fehlende Pagination + künstlicher ID-Filter) #262). Der
shopimport.php-Caller-Loop iteriert.(konsistent mit Pre-Bug: WooCommerce Bestellimport verliert Aufträge bei >20 neuen Bestellungen (fehlende Pagination + künstlicher ID-Filter) #262-Verhalten).
ab_nummer-Cursor wirdeinmalig in einen Timestamp übersetzt (läuft in Count- und Fetch-Pfad).
DatabaseServicewoverfügbar, sonst
DB->Select/Update(war per Cron sonst nicht erreichbar).Nicht in diesem PR:
authenticate()-Logik ist unverändert gegenüber Base (SSL-only-Gating).Test Plan
End-to-End gegen zwei WC-Versionen (Docker-Testumgebung):
maxmanuell-CapSkipped (P3): T7 (
ab_nummer-Transition mit Legacy-Daten — via Code-Reviewstatisch verifiziert), T8 (Abort-Recovery — via Architektur verifiziert), T10
(800-Orders-URL-Länge — indirekt: längste gemessene URL 223 B vs. ~12 KB im Alt-Hack).
Known Limitations
älter als 30 Tage und ohne
ab_nummer-Cursor übersehen diese Orders. UI-Reset-Button als Folge-PR.
Upgrade, nicht OpenXE-spezifisch.
Follow-up Issues
Aus der Gesamt-Analyse entstanden separate Issues:
Reconcile-Pfad relevant auch mit Webhooks)
Review-Iterationen
Mehrere externe Code-Reviews mit Findings, alle adressiert und in der Commit-
Historie nachvollziehbar:
$runStartTimestamp-Stabilisierung,return nullbei leergetKonfig-Doppelaufruf inImportGetAuftragentfernt-1s-Offset gegen Doppel-Subtraktion nach ab_nummer-MigrationRollback
Branch in toto reverten. Die
letzter_import_timestamp/letzter_import_order_ids-Keys in
einstellungen_jsonwerden von der alten Version ignoriert. KeineDB-Schema-Änderung, kein Migrations-Risiko.