Skip to content

Add delete carrier endpoint#160

Open
luigimassa wants to merge 19 commits into
PrestaShop:devfrom
bwlab:add-delete-carrier-endpoint
Open

Add delete carrier endpoint#160
luigimassa wants to merge 19 commits into
PrestaShop:devfrom
bwlab:add-delete-carrier-endpoint

Conversation

@luigimassa
Copy link
Copy Markdown

Questions Answers
Description? Add CQRSDelete operation on Carrier resource mapped to DeleteCarrierCommand, CarrierConstraintException → 422 to exception status mapping, filtersMapping for carrierId and isFree on CarrierList
Type? new feature
BC breaks? no
Deprecations? no
Fixed ticket?
Sponsor company bwlab (Luigi Massa)
How to test? Send DELETE /carriers/{carrierId} with a valid carrier ID and verify 204 response; Send DELETE /carriers/{carrierId} with a non-existent ID and verify 404 response; Send DELETE /carriers/{carrierId} with an invalid ID and verify 422 response;

luigimassa and others added 17 commits March 5, 2026 17:20
| Questions     | Answers
| ------------- | -------------------------------------------
| Description?  | Add GetRequiredFieldsForAddress CQRS endpoint
| Type?         | new feature
| BC breaks?    | no
| Deprecations? | no
| Fixed ticket? | N/A
| Sponsor company | @PrestaShopCorp
| How to test?  | Ask a dev
Expose ToggleCarrierStatusCommand and ToggleCarrierIsFreeCommand via
PUT /carriers/{carrierId}/toggle-status and
PUT /carriers/{carrierId}/toggle-is-free endpoints with carrier_write scope.
Add integration tests for both toggle endpoints.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add BulkCarriers API resource with PUT /carriers/bulk-set-status endpoint
that maps to BulkToggleCarrierStatusCommand, following the same pattern
used for taxes and zones bulk status toggle.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without this constraint, passing carrierId=0 causes a CarrierConstraintException
thrown during command construction (before the command bus), which bypasses
the exceptionToStatus mapping and returns 500 instead of a proper error.
The Assert\All([Assert\Positive()]) constraint catches invalid IDs during
Symfony validation phase and returns a 400 Bad Request with details.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nd instantiation

CQRSCommand/CQRSUpdate automatically sets input to the CQRS command class,
causing CQRSApiNormalizer to instantiate BulkToggleCarrierStatusCommand
directly during the DeserializeListener phase. This bypasses the DTO
and passes raw string values from JSON to the command constructor, where
(int) cast on non-numeric strings produces 0, triggering CarrierConstraintException.

Setting input: BulkCarriers::class forces the deserialization to use the
DTO class, letting the CommandProcessor handle command instantiation
with proper type handling.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rmalizer flow

Removing input: BulkCarriers::class lets CQRSCommand auto-set input to
BulkToggleCarrierStatusCommand, so CQRSApiNormalizer validates BulkCarriers
constraints (Assert\Positive on carrierIds) before applying CQRSCommandMapping
([enabled] -> [expectedStatus]) and instantiating the command.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Non-numeric values (e.g. strings) in carrierIds array were bypassing
Assert\Positive and causing CarrierConstraintException during command
instantiation, resulting in HTTP 500 instead of 422.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ping

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ps-jarvis
Copy link
Copy Markdown

Hello @luigimassa!

This is your first pull request on ps_apiresources repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

Comment thread src/ApiPlatform/Resources/Carrier/Carrier.php Outdated
Comment thread src/ApiPlatform/Resources/Carrier/Carrier.php
Comment thread src/ApiPlatform/Resources/Carrier/CarrierRanges.php
Comment thread TODO.md Outdated
Comment thread tests/Integration/ApiPlatform/CarrierEndpointTest.php
Comment thread src/ApiPlatform/Resources/Address/Address.php
luigimassa and others added 2 commits March 30, 2026 07:40
Replace individual property assertions with a single assertEquals
on the entire carrier array, following the project test conventions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use property names instead of getter method names in QUERY_MAPPING,
and only map properties where names differ between QueryResult and API.
Remove TODO.md as it should not be committed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nicosomb
Copy link
Copy Markdown
Contributor

nicosomb commented May 7, 2026

@luigimassa CI is red, could you have a look please?

@ps-jarvis ps-jarvis moved this from Ready for review to Waiting for author in PR Dashboard May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Waiting for author

Development

Successfully merging this pull request may close these issues.

5 participants