Skip to content

Commit

Permalink
Fix author edit_content permissions
Browse files Browse the repository at this point in the history
We were missing a third argument in order to be able to use this
permission with the `.accessible_by` scope.
  • Loading branch information
tvdeyen committed Sep 1, 2022
1 parent 85f5dc0 commit c98605a
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 13 deletions.
11 changes: 3 additions & 8 deletions app/controllers/alchemy/api/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,9 @@ class Api::PagesController < Api::BaseController
# Returns all pages as json object
#
def index
# Fix for cancancan not able to merge multiple AR scopes for logged in users
if can? :edit_content, Alchemy::Page
@pages = Alchemy::Page.all
else
language = Alchemy::Language.find_by(id: params[:language_id]) || Alchemy::Language.current
@pages = Alchemy::Page.accessible_by(current_ability, :index)
@pages = @pages.where(language: language)
end
language = Alchemy::Language.find_by(id: params[:language_id]) || Alchemy::Language.current
@pages = Alchemy::Page.accessible_by(current_ability, :index)
@pages = @pages.where(language: language)
@pages = @pages.includes(*page_includes)
@pages = @pages.ransack(params[:q]).result

Expand Down
4 changes: 3 additions & 1 deletion lib/alchemy/permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ def alchemy_author_rules
can :manage, Alchemy::Node
can [:read, :url], Alchemy::Picture
can [:read, :autocomplete], Alchemy::Tag
can(:edit_content, Alchemy::Page) { |p| p.editable_by?(@user) }
can :edit_content, Alchemy::Page, Alchemy::Page.all do |page|
page.editable_by?(@user)
end
end
end

Expand Down
8 changes: 4 additions & 4 deletions spec/controllers/alchemy/api/pages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,10 @@ module Alchemy
let(:site_2) { create(:alchemy_site) }
let(:language_2) { create(:alchemy_language, site: site_2) }
let!(:site_2_page) { create(:alchemy_page, :public, language: language_2) }
let!(:unpublished_page) { create(:alchemy_page, language: default_language) }

context "as guest user" do
it "only returns pages for current site" do
it "only returns public pages for current site" do
get :index, format: :json
expect(result["pages"].map { |r| r["id"] }).to match_array([
page.parent_id,
Expand All @@ -101,13 +102,12 @@ module Alchemy
authorize_user(build(:alchemy_dummy_user, :as_author))
end

it "returns all pages" do
it "returns all pages for current site" do
get :index, format: :json
expect(result["pages"].map { |r| r["id"] }).to match_array([
page.parent_id,
page.id,
site_2_page.parent_id,
site_2_page.id,
unpublished_page.id,
])
end
end
Expand Down

4 comments on commit c98605a

@dbwinger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tvdeyen Can you explain this change more? It reverts #2258 so I can no longer link to a page in another site or language. Is that intentional? If so, is the recommended way to do that now using the "External" tab? I'd like to submit at PR to send the language_id param from the page selector on the link overlay so pages for the language of the page being edited are returned, but I wanted to make sure I understand this change first.

@tvdeyen
Copy link
Member Author

@tvdeyen tvdeyen commented on c98605a Mar 20, 2023 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbwinger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tvdeyen No problem! See PR #2439 , but when you get a chance, please explain this one too, because that PR only solves one of the problems I see. Take rest and get well soon!

@tvdeyen
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #2500 (comment) for further info

Please sign in to comment.