Skip to content

DGS-444, DGS-446: courier request pickup time slots + response parsing fix - #150

Closed
GantasG wants to merge 4 commits into
mainfrom
DGS-446-courier-request-pickup-time-slots
Closed

DGS-444, DGS-446: courier request pickup time slots + response parsing fix#150
GantasG wants to merge 4 commits into
mainfrom
DGS-446-courier-request-pickup-time-slots

Conversation

@GantasG

@GantasG GantasG commented Aug 31, 2026

Copy link
Copy Markdown

Tickets

  • DGS-446 — Courier Request fails and crashes: module still sends single pickupTime, DPD API now requires pickupTimeFrom/pickupTimeTo (Critical)
  • DGS-444 — default pick-up time isn't aligned to DPD's allowed time slots (Major)

One branch for both: they share the root-cause chain, and the slot work that fixes DGS-446 subsumes DGS-444.

What changed

Request side:

  • The 'Desired pick-up time' / 'Last pick-up time' free datetime inputs are replaced with a Pick-up date field and two slot dropdowns constrained to DPD's whitelists, so an invalid value can't be submitted at all (DGS-444's suggested fix).
  • Defaults are slot-aligned: next available slot today (e.g. 11:20 now → 12:00), or tomorrow 08:00 after the last slot — instead of the raw 'now + 30 min' second-precision value DPD always rejected.
  • CourierRequestValidator::validatePickupTimeSlots() re-checks the whitelist server-side (a tampered POST gets a clear error, not an API round-trip).
  • The API payload now carries pickupDate/pickupTimeFrom/pickupTimeTo alongside the legacy pickupTime/senderWorkUntil datetimes — kept for DPD endpoints still on the old contract (EE/LV) and for the module's own DPDCourierRequest entity, whose stored columns are composed from the slot fields.

Response side (the crash):

  • New CourierRequestResponseParser accepts both response shapes: legacy <p>DONE / 402 Error: strings and the new JSON object, whose body->message (or errlog) is surfaced to the merchant. Previously the stdClass reached strpos() and fatally crashed the admin page on PHP 8 (TypeError, uncatchable by the controller's catch (Exception)).

Depends on

⚠️ Invertus/dpdbaltics-api#6 — adds the slot setters to CourierRequestRequest (the payload is built by that package's jsonSerialize(), so the new params can't reach the wire from module code alone). Merge that into developer first, then composer update invertus/dpdbaltics-api here to bump composer.lock (will push that bump once the api PR lands). Until then a fresh composer install of this branch runs the old DTO and courier submit would fatal on the missing setters.

Reproduction & verification (local PS 8.2.7 / PHP 8.1, DPD live endpoint stubbed with wiremock replaying the exact live rejection)

Both bugs reproduced on main before the fix:

  • Default pick-up time auto-filled as 2026-08-31 11:35:26 (raw now+30min, no slot) — DGS-444 confirmed.
  • Submitting a fully valid in-slot time → request rejected (module sends single pickupTime only) → HTTP 500 with strpos(): Argument #1 ($haystack) must be of type string, stdClass given — DGS-446 confirmed byte-for-byte.

After the fix, against the same stubs:

  • Form shows date + slot dropdowns, default snapped to the next valid slot (12:00).
  • DPD rejection renders as a normal BO error showing DPD's actual message — no 500.
  • Success stub (legacy <p>DONE): 'Successful creation' confirmation, record saved with composed datetimes, list view unchanged.
  • Tampered POST with pick_up_time_from=11:35 rejected server-side with the whitelist message, no API call.

Captured outgoing request after the fix (wiremock):

pickupTime=2026-08-31 12:00:00   senderWorkUntil=2026-08-31 17:00:00   (legacy, kept)
pickupDate=2026-08-31   pickupTimeFrom=12:00   pickupTimeTo=17:00     (new)

Unit tests added (run green on PHP 7.2 via the module's PHPUnit 5.7): CourierRequestResponseParserTest (8 tests, incl. the stdClass regression fixture DGS-446 asks for), PickupTimeSlotUtilityTest (4), CourierRequestValidatorTest (6). The pre-existing tests/Unit suite is broken on main (network-dependent API tests, _PS_VERSION_ undefined) — untouched.

Browser QA: BO form smoke + interaction pass done on PS 8.2.7 (defaults, valid submit, DPD-rejection submit, tampered-value submit, success submit). Module list view after save verified. FO untouched by this change.

Verdict: ship with follow-ups (the two items below).

Needs human verification

  • One real submission against the live LT endpoint (integracijos.dpd.lt) with production credentials. The new param names (pickupDate/pickupTimeFrom/pickupTimeTo) match DPD's current API data model and the live error message, but DPD's ws-mapper docs for the new contract aren't public, and the sandbox doesn't enforce it (per both tickets), so the exact wire naming is unverified against production. If DPD expects different naming, the module now shows DPD's answer instead of crashing, so the failure mode is a readable error.
  • Success-response shape of the new contract: parser treats legacy DONE strings as success and any JSON object as a surfaced error. If the live endpoint starts answering success as JSON, the success branch needs the shape added.

Confidence: 75%

Notes

  • wiremock/mappings/ carries the stub replaying DPD's live rejection — the workspace's wiremock container mounts this dir, so the repro stays reproducible for QA.
  • DB schema untouched; pick_up_time/sender_work_until columns still store full datetimes composed from the slot fields.

@GantasG
GantasG force-pushed the DGS-446-courier-request-pickup-time-slots branch from 8ddf281 to 1fe1ed6 Compare August 31, 2026 09:42
…ontract

DPD's live pickupOrderSave_ endpoint now rejects any pickup time that is not
one of its fixed pickupTimeFrom/pickupTimeTo slots, and answers with a JSON
object instead of the legacy plain-text response.

Request side (DGS-446 + DGS-444):
- The free-form 'Desired pick-up time' / 'Last pick-up time' datetime inputs
  are replaced with a pick-up date field and two slot dropdowns constrained
  to DPD's whitelists, so an invalid value can no longer be submitted.
- Defaults are slot-aligned (next available slot today, or tomorrow 08:00)
  instead of the raw 'now + 30 min' value DPD always rejected.
- CourierRequestValidator rejects non-whitelisted slots server-side.
- The API request now carries pickupDate/pickupTimeFrom/pickupTimeTo alongside
  the legacy pickupTime/senderWorkUntil datetimes, which are kept for DPD
  endpoints still running the old contract and for the module's own entity.

Response side (DGS-446):
- CourierRequestResponseParser accepts both response shapes: the legacy
  '<p>DONE' / '402 Error:' strings and the new JSON object, whose
  body->message (or errlog) is surfaced to the merchant. Previously the JSON
  object reached strpos() and fatally crashed the admin page on PHP 8.

Requires invertus/dpdbaltics-api with CourierRequestRequest slot setters
(see the companion dpdbaltics-api PR); composer.lock to be bumped once that
lands on the developer branch.
@GantasG
GantasG force-pushed the DGS-446-courier-request-pickup-time-slots branch from 1fe1ed6 to ef461e8 Compare August 31, 2026 09:47
GantasG and others added 3 commits August 31, 2026 12:55
actions/cache@v1 is auto-failed by GitHub since its shutdown, so both jobs
died before running on any PR. Bump checkout/cache/auto-commit to current
versions, pin PHP 7.4 (autoindex and header-stamp are PHP 5/7-era tools),
and install with --prefer-source because the lock file's legacy.zip dist
URLs are no longer served by GitHub.
@GantasG

GantasG commented Aug 31, 2026

Copy link
Copy Markdown
Author

Closing — wrong repo: the live merchants run the module from DPDBaltics/PrestaShop (v3.3.1, the version in DGS-444/DGS-446). The fix is being re-applied and re-verified there. The companion API-package PR Invertus/dpdbaltics-api#6 stays open — DPDBaltics/PrestaShop depends on the same invertus/dpdbaltics-api package.

@GantasG GantasG closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant