Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing reasons in all_details #278

Open
inkstak opened this issue Dec 10, 2024 · 3 comments
Open

Missing reasons in all_details #278

inkstak opened this issue Dec 10, 2024 · 3 comments

Comments

@inkstak
Copy link
Contributor

inkstak commented Dec 10, 2024

Tell us about your environment

Ruby Version: 3.3.4
Rails Version: 7.1.4.1
Action Policy Version: 0.7.1

Reproduction Script:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true, quiet: false) do
  source "https://rubygems.org"

  gem "rails", "~> 7.1.0"
  gem "action_policy"
  gem "rspec"
end

require "rails"
require "action_controller/railtie"
require "action_policy"
require "rspec/autorun"

class TestApp < Rails::Application
  config.logger = Logger.new("/dev/null")
  config.eager_load = false
end

class User
end

class UserPolicy < ActionPolicy::Base
  def index?
    deny!(:permission_required)
  end
end

RSpec.describe "all_details" do
  let(:policy) { UserPolicy.new(nil, user: User.new) }

  it "returns result.reasons.details" do
    policy.apply(:index?)

    expect(policy.result.reasons.details).to eq(user: [:permission_required])
  end

  it "returns result.all_details" do
    policy.apply(:index?)

    expect(policy.result.all_details).to include(:permission_required)
  end
end

What did you do? / What did you expect to happen?

I would like to use the all_details method as described in documentation to catch unauthorized exception and return the appropriate status :

rescue_from ActionPolicy::Unauthorized do |ex|
  if ex.result.all_details[:not_found]
    head :not_found
  else
    head :unauthorized
  end
end

What actually happened?

all_details seems to exclude some reasons.

@inkstak
Copy link
Contributor Author

inkstak commented Dec 10, 2024

I'm investigating the source code and I'm starting to understand my mistakes : I confused the details & the reasons.

The documentation seems (to me) a little ambiguous because it explains that to access the reasons you should call result.reasons.details.

But when you call results.all_details you only get details without reasons.

@inkstak
Copy link
Contributor Author

inkstak commented Dec 11, 2024

I've made a first PR to update documentation by adding example about all_details output.

I keep thinking that differences between result.reasons.details and result.all_details are confusing.
What do you think about promoting another method in documentation like result.reasons.to_h ?

p ex.result.reasons.to_h #=> { stage: [:show?] }
p ex.result.reasons.to_h #=> { stage: [{show?: {title: "Onboarding"}] }

See: 5a003d9

@palkan
Copy link
Owner

palkan commented Dec 13, 2024

I keep thinking that differences between result.reasons.details and result.all_details are confusing.

I agree. That's a result of incorporating various feature requests into a design that didn't expect that 🙂 (And that's one of the reasons why we don't have 1.0—I'm still not sure how to resolve this confusion).

What do you think about promoting another method in documentation like result.reasons.to_h ?

I'm open to it. Anything that could help other users worth being mentioned in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants