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 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 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