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

Fix Permissions::Order spec in rails 4 #5151

Merged
merged 1 commit into from Apr 6, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Apr 3, 2020

What? Why?

Fixes permissions::order service spec in the rails 4 branch.

The problem was introduced here by relying on order (from factories) that have products by the same producer required in the test:
https://github.com/openfoodfoundation/openfoodnetwork/pull/5030/files#diff-610e1eeeabb29a85e6d4a271d2bd7f56R75
In rails 4 the products in these orders dont have the same producer as required and so the test breaks.
We move the test to a simpler context above. It's still valid and it just validates what it is meant to validate.
See commit messages for more details.

What should we test?

green build

Release notes

Changelog Category: Changed
Adapted test code to rails 4 related to order permissions.

…ave to set the producer of the products in the order

This is working in master by chance of the factories but breaks in rails 4 because the orders in this test dont have products supplied by the producer which is a necessary condition in the context where it was
@luisramos0 luisramos0 self-assigned this Apr 3, 2020
@luisramos0 luisramos0 changed the title Move search params test case to a different context so that we dont h… Fix Permissions::Order spec in rails 4 Apr 3, 2020
Copy link
Contributor

@Matt-Yorkley Matt-Yorkley left a comment

Choose a reason for hiding this comment

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

👍

@sauloperez sauloperez merged commit 924e816 into openfoodfoundation:master Apr 6, 2020
@luisramos0 luisramos0 deleted the fix_order_perms branch April 6, 2020 19:16
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.

None yet

3 participants