-
-
Notifications
You must be signed in to change notification settings - Fork 719
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 bug #12835 for producer reports #12847
Fix bug #12835 for producer reports #12847
Conversation
bdaecfb
to
f7a1f97
Compare
The key here is the enterprise_relationship. This is required for the supplier to have permission to see the orders. Curiously, the unit test still passes. All will be revealed in the next commit..
f7a1f97
to
0b22601
Compare
We missed this in c31416c, oops.
0b22601
to
a9ad6a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but the Ability
spec should also be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that we have a spec for it now. And thanks for fixing my mistakes.
Hey @dacook , I've ran a quick test on this one: The issue is fixed, reports are accessible to Merging. |
What? Why?
Although it was hard to get the tests right, in the end it turned out to be a simple fix!
What should we test?
As per issue: