passing optional params to can?

[slashmili]

Hey,

So I have this setup:

GroupController => /groups/:group_id
UserController => /groups/:group_id/users
AppController => /groups/:group_id/apps

I want to allow only admins access to users :

defimpl Canada.Can, for: MyApp.User do
  alias MyApp.{User, Group}

  def can?(%User{} = user, _, %Group{} = group) do
    Group.is_admin?(group, user)
  end

  def can?(_, _, User), do: false
end

This works well I use it with Canary so I add this plug to my UserController

defmodule UserController do
...
plug :authorize_resource, model: Group, persisted: true, id_name: "group_id"
...

Now if I want to limit access to AppController to only the users that belongs to that group,
I can add same plug to AppController :

defmodule AppController do
...
plug :authorize_resource, model: Group, persisted: true, id_name: "group_id"
...
end

And I need to add a new can? to implemention:

defimpl Canada.Can, for: MyApp.User do
  alias MyApp.{User, Group}

  def can?(%User{} = user, _, %Group{} = group) do
    Group.is_admin?(group, user)
  end

  def can?(%User{} = user, _, %Group{} = group) do
    Group.is_memeber?(group, user)
  end

  def can?(_, _, User), do: false
end

But As you can see the new can? has the same pattern as previous function.

What do you think if we add another optional param to to can?

Then I can use it like this:

defmodule UserController do
...
plug :authorize_resource, model: Group, persisted: true, id_name: "group_id", only_admin: true
...
end

defimpl Canada.Can, for: MyApp.User do
  alias MyApp.{User, Group}

  def can?(%User{} = user, _, %Group{} = group, %{only_admin: true}) do
    Group.is_admin?(group, user)
  end

  def can?(%User{} = user, _, %Group{} = group, _) do
    Group.is_memeber?(group, user)
  end

  def can?(_, _, User), do: false
end

I know that you are quite busy, let me know what do you think and if you agree with the change I will create PR.

[OvermindDL1]

The def can?(%User{} = user, _, %Group{} = group, %{only_admin: true}) do line should probably be def can?(%User{} = user, _, %Group{} = group, [only_admin: true]) do as a keyword list is being passed to the main thing. However I'm not sure this is necessary as it is easy to wrap one or the other bits up in another struct and dispatch based on that (which is what I do, handles admin mapping, extra options, categories, etc...).

[jarednorman]

I tend to favour @OvermindDL1's solution here.

[slashmili]

@OvermindDL1 Yeah but pattern matching on keyword looks ugly, to make the point I skip the reality :)

hmm, as your solution I understand that it's possible to wrap it around but if you use it in Canary the third option is always a Ecto Schema but of course it might be the shortcoming of Canary.