Skip to content

Commit

Permalink
Refactor TaskStatusService
Browse files Browse the repository at this point in the history
Make each status method private, and rename missing_sections to
incomplete_tasks to be clearer about what the method does
  • Loading branch information
jamie-o-wilkinson committed Sep 18, 2023
1 parent fd0fb92 commit 7335175
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 55 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v1/forms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def make_live
form.make_live!
render json: { success: true }.to_json, status: :ok
else
render json: form.missing_sections.to_json, status: :forbidden
render json: form.incomplete_tasks.to_json, status: :forbidden
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def name=(val)
end

def as_json(options = {})
options[:methods] ||= %i[live_at start_page has_draft_version has_live_version has_routing_errors ready_for_live missing_sections task_statuses]
options[:methods] ||= %i[live_at start_page has_draft_version has_live_version has_routing_errors ready_for_live incomplete_tasks task_statuses]
super(options)
end

Expand Down Expand Up @@ -83,7 +83,7 @@ def ready_for_live
task_status_service.mandatory_tasks_completed?
end

delegate :missing_sections, to: :task_status_service
delegate :incomplete_tasks, to: :task_status_service

delegate :task_statuses, to: :task_status_service

Expand Down
54 changes: 26 additions & 28 deletions app/service/task_status_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,31 @@ def initialize(form:)
@form = form
end

def mandatory_tasks_completed?
incomplete_tasks.empty?
end

def incomplete_tasks
{ missing_pages: pages_status,
missing_what_happens_next: what_happens_next_status,
missing_privacy_policy_url: privacy_policy_status,
missing_contact_details: support_contact_details_status }.reject { |_k, v| v == :completed }.map { |k, _v| k }
end

def task_statuses
{
name_status:,
pages_status:,
declaration_status:,
what_happens_next_status:,
privacy_policy_status:,
support_contact_details_status:,
make_live_status:,
}
end

private

def name_status
:completed
end
Expand Down Expand Up @@ -53,35 +78,8 @@ def support_contact_details_status

def make_live_status
if @form.has_draft_version
if mandatory_tasks_completed?
return :not_started
else
return :cannot_start
end
return mandatory_tasks_completed? ? :not_started : :cannot_start
end
return :completed if @form.has_live_version
end

def mandatory_tasks_completed?
missing_sections.empty?
end

def missing_sections
{ missing_pages: pages_status,
missing_what_happens_next: what_happens_next_status,
missing_privacy_policy_url: privacy_policy_status,
missing_contact_details: support_contact_details_status }.reject { |_k, v| v == :completed }.map { |k, _v| k }
end

def task_statuses
{
name_status:,
pages_status:,
declaration_status:,
what_happens_next_status:,
privacy_policy_status:,
support_contact_details_status:,
make_live_status:,
}
end
end
6 changes: 3 additions & 3 deletions spec/models/form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -312,20 +312,20 @@
end
end

describe "#missing_sections" do
describe "#incomplete_tasks" do
context "when a form is complete and ready to be made live" do
let(:completed_form) { build :form, :live }

it "returns no missing sections" do
expect(completed_form.missing_sections).to be_empty
expect(completed_form.incomplete_tasks).to be_empty
end
end

context "when a form is incomplete and should still be in draft state" do
let(:new_form) { build :form, :new_form }

it "returns a set of keys related to missing fields" do
expect(new_form.missing_sections).to match_array(%i[missing_pages missing_privacy_policy_url missing_contact_details missing_what_happens_next])
expect(new_form.incomplete_tasks).to match_array(%i[missing_pages missing_privacy_policy_url missing_contact_details missing_what_happens_next])
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/request/api/v1/forms_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@
created_at: form1.created_at.as_json,
updated_at: form1.updated_at.as_json,
has_routing_errors: false,
missing_sections: %w[missing_pages missing_what_happens_next missing_privacy_policy_url missing_contact_details],
incomplete_tasks: %w[missing_pages missing_what_happens_next missing_privacy_policy_url missing_contact_details],
ready_for_live: false,
task_statuses: { declaration_status: "not_started", make_live_status: "cannot_start", name_status: "completed", pages_status: "not_started", privacy_policy_status: "not_started", support_contact_details_status: "not_started", what_happens_next_status: "not_started" },
)
Expand Down
40 changes: 20 additions & 20 deletions spec/service/task_status_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
let(:form) { build(:form, :new_form) }

it "returns the correct default value" do
expect(task_status_service.name_status).to eq :completed
expect(task_status_service.task_statuses[:name_status]).to eq :completed
end
end

Expand All @@ -21,22 +21,22 @@
let(:form) { build(:form, :new_form) }

it "returns the correct default value" do
expect(task_status_service.pages_status).to eq :not_started
expect(task_status_service.task_statuses[:pages_status]).to eq :not_started
end
end

context "with a form which has pages" do
let(:form) { build(:form, :new_form, :with_pages, question_section_completed: false) }

it "returns the in progress status" do
expect(task_status_service.pages_status).to eq :in_progress
expect(task_status_service.task_statuses[:pages_status]).to eq :in_progress
end

context "and questions marked completed" do
let(:form) { build(:form, :new_form, :with_pages, question_section_completed: true) }

it "returns the completed status" do
expect(task_status_service.pages_status).to eq :completed
expect(task_status_service.task_statuses[:pages_status]).to eq :completed
end
end
end
Expand All @@ -47,31 +47,31 @@
let(:form) { build(:form, :new_form) }

it "returns the correct default value" do
expect(task_status_service.declaration_status).to eq :not_started
expect(task_status_service.task_statuses[:declaration_status]).to eq :not_started
end
end

context "with a form which has no declaration content and is marked incomplete" do
let(:form) { build(:form, declaration_section_completed: false) }

it "returns the not started status" do
expect(task_status_service.declaration_status).to eq :not_started
expect(task_status_service.task_statuses[:declaration_status]).to eq :not_started
end
end

context "with a form which has declaration content and is marked incomplete" do
let(:form) { build(:form, declaration_text: "I understand the implications", declaration_section_completed: false) }

it "returns the in progress status" do
expect(task_status_service.declaration_status).to eq :in_progress
expect(task_status_service.task_statuses[:declaration_status]).to eq :in_progress
end
end

context "with a form which has a declaration marked complete" do
let(:form) { build(:form, declaration_section_completed: true) }

it "returns the completed status" do
expect(task_status_service.declaration_status).to eq :completed
expect(task_status_service.task_statuses[:declaration_status]).to eq :completed
end
end
end
Expand All @@ -81,15 +81,15 @@
let(:form) { build(:form, :new_form) }

it "returns the correct default value" do
expect(task_status_service.what_happens_next_status).to eq :not_started
expect(task_status_service.task_statuses[:what_happens_next_status]).to eq :not_started
end
end

context "with a form which has a what happens next section" do
let(:form) { build(:form, :new_form, what_happens_next_text: "We usually respond to applications within 10 working days.") }

it "returns the in progress status" do
expect(task_status_service.what_happens_next_status).to eq :completed
expect(task_status_service.task_statuses[:what_happens_next_status]).to eq :completed
end
end
end
Expand All @@ -99,15 +99,15 @@
let(:form) { build(:form, :new_form) }

it "returns the correct default value" do
expect(task_status_service.privacy_policy_status).to eq :not_started
expect(task_status_service.task_statuses[:privacy_policy_status]).to eq :not_started
end
end

context "with a form which has a privacy policy section" do
let(:form) { build(:form, :new_form, privacy_policy_url: Faker::Internet.url(host: "gov.uk")) }

it "returns the in progress status" do
expect(task_status_service.privacy_policy_status).to eq :completed
expect(task_status_service.task_statuses[:privacy_policy_status]).to eq :completed
end
end
end
Expand All @@ -117,15 +117,15 @@
let(:form) { build(:form, :new_form) }

it "returns the correct default value" do
expect(task_status_service.support_contact_details_status).to eq :not_started
expect(task_status_service.task_statuses[:support_contact_details_status]).to eq :not_started
end
end

context "with a form which has contact details set" do
let(:form) { build(:form, :new_form, :with_support) }

it "returns the in progress status" do
expect(task_status_service.support_contact_details_status).to eq :completed
expect(task_status_service.task_statuses[:support_contact_details_status]).to eq :completed
end
end
end
Expand All @@ -135,23 +135,23 @@
let(:form) { build(:form, :new_form) }

it "returns the correct default value" do
expect(task_status_service.make_live_status).to eq :cannot_start
expect(task_status_service.task_statuses[:make_live_status]).to eq :cannot_start
end
end

context "with a form which is ready to go live" do
let(:form) { build(:form, :ready_for_live) }

it "returns the not started status" do
expect(task_status_service.make_live_status).to eq :not_started
expect(task_status_service.task_statuses[:make_live_status]).to eq :not_started
end
end

context "with a live form" do
let(:form) { create(:form, :live) }

it "returns the completed status" do
expect(task_status_service.make_live_status).to eq :completed
expect(task_status_service.task_statuses[:make_live_status]).to eq :completed
end
end
end
Expand All @@ -175,20 +175,20 @@
end
end

describe "#missing_sections" do
describe "#incomplete_tasks" do
context "when mandatory tasks are complete" do
let(:form) { build :form, :live }

it "returns no missing sections" do
expect(task_status_service.missing_sections).to be_empty
expect(task_status_service.incomplete_tasks).to be_empty
end
end

context "when a form is incomplete and should still be in draft state" do
let(:form) { build :form, :new_form }

it "returns a set of keys related to missing fields" do
expect(task_status_service.missing_sections).to match_array(%i[missing_pages missing_privacy_policy_url missing_contact_details missing_what_happens_next])
expect(task_status_service.incomplete_tasks).to match_array(%i[missing_pages missing_privacy_policy_url missing_contact_details missing_what_happens_next])
end
end
end
Expand Down

0 comments on commit 7335175

Please sign in to comment.