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

[Fixture] Quick fixes for channel not found bug #4095

Merged
merged 1 commit into from
Feb 10, 2016

Conversation

Zales0123
Copy link
Member

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets #4094
License MIT
Doc PR

After last updates, problem with fixtures occurred, with ChannelNotFoundException. There were two sources of this evil:

  • OrderChannelListener which got channel from ChannelContext (which led to exception) and set it on order
  • PromotionProcessor which found active promotions by channel (the same case as above)

Quick fixes for these problems are:

  • set channel on Order in OrderChannelListener only if it is not set yet (we're setting channel on each order in LoadOrdersData
  • pass order's channel to findActiveByChannel repository method, rather than this from ChannelContext

Of course, these are a little bit workarounds, but fortunately there is huge checkout and promotion's refactoring upcoming, which should kill these nasty bugs forever ;)

@michalmarcinkowski
Copy link
Contributor

Good job! 👍

@pjedrzejewski
Copy link
Member

Good to go when green.

michalmarcinkowski added a commit that referenced this pull request Feb 10, 2016
[Fixture] Quick fixes for channel not found bug
@michalmarcinkowski michalmarcinkowski merged commit 34008ae into Sylius:master Feb 10, 2016
@michalmarcinkowski
Copy link
Contributor

Thanks Mateusz!

@pjedrzejewski
Copy link
Member

During review I noticed another issue, reported in #4097.

@michalmarcinkowski
Copy link
Contributor

@pjedrzejewski good catch!

@Zales0123 Zales0123 deleted the fixtures-channel-bug branch October 28, 2016 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants