Add update() (full-replace PUT), CustomerRequest::fromResponse(), typed Customer fields#4
Merged
Merged
Conversation
Addresses end-user DX feedback: - Client::put() + ClientInterface::put() (shared send() pipeline with post()) - ResourceEndpoint::updateOne() + update() on CustomersEndpoint and DraftOrdersEndpoint (e-conomic full-replace PUT) - CustomerRequest::fromResponse(Customer) read-modify-write prefill helper; reference objects and untyped scalars extracted from $raw, malformed raw data throws instead of silently dropping (full-replace would clear it) - All Payload request DTOs are now mutable (final class, no readonly) so the RMW flow is prefill -> assign -> update(); rector skips ReadOnly* rules for src/Request/Customer and src/Request/Order - Customer response: typed telephoneAndFaxNumber/mobilePhone, lastUpdated is now ?DateTimeImmutable (wire string stays in $raw['lastUpdated']) - Fix pre-existing CoversClass on the EconomicException interface (rejected by PHPUnit 11 under a coverage driver, would fail CI's mutation job) - Delete AUDIT.md (closeout note says safe to delete; was already staged) BC notes for next pre-release tag: Customer::$lastUpdated type change, ClientInterface gained put(), request DTOs no longer readonly.
Verified the full-replace PUT claims against the official e-conomic REST
docs. PUT semantics ("overwrite the full object and must include the full
entity") and absent-field-clears ("To null a property you must exclude it
from your JSON on the write operation") are confirmed verbatim.
One nuance surfaced: eInvoicingDisabledByDefault "is updatable only by
using PATCH to /customers/:customerNumber" — the API's sole exception to
its no-PATCH-on-JSON rule. Its value in a PUT body is ignored and it is
not cleared by omission, so mutating it on a prefilled request has no
effect through update(). Documented on CustomersEndpoint::update(),
CustomerRequest::fromResponse(), the README RMW section, and CLAUDE.md.
CustomerRequest::fromResponse() and its five raw-extraction helpers are replaced by a single generic Payload::fromResponse(Resource): static that maps $response->raw into the calling DTO via a dedicated Valinor mapper: - Payload becomes an abstract base class (was a marker interface); the eight request DTOs now extend it instead of implementing it - Identifier::fromReference(array) infers the JSON field name from the reference object's single scalar <x>Number key, so one constructor covers every reference type and identifiers round-trip back to the exact shape the server produced - the request mapper is strict (no scalar casting - malformed raw data throws, never silently dropped), allows superfluous keys (raw carries the full response body), and converts the SDK's guard exceptions into mapping errors with node-path context via filterExceptions() - MappingError is wrapped in InvalidArgumentException carrying the "requires a response fetched through the SDK" hint with the Valinor error preserved as $previous DraftOrderRequest::fromResponse() falls out for free, closing the read-modify-write gap for orders: nested Payloads (Recipient, Delivery, References, Line) and their Identifiers map recursively. Covered by a full-body order round-trip test mirroring the customers one, plus direct Identifier::fromReference() unit tests. Trade-off: the parameter type widens from Customer to Resource (PHP cannot narrow parameter types in overrides), so passing the wrong resource class is a runtime mapping error rather than a compile-time type error.
The Unit tests (8.4, lowest) job has been failing since before this branch: on valinor <= 2.2.1 the RawStamper converter never fires, so $raw is empty on every mapped Resource (breaking ResourceRawTest on 1.x, and cascading into the new fromResponse() pipeline here), and mapping error paths/messages differ. Bisected the floor empirically: 2.0.0, 2.1.x, 2.2.0 and 2.2.1 all fail in different ways; 2.2.2 is the first version where the full suite passes (2.2.2 ships "handle object arguments default value", which the DTOs' defaulted constructor args rely on). Verified both resolution ends locally: --prefer-lowest (2.2.2) and highest (2.4.0) — phpunit, PHPStan, ECS, Rector and composer validate/normalize all green.
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
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
Implements four end-user DX feedback items, in their priority order:
update()support — newClient::put()/ClientInterface::put()(mirrorspost(), both delegate to a shared privatesend()),ResourceEndpoint::updateOne()pipeline, and typed one-linersCustomersEndpoint::update(int, CustomerRequest): CustomerandDraftOrdersEndpoint::update(int, DraftOrderRequest): Order.telephoneAndFaxNumber/mobilePhoneare now?stringproperties on the responseCustomer(previously$raw-only).Payload::fromResponse(Resource $response): static, inherited by every request DTO: the response's full decoded body ($raw) is mapped into the request DTO via a dedicated Valinor mapper. Reference objects becomeIdentifierinstances via the newIdentifier::fromReference()(field name inferred from the reference's single scalar<x>Numberkey, so identifiers round-trip back to the exact shape the server produced); server-computed fields, HATEOAS links and unmodeled schema fields are dropped as superfluous. The mapper is strict — present-but-malformed raw data throws (with node-path context) instead of being silently dropped, because a dropped field would be cleared server-side by the full-replace PUT. This works for orders too:DraftOrderRequest::fromResponse($order)maps nested Payloads (Recipient,Delivery,References,Line) recursively. To make mutation-after-prefill work, allPayloadrequest DTOs are mutable (final class, noreadonly);Payloaditself became an abstract base class (was a marker interface);rector.phpskips the ReadOnly rectors forsrc/Request/Customerandsrc/Request/Order.Customer::$lastUpdatedis now?\DateTimeImmutable— Valinor's default date handling maps e-conomic's2020-02-19T09:18:09Zformat with no builder changes; wire string remains at$raw['lastUpdated'].Also in this PR:
Payloadtransformer is mandatory, not cosmetic, since e-conomic rejects explicit nulls.eInvoicingDisabledByDefaultis "updatable only by using PATCH to /customers/:customerNumber" (the API's sole exception to its no-PATCH-on-JSON rule) — its value in a PUT body is ignored, so mutating it on a prefilled request has no effect throughupdate().#[CoversClass(EconomicException::class)]on an interface inExceptionHierarchyTest— PHPUnit 11 rejects interfaces as coverage targets, which would fail the CI mutation job.AUDIT.md(its closeout note says it is safe to delete; the deletion was already staged).supportDateFormats()caveat. CLAUDE.md updated to match.BC notes (pre-release
1.x, latest tagv1.0.0-alpha)Customer::$lastUpdatedchanged type?string→?\DateTimeImmutableClientInterfacegainedput()readonly;Payloadis an abstract class instead of an interface (implements Payload→extends Payload)fromResponse()takesResource(inherited generic) — passing the wrong resource class is a runtime mapping error, not a compile-time type errorTest plan
fromResponse(), mutate, PUT, andassertSamethe entire serialized body; directIdentifier::fromReference()unit tests;put()cases inClientTest; phone/lastUpdatedmapping cases