-
Notifications
You must be signed in to change notification settings - Fork 25
Specific prices #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Specific prices #110
Conversation
nicosomb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About src/ApiPlatform/Normalizer/DateTimeImmutableNormalizer.php, we plan to change the fields, to make them as nullable.
So this kind of hack won't be useful anymore.
@boherm @jolelievre @tleon are we ok with that?
f8e1c4f to
ab9bdf5
Compare
|
I’ve updated my code to follow the new conventions that require endpoint names to be plural. |
|
Hello @Jeremie-Kiwik From what I see, most changes are ok, but there is a conflict in services.yml that has to be resolved before continuing, which probably appeared once we merged your other PR. |
ab9bdf5 to
4793219
Compare
|
@kpodemski I rebased the PR and merged services.yml. It should be OK now! |
jolelievre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry many PRs have passed similar to this one but this module is not supposed to contain serialization logic nor providers
The core is meant to include all this generic logic and the module is only there for configuration, very often the core alreay allows handling use cases with the proper configuration and if it can't then the core should evolve not the module
Can you explain the reason why you needed a dedicated normalizer and a specific provider?
|
Hello @Jeremie-Kiwik Just a quick heads-up: reviews on pending Admin API PRs will start in the coming days. We first took some time to clarify and unify the Admin API contribution rules and ADR expectations. With that work done, the team will now review existing PRs based on those updates. Please note that, based on the updated standards described here: some aspects of this PR currently do not meet the requirements. For this reason, I've added the Invalid label for now. I see that @jolelievre asked about some of those aspects already. Could you please take a moment to answer us? Thanks for your patience. |
Endpoints summary
POST /admin-api/products/specific-prices
PATCH /admin-api/products/specific-prices/{specificPriceId}
DEL /admin-api/products/specific-prices/{specificPriceId}
GET /admin-api/products/{productId}/specific-prices
GET /admin-api/products/specific-prices/{specificPriceId}
PATCH /admin-api/products/{productId}/specific-price-priorities
DEL /admin-api/products/{productId}/specific-price-priorities
EDIT 2025-11-26
I have updated this PR to follow the new conventions, which require endpoint names to be in the plural form.
Discussions
JSON keys standardization:
reductionValuevsreductionAmountPrestaShop core presents an inconsistency in property names:
GetSpecificPriceForEditing(used for GET/UPDATE) exposesreductionAmountviagetReductionAmount()GetSpecificPriceList(used for LIST) exposesreductionValueviagetReductionValue()AddSpecificPriceCommand,EditSpecificPriceCommand) usereductionValueas parameter insetReduction()Technical choice: Use
reductionValueeverywhere in the API (input and output) to:reductionValueImplementation: Mapping
[reductionAmount]→[reductionValue]inQUERY_MAPPINGto transformGetSpecificPriceForEditingresponse to API Resource.Specific price priority: hard-coding choices
Specific price priorities are declared in
PriorityList::AVAILABLE_PRIORITIES.My implementation limits allowed values via an
Assert\Choiceconstraint with a hard-coded list:id_groupid_currencyid_countryid_shopThat way, the admin API validation provides clear and immediate error feedback. And priorities are stable business constants in PrestaShop.
As this is my own opinion, feel free to discuss this point!
Notice: mapping
reductionType/reductionValuefor partial updatesThe PATCH endpoint uses
EditSpecificPriceCommand::setReduction(string $reductionType, string $reductionValue)which expects two distinct parameters.Technical choice: Transformation via
UPDATE_COMMAND_MAPPING:[reductionType]→[reduction][reductionType][reductionValue]→[reduction][reductionValue]The
CQRSApiNormalizerautomatically detects methods with multiple parameters and callssetReduction()with values from thereductionarray. This approach allows handling partial updates while respecting the CQRS command signature.How to test
Create an API Client with these scopes :
product_read&product_writeRequest an access token
Create a specific price
POST/admin-api/products/specific-prices{ "productId": 5, "reductionType": "percentage", "reductionValue": "10.5", "includeTax": true, "fixedPrice": "-1", "fromQuantity": 1, "dateTimeFrom": "0000-00-00T00:00:00+00:00", "dateTimeTo": "0000-00-00T00:00:00+00:00" }HTTP Code :
201HTTP Body : the created specific price, with an ID that we will use as
{createdID}for other callsUpdate a specific price
PATCH/admin-api/products/specific-prices/{createdID}{ "reductionType": "percentage", "reductionValue": "20.0", "includesTax": true, "fromQuantity": 5, "dateTimeFrom": "2025-01-01T00:00:00+00:00", "dateTimeTo": "2025-12-31T23:59:59+00:00" }HTTP Code :
200HTTP Body : updated specific price, with updated values
Get a specific price
GET/admin-api/products/specific-prices/{createdID}HTTP Code :
200HTTP Body : the specific price with ID
{createdID}List specific prices for a product
GET/admin-api/products/{productId}/specific-pricesHTTP Code :
200HTTP Body : array of specific prices for the product
Set specific price priority for a product
PATCH/admin-api/products/{productId}/specific-price-priorities{ "priorities": [ "id_shop", "id_country", "id_currency", "id_group" ] }HTTP Code :
204HTTP Body : (no content)
Remove specific price priority (reset to default)
DELETE/admin-api/products/{productId}/specific-price-prioritiesHTTP Code :
204Result: specific price priority is reset to default for the product
Delete a specific price
DELETE/admin-api/products/specific-prices/{createdID}HTTP Code :
204Result: specific price with ID
{createdID}is deleted