0

In my App user to have full access to the app needs to pass survey after account confirmation. Survey has 2 steps:

  • questions (in which the user answers questions; controller: TestResultsController, model: TestResult)
  • experience level (inside of which user specify his level of experience; controller: ExperienceLevelsController, updates current_user.experience_level)

Business requirements:

When the user answers the questions it's redirected to redirect_to edit_users_experience_level_path(current_user) where he sets his experience level (it's inside of ExperienceLevelsController and method update). If the user completes the survey but will give up on completing the user experience and come back to it later it would be logical to display only the experience level page. To do so I've prepared below policies:

class TestResultPolicy < ApplicationPolicy
  def new?
    return false if passed?

    if without_result?
      redirect_to edit_users_experience_level_path(current_user)
    elsif passed?
      active?
    end
  end

  def create?
    new?
  end

  private

  def passed?
    user.test_results.where(test_result: 'passed').any?
  end

  def without_result?
    user.test_results.last.result.nil?
  end
end

Is it a good way to define redirection inside of Pundit policy? I know I could use user_not_authorized but I'm using it already inside of ApplicationController where I redirect to identity_unconfirmed_path or root_path:

class ApplicationController < ActionController::Base
  include Pundit
  before_action :set_paper_trail_whodunnit

  rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized

  private

  def user_not_authorized
    flash[:alert] = 'You are not authorized to perform this action.'
    return redirect_to(request.referrer || root_path) unless current_user.northrow_status == 'failed'

    redirect_to identity_unconfirmed_path
  end
end

So again, should I use redirect flow inside pundit policy or isn't this a good practice?

mr_muscle
  • 2,536
  • 18
  • 61
  • It is anti pattern, when you redirect a user it should always be directly from the controller, or if this is a general rule the application controller. Imagine a new developer trying to find this while ripping his hair off. – TTD May 04 '21 at 14:23

1 Answers1

0

Hey – structuring code like this makes it hard to test and to find bugs should they arise. A good rule of thumb to apply here is Separation of concerns, ideally, your Policies should be doing just that, policing.

A more ideal thing to introduce here is a value boundary, where you connect the logic in the policy and controller via values. What I mean is, simply returning a value from the policy and acting on that value in the controller is a much better pattern to uphold, in this case, a simple true of false

Another thing you can consider is raising an exception and rescuing from it in the controller to execute the redirect(may want to consider this more carefully).

What I would do? Use the value boundary approach. Return true or false from the new? method, use it to proceed in whatever direction in the controller

Edit: Just learned that true or false values cannot be returned from a policy, however, it will raise a Pundit::NotAuthorizedError, and in that case we can probably just rescue from the error and redirect the user.

  • Returning `true` or `false` from the pundit policy? `authorize` raises a `Pundit::NotAuthorizedError` if a user has no permission to perform a certain action. It will never return true or false. – mr_muscle May 04 '21 at 20:36
  • Great to know @mr_muscle, not extremely accustomed to the nuances of Pundit. In that case, we can nicely rescue from the error and redirect? – Damilare Olusakin May 04 '21 at 20:43
  • Instead of rescue from the error and redirect, you could override `user_not_authorized` method like I wrote. But I don't think it's going to be easy to maintain in the long run. Now I'm thinking to use separate helper just for redirection depends on user steps. – mr_muscle May 04 '21 at 20:58