[ApiBundle] Added remaining APIs for the Promotion resource. #15524

Wojdylak · 2023-11-13T14:39:28Z

Q	A
Branch?	1.13
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Related tickets	N/A
License	MIT

github-actions · 2023-11-13T14:53:00Z

Bunnyshell Preview Environment deleted

Available commands:

/bns:deploy to redeploy the environment

sonarcloud · 2023-11-13T21:19:43Z

SonarCloud Quality Gate failed.

6 Bugs
0 Vulnerabilities
0 Security Hotspots
15 Code Smells

No Coverage information
0.6% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

src/Sylius/Bundle/CoreBundle/Fixture/Factory/PromotionExampleFactory.php

src/Sylius/Behat/Context/Api/Admin/ManagingPromotionsContext.php

src/Sylius/Bundle/ApiBundle/Resources/config/api_resources/PromotionTranslation.xml

src/Sylius/Bundle/ApiBundle/Resources/config/app/config.yaml

GSadee · 2023-11-16T09:42:24Z

src/Sylius/Bundle/ApiBundle/Resources/config/serialization/Promotion.xml

+            <group>admin:promotion:create</group>
+            <group>admin:promotion:update</group>
+            <denormalization_context>
+                <entry name="disable_type_enforcement">true</entry>


Why are you disabling it on the priority field?

In the database, the priority column is not nullable, so serialization throws an error on null values. However, we want to allow sending null, because the setPriority method accepts null.

The question is whether we want to accept a null value if the db doesn't accept it. IMO allowing setting null is not a good choice.

IMO, if the database doesn't accept the null value, we shouldn't accept it during serialization

Setter allows setting a null value. If I set the value to null, it receives the last priority. If we remove the ability to set null, we would know what the last priority is.

#15551 I have removed possibility to set null value. We can achieve the same goal to pass a value of minus one.

src/Sylius/Bundle/CoreBundle/Fixture/Factory/PromotionExampleFactory.php

UPGRADE-1.13.md

tests/Api/Responses/Expected/admin/promotion/put_promotion_with_null_priority_response.json

NoResponseMate · 2023-11-17T12:52:52Z

src/Sylius/Bundle/CoreBundle/Fixture/Factory/PromotionExampleFactory.php

                }

-                return $rules;
+                return empty($rules) ? [[]] : $rules;


I can see that it's been like this before, but is there a particular reason why it's not a simple array?

Ok, it has been working in a way that if I pass an empty array, then this normalizer returns an empty array within an array, which previously created a promotion with at least one action/rule. For the purpose of using this in Behat tests, where I need to create a promotion without any actions/rules, I've added the option to pass a null value. In this case, it will return an empty array, so no actions/rules are added. If someone passes an empty array, the behavior will remain as it was before.

…motionContext class

…y class

phpstan-baseline.neon

src/Sylius/Bundle/ApiBundle/OpenApi/Documentation/PromotionDocumentationModifier.php

jakubtobiasz · 2023-11-21T15:22:16Z

src/Sylius/Bundle/ApiBundle/Resources/config/serialization/Promotion.xml

+            <group>admin:promotion:create</group>
+            <group>admin:promotion:update</group>
+            <denormalization_context>
+                <entry name="disable_type_enforcement">true</entry>


The question is whether we want to accept a null value if the db doesn't accept it. IMO allowing setting null is not a good choice.

src/Sylius/Bundle/CoreBundle/Fixture/Factory/PromotionExampleFactory.php

features/promotion/managing_promotions/editing_promotion.feature

GSadee · 2023-11-22T05:24:34Z

src/Sylius/Bundle/ApiBundle/Resources/config/serialization/Promotion.xml

+            <group>admin:promotion:create</group>
+            <group>admin:promotion:update</group>
+            <denormalization_context>
+                <entry name="disable_type_enforcement">true</entry>


IMO, if the database doesn't accept the null value, we shouldn't accept it during serialization

src/Sylius/Bundle/CoreBundle/Fixture/Factory/PromotionExampleFactory.php

GSadee · 2023-11-22T05:28:29Z

Thanks, Karol! 🥇

Wojdylak requested a review from a team as a code owner November 13, 2023 14:39

probot-autolabeler bot added the API APIs related issues and PRs. label Nov 13, 2023

probot-autolabeler bot added the Maintenance CI configurations, READMEs, releases, etc. label Nov 13, 2023

Wojdylak force-pushed the SYL-3164-api-cover-cart-promotion-management branch 2 times, most recently from a73d2df to 862b8a6 Compare November 13, 2023 15:21

Wojdylak force-pushed the SYL-3164-api-cover-cart-promotion-management branch 6 times, most recently from b9b1b56 to f9bf245 Compare November 15, 2023 10:44

GSadee reviewed Nov 16, 2023

View reviewed changes

Wojdylak force-pushed the SYL-3164-api-cover-cart-promotion-management branch 2 times, most recently from 1cce2ff to 1c5fc26 Compare November 16, 2023 20:35

NoResponseMate reviewed Nov 17, 2023

View reviewed changes

Wojdylak force-pushed the SYL-3164-api-cover-cart-promotion-management branch from 1c5fc26 to 2aecfb9 Compare November 19, 2023 18:20

Wojdylak added 12 commits November 20, 2023 11:36

[ApiBundle] Added remaining APIs for the Promotion resource.

df6a93f

[ApiBundle] Update Upgrade file

27414a9

[ApiBundle] Add behat tests for promotion resource

5d82fd1

[ApiBundle] Update phpstan baseline

d07a1d8

[ApiBundle] Add search response to PromotionTest

9adb92f

[ApiBundle] Add documentation of Promotion resource

296cc71

[ApiBundle] Add phpdoc PromotionDocumentationModifier

1c32900

[ApiBundle] Add contract tests for Promotion resources

214f066

[Behat] Change TestPromotionFactory to PromotionExampleFactory at Pro…

ebc64d1

…motionContext class

[Behat] Change way of create coupon at PromotionContext

247f9f3

[Behat] Change way of create rules at PromotionContext

4b7242e

[Behat] Change default description at PromotionContext

1d9ae8a

Wojdylak added 5 commits November 20, 2023 11:37

[Behat] Fix promotions_filter_validation.feature

ace6ff4

[Behat] Change steps in ManagingPromotionsContext class

d082a91

[ApiBundle] Remove normalization context from PromotionTranslation

c5c3691

Change deprecation message in PromotionExampleFactory

623fd0e

Change deprecation message in upgrade file for PromotionExampleFactor…

70299c1

…y class

Wojdylak force-pushed the SYL-3164-api-cover-cart-promotion-management branch from 2aecfb9 to 70299c1 Compare November 20, 2023 10:37

Rafikooo approved these changes Nov 21, 2023

View reviewed changes

jakubtobiasz reviewed Nov 21, 2023

View reviewed changes

GSadee approved these changes Nov 22, 2023

View reviewed changes

GSadee merged commit d3ad385 into Sylius:1.13 Nov 22, 2023
25 checks passed

Wojdylak mentioned this pull request Nov 27, 2023

Resolve actions and rules configuration validation #15551

Merged

Wojdylak deleted the SYL-3164-api-cover-cart-promotion-management branch November 30, 2023 07:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ApiBundle] Added remaining APIs for the Promotion resource. #15524

[ApiBundle] Added remaining APIs for the Promotion resource. #15524

Wojdylak commented Nov 13, 2023

github-actions bot commented Nov 13, 2023 •

edited

sonarcloud bot commented Nov 13, 2023

GSadee Nov 16, 2023

Wojdylak Nov 16, 2023

jakubtobiasz Nov 21, 2023

GSadee Nov 22, 2023

Wojdylak Nov 22, 2023

Wojdylak Nov 24, 2023

NoResponseMate Nov 17, 2023

Wojdylak Nov 20, 2023

jakubtobiasz Nov 21, 2023

GSadee Nov 22, 2023

GSadee commented Nov 22, 2023

[ApiBundle] Added remaining APIs for the Promotion resource. #15524

[ApiBundle] Added remaining APIs for the Promotion resource. #15524

Conversation

Wojdylak commented Nov 13, 2023

github-actions bot commented Nov 13, 2023 • edited

Bunnyshell Preview Environment deleted

sonarcloud bot commented Nov 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GSadee commented Nov 22, 2023

github-actions bot commented Nov 13, 2023 •

edited