From c2c761de4f9311b8bd2adf33a870b939af6096bd Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 13 Aug 2026 14:20:01 +0100 Subject: [PATCH 1/3] Change unique indexes on form_documents table Allow: - One form_document with the same language and version per form - One form_document with the same language and a null version per form to account for draft forms The indexes are not added concurrently, as the benefits of adding them in a transaction outweigh the potental downsides of locking the table for a short time. --- ...change_unique_indexes_on_form_documents.rb | 19 +++++++++++++++++++ db/schema.rb | 5 +++-- 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20260813112443_change_unique_indexes_on_form_documents.rb diff --git a/db/migrate/20260813112443_change_unique_indexes_on_form_documents.rb b/db/migrate/20260813112443_change_unique_indexes_on_form_documents.rb new file mode 100644 index 000000000..de17dd089 --- /dev/null +++ b/db/migrate/20260813112443_change_unique_indexes_on_form_documents.rb @@ -0,0 +1,19 @@ +class ChangeUniqueIndexesOnFormDocuments < ActiveRecord::Migration[8.1] + def up + remove_index :form_documents, name: "index_form_documents_on_form_id_tag_and_language" + + # Only one form_document with the same version per language per form + add_index :form_documents, %i[form_id version language], name: "index_form_documents_on_form_id_version_and_language", unique: true + + # Only one form_document with a null version (the draft form document) per language per form + add_index :form_documents, %i[form_id language], name: "index_form_documents_only_one_draft_per_language", unique: true, where: "version IS NULL" + end + + def down + remove_index :form_documents, name: "index_form_documents_only_one_draft_per_language" + + remove_index :form_documents, name: "index_form_documents_on_form_id_version_and_language" + + add_index :form_documents, %i[form_id tag language], name: "index_form_documents_on_form_id_tag_and_language", unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index ff473944b..d2b62dabc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_08_12_141413) do +ActiveRecord::Schema[8.1].define(version: 2026_08_13_112443) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -125,7 +125,8 @@ t.text "tag", null: false, comment: "The tag for the form, for example: 'live' or 'draft'" t.datetime "updated_at", null: false t.integer "version" - t.index ["form_id", "tag", "language"], name: "index_form_documents_on_form_id_tag_and_language", unique: true + t.index ["form_id", "language"], name: "index_form_documents_only_one_draft_per_language", unique: true, where: "(version IS NULL)" + t.index ["form_id", "version", "language"], name: "index_form_documents_on_form_id_version_and_language", unique: true t.index ["form_id"], name: "index_form_documents_on_form_id" end From 121125409fae38b6ed358710cd47eaf26a183048 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 13 Aug 2026 14:20:20 +0100 Subject: [PATCH 2/3] Delete specs for data migration that has been run Delete the tests for the data migration rake task to backfill versons as the new indexes do not allow for multiple form_documents with the same language and a null version per form. This rake task has been run in all environments - but we'll keep it around slightly longer so devs can run it if they need to backfill their local databases. --- spec/lib/tasks/data_migrations.rake_spec.rb | 67 --------------------- 1 file changed, 67 deletions(-) delete mode 100644 spec/lib/tasks/data_migrations.rake_spec.rb diff --git a/spec/lib/tasks/data_migrations.rake_spec.rb b/spec/lib/tasks/data_migrations.rake_spec.rb deleted file mode 100644 index 9d3e7e787..000000000 --- a/spec/lib/tasks/data_migrations.rake_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require "rails_helper" - -RSpec.describe "data_migrations.rake", type: :task do - describe "data_migrations:set_version_on_form_documents" do - subject(:task) do - Rake::Task["data_migrations:set_version_on_form_documents"] - end - - context "when the version is not set for form documents" do - let!(:live_form) { create :form, :live, :with_welsh_translation } - let!(:archived_form) { create :form, :archived, :with_welsh_translation } - let!(:draft_form) { create :form, :with_welsh_translation } - - before do - live_form.live_form_document.update!(version: nil) - live_form.live_welsh_form_document.update!(version: nil) - archived_form.archived_form_document.update!(version: nil) - archived_form.archived_welsh_form_document.update!(version: nil) - end - - it "sets version to 1 for all live and archived form documents" do - task.invoke - - expect(live_form.live_form_document.reload.version).to eq(1) - expect(live_form.live_welsh_form_document.reload.version).to eq(1) - expect(archived_form.archived_form_document.reload.version).to eq(1) - expect(archived_form.archived_welsh_form_document.reload.version).to eq(1) - end - - it "does not set version for draft form documents" do - task.invoke - - expect(draft_form.draft_form_document.reload.version).to be_nil - expect(draft_form.draft_welsh_form_document.reload.version).to be_nil - end - - it "updates latest_form_document_id to the English version" do - task.invoke - - expect(live_form.reload.latest_form_document_id).to eq(live_form.live_form_document.id) - expect(archived_form.reload.latest_form_document_id).to eq(archived_form.archived_form_document.id) - end - - it "does not update the latest_form_document_id for draft forms" do - task.invoke - - expect(draft_form.reload.latest_form_document_id).to be_nil - end - end - - context "when the version is set for form documents" do - let!(:live_form) { create :form, :live, :with_welsh_translation } - - before do - live_form.live_form_document.update!(version: 2) - live_form.live_welsh_form_document.update!(version: 2) - end - - it "does not change the version for form documents that already have a version set" do - task.invoke - - expect(live_form.live_form_document.reload.version).to eq(2) - expect(live_form.live_welsh_form_document.reload.version).to eq(2) - end - end - end -end From 36f62287e80f3f65e63ebec221381a36333716c7 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Thu, 13 Aug 2026 14:20:57 +0100 Subject: [PATCH 3/3] Update specs to account for new unique indexes on FormDocument --- spec/models/form_document_spec.rb | 21 ++++++++++++++++--- .../api/form_documents_controller_spec.rb | 4 ++-- .../revert_draft_form_service_spec.rb | 6 +++--- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/spec/models/form_document_spec.rb b/spec/models/form_document_spec.rb index 7761888e3..3948058e8 100644 --- a/spec/models/form_document_spec.rb +++ b/spec/models/form_document_spec.rb @@ -46,8 +46,23 @@ expect(form_document.form).to be_a(Form) end - it "tags must be unique for a given form" do - form_document = create(:form_document, tag: "live") - expect { create(:form_document, form: form_document.form, tag: "live") }.to raise_error(ActiveRecord::RecordNotUnique) + it "raises an error if a form document with the same version exists for the form" do + form_document = create(:form_document, :live, version: 1) + expect { create(:form_document, :archived, form: form_document.form, version: 1) }.to raise_error(ActiveRecord::RecordNotUnique) + end + + it "raises an error if a draft form document already exists for the form" do + form = create(:form) # also creates a draft form document + expect { create(:form_document, :draft, form: form) }.to raise_error(ActiveRecord::RecordNotUnique) + end + + it "allows creating a form with the same version and a different language" do + form_document = create(:form_document, :live, language: "en") + expect { create(:form_document, :live, form: form_document.form, language: "cy") }.not_to raise_error + end + + it "allows creating a draft form with a different language" do + form = create(:form) # also creates a draft form document + expect { create(:form_document, :draft, form: form, language: "cy") }.not_to raise_error end end diff --git a/spec/requests/api/form_documents_controller_spec.rb b/spec/requests/api/form_documents_controller_spec.rb index 0711e0fa9..d616e477d 100644 --- a/spec/requests/api/form_documents_controller_spec.rb +++ b/spec/requests/api/form_documents_controller_spec.rb @@ -123,8 +123,8 @@ let(:form) { create :form } before do - create :form_document, form: form, tag: "live", language: "en", content: { form_id: form.id.to_s, language: "en" } - create :form_document, form: form, tag: "live", language: "cy", content: { form_id: form.id.to_s, language: "cy" } + create :form_document, :live, form: form, language: "en", content: { form_id: form.id.to_s, language: "en" } + create :form_document, :live, form: form, language: "cy", content: { form_id: form.id.to_s, language: "cy" } end it "when not given a language, defaults to english returns the live form document in english" do diff --git a/spec/services/revert_draft_form_service_spec.rb b/spec/services/revert_draft_form_service_spec.rb index 9f13316e1..df7b53ab9 100644 --- a/spec/services/revert_draft_form_service_spec.rb +++ b/spec/services/revert_draft_form_service_spec.rb @@ -95,7 +95,7 @@ def revert_draft(tag) goto_page_id: live_form.pages.last.id, routing_page_id: live_form.pages.first.id, ) - FormDocument.create!(form: live_form, tag: "live", content: live_form.as_form_document(live_at: live_form.updated_at)) + create(:form_document, :live, form: live_form, content: live_form.as_form_document(live_at: live_form.updated_at)) live_form.update!(state: :live_with_draft) end @@ -149,7 +149,7 @@ def revert_draft(tag) exit_page_heading: "You cannot continue", exit_page_markdown: "Please contact us", ) - FormDocument.create!(form: live_form, tag: "live", content: live_form.as_form_document(live_at: live_form.updated_at)) + create(:form_document, :live, form: live_form, content: live_form.as_form_document(live_at: live_form.updated_at)) live_form.update!(state: :live_with_draft) end @@ -563,7 +563,7 @@ def revert_draft(tag) goto_page_id: archived_form.pages.last.id, routing_page_id: archived_form.pages.first.id, ) - FormDocument.create!(form: archived_form, tag: "archived", content: archived_form.as_form_document(live_at: archived_form.updated_at)) + create(:form_document, :archived, form: archived_form, content: archived_form.as_form_document(live_at: archived_form.updated_at)) archived_form.update!(state: :archived_with_draft) end