Merge pull request #1597 from alphagov/remove-page-slug

theseanything · web-flow · commit 159fcce19fae · 2025-08-21T11:04:50.000+01:00
Access page_slug from page inside Step class
diff --git a/app/controllers/forms/add_another_answer_controller.rb b/app/controllers/forms/add_another_answer_controller.rb
@@ -4,7 +4,7 @@ class AddAnotherAnswerController < PageController
 
     def show
       @rows = rows
-      back_link(@step.page_slug)
+      back_link(@step.id)
       @add_another_answer_input = AddAnotherAnswerInput.new
     end
 
@@ -13,7 +13,7 @@ def save
 
       if @add_another_answer_input.invalid?
         @rows = rows
-        back_link(@step.page_slug)
+        back_link(@step.id)
         return render :show
       end
 
@@ -28,18 +28,18 @@ def save
 
     def add_another_path
       if changing_existing_answer
-        form_change_answer_path(@form.id, @form.form_slug, @step.page_slug, answer_index: @step.next_answer_index)
+        form_change_answer_path(@form.id, @form.form_slug, @step.id, answer_index: @step.next_answer_index)
       else
-        form_page_path(@form.id, @form.form_slug, @step.page_slug, answer_index: @step.next_answer_index)
+        form_page_path(@form.id, @form.form_slug, @step.id, answer_index: @step.next_answer_index)
       end
     end
 
     def rows
       @step.questions.map.with_index(1) do |question, answer_index|
-        actions = [{ text: t("forms.add_another_answer.rows.change"), href: form_change_answer_path(@form.id, @form.form_slug, @step.page_slug, answer_index:), visually_hidden_text: I18n.t("forms.add_another_answer.rows.action_hidden_text", answer_index:) }]
+        actions = [{ text: t("forms.add_another_answer.rows.change"), href: form_change_answer_path(@form.id, @form.form_slug, @step.id, answer_index:), visually_hidden_text: I18n.t("forms.add_another_answer.rows.action_hidden_text", answer_index:) }]
 
         unless @step.min_answers?
-          actions << { text: t("forms.add_another_answer.rows.remove"), href: form_remove_answer_path(@form.id, @form.form_slug, @step.page_slug, answer_index:, changing_existing_answer:), visually_hidden_text: I18n.t("forms.add_another_answer.rows.action_hidden_text", answer_index:) }
+          actions << { text: t("forms.add_another_answer.rows.remove"), href: form_remove_answer_path(@form.id, @form.form_slug, @step.id, answer_index:, changing_existing_answer:), visually_hidden_text: I18n.t("forms.add_another_answer.rows.action_hidden_text", answer_index:) }
         end
 
         {
@@ -61,9 +61,9 @@ def add_another_input_params
     def redirect_if_not_repeating
       unless @step.is_a?(RepeatableStep)
         if changing_existing_answer
-          redirect_to form_change_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug)
+          redirect_to form_change_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id)
         else
-          redirect_to form_page_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug)
+          redirect_to form_page_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id)
         end
       end
     end
diff --git a/app/controllers/forms/check_your_answers_controller.rb b/app/controllers/forms/check_your_answers_controller.rb
@@ -100,7 +100,7 @@ def back_link
       previous_step = current_context.previous_step(CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG)
 
       if previous_step.present?
-        previous_step.repeatable? ? add_another_answer_path(form_id: current_context.form.id, form_slug: current_context.form.form_slug, page_slug: previous_step.page_slug) : form_page_path(current_context.form.id, current_context.form.form_slug, previous_step.page_id)
+        previous_step.repeatable? ? add_another_answer_path(form_id: current_context.form.id, form_slug: current_context.form.form_slug, page_slug: previous_step.id) : form_page_path(current_context.form.id, current_context.form.form_slug, previous_step.page_id)
       end
     end
   end
diff --git a/app/controllers/forms/exit_pages_controller.rb b/app/controllers/forms/exit_pages_controller.rb
@@ -1,9 +1,9 @@
 module Forms
   class ExitPagesController < PageController
     def show
-      return redirect_to form_page_path(@form.id, @form.form_slug, current_context.next_page_slug) unless current_context.can_visit?(@step.page_slug)
+      return redirect_to form_page_path(@form.id, @form.form_slug, current_context.next_page_slug) unless current_context.can_visit?(@step.id)
 
-      @back_link = form_page_path(@form.id, @form.form_slug, @step.page_slug)
+      @back_link = form_page_path(@form.id, @form.form_slug, @step.id)
       @condition = @step.routing_conditions.first
     end
   end
diff --git a/app/controllers/forms/page_controller.rb b/app/controllers/forms/page_controller.rb
@@ -9,10 +9,10 @@ def set_request_logging_attributes
     end
 
     def show
-      return redirect_to form_page_path(@form.id, @form.form_slug, current_context.next_page_slug) unless current_context.can_visit?(@step.page_slug)
+      return redirect_to form_page_path(@form.id, @form.form_slug, current_context.next_page_slug) unless current_context.can_visit?(@step.id)
       return redirect_to review_file_page if answered_file_question?
 
-      back_link(@step.page_slug)
+      back_link(@step.id)
       setup_instance_vars_for_view
     end
 
@@ -53,7 +53,7 @@ def answer_index
 
     def setup_instance_vars_for_view
       @is_question = true
-      @question_edit_link = "#{Settings.forms_admin.base_url}/forms/#{@form.id}/pages/#{@step.page_slug}/edit/question"
+      @question_edit_link = "#{Settings.forms_admin.base_url}/forms/#{@form.id}/pages/#{@step.id}/edit/question"
       @save_url = save_url
     end
 
@@ -67,20 +67,20 @@ def back_link(page_slug)
       if changing_existing_answer
         @back_link = check_your_answers_path(form_id: current_context.form.id)
       elsif previous_step
-        @back_link = previous_step.repeatable? ? add_another_answer_path(form_id: current_context.form.id, form_slug: current_context.form.form_slug, page_slug: previous_step.page_slug) : form_page_path(@form.id, @form.form_slug, previous_step.page_id)
+        @back_link = previous_step.repeatable? ? add_another_answer_path(form_id: current_context.form.id, form_slug: current_context.form.form_slug, page_slug: previous_step.id) : form_page_path(@form.id, @form.form_slug, previous_step.page_id)
       end
     end
 
     def redirect_post_save
       return redirect_to review_file_page, success: t("banner.success.file_uploaded") if answered_file_question?
-      return redirect_to exit_page_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug) if @step.exit_page_condition_matches?
+      return redirect_to exit_page_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id) if @step.exit_page_condition_matches?
 
       redirect_to next_page
     end
 
     def redirect_if_not_answered_file_question
       unless @step.question.is_a?(Question::File) && @step.question.file_uploaded?
-        redirect_to form_page_path(@form.id, @form.form_slug, @step.page_slug)
+        redirect_to form_page_path(@form.id, @form.form_slug, @step.id)
       end
     end
 
@@ -89,7 +89,7 @@ def answered_file_question?
     end
 
     def review_file_page
-      review_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, changing_existing_answer:)
+      review_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer:)
     end
 
     def next_page
@@ -102,15 +102,15 @@ def next_page
 
     def next_step_path
       if should_show_add_another?(@step)
-        return add_another_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug)
+        return add_another_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id)
       end
 
       next_step_in_form_path
     end
 
     def next_step_changing
       if should_show_add_another?(@step)
-        return change_add_another_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug)
+        return change_add_another_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id)
       end
 
       check_answers_path
@@ -160,7 +160,7 @@ def admin_edit_condition_url(form_id, page_id)
     end
 
     def is_first_page?
-      current_context.form.start_page == @step.id
+      current_context.form.start_page.to_s == @step.id
     end
 
     def save_url
diff --git a/app/controllers/forms/remove_answer_controller.rb b/app/controllers/forms/remove_answer_controller.rb
@@ -31,9 +31,9 @@ def remove_answer
 
     def next_page_after_removing
       if @step.question.is_optional? && @step.show_answer.blank?
-        form_page_path(@form.id, @form.form_slug, @step.page_slug, changing_existing_answer:)
+        form_page_path(@form.id, @form.form_slug, @step.id, changing_existing_answer:)
       else
-        add_another_answer_path(@form.id, @form.form_slug, @step.page_slug, changing_existing_answer:)
+        add_another_answer_path(@form.id, @form.form_slug, @step.id, changing_existing_answer:)
       end
     end
   end
diff --git a/app/controllers/forms/remove_file_controller.rb b/app/controllers/forms/remove_file_controller.rb
@@ -21,7 +21,7 @@ def destroy
         return redirect_to redirect_after_delete_path, success: t("banner.success.file_removed")
       end
 
-      redirect_to review_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, changing_existing_answer:)
+      redirect_to review_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer:)
     end
 
   private
@@ -32,15 +32,15 @@ def remove_input_params
 
     def redirect_after_delete_path
       if changing_existing_answer
-        return form_change_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug)
+        return form_change_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id)
       end
 
-      form_page_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug)
+      form_page_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id)
     end
 
     def setup_urls
-      @back_link = review_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, changing_existing_answer:)
-      @remove_file_url = remove_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, changing_existing_answer:)
+      @back_link = review_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer:)
+      @remove_file_url = remove_file_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer:)
     end
   end
 end
diff --git a/app/controllers/forms/review_file_controller.rb b/app/controllers/forms/review_file_controller.rb
@@ -3,9 +3,9 @@ class ReviewFileController < PageController
     before_action :redirect_if_not_answered_file_question
 
     def show
-      back_link(@step.page_slug)
-      @remove_file_confirmation_url = remove_file_confirmation_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, changing_existing_answer:)
-      @continue_url = review_file_continue_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, changing_existing_answer:)
+      back_link(@step.id)
+      @remove_file_confirmation_url = remove_file_confirmation_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer:)
+      @continue_url = review_file_continue_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer:)
     end
 
     def continue
diff --git a/app/lib/flow/journey.rb b/app/lib/flow/journey.rb
@@ -29,12 +29,12 @@ def initialize(answer_store:, form:)
     end
 
     def find_or_create(page_slug)
-      step = completed_steps.find { |s| s.page_slug == page_slug }
+      step = completed_steps.find { |s| s.id == page_slug }
       step || @step_factory.create_step(page_slug)
     end
 
     def previous_step(page_slug)
-      index = completed_steps.find_index { |step| step.page_slug == page_slug }
+      index = completed_steps.find_index { |step| step.id == page_slug }
       return nil if completed_steps.empty? || index&.zero?
 
       return completed_steps.last if index.nil?
@@ -45,7 +45,7 @@ def previous_step(page_slug)
     def next_page_slug
       return nil if completed_steps.last&.end_page?
 
-      completed_steps.last&.next_page_slug_after_routing || @step_factory.start_step.page_slug
+      completed_steps.last&.next_page_slug_after_routing || @step_factory.start_step.id
     end
 
     def next_step
@@ -55,7 +55,7 @@ def next_step
     end
 
     def can_visit?(page_slug)
-      (completed_steps.map(&:page_slug).include? page_slug) || page_slug == next_page_slug
+      (completed_steps.map(&:id).include? page_slug) || page_slug == next_page_slug
     end
 
     def all_steps
@@ -111,7 +111,7 @@ def each_step_with_routing
           break if visited_page_slugs.include?(next_page_slug)
 
           yielder << current_step
-          visited_page_slugs << current_step.page_slug
+          visited_page_slugs << current_step.id
 
           break if next_page_slug.nil?
 
diff --git a/app/lib/flow/step_factory.rb b/app/lib/flow/step_factory.rb
@@ -26,7 +26,7 @@ def create_step(page_slug_or_start)
 
       question = QuestionRegister.from_page(page)
 
-      step_class(page).new(question:, page:, page_slug:)
+      step_class(page).new(question:, page:)
     end
 
     def start_step
diff --git a/app/models/step.rb b/app/models/step.rb
@@ -1,20 +1,17 @@
 class Step
   attr_accessor :page, :question
-  attr_reader :page_slug
 
   GOTO_PAGE_ERROR_NAMES = %w[cannot_have_goto_page_before_routing_page goto_page_doesnt_exist].freeze
 
-  def initialize(page:, question:, page_slug:)
+  def initialize(page:, question:)
     @page = page
     @question = question
-
-    @page_slug = page_slug
   end
 
   alias_attribute :id, :page_id
 
   def page_id
-    page&.id
+    page&.id.to_s
   end
 
   def page_number
diff --git a/app/services/log_event_service.rb b/app/services/log_event_service.rb
@@ -59,6 +59,6 @@ def skipped_question?
   end
 
   def is_starting_form?
-    @current_context.form.start_page == @step.id
+    @current_context.form.start_page.to_s == @step.id
   end
 end
diff --git a/app/views/forms/add_another_answer/show.html.erb b/app/views/forms/add_another_answer/show.html.erb
@@ -6,7 +6,7 @@
   <% end %>
 <% end %>
 
-<%= form_with(model: @add_another_answer_input , method: :post, url: add_another_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, changing_existing_answer: @changing_existing_answer)) do |f| %>
+<%= form_with(model: @add_another_answer_input , method: :post, url: add_another_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, changing_existing_answer: @changing_existing_answer)) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-full">
       <% if @add_another_answer_input&.errors.any? %>
diff --git a/app/views/forms/remove_answer/show.html.erb b/app/views/forms/remove_answer/show.html.erb
@@ -1,8 +1,8 @@
 <% content_for :back_link do %>
-  <%= link_to t("forms.back"), add_another_answer_path(@form.id, @form.form_slug, @step.page_slug), class: "govuk-back-link" %>
+  <%= link_to t("forms.back"), add_another_answer_path(@form.id, @form.form_slug, @step.id), class: "govuk-back-link" %>
 <% end %>
 
-<%= form_with(model: @remove_input , method: :delete, url: delete_form_remove_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.page_slug, answer_index: @step.answer_index, changing_existing_answer: @changing_existing_answer, mode: @mode)) do |f| %>
+<%= form_with(model: @remove_input , method: :delete, url: delete_form_remove_answer_path(form_id: @form.id, form_slug: @form.form_slug, page_slug: @step.id, answer_index: @step.answer_index, changing_existing_answer: @changing_existing_answer, mode: @mode)) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <% if @remove_input&.errors.any? %>
diff --git a/spec/factories/models/repeatable_steps.rb b/spec/factories/models/repeatable_steps.rb
@@ -1,9 +1,8 @@
 FactoryBot.define do
   factory :repeatable_step, class: "RepeatableStep" do
     page { association :page }
-    sequence(:page_slug) { |n| "page-#{n}" }
     question { build(:full_name_question) }
 
-    initialize_with { new(question:, page:, page_slug:) }
+    initialize_with { new(question:, page:) }
   end
 end
diff --git a/spec/factories/models/steps.rb b/spec/factories/models/steps.rb
@@ -1,9 +1,8 @@
 FactoryBot.define do
   factory :step, class: "Step" do
     page { association :page }
-    sequence(:page_slug) { |n| "page-#{n}" }
     question { build(:full_name_question) }
 
-    initialize_with { new(question:, page:, page_slug:) }
+    initialize_with { new(question:, page:) }
   end
 end
diff --git a/spec/lib/flow/context_spec.rb b/spec/lib/flow/context_spec.rb
@@ -29,10 +29,10 @@
   end
 
   [
-    ["no input", { answers: [], request_step: 1 }, { next_page_slug: "2", start_id: 1, next_incomplete_page_id: "1", current_step_id: 1, previous_step_id: nil }],
-    ["first question complete, request second step", { answers: [{ text: "q1 answer" }], request_step: 2 }, { next_page_slug: CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG, start_id: 1, previous_step_id: 1, next_incomplete_page_id: "2", current_step_id: 2 }],
-    ["first question complete, request first step", { answers: [{ text: "q1 answer" }], request_step: 1 }, { next_page_slug: "2", start_id: 1, previous_step_id: nil, next_incomplete_page_id: "2", current_step_id: 1 }],
-    ["all questions complete, request second step", { answers: [{ text: "q1 answer" }, { text: "q2 answer" }], request_step: 2 }, { next_page_slug: CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG, start_id: 1, previous_step_id: 1, next_incomplete_page_id: CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG, current_step_id: 2 }],
+    ["no input", { answers: [], request_step: 1 }, { next_page_slug: "2", start_id: 1, next_incomplete_page_id: "1", current_step_id: "1", previous_step_id: nil }],
+    ["first question complete, request second step", { answers: [{ text: "q1 answer" }], request_step: 2 }, { next_page_slug: CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG, start_id: 1, previous_step_id: "1", next_incomplete_page_id: "2", current_step_id: "2" }],
+    ["first question complete, request first step", { answers: [{ text: "q1 answer" }], request_step: 1 }, { next_page_slug: "2", start_id: 1, previous_step_id: nil, next_incomplete_page_id: "2", current_step_id: "1" }],
+    ["all questions complete, request second step", { answers: [{ text: "q1 answer" }, { text: "q2 answer" }], request_step: 2 }, { next_page_slug: CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG, start_id: 1, previous_step_id: "1", next_incomplete_page_id: CheckYourAnswersStep::CHECK_YOUR_ANSWERS_PAGE_SLUG, current_step_id: "2" }],
   ].each do |variation, input, expected_output|
     context "with #{variation}" do
       before do
@@ -58,7 +58,7 @@
       end
 
       it "has the correct previous step" do
-        expect(@context.previous_step(@step.page_slug)&.page_id).to eq(expected_output[:previous_step_id])
+        expect(@context.previous_step(@step.id)&.page_id).to eq(expected_output[:previous_step_id])
       end
 
       it "has the correct next incomplete step" do
diff --git a/spec/lib/flow/step_factory_spec.rb b/spec/lib/flow/step_factory_spec.rb
@@ -26,7 +26,7 @@
         expect(step).to be_a(Step)
         expect(step.question).to eq(question)
         expect(step.next_page_slug).to eq("page-2")
-        expect(step.page_slug).to eq("page-1")
+        expect(step.id).to eq("page-1")
       end
 
       context "when it is the last page" do
@@ -49,7 +49,7 @@
       end
 
       it "a RepeatingStep is created" do
-        step = factory.create_step(page.page_slug)
+        step = factory.create_step(page.id)
         expect(step).to be_a(RepeatableStep)
       end
     end
@@ -71,7 +71,7 @@
       it "creates a step for the start page" do
         step = factory.create_step(Flow::StepFactory::START_PAGE)
         expect(step).to be_a(Step)
-        expect(step.page_slug).to eq("page-1")
+        expect(step.id).to eq("page-1")
       end
     end
   end
diff --git a/spec/models/repeatable_step_spec.rb b/spec/models/repeatable_step_spec.rb
@@ -1,7 +1,7 @@
 require "rails_helper"
 
 RSpec.describe RepeatableStep, type: :model do
-  subject(:repeatable_step) { described_class.new(question:, page:, page_slug: page.id) }
+  subject(:repeatable_step) { described_class.new(question:, page:) }
 
   let(:form) { build :form, id: 1, form_slug: "form-slug", pages: [page, build(:page, id: 2)] }
   let(:page) { build :page }
diff --git a/spec/models/step_spec.rb b/spec/models/step_spec.rb
diff --git a/spec/services/log_event_service_spec.rb b/spec/services/log_event_service_spec.rb
