Add rule condition to include other rules (for structured nesting). #2487

kleinmann · 2022-05-14T21:37:49Z

1. Why is this change necessary?

Lots of shops have repeating rules that they use in multiple rules, e.g. a rule excluding certain products from promotions which is used in different combinations with other conditions.

This PR adds a new rule condition to embed a separate rule to allow for extraction of common rule conditions and easier maintenance.

2. What does this change do, exactly?

It adds a new rule condition and changes the rule indexing such that nested rules are embedded in their corresponding NestedRule instances.
It also validates rule conditions to prevent creation of cycles in the nested rules.

3. Describe each step to reproduce the issue or behaviour.

Create a rule with the new condition "Active rule" and select any other rule.
This rule is seen as valid, if the linked rule is either valid or invalid (depending on the operator chosen).
Use this new rule like any other rule, no special handling needed 🙂

4. Please link to the relevant issues (if any).

5. Checklist

I have written tests and verified that they fail without my change
I have created a changelog file with all necessary information about my changes
I have written or adjusted the documentation according to my changes
This change has comments for package types, values, functions, and non-obvious lines of code
I have read the contribution requirements and fulfil them.

shyim · 2022-05-14T21:50:41Z

src/Core/Content/Rule/DataAbstractionLayer/RulePayloadUpdater.php

-                ]);
-            }
+        while (!empty($unprocessedRules)) {
+            foreach ($rules as $id => &$rule) {


this should be unreferenced using unset

shyim · 2022-05-14T21:51:22Z

src/Core/Content/Rule/DataAbstractionLayer/RulePayloadUpdater.php

+
+                foreach ($rule as &$condition) {
+                    if ($condition['type'] === NestedRule::NAME) {
+                        $value = json_decode($condition['value'], true);


throw on error missing

shyim · 2022-05-14T21:52:36Z

src/Core/Content/Rule/DataAbstractionLayer/RulePayloadUpdater.php

+
+            if ($command instanceof UpdateCommand) {
+                if ($command->hasField('value')) {
+                    $value = json_decode($command->getPayload()['value'], true);


throw on error

I opted to skip requesting the changeset, if it's not the value we need, instead.

shyim · 2022-05-14T21:56:58Z

src/Core/Content/Rule/NestedRuleValidator.php

+        ?string $code = null
+    ): ConstraintViolationInterface {
+        return new ConstraintViolation(
+            str_replace(array_keys($parameters), array_values($parameters), $messageTemplate),


array_values is not needed

shyim · 2022-05-14T21:59:07Z

src/Core/Framework/Test/Rule/NestedRuleTest.php

+    /**
+     * @var EntityRepositoryInterface
+     */
+    private $ruleRepository;


shyim · 2022-05-14T21:59:22Z

src/Core/Framework/Test/Rule/NestedRuleTest.php

+
+    public function testIfRuleIsConsistent(): void
+    {
+        $ruleId = Uuid::randomHex();


idscollection

codecov · 2022-05-14T22:29:00Z

Codecov Report

Merging #2487 (f4865b1) into trunk (42fb922) will decrease coverage by 0.02%.
The diff coverage is 58.95%.

@@            Coverage Diff             @@
##            trunk    #2487      +/-   ##
==========================================
- Coverage   61.95%   61.93%   -0.03%     
==========================================
  Files        3467     3469       +2     
  Lines       75624    75778     +154     
==========================================
+ Hits        46854    46932      +78     
- Misses      28770    28846      +76

Impacted Files	Coverage Δ
src/Core/Framework/Rule/NestedRule.php	`41.66% <41.66%> (ø)`
src/Core/Content/Rule/NestedRuleValidator.php	`48.27% <48.27%> (ø)`
...t/Rule/DataAbstractionLayer/RulePayloadUpdater.php	`65.95% <66.99%> (-8.59%)`	⬇️
src/Core/Checkout/Cart/PriceDefinitionFactory.php	`40.00% <0.00%> (-15.56%)`	⬇️
...ore/Checkout/Cart/Delivery/Struct/DeliveryDate.php	`57.57% <0.00%> (-9.10%)`	⬇️
...efront/Framework/Cache/CacheResponseSubscriber.php	`86.17% <0.00%> (-4.26%)`	⬇️
...ataAbstractionLayer/DefinitionInstanceRegistry.php	`92.85% <0.00%> (-1.79%)`	⬇️
...tent/ContactForm/SalesChannel/ContactFormRoute.php	`95.58% <0.00%> (-1.43%)`	⬇️
...out/Cart/Order/Transformer/LineItemTransformer.php	`98.59% <0.00%> (-1.41%)`	⬇️
...Flow/Dispatching/Action/GenerateDocumentAction.php	`89.55% <0.00%> (-1.22%)`	⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42fb922...f4865b1. Read the comment docs.

* Detect cycles with existing rules in validation * Preprocess rules during indexing and embed nested rules inside NestedRule instance Co-authored-by: Uwe Kleinmann <u.kleinmann@kellerkinder.de> Co-authored-by: Jan Mallwitz <mallwitz@botschaft.digital>

shopwareBot · 2022-05-19T07:53:15Z

Hello,

thank you for creating this pull request.
I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/NEXT-21684

Please use this issue to track the state of your pull request.

taltholtmann · 2022-05-23T07:21:43Z

This MR breaks the rule awareness feature: shopware/docs#446 and the app script conditions feature.

lernhart · 2022-05-23T09:14:53Z

Hey @kleinmann.

I am sorry, that you PR is breaking an upcoming feature. Even I did not see the rule awareness feature coming.
It is planned to be released in the next minor of Shopware 6.4 (6.4.12.0).
As I have not a lot of Information yet, I can only encourage you to wait for the release and create a new PR, refactored to work with the new feature.

For now, I am closing this PR, thank you for contributing though 🎉

kleinmann force-pushed the feature/nested-rule branch from 78df06a to b27ab58 Compare May 14, 2022 21:42

shyim reviewed May 14, 2022

View reviewed changes

kleinmann force-pushed the feature/nested-rule branch from b27ab58 to 8dff736 Compare May 15, 2022 10:22

kleinmann force-pushed the feature/nested-rule branch from 8dff736 to f4865b1 Compare May 16, 2022 12:54

keulinho assigned lernhart May 17, 2022

shopwareBot added the Scheduled label May 19, 2022

lernhart closed this May 23, 2022

mitelg added Declined and removed Scheduled labels May 23, 2022

shopware-issue-bot bot mentioned this pull request Jun 16, 2023

Rule system v2 #3142

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rule condition to include other rules (for structured nesting). #2487

Add rule condition to include other rules (for structured nesting). #2487

kleinmann commented May 14, 2022

shyim May 14, 2022

kleinmann May 16, 2022

shyim May 14, 2022

kleinmann May 16, 2022

shyim May 14, 2022

kleinmann May 16, 2022

shyim May 14, 2022

kleinmann May 16, 2022

shyim May 14, 2022

kleinmann May 16, 2022

shyim May 14, 2022

kleinmann May 16, 2022

codecov bot commented May 14, 2022 •

edited

Loading

shopwareBot commented May 19, 2022

taltholtmann commented May 23, 2022 •

edited

Loading

lernhart commented May 23, 2022 •

edited

Loading

Add rule condition to include other rules (for structured nesting). #2487

Add rule condition to include other rules (for structured nesting). #2487

Conversation

kleinmann commented May 14, 2022

1. Why is this change necessary?

2. What does this change do, exactly?

3. Describe each step to reproduce the issue or behaviour.

4. Please link to the relevant issues (if any).

5. Checklist

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 14, 2022 • edited Loading

Codecov Report

shopwareBot commented May 19, 2022

taltholtmann commented May 23, 2022 • edited Loading

lernhart commented May 23, 2022 • edited Loading

codecov bot commented May 14, 2022 •

edited

Loading

taltholtmann commented May 23, 2022 •

edited

Loading

lernhart commented May 23, 2022 •

edited

Loading