Skip to content

Commit

Permalink
Add AlbumPolicy::Scope & use in ArtistsController#show
Browse files Browse the repository at this point in the history
I think this is more idiomatic use of Pundit and it simplifies
`ArtistsController#show` nicely which will make adding an Atom feed for
that action easier.

I suspect there's a nicer way to implement the 2nd expression in the
`if` statement in `AlbumPolicy::Scope#resolve`, but the one I've gone
with seems to result in a sensible number of queries.
  • Loading branch information
floehopper committed Jan 9, 2024
1 parent 2aa44ef commit dff58b9
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 5 deletions.
6 changes: 1 addition & 5 deletions app/controllers/artists_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ def index
def show
skip_authorization

@albums = if pundit_user.admin? || pundit_user.artists.include?(@artist)
Album.where(artist: @artist)
else
Album.published.where(artist: @artist)
end
@albums = policy_scope(@artist.albums)
end

def new
Expand Down
10 changes: 10 additions & 0 deletions app/policies/album_policy.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
# frozen_string_literal: true

class AlbumPolicy < ApplicationPolicy
class Scope < Scope
def resolve
if user.admin? || scope.map(&:artist).uniq.all? { |a| user.artists.include?(a) }
scope.all
else
scope.published
end
end
end

def show?
return record.published? unless user.signed_in?

Expand Down
53 changes: 53 additions & 0 deletions test/policies/album_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ class AlbumPolicyTest < ActiveSupport::TestCase
assert_not policy.request_publication?
end

test 'an admin scope' do
user = build(:user, admin: true)
unpublished_album = create(:album, publication_status: :unpublished)
pending_album = create(:album, publication_status: :pending)
published_album = create(:album, publication_status: :published)

scope = AlbumPolicy::Scope.new(user, Album.all)

assert_includes scope.resolve, unpublished_album
assert_includes scope.resolve, pending_album
assert_includes scope.resolve, published_album
end

test 'a user' do
user = build(:user)
album = build(:album)
Expand Down Expand Up @@ -48,6 +61,20 @@ class AlbumPolicyTest < ActiveSupport::TestCase
assert policy.request_publication?
end

test 'scope for a user with albums belonging to their artist' do
user = build(:user)
artist = create(:artist, user:)
unpublished_album = create(:album, artist:, publication_status: :unpublished)
pending_album = create(:album, artist:, publication_status: :pending)
published_album = create(:album, artist:, publication_status: :published)

scope = AlbumPolicy::Scope.new(user, Album.all)

assert_includes scope.resolve, unpublished_album
assert_includes scope.resolve, pending_album
assert_includes scope.resolve, published_album
end

test 'non-signed-in user' do
non_signed_in_user = NullUser.new
unpublished_album = create(:album, publication_status: :unpublished)
Expand All @@ -59,6 +86,19 @@ class AlbumPolicyTest < ActiveSupport::TestCase
assert AlbumPolicy.new(non_signed_in_user, published_album).show?
end

test 'non-signed-in user scope' do
user = NullUser.new
unpublished_album = create(:album, publication_status: :unpublished)
pending_album = create(:album, publication_status: :pending)
published_album = create(:album, publication_status: :published)

scope = AlbumPolicy::Scope.new(user, Album.all)

assert_not_includes scope.resolve, unpublished_album
assert_not_includes scope.resolve, pending_album
assert_includes scope.resolve, published_album
end

test 'signed-in user' do
user = build(:user)
unpublished_album = create(:album, publication_status: :unpublished)
Expand All @@ -69,4 +109,17 @@ class AlbumPolicyTest < ActiveSupport::TestCase
assert_not AlbumPolicy.new(user, pending_album).show?
assert AlbumPolicy.new(user, published_album).show?
end

test 'signed-in user scope' do
user = build(:user)
published_album = create(:album, publication_status: :published)
pending_album = create(:album, publication_status: :pending)
unpublished_album = create(:album, publication_status: :unpublished)

scope = AlbumPolicy::Scope.new(user, Album.all)

assert_not_includes scope.resolve, unpublished_album
assert_not_includes scope.resolve, pending_album
assert_includes scope.resolve, published_album
end
end

0 comments on commit dff58b9

Please sign in to comment.