fixed context passing in role resolver with isOwner

[DaGaMs]

Description

With the move to explicit context passing of an options object, internal calls to find etc need to be updated to pass the necessary information along, too. In this case, isOwner calls findById on a Model, but does not pass an access token in options.

Related issues

• findById does not include "options", but find does? #3209
• Inject remoting context to options arg #3023

Checklist

• New tests added or existing tests modified to cover all changes
• Code conforms with the style
  guide

[slnode]

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[bajtos]

@slnode ok to test

[bajtos]

Hello @DaGaMs, thank you for the pull request! Please add a test to verify this change - the test should be failing against the current master and pass with your changes in place.

[bajtos]

You should pass full options object, as it may contain additional information beyond accessToken, for example the current transaction to use when talking to an SQL datastore.

[bajtos]

@DaGaMs also make sure that npm test passes on your machine.

@ebarault Since this is related to Role.isOwner method which you were changing recently, could you please take a look at the changes proposed in this pull request?

[DaGaMs]

I had a quick look at the tests, but unfortunately it was a bit over my head to figure out how to set up a test environment with a specific set of ACL rules and an additional access hook

[ebarault]

@bajtos: yes, i'll follow.
@DaGaMs: could you please first follow @bajtos comment in code review?

[bajtos]

I had a quick look at the tests, but unfortunately it was a bit over my head to figure out how to set up a test environment with a specific set of ACL rules and an additional access hook

I'll see. I'll try to find some time this or next week to provide you with an example showing how to write such test.

[DaGaMs]

There is a test project in #3209 that reproduces the issue

…

-- Benjamin Schuster-Böckler ben@evershelfapp.com http://twitter.com/evershelf snap. search. share.

On 22 Feb 2017, at 13:22, Miroslav Bajtoš ***@***.***> wrote: I had a quick look at the tests, but unfortunately it was a bit over my head to figure out how to set up a test environment with a specific set of ACL rules and an additional access hook I'll see. I'll try to find some time this or next week to provide you with an example showing how to write such test. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[bajtos]

Hello, I have the test ready. I wanted to commit it directly into this pull request, but unfortunately committing into other people's master branch seems to not work well. @DaGaMs in the future, could you please create a feature branch for your pull requests?

I have opened a new pull request which contains the added test where I also made some small improvements in the contributed fix - see #3230.