Skip to content
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

[Orders][API] Validate endpoints #15064

Merged
merged 13 commits into from Jul 5, 2023
Merged

[Orders][API] Validate endpoints #15064

merged 13 commits into from Jul 5, 2023

Conversation

Rafikooo
Copy link
Contributor

@Rafikooo Rafikooo commented May 28, 2023

Q A
Branch? 1.12 and 1.13
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets #14764
License MIT

We are missing validation of the order checkout process and some URI parameters across order endpoints

@Rafikooo Rafikooo added the Bug Confirmed bugs or bugfixes. label May 28, 2023
@Rafikooo Rafikooo requested a review from a team as a code owner May 28, 2023 23:06
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label May 28, 2023
@Rafikooo Rafikooo changed the title [Orders][API] Validate endpoints [WIP][Orders][API] Validate endpoints May 28, 2023
@Rafikooo Rafikooo changed the title [WIP][Orders][API] Validate endpoints [Orders][API] Validate endpoints Jun 3, 2023
@@ -68,6 +68,7 @@ sylius:
order:
currency_code:
not_valid: The currency code you entered is invalid.
invalid_state_transition: 'Cannot complete as order is in a wrong state. Current: %currentState%. Possible transitions: %possibleTransitions%.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is shown to the customer, then it needs some rewording to make it less technical.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is intended for developers, the question is whether I should put it in ApiBundle, since this message is API-oriented

{
Assert::isInstanceOf($value, OrderTokenValueAwareInterface::class);

/** @var CheckoutCompletion $constraint */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be needed as there is an assertion below

Suggested change
/** @var CheckoutCompletion $constraint */

@GSadee GSadee merged commit 6f6fe74 into Sylius:1.12 Jul 5, 2023
23 checks passed
@GSadee
Copy link
Member

GSadee commented Jul 5, 2023

Thank you, Rafał! 🥇

@Rafikooo Rafikooo mentioned this pull request Jul 5, 2023
jakubtobiasz added a commit that referenced this pull request Jul 5, 2023
This PR was merged into the 1.13 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.13 <!-- see the comment below -->                  |
| Bug fix?        | no                                                      |
| New feature?    | no                                                      |
| BC breaks?      | no                                                      |
| Deprecations?   | no <!-- don't forget to update the UPGRADE-*.md file --> |
| Related tickets | #15064
| License         | MIT                                                          |

<!--
 - Bug fixes must be submitted against the 1.12 branch
 - Features and deprecations must be submitted against the 1.13 branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------
  [PHPStan] Add missing @param annotations
  [ECS] Apply ecs fixes
  [Units] Adjust checkout completion responses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. Bug Confirmed bugs or bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants