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

Add shop constraint on SetAssociatedProductCategoriesCommand and RemoveAllAssociatedProductCategoriesCommand #31047

Merged

Conversation

jolelievre
Copy link
Contributor

@jolelievre jolelievre commented Jan 26, 2023

Questions Answers
Branch? develop
Description? Handle shop constraint for product categories commands, depending on the provided shop constraint the commands only removed categories matching the defined constraints. The query for product editing also returns only the categories associated to the provided shop constraint. Thus when switching from a shop context to another the user can only see relevant categories and edit them without risking to delete associations on another shop.
Note: this PR is blocking for the DuplicateProductCommand in multishop mode
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
How to test? CI test green
Fixed ticket? ~
Related PRs ~
Sponsor company ~

@prestonBot prestonBot added develop Branch Bug fix Type: Bug fix BC break Type: Introduces a backwards-incompatible break labels Jan 26, 2023
@prestonBot prestonBot added the Improvement Type: Improvement label Jan 27, 2023
@jolelievre jolelievre marked this pull request as ready for review January 27, 2023 01:54
@jolelievre jolelievre requested a review from a team as a code owner January 27, 2023 01:54
@jolelievre jolelievre changed the title Add shop constraint on SetAssociatedProductCategoriesCommand Add shop constraint on SetAssociatedProductCategoriesCommand and RemoveAllAssociatedProductCategoriesCommand Jan 27, 2023
And I edit category "women" with following details:
| associated shops | shop2 |

Scenario: I assign product to categories they are the same on all shops, but default one can be different
Copy link
Contributor

Choose a reason for hiding this comment

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

hard to understand the scenario, wording could be improved

| id reference | name | is default |
| women | Women | true |
When I copy product product1 from shop shop2 to shop shop1
# Women is not associated to shop1, so shop1's default category is used as default, thus it is also part of shop2's categories
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure i get the comment idea, it is probably clear to yo, because you already know about this behavior

zuk3975
zuk3975 previously approved these changes Jan 27, 2023
Copy link
Contributor

@zuk3975 zuk3975 left a comment

Choose a reason for hiding this comment

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

IMO some wording (and comments) in behat could be improved, it is hard to understand the idea, but it is not blocking

zuk3975
zuk3975 previously approved these changes Jan 27, 2023
@jolelievre jolelievre merged commit 16cdd15 into PrestaShop:develop Jan 27, 2023
@jolelievre jolelievre deleted the product-categories-multishop branch January 27, 2023 18:00
@Progi1984 Progi1984 added this to the 8.1.0 milestone Feb 13, 2023
@kpodemski kpodemski removed the BC break Type: Introduces a backwards-incompatible break label Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug fix Type: Bug fix develop Branch Improvement Type: Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants