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 a bug with has-one relationship authorizers #124

Merged
merged 1 commit into from
Mar 29, 2019

Conversation

brianswko
Copy link
Contributor

@brianswko brianswko commented Mar 29, 2019

Fixes a bug introduced in #119

Any has-one relationship authorizers e.g. ArticlePolicy#create_with_author?(author) are currently breaking since the default authorizer is calling the method with [author] instead of author.

I was trying to be too clever in that PR simplifying code and didn't realize that there may be code elsewhere that relied on has-one relationships sending related records back as an actual record instead of potentially a single-object array.

I'd like to add a spec to cover this case since there are no (non-stub setup reliant) specs covering this behavior, but am unsure how to structure it. I believe this can go out on its own now, or am happy to take some feedback as to where to add a test for this.

While there was a major version bump to signify breaking changes, this was not an intended breaking change.

Getting sent to has-one relationship authorizers
@valscion
Copy link
Member

Thanks!

One way to go about without stubbing things could be to create some sort of minitest-like spec where all the relevant classes would be defined in the test file itself. Then those tests could go through all the layers without relying on any stubbing mechanisms.

Even having one such test would be a huge boon to the development ability.

I'd be interested to see how it would look like with minitest. RSpec would be also be OK. If only we could have all the relevant classes under test defined in the same file, that'd be nice — once that file gets unwieldy, we could figure out how to split it.

@valscion valscion merged commit 3f76cfa into venuu:master Mar 29, 2019
@valscion
Copy link
Member

I released this as v3.0.1 — and would be amazing to have tests that wouldn't rely on stubbing 😅

brianswko added a commit to brianswko/jsonapi-authorization that referenced this pull request Oct 2, 2019
Getting sent to has-one relationship authorizers
brianswko added a commit to brianswko/jsonapi-authorization that referenced this pull request Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants