Change order of args in verify_record/3 and add delegation as do?/3

Author: vincentvanbush

CHANGELOG.md

@@ -6,6 +6,21 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+- [Breaking] Change order of args in `Permit.verify_record/3` and add delegation as `do?/3` when doing `use Permit`.
+
+  This way, permisisons to perform a dynamically computed action can be checked like this:
+  ```elixir
+  action = :read
+  can(user) |> do?(action, %Item{id: 1})
+  ```
+  instead of the previously required, rather clumsy, and undocumented notation:
+
+  ```elixir
+  # Note: this is the previously used argument order; as of now, arguments 2 and 3 have been reversed.
+  can(user) |> Permit.verify_record(%Item{id: 1}, action)
+  ```
+  If you happened to use `Permit.verify_record(3)`, the way to migrate is swapping arguments 2 and 3 in all calls, or - better still - migrating to the `do?/3` syntactic sugar.
+
 ## [v0.2.1]
 ### Fixed
 - Fix runtime and compile-time warnings (#38).

lib/permit.ex

@@ -122,7 +122,7 @@ defmodule Permit do
           @spec unquote(predicate)(Permit.Context.t(), Types.object_or_resource_module()) ::
                   boolean()
           def unquote(predicate)(authorization, resource) do
-            Permit.verify_record(authorization, resource, unquote(name))
+            Permit.verify_record(authorization, unquote(name), resource)
           end
         end
       end)
@@ -153,6 +153,10 @@ defmodule Permit do
       defoverridable resolver_module: 0
 
       unquote(predicates)
+
+      defdelegate do?(authorization, action, resource_or_module),
+        to: Permit,
+        as: :verify_record
     end
   end
 
@@ -166,17 +170,17 @@ defmodule Permit do
   end
 
   @doc false
-  @spec verify_record(Permit.Context.t(), Types.object_or_resource_module(), Types.action_group()) ::
+  @spec verify_record(Permit.Context.t(), Types.action_group(), Types.object_or_resource_module()) ::
           boolean()
   def verify_record(
         %{
           permissions: permissions,
           subject: subject
         } = _authorization,
-        record,
-        action
+        action,
+        resource_or_module
       ) do
-    Permissions.granted?(permissions, action, record, subject)
+    Permissions.granted?(permissions, action, resource_or_module, subject)
   end
 
   defp add_predicate_name(atom),

lib/permit/actions/traversal.ex

@@ -37,12 +37,14 @@ defmodule Permit.Actions.Traversal do
     disj_function = funs[:disj]
     value_function = funs[:value]
 
+    # case Map.get(forest, :action_name, []) do
     case forest[action_name] do
       # action_name can be directly checked
       # in this case, the entire trace should be verified.
       # For example, if :see implies :read implies :index,
       # then asking for :index verifies whether explicitly
       # given is the permission to :see, or :read, or :index
+
       [] ->
         [action_name | trace]
         |> Enum.map(value_function)
@@ -54,7 +56,12 @@ defmodule Permit.Actions.Traversal do
         |> Enum.map(&traverse(f, &1, funs, [action_name | trace]))
         |> conj_function.()
 
+      # TODO: dubious after the introduction of reading out actions from the router
+      #
+      # Action not defined in the action definitions file. Assume it's an action
+      # that doesn't map to any different action in the forest.
       nil ->
+        # traverse(%{f | forest: Map.put(forest, action_name, [])}, action_name, funs, trace)
         throw({:not_defined, action_name})
     end
   end

lib/permit/resolver_base.ex

@@ -92,7 +92,7 @@ defmodule Permit.ResolverBase do
   def authorized?(subject, authorization_module, resource_or_module, action) do
     auth = authorization_module.can(subject)
     actions_module = authorization_module.actions_module()
-    verify_fn = &Permit.verify_record(auth, resource_or_module, &1)
+    verify_fn = &Permit.verify_record(auth, &1, resource_or_module)
 
     Permit.Actions.verify_transitively!(actions_module, action, verify_fn)
   end

test/permit/permit_test.exs

@@ -2,6 +2,14 @@ defmodule Permit.PermitTest do
   @moduledoc false
   use Permit.Case
 
+  defmodule TestUser do
+    defstruct [:id]
+  end
+
+  defmodule TestItem do
+    defstruct [:id]
+  end
+
   defmodule TestActions do
     use Permit.Actions
 
@@ -21,7 +29,7 @@ defmodule Permit.PermitTest do
     use Permit.Permissions,
       actions_module: TestActions
 
-    def can(_role), do: permit()
+    def can(_role), do: permit() |> a(TestItem)
   end
 
   defmodule TestAuthorization do
@@ -45,5 +53,19 @@ defmodule Permit.PermitTest do
         assert predicate in TestAuthorization.__info__(:functions)
       end)
     end
+
+    test "should delegate do?/3 to Permit.verify_record/3" do
+      assert TestAuthorization.can(%TestUser{id: 1})
+             |> TestAuthorization.do?(:a, TestItem)
+
+      assert TestAuthorization.can(%TestUser{id: 1})
+             |> TestAuthorization.do?(:a, %TestItem{id: 1})
+
+      refute TestAuthorization.can(%TestUser{id: 1})
+             |> TestAuthorization.do?(:b, TestItem)
+
+      refute TestAuthorization.can(%TestUser{id: 1})
+             |> TestAuthorization.do?(:b, %TestItem{id: 1})
+    end
   end
 end