platanus / potassium

A Rails application generator by Platanus, inspired by Suspenders
MIT License
232 stars 17 forks source link

F/pundit page policy #453

Closed aaguilarg closed 1 year ago

aaguilarg commented 1 year ago

Context

Potassium-generated applications with Pundit use that gem to allow access to certain resources. Sometimes we add roles to filter which pages can a user see on active admin, and if they don't have permission to see anything (not even the dashboard) the app enters on a redirect loop, leading into an exception.

Also, every Potassium-generated policy files come un-tested.

What has been done

Summary: PagePolicy has been edited, now when de page is the Dashboard, is always shown to the user. Also, policy tests are now generated with Potassium.

Commits:

  1. Add Pundit shared examples, which will be useful for every policy spec created in this PR
  2. Add the "Always show dashboard" logic to the PagePolicy
  3. Add missing tests to the policies added in the pundit recipe
  4. Modify the pundit recipe and test that every file is copied correctly

Extra info

EDIT: what I describe below is now fixed, I added a force: true to the factory file copy, and now it works perfectly.

All the new generated tests fails on a new app, but this is because de AdminUser factory is created with empty attributes. If the email and password are added, every test pass.

FactoryBot.define do
  factory :admin_user do
    email { Faker::Internet.unique.email }
    password { 'password' }
  end
end

My doubt is: is it okay to leave it like that, with failing tests? I tried to modify the AdminUser factory file, but when the pundit recipe is ran, that file is not created yet. Also I tried to copy the AdminUser factory file (like the policies files are copied), but that generates a situation where user input is needed. This is because when the AdminUser factory file is trying to be created, it raises a warning that already exists, and user input is needed in order to decide to skip, abort, overwrite, etc.