From 4d263ea3cdbbfee8e770d47b670bedff513618d9 Mon Sep 17 00:00:00 2001 From: Sascha Karnatz Date: Fri, 29 Nov 2024 17:15:48 +0100 Subject: [PATCH] Use an inner select for CanCanCan abilities The previous solution only worked if the pg_search_document had a page_id. If another search document without a page relation was added and an ability was present, the document couldn't be found, because the generate SQL had an inner join that was using the page_id. The new solution is using a sub select and only filters pg_search_documents if the page_id is present on that document entries. --- lib/alchemy-pg_search.rb | 9 ++------- spec/lib/alchemy-pg_search_spec.rb | 10 ++++++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/alchemy-pg_search.rb b/lib/alchemy-pg_search.rb index 61fbc5d..4b7f7e1 100644 --- a/lib/alchemy-pg_search.rb +++ b/lib/alchemy-pg_search.rb @@ -60,13 +60,8 @@ def self.search(query, ability: nil) query = ::PgSearch.multisearch(query).includes(:searchable) if ability - # left_joins method is not usable here, because the order of the joins are incorrect - # and would result in a SQL error. We can receive the correct query order with these - # odd left join string - # Ref: https://guides.rubyonrails.org/active_record_querying.html#using-a-string-sql-fragment - query = query - .joins("LEFT JOIN alchemy_pages ON alchemy_pages.id = pg_search_documents.page_id") - .merge(Alchemy::Page.accessible_by(ability, :read)) + inner_ability_select = Alchemy::Page.select(:id).merge(Alchemy::Page.accessible_by(ability, :read)) + query = query.where("page_id IS NULL OR page_id IN (#{inner_ability_select.to_sql})") end query diff --git a/spec/lib/alchemy-pg_search_spec.rb b/spec/lib/alchemy-pg_search_spec.rb index a8339ad..de0ae4b 100644 --- a/spec/lib/alchemy-pg_search_spec.rb +++ b/spec/lib/alchemy-pg_search_spec.rb @@ -101,6 +101,16 @@ it 'should find two pages' do expect(result.length).to eq(2) end + + context "with other search documents" do + let!(:other_search_document) do + PgSearch::Document.new(content: "Page").save(validate: false) + end + + it 'should find three pages' do + expect(result.length).to eq(3) + end + end end end end