Repository navigation
feat: validate from the generated interfaces and adopt the shared base - #15
Conversation
…eplace fulfillment_address and fulfillment_option_id with fulfillment_details and selected_fulfillment_options, and move the payment provider into capabilities.payment.handlers where the spec keeps it.
…ts, and serve the well-known ACP discovery document.
…s[] breakdown, and carry the quantity the spec requires on LineItem rather than only on Item.
…tial token from where the spec keeps them, and serve the JSON Schemas the payment handler advertises.
…r while the original request is in flight, 422 when a key is reused with a different body. Drops the two error types the spec's enum does not carry.
…pping the code field MessageInfo forbids and replacing cart_not_active with the conflict code the enum actually allows.
…ule was restating verbatim. Fixes an annotation that cast a Magento sales order to the ACP order DTO.
…al, tax and total into the typed totals[] breakdown the spec declares.
…ctly, deleting the module's response interfaces and DTOs which had become verbatim subsets.
…xtend the generated one so it adds only the quantity the spec's schema omits.
…ng; it resolved only because the root project installed it.
…bit_AgenticCore. The buyer now gates on email, which the schema requires, instead of demanding both names as well.
… the abandoned-claim takeover it never had. Data patch maps its column names and its zero-instead-of-null in-flight status, and the module finally gets a schema whitelist.
…n map becomes two routes with different methods, and its any-shared-segment match is replaced by an anchored pattern.
…cts go with it: a single-word name no longer discards the address, an absent field no longer erases a populated one, the selected method reaches the shipping assignment, an unknown SKU is a message rather than rejecting the whole cart, and errored carrier rates are no longer offered. Also binds nine spec types the module built through factories without a preference, which made discovery and any session carrying fulfillment details fail at runtime.
…ove ACP onto it. Order placement no longer waits on the receiver, and a failed delivery is retried instead of logged and lost.
…CP stops inferring a placed order from the reserved increment id, so an abandoned payment no longer reports as completed.
…rays. The spec runtime returns null for anything that is not already the right instance, so a submitted delivery address was silently discarded and the payment credential token — two levels down — was never readable, which made completion refuse every request. Its validator also still required the pre-adoption flat token and provider pair rather than handler_id and instrument.
…e sender hardcoded one protocol's Merchant-Signature and Request-Id, which the second consumer cannot use — it signs per RFC 9421 with Webhook-Id and Webhook-Timestamp instead. The queue now asks a DeliveryHeadersProvider per attempt and knows nothing about signing.
…second module delivers with different headers.
…link after ac_order_id lost its writer
…he golden fixtures against the schema
…dropping the hand-written constraints
…yed rather than refused
✅ Magebit Code Review — Review completeReviewed up to No issues met the confidence threshold to publish. Findings checklist
Changed files (174).github/
Api/
Controller/
+124 more files… Reviewed by |
![magebit-automation[bot]](https://avatars.githubusercontent.com/in/3754365?s=60&v=4){kind=small}
|
Five of the six findings are addressed. One does not hold up. Coupon wiped by an update that omits discounts — fixed. Migrated idempotency replies never decrypted — this one is wrong, on both premises. The decrypt The replay parses as JSON and is identical to the first response. A migrated row takes the same Configurable children loaded in a loop — fixed. Child ids for the whole page are collected, then Feed loads every id before paging — fixed, the window is on the select. This finding was stronger Unused fixture — deleted. Commit subjects — correct, twenty-two of forty-one, all from earlier phases of this work. Not One thing the fix surfaced that was not in the report: the spec interface still carries a deprecated Verification: |
1 participant
Brings this module onto the pinned specification target and the shared base.
developalreadycarries an earlier slice of this branch through #14; this is everything since, and the branch is a
content superset of
develop, so nothing is lost.Requests are now checked against the specification, not against hand-written rules
The six request classes carried 509 lines of Symfony constraint trees restating rules the JSON
schema already states. Those are gone, and
symfony/validatoris gone from the dependencies withthem. A decoded body is now checked against the generated interface: a getter that cannot return
null is a required field, its return type is the field's type, its value constants are the values
allowed, and a new
CONSTRAINTSconstant carries lengths, patterns, formats, bounds and listcardinality straight from the schema.
The request classes were also rebuilt on the generated data objects, the way the other module's
already were. That removed
FulfillmentDetailsBuilder,PaymentDataBuilderandAuthenticationResultBuilderoutright: the shared hydrator walks the whole tree from the declaredreturn types, which is what all three did by hand.
Two bugs this exposed
still in flight rather than replayed. Every write now goes through one
respond()helper thatstores before sending, which is what stops it being forgotten again.
answer was discarded unless it was a replay. One decision per request now.
Taken from the shared base
Idempotency arbitration, what a quote still needs before an order can be placed, the stock check
(the two copies were byte-identical), the buyer writer, the total label, the scheduled dispatch and
the JSON endpoint plumbing. The two cart validators collapsed into one that words the shared
findings, and it gained the other module's region handling, which names an unresolvable region
instead of letting it surface at completion.
One deliberate tightening
A cart line item must now carry a quantity. It never was in the specification's own
Item, and thecart side used to default a missing one to a single unit while the checkout side refused it.
Guessing a quantity is worse than saying so, so both refuse it now and name the position that is
wrong.
Notes for review
d/checkpasses: phpcs, phpstan and 156 unit tests. Net −1,879 lines.Item.quantitydefect is documented in the specification library README; it is why the modulenarrows the item type rather than trusting the schema here.
CONSTRAINTSconstant thegenerator now emits; see feat: carry the schema's own validation keywords onto the generated interfaces acp-php-spec#3.
targets was superseded upstream and cannot be exercised from development.
cut until the new tables are confirmed populated on a real install.