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.

Sign up for GitHub

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

Jump to bottom

[BUG] Check action type for fixed promotion #13393

Merged
merged 7 commits into from
Dec 16, 2021
Merged

[BUG] Check action type for fixed promotion #13393

merged 7 commits into from
Dec 16, 2021

Conversation

SirDomin
Copy link
Contributor

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

@SirDomin SirDomin requested a review from a team as a code owner December 14, 2021 22:11
@SirDomin SirDomin changed the title [WIP][BUG] Check action type for fixed promotion [BUG] Check action type for fixed promotion Dec 15, 2021
@SirDomin SirDomin force-pushed the multiple-channels-fixed-discount-validation branch from be91dff to 1102193 Compare December 15, 2021 21:24
@SirDomin SirDomin force-pushed the multiple-channels-fixed-discount-validation branch from 1102193 to 8c25381 Compare December 15, 2021 21:43
GSadee
GSadee reviewed Dec 16, 2021
View reviewed changes
features/promotion/managing_catalog_promotions/validating_catalog_promotion_creation.feature Outdated Show resolved Hide resolved
src/Sylius/Bundle/PromotionBundle/Form/Type/CatalogPromotionActionType.php
Comment on lines +77 to +83
if ($data['type'] === CatalogPromotionActionInterface::TYPE_FIXED_DISCOUNT) {
foreach ($data['configuration'] as $channelConfiguration) {
if ($channelConfiguration['amount'] === '') {
return;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Should we be so hardcoded to one action type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thats only place where this error occurs, so i had to handle only when channelConfiguration is available.

src/Sylius/Bundle/PromotionBundle/Form/Type/CatalogPromotionActionType.php
$form = $event->getForm();

$actionConfigurationType = $this->actionConfigurationTypes[$data->getType()];
$form->add('configuration', $actionConfigurationType, [
Copy link
Member

Choose a reason for hiding this comment

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

Why is it needed to be added on submit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

without adding it to sumit form resets and we dont have inputs for selected action type

src/Sylius/Bundle/CoreBundle/Resources/translations/validators.en.yml Outdated Show resolved Hide resolved
@GSadee GSadee added the Bug Confirmed bugs or bugfixes. label Dec 16, 2021
@GSadee GSadee changed the base branch from master to 1.11 December 16, 2021 06:12
@GSadee
Copy link
Member

GSadee commented Dec 16, 2021

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "multiple-channels-fixed-discount-validation" to update your local branch.

Feel free to ask for assistance when you get stuck 👍

@GSadee GSadee force-pushed the multiple-channels-fixed-discount-validation branch from 8c25381 to f739862 Compare December 16, 2021 06:12
@SirDomin SirDomin force-pushed the multiple-channels-fixed-discount-validation branch from f739862 to bd5f34b Compare December 16, 2021 08:52
@SirDomin SirDomin force-pushed the multiple-channels-fixed-discount-validation branch from bd5f34b to aa029f5 Compare December 16, 2021 09:24
@GSadee GSadee merged commit 011c6ae into Sylius:1.11 Dec 16, 2021
@GSadee
Copy link
Member

GSadee commented Dec 16, 2021

Thank you, @SirDomin! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GSadee GSadee GSadee approved these changes

Assignees
No one assigned
Labels
Bug Confirmed bugs or bugfixes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@SirDomin @GSadee