Skip to content

Commit

Permalink
Merge pull request #9961 from alphagov/content-modelling/926-bug-serv…
Browse files Browse the repository at this point in the history
…er-error-when-searching-for-certain-keywords

(926) Fix server error when searching for certain keywords
  • Loading branch information
pezholio authored Feb 21, 2025
2 parents 98c46dc + b4a8866 commit f9bf6b5
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
class AddFulltextIndexToContentBlockEditions < ActiveRecord::Migration[8.0]
INDEX_NAME = "title_details_instructions_to_publishers".freeze

def up
change_table :content_block_editions, bulk: true do |t|
t.virtual :details_for_indexing, type: :text, as: "JSON_UNQUOTE(details)", stored: true
t.index %i[title details_for_indexing instructions_to_publishers], name: INDEX_NAME, type: :fulltext
end
end

def down
change_table :content_block_editions, bulk: true do |t|
t.remove :details_for_indexing
end
remove_index :content_block_editions, name: INDEX_NAME
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2025_01_29_095918) do
ActiveRecord::Schema[8.0].define(version: 2025_02_20_131003) do
create_table "assets", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
t.string "asset_manager_id", null: false
t.string "variant", null: false
Expand Down Expand Up @@ -236,7 +236,9 @@
t.text "internal_change_note"
t.text "change_note"
t.boolean "major_change"
t.virtual "details_for_indexing", type: :text, as: "json_unquote(`details`)", stored: true
t.index ["document_id"], name: "index_content_block_editions_on_document_id"
t.index ["title", "details_for_indexing", "instructions_to_publishers"], name: "title_details_instructions_to_publishers", type: :fulltext
t.index ["user_id"], name: "index_content_block_editions_on_user_id"
end

Expand Down
3 changes: 3 additions & 0 deletions features/support/env.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,6 @@

# Require step definitions contained within engines
Dir.glob(Rails.root.join("lib/engines/**/features/step_definitions/*.rb")).each(&method(:require))

# Require support files contained within engines
Dir.glob(Rails.root.join("lib/engines/**/features/support/*.rb")).each(&method(:require))
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ module ContentBlock::Document::Scopes::SearchableByKeyword
extend ActiveSupport::Concern

SQL = <<-SQL.freeze
content_block_editions.title REGEXP :pattern OR#{' '}
content_block_editions.details REGEXP :pattern OR#{' '}
content_block_editions.instructions_to_publishers REGEXP :pattern
MATCH(
content_block_editions.title,#{' '}
content_block_editions.details_for_indexing,#{' '}
content_block_editions.instructions_to_publishers
) AGAINST (:pattern IN BOOLEAN MODE)
SQL

included do
Expand All @@ -14,8 +16,8 @@ module ContentBlock::Document::Scopes::SearchableByKeyword
split_keywords = keywords.split
pattern = split_keywords.map { |k|
escaped_word = Regexp.escape(k)
"(?=.*#{escaped_word})"
}.join
"+#{escaped_word}"
}.join(" ")
joins(:latest_edition)
.where(SQL, pattern:)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,23 @@ Feature: Search for a content object
And I click to view results
Then I should see the details for all documents from my organisation

@disable_transactions
Scenario: GDS Editor searches for a content object by keyword in instructions to publishers
When I visit the Content Block Manager home page
And I enter the keyword "GDS"
And I click to view results
Then I should see the content block with title "an address" returned
And "1" content blocks are returned in total

@disable_transactions
Scenario: GDS Editor searches for a content object by keyword in title
When I visit the Content Block Manager home page
And I enter the keyword "example search"
And I click to view results
Then I should see the content block with title "example search title" returned
And "1" content blocks are returned in total

@disable_transactions
Scenario: GDS Editor searches for a content object by keyword in details
When I visit the Content Block Manager home page
And I enter the keyword "ABC123"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Around("@disable_transactions") do |_scenario, block|
Cucumber::Rails::World.use_transactional_tests = false
DatabaseCleaner.strategy = :truncation
DatabaseCleaner.start

block.call

DatabaseCleaner.clean
end
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,22 @@
class ContentBlockManager::SearchableByKeywordTest < ActiveSupport::TestCase
extend Minitest::Spec::DSL

# Because our tests run in a transaction by default, and this functionality relies on
# database indexes, the indexes never get created, so we need to disable transactions
# and ensure that DatabaseCleaner cleans up after each test.
self.use_transactional_tests = false
DatabaseCleaner.strategy = :truncation

before(:each) do
DatabaseCleaner.start
end

after(:each) do
DatabaseCleaner.clean
end

describe ".with_keyword" do
test "should find documents with title containing keyword" do
it "should find documents with title containing keyword" do
document_with_first_keyword = create(:content_block_document, :email_address)
_edition_with_first_keyword = create(:content_block_edition,
:email_address,
Expand All @@ -14,21 +28,23 @@ class ContentBlockManager::SearchableByKeywordTest < ActiveSupport::TestCase
document_without_first_keyword = create(:content_block_document, :email_address)
_edition_without_first_keyword = create(:content_block_edition, :email_address, document: document_without_first_keyword,
title: "this document is about muppets")

assert_equal [document_with_first_keyword], ContentBlockManager::ContentBlock::Document.with_keyword("klingons")
end

test "should find documents with title containing keywords not in order" do
it "should find documents with title containing keywords not in order" do
document_with_first_keyword = create(:content_block_document, :email_address)
_edition_with_first_keyword = create(:content_block_edition,
:email_address,
document: document_with_first_keyword,
details: { "email_address" => "hello@hello.com" },
title: "klingons and such")
_document_without_first_keyword = create(:content_block_document, :email_address)

assert_equal [document_with_first_keyword], ContentBlockManager::ContentBlock::Document.with_keyword("such klingons")
end

test "should find documents with latest edition's details containing keyword" do
it "should find documents with latest edition's details containing keyword" do
document_with_first_keyword = create(:content_block_document, :email_address)
_edition_with_first_keyword = create(:content_block_edition,
document: document_with_first_keyword,
Expand All @@ -39,10 +55,11 @@ class ContentBlockManager::SearchableByKeywordTest < ActiveSupport::TestCase
document: document_without_first_keyword,
details: { "something" => "something" },
title: "this document is about muppets")

assert_equal [document_with_first_keyword], ContentBlockManager::ContentBlock::Document.with_keyword("foo bar")
end

test "should find documents with instructions to publishers containing keyword" do
it "should find documents with instructions to publishers containing keyword" do
document_with_first_keyword = create(:content_block_document, :email_address)
_edition_with_first_keyword = create(:content_block_edition,
document: document_with_first_keyword,
Expand All @@ -53,10 +70,11 @@ class ContentBlockManager::SearchableByKeywordTest < ActiveSupport::TestCase
document: document_without_first_keyword,
instructions_to_publishers: "bar",
title: "this document is about muppets")

assert_equal [document_with_first_keyword], ContentBlockManager::ContentBlock::Document.with_keyword("foo")
end

test "should find documents with details or title containing keyword" do
it "should find documents with details or title containing keyword" do
document_with_keyword_in_details = create(:content_block_document, :email_address)
_edition_with_keyword = create(:content_block_edition,
document: document_with_keyword_in_details,
Expand All @@ -67,6 +85,7 @@ class ContentBlockManager::SearchableByKeywordTest < ActiveSupport::TestCase
document: document_with_keyword_in_title,
details: { "something" => "something" },
title: "this document is about bar foo")

assert_equal [document_with_keyword_in_details, document_with_keyword_in_title], ContentBlockManager::ContentBlock::Document.with_keyword("foo bar")
end
end
Expand Down

0 comments on commit f9bf6b5

Please sign in to comment.