-
Notifications
You must be signed in to change notification settings - Fork 25
Add manufacturer endpoints #63
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?
Conversation
|
Hello @MattKelvin! This is your first pull request on ps_apiresources repository of the PrestaShop project. Thank you, and welcome to this Open Source community! |
|
Hello @MattKelvin, |
|
It would be amazing ! Let's wait for the result of your PR 🙏 Thank you :) |
|
Perfect, thank you! |
|
@MattKelvin the PR has been merged: #61, so if you want you can use the service |
|
Hello @Codencode I just did it. What do you think about my implementation ? Thank you ! :) |
|
Hello @MattKelvin, I think you did a good job, congratulations! |
tleon
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.
Thx :)
SiraDIOP
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.
Hello @MattKelvin,
Thanks for your pr i tested but At the admin API level, I don't have the manufacturer endpoint, but I do have it in Swagger.
Uploading Enregistrement de l’écran 2025-10-13 à 18.47.51.mov…
Do you think I've missed something?
Thanks
|
Hi @SiraDIOP, Thanks for testing this PR. 🙏 Can you try to clear the PrestaShop cache and do a |
|
Hello @MattKelvin, Thanks, Thanks :) |
|
Thanks @SiraDIOP! Else I can disable the endpoint which need it (GET /manufacturer/1/details/1) Thank you! |
|
I added a "Blocked by Core" label. That also means that |
10ef284
|
@MattKelvin tests are red |
|
@nicosomb, just rebased it thanks! |
|
Hello @MattKelvin Could you remove |
|
Hello @MattKelvin There's still one merge commit inside your pull request. To keep the git history clean, we need to remove it. |
c6def0f to
5ef7a32
Compare
|
Hello @kpodemski Sorry for the delay. I tried to remove it. Can you launch the tests? :) Thanks! |
|
Hello @MattKelvin 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. Thanks for your patience. Feedback will follow directly on the PR. |






Any advices welcome :)
New endpoints
How to test
modules/ps_apiresourcesand run:composer installcomposer clear-test-cache-- manufacturer_read
-- manufacturer_write
-- YOUR_CLIENT_ID
-- YOUR_CLIENT_SECRET
-- YOUR_DOMAIN_NAME
-- YOUR_ACCESS_TOKEN
To execute tests
Thank you