search

ziffmedia / nova-select-plus

A Laravel Nova Select Field
MIT License
95 stars 26 forks source link

Implement authorizable #1

Open robindrost opened 4 years ago

robindrost commented 4 years ago

Hello,

Is it possible to implement the Laravel\Nova\Authorizable trait so the attachModelName policy methods are respected? Or implement the authorize function from the BelongsToManyField?

Now I'm able to see all the possible relationship values even if I have no acces to see them.

Thanks in advance!

ralphschindler commented 4 years ago

This is a great idea, I will either document or build the necessary features within a week.

ralphschindler commented 4 years ago

I am actually unsure how this should work.

Nova's BelongsToMany models that relationship as it's own CRUD view. That means that authorization can happen over every single relation, and they can be shown independently of each other (it uses attach and detach to make/remove the relations). In the case of nova-select-plus, this uses $model->sync(...) to build the relationships.

For example, let's say $user1 (of Type App\User) can mark they can live in one ore many states (type App\State). Let's say $user1 is setup to have states Louisiana, Texas, Mississippi. But then when inside Nova, after a policy check, they are not able to see Texas. What should happen? Should the value that is already associated to their $user->hasLivedInStates belongsToMany relation not be visible? If it is not visible, is it removed when ->sync() is called simply because it is not in the next set of states passed when ->save() is called?

I don't know how we would go about separating all the values that are able to be removed but not visible from the list of things that is passed in.

robindrost commented 4 years ago

If it is not visible (the user has no right to select it) it should be removed / not visible.

This package: https://github.com/dillingham/nova-attach-many

Loops over the results and filters them based on the attach* policy method if im correctly. (https://github.com/dillingham/nova-attach-many/blob/master/src/Http/Controllers/AttachController.php)

Is that an option? Or would that be way too heavy?

ralphschindler commented 4 years ago

It may not be visible to the current user, but it may be visible to another user who may have set that particular relationship previously (it was visible to that user).

Re: "it should be removed": that is effectively allowing the current user to "detach" the relationship (implicitly, since it's not available to them through the policy), which I think is also the wrong thing to do.

Let me see if I can come up with another real life scenario to test this against. I feel like there is a problem we have to solve with regards to maintaining the state of relationships that a user may not have access to see (or change)...

techdaddies-kevin commented 4 years ago

I think the solution to this problem would be something like this:

If the model has an association already set up and the current user doesn't have permission to remove that association, it should be displayed in a read only fashion, then when syncing, be sure to add that read only association to the sync array before syncing. It feels kind of hacky, but it solves the root issue.