From 8c2bb4a556679fe7b1945e2d36d62397bfd8ed2a Mon Sep 17 00:00:00 2001 From: Roger Lam Date: Tue, 26 May 2026 12:32:37 -0700 Subject: [PATCH 1/6] Show continue on question page if any tasks were started --- app/models/strata/flows/question_page.rb | 7 +++ app/models/strata/flows/task.rb | 2 +- .../app/previews/sample_task_list_preview.rb | 10 ++++ .../models/strata/flows/question_page_spec.rb | 58 +++++++++++++++++++ spec/models/strata/flows/task_spec.rb | 37 +++++++++--- 5 files changed, 105 insertions(+), 9 deletions(-) diff --git a/app/models/strata/flows/question_page.rb b/app/models/strata/flows/question_page.rb index da8b5720..09ebbedb 100644 --- a/app/models/strata/flows/question_page.rb +++ b/app/models/strata/flows/question_page.rb @@ -22,6 +22,13 @@ def completed?(record) record.valid?(@name.to_sym) end + def started?(record) + @fields.any? do |field| + field_names = field.is_a?(Hash) ? field.keys : [ field ] + field_names.any? { |name| record.public_send(name).present? } + end + end + def edit_pathname "edit_#{@name}" end diff --git a/app/models/strata/flows/task.rb b/app/models/strata/flows/task.rb index e5924ac8..92ef541a 100644 --- a/app/models/strata/flows/task.rb +++ b/app/models/strata/flows/task.rb @@ -12,7 +12,7 @@ def initialize(name, depends_on: nil, pages: []) end def started?(record) - @pages.any? { |page| page.completed?(record) } + @pages.any? { |page| page.started?(record) } end def completed?(record) diff --git a/spec/dummy/app/previews/sample_task_list_preview.rb b/spec/dummy/app/previews/sample_task_list_preview.rb index c33222e9..d1584dcf 100644 --- a/spec/dummy/app/previews/sample_task_list_preview.rb +++ b/spec/dummy/app/previews/sample_task_list_preview.rb @@ -32,6 +32,16 @@ def depends_on_no_tasks_completed render Strata::Flows::TaskListComponent.new(flow:, show_step_label: true) end + # @label First page empty, later page has data + # @note Personal Information has data on the second page (Date of Birth) but the first page (Name) is empty. Shows "Continue" because some sub-page has data, and routes to the first incomplete page. + def first_page_empty_later_page_has_data + flow = SampleFlow.new(FactoryBot.build_stubbed( + :sample_application_form, + date_of_birth: Date.new(1990, 5, 15) + )) + render Strata::Flows::TaskListComponent.new(flow:, show_step_label: true) + end + # @label First task completed # @note Personal Information is completed, unlocking Employment Details. Leave Details still shows "Cannot start yet" because it depends on Employment Details. def depends_on_first_task_completed diff --git a/spec/models/strata/flows/question_page_spec.rb b/spec/models/strata/flows/question_page_spec.rb index 9a1504fb..efdbf0c8 100644 --- a/spec/models/strata/flows/question_page_spec.rb +++ b/spec/models/strata/flows/question_page_spec.rb @@ -36,6 +36,13 @@ expect(page).to be_completed(record) end + it "is started when the field has a value" do + expect(page).not_to be_started(record) + + record.first_name = "Mary" + expect(page).to be_started(record) + end + it "returns the correct pathnames" do expect(page.edit_pathname).to eq("edit_first_name") expect(page.update_pathname).to eq("update_first_name") @@ -63,6 +70,57 @@ end end + describe "#started? with a multi-parameter field" do + before do + test_model_class = Class.new(ActiveRecord::Base) do + self.table_name = "test_records" + include Strata::Attributes + + strata_attribute :date_of_birth, :memorable_date + end + + stub_const("DobModel", test_model_class) + end + + let(:record) { DobModel.new } + let(:page) { described_class.new("dob", fields: [ date_of_birth: [ :month, :day, :year ] ]) } + + it "is not started when the field has no value" do + expect(page).not_to be_started(record) + end + + it "is started when the field has a value" do + record.date_of_birth = Date.new(1990, 5, 15) + expect(page).to be_started(record) + end + end + + describe "#started? with multiple fields" do + before do + multi_field_class = Class.new do + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :first_name, :string + attribute :last_name, :string + end + + stub_const("MultiFieldModel", multi_field_class) + end + + let(:record) { MultiFieldModel.new } + let(:page) { described_class.new("name", fields: [ :first_name, :last_name ]) } + + it "is not started when no fields have values" do + expect(page).not_to be_started(record) + end + + it "is started when any field has a value" do + record.last_name = "Smith" + expect(page).to be_started(record) + end + end + describe "#attributes" do before do test_model_class = Class.new(ActiveRecord::Base) do diff --git a/spec/models/strata/flows/task_spec.rb b/spec/models/strata/flows/task_spec.rb index eafd2a70..2891da85 100644 --- a/spec/models/strata/flows/task_spec.rb +++ b/spec/models/strata/flows/task_spec.rb @@ -9,18 +9,20 @@ include ActiveModel::Attributes attribute :first_name, :string + attribute :last_name, :string validates :first_name, presence: true, on: :first_name + validates :last_name, presence: true, on: :last_name end stub_const("TestModel", test_model_class) end - let(:incomplete_page) { Strata::Flows::QuestionPage.new("first_name") } - let(:complete_page) { Strata::Flows::QuestionPage.new("last_name") } + let(:first_name_page) { Strata::Flows::QuestionPage.new("first_name") } + let(:last_name_page) { Strata::Flows::QuestionPage.new("last_name") } let(:record) { TestModel.new } describe "an unstarted task" do - let(:task) { described_class.new("name", pages: [ incomplete_page ]) } + let(:task) { described_class.new("name", pages: [ first_name_page ]) } it "is not started or completed" do expect(task).not_to be_started(record) @@ -28,13 +30,15 @@ end it "returns the first page path" do - allow(incomplete_page).to receive(:edit_path).and_return("edit_path") + allow(first_name_page).to receive(:edit_path).and_return("edit_path") expect(task.path(record)).to eq("edit_path") end end describe "a started task" do - let(:task) { described_class.new("name", pages: [ complete_page, incomplete_page ]) } + let(:task) { described_class.new("name", pages: [ first_name_page, last_name_page ]) } + + before { record.first_name = "Mary" } it "is started but not completed" do expect(task).to be_started(record) @@ -42,13 +46,15 @@ end it "returns the first incomplete page path" do - allow(incomplete_page).to receive(:edit_path).and_return("edit_path") + allow(last_name_page).to receive(:edit_path).and_return("edit_path") expect(task.path(record)).to eq("edit_path") end end describe "a completed task" do - let(:task) { described_class.new("name", pages: [ complete_page ]) } + let(:task) { described_class.new("name", pages: [ first_name_page ]) } + + before { record.first_name = "Mary" } it "is started and completed" do expect(task).to be_started(record) @@ -56,7 +62,22 @@ end it "returns the first page path" do - allow(complete_page).to receive(:edit_path).and_return("edit_path") + allow(first_name_page).to receive(:edit_path).and_return("edit_path") + expect(task.path(record)).to eq("edit_path") + end + end + + describe "a task with data on a later page but not the first page" do + let(:task) { described_class.new("name", pages: [ first_name_page, last_name_page ]) } + + before { record.last_name = "Smith" } + + it "is started" do + expect(task).to be_started(record) + end + + it "returns the first incomplete page path" do + allow(first_name_page).to receive(:edit_path).and_return("edit_path") expect(task.path(record)).to eq("edit_path") end end From cf9337fc076a33063a0dff4ea8b60b0db1d2f9c5 Mon Sep 17 00:00:00 2001 From: Roger Lam Date: Tue, 26 May 2026 13:31:50 -0700 Subject: [PATCH 2/6] Rename tests --- spec/models/strata/flows/task_spec.rb | 73 ++++++++++++++------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/spec/models/strata/flows/task_spec.rb b/spec/models/strata/flows/task_spec.rb index 2891da85..dfc7c005 100644 --- a/spec/models/strata/flows/task_spec.rb +++ b/spec/models/strata/flows/task_spec.rb @@ -21,64 +21,69 @@ let(:last_name_page) { Strata::Flows::QuestionPage.new("last_name") } let(:record) { TestModel.new } - describe "an unstarted task" do - let(:task) { described_class.new("name", pages: [ first_name_page ]) } + describe "#started?" do + let(:task) { described_class.new("name", pages: [ first_name_page, last_name_page ]) } - it "is not started or completed" do + it "is false when no pages have data" do expect(task).not_to be_started(record) - expect(task).not_to be_completed(record) end - it "returns the first page path" do - allow(first_name_page).to receive(:edit_path).and_return("edit_path") - expect(task.path(record)).to eq("edit_path") + it "is true when the first page has data" do + record.first_name = "Mary" + expect(task).to be_started(record) + end + + it "is true when only a later page has data" do + record.last_name = "Smith" + expect(task).to be_started(record) end end - describe "a started task" do + describe "#completed?" do let(:task) { described_class.new("name", pages: [ first_name_page, last_name_page ]) } - before { record.first_name = "Mary" } + it "is false when no pages have data" do + expect(task).not_to be_completed(record) + end - it "is started but not completed" do - expect(task).to be_started(record) + it "is false when only some pages are complete" do + record.first_name = "Mary" expect(task).not_to be_completed(record) end - it "returns the first incomplete page path" do - allow(last_name_page).to receive(:edit_path).and_return("edit_path") - expect(task.path(record)).to eq("edit_path") + it "is true when all pages are complete" do + record.first_name = "Mary" + record.last_name = "Smith" + expect(task).to be_completed(record) end end - describe "a completed task" do - let(:task) { described_class.new("name", pages: [ first_name_page ]) } - - before { record.first_name = "Mary" } + describe "#path" do + let(:task) { described_class.new("name", pages: [ first_name_page, last_name_page ]) } - it "is started and completed" do - expect(task).to be_started(record) - expect(task).to be_completed(record) + before do + allow(first_name_page).to receive(:edit_path).and_return("first_name_path") + allow(last_name_page).to receive(:edit_path).and_return("last_name_path") end - it "returns the first page path" do - allow(first_name_page).to receive(:edit_path).and_return("edit_path") - expect(task.path(record)).to eq("edit_path") + it "returns the first page path when no pages have data" do + expect(task.path(record)).to eq("first_name_path") end - end - - describe "a task with data on a later page but not the first page" do - let(:task) { described_class.new("name", pages: [ first_name_page, last_name_page ]) } - before { record.last_name = "Smith" } + it "returns the first incomplete page path when in progress" do + record.first_name = "Mary" + expect(task.path(record)).to eq("last_name_path") + end - it "is started" do - expect(task).to be_started(record) + it "returns the first incomplete page path when only a later page has data" do + record.last_name = "Smith" + expect(task.path(record)).to eq("first_name_path") end - it "returns the first incomplete page path" do - allow(first_name_page).to receive(:edit_path).and_return("edit_path") - expect(task.path(record)).to eq("edit_path") + it "returns the first page path when all pages are complete" do + record.first_name = "Mary" + record.last_name = "Smith" + expect(task.path(record)).to eq("first_name_path") end end From 7dd09b82ec5cf14e0a38bba06b825b7e5310238c Mon Sep 17 00:00:00 2001 From: Roger Lam Date: Tue, 26 May 2026 13:53:49 -0700 Subject: [PATCH 3/6] Add test for untouched optional field --- spec/models/strata/flows/task_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/models/strata/flows/task_spec.rb b/spec/models/strata/flows/task_spec.rb index dfc7c005..10205381 100644 --- a/spec/models/strata/flows/task_spec.rb +++ b/spec/models/strata/flows/task_spec.rb @@ -10,6 +10,7 @@ attribute :first_name, :string attribute :last_name, :string + attribute :middle_name, :string validates :first_name, presence: true, on: :first_name validates :last_name, presence: true, on: :last_name end @@ -37,6 +38,14 @@ record.last_name = "Smith" expect(task).to be_started(record) end + + it "is false when a page has only an untouched optional field" do + optional_page = Strata::Flows::QuestionPage.new("middle_name") + task = described_class.new("name", pages: [ optional_page ]) + + expect(optional_page).to be_completed(record) + expect(task).not_to be_started(record) + end end describe "#completed?" do From ce050486acf3c6e355e730f4f721ae91ff143c1c Mon Sep 17 00:00:00 2001 From: Roger Lam Date: Tue, 26 May 2026 14:37:16 -0700 Subject: [PATCH 4/6] fix bug when using accepts_nested_attributes_for --- app/models/strata/flows/question_page.rb | 9 ++++- .../models/strata/flows/question_page_spec.rb | 40 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/models/strata/flows/question_page.rb b/app/models/strata/flows/question_page.rb index 09ebbedb..84f65068 100644 --- a/app/models/strata/flows/question_page.rb +++ b/app/models/strata/flows/question_page.rb @@ -25,7 +25,14 @@ def completed?(record) def started?(record) @fields.any? do |field| field_names = field.is_a?(Hash) ? field.keys : [ field ] - field_names.any? { |name| record.public_send(name).present? } + field_names.any? do |name| + # Fields for accepts_nested_attributes_for associations are declared as + # :foo_attributes (matching the foo_attributes= setter used by form + # params), but only the bare association reader (record.foo) exists. + # Strip the suffix so we probe the actual association. + attribute_name = name.to_s.delete_suffix("_attributes") + record.public_send(attribute_name).present? + end end end diff --git a/spec/models/strata/flows/question_page_spec.rb b/spec/models/strata/flows/question_page_spec.rb index efdbf0c8..3d375304 100644 --- a/spec/models/strata/flows/question_page_spec.rb +++ b/spec/models/strata/flows/question_page_spec.rb @@ -95,6 +95,46 @@ end end + describe "#started? with a nested-attributes field" do + before do + child_class = Class.new(ActiveRecord::Base) do + self.table_name = "test_application_forms" + end + stub_const("NestedAttrsForm", child_class) + + parent_class = Class.new(ActiveRecord::Base) do + self.table_name = "users" + has_many :forms, class_name: "NestedAttrsForm", foreign_key: :user_id + accepts_nested_attributes_for :forms + end + stub_const("NestedAttrsUser", parent_class) + end + + let(:record) { NestedAttrsUser.new(first_name: "Alice", last_name: "Smith") } + + context "with the hash form" do + let(:page) { described_class.new("forms", fields: [ forms_attributes: [ :test_string ] ]) } + + it "is not started when no nested records have been assigned" do + expect(page).not_to be_started(record) + end + + it "is started after nested attributes are assigned" do + record.forms_attributes = [ { test_string: "hello" } ] + expect(page).to be_started(record) + end + end + + context "with the symbol form" do + let(:page) { described_class.new("forms", fields: [ :forms_attributes ]) } + + it "is started after nested attributes are assigned" do + record.forms_attributes = [ { test_string: "hello" } ] + expect(page).to be_started(record) + end + end + end + describe "#started? with multiple fields" do before do multi_field_class = Class.new do From f5a513eaabc8e3bb71baff96503dc504183784f0 Mon Sep 17 00:00:00 2001 From: lamroger-nava <164910391+lamroger-nava@users.noreply.github.com> Date: Tue, 26 May 2026 15:31:10 -0700 Subject: [PATCH 5/6] More safely access attributes started? calls record.public_send(attribute_name) unconditionally. Flows can define fields that don't exist on the record (e.g., :flow_only_field, :resume, or :income_records_attributes in spec/dummy/app/flows/test_application_form_flow.rb), which will raise NoMethodError and break task list rendering. Consider guarding with respond_to? (and only stripping _attributes when needed), treating unknown fields as "not started" instead of crashing. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- app/models/strata/flows/question_page.rb | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/app/models/strata/flows/question_page.rb b/app/models/strata/flows/question_page.rb index 84f65068..54718d0a 100644 --- a/app/models/strata/flows/question_page.rb +++ b/app/models/strata/flows/question_page.rb @@ -26,16 +26,26 @@ def started?(record) @fields.any? do |field| field_names = field.is_a?(Hash) ? field.keys : [ field ] field_names.any? do |name| - # Fields for accepts_nested_attributes_for associations are declared as - # :foo_attributes (matching the foo_attributes= setter used by form - # params), but only the bare association reader (record.foo) exists. - # Strip the suffix so we probe the actual association. - attribute_name = name.to_s.delete_suffix("_attributes") - record.public_send(attribute_name).present? + attribute_name = started_field_name(record, name) + attribute_name && record.public_send(attribute_name).present? end end end + def started_field_name(record, name) + attribute_name = name.to_s + return attribute_name if record.respond_to?(attribute_name) + + # Fields for accepts_nested_attributes_for associations are declared as + # :foo_attributes (matching the foo_attributes= setter used by form + # params), but only the bare association reader (record.foo) exists. + # Only strip the suffix when probing a nested-attributes field. + return unless attribute_name.end_with?("_attributes") + + association_name = attribute_name.delete_suffix("_attributes") + association_name if record.respond_to?(association_name) + end + def edit_pathname "edit_#{@name}" end From 665d9d67b7c285f0aee9482f71b0b670d5bf8fec Mon Sep 17 00:00:00 2001 From: Roger Lam Date: Tue, 26 May 2026 15:50:30 -0700 Subject: [PATCH 6/6] Account for boolean fields --- app/models/strata/flows/question_page.rb | 5 +- .../models/strata/flows/question_page_spec.rb | 73 +++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/app/models/strata/flows/question_page.rb b/app/models/strata/flows/question_page.rb index 54718d0a..b4a76df4 100644 --- a/app/models/strata/flows/question_page.rb +++ b/app/models/strata/flows/question_page.rb @@ -27,7 +27,10 @@ def started?(record) field_names = field.is_a?(Hash) ? field.keys : [ field ] field_names.any? do |name| attribute_name = started_field_name(record, name) - attribute_name && record.public_send(attribute_name).present? + value = record.public_send(attribute_name) if attribute_name + # An explicit boolean false is a real user-supplied answer (e.g. a "No" + # on a yes/no question), but `false.present?` is falsy in Rails. + value == false || value.present? end end end diff --git a/spec/models/strata/flows/question_page_spec.rb b/spec/models/strata/flows/question_page_spec.rb index 3d375304..bfe5d031 100644 --- a/spec/models/strata/flows/question_page_spec.rb +++ b/spec/models/strata/flows/question_page_spec.rb @@ -70,6 +70,79 @@ end end + describe "#started? with a boolean field" do + before do + bool_class = Class.new do + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :consents, :boolean + end + + stub_const("BooleanModel", bool_class) + end + + let(:record) { BooleanModel.new } + let(:page) { described_class.new("consents") } + + it "is not started when the field is nil" do + expect(page).not_to be_started(record) + end + + it "is started when the field is true" do + record.consents = true + expect(page).to be_started(record) + end + + it "is started when the field is explicitly false" do + record.consents = false + expect(page).to be_started(record) + end + + context "when mixed with a string field that is blank" do + before do + mixed_class = Class.new do + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :consents, :boolean + attribute :note, :string + end + + stub_const("MixedBooleanModel", mixed_class) + end + + let(:record) { MixedBooleanModel.new } + let(:page) { described_class.new("preferences", fields: [ :note, :consents ]) } + + it "is started when only the boolean field is explicitly false" do + record.consents = false + expect(page).to be_started(record) + end + end + end + + describe "#started? when a field is not defined on the record" do + let(:page) { described_class.new("ghost", fields: [ :nonexistent_field ]) } + + it "does not raise" do + expect { page.started?(record) }.not_to raise_error + end + + it "is not started" do + expect(page).not_to be_started(record) + end + + context "when mixed with a defined field that has a value" do + let(:page) { described_class.new("mixed", fields: [ :nonexistent_field, :first_name ]) } + + it "is started" do + record.first_name = "Mary" + expect(page).to be_started(record) + end + end + end + describe "#started? with a multi-parameter field" do before do test_model_class = Class.new(ActiveRecord::Base) do