Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(934) Fix the rates step getting skipped when going back #9971

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
module Workflow
class Step < Data.define(:name, :show_action, :update_action)
class Step < Data.define(:name, :show_action, :update_action, :included_in_create_journey)
SUBSCHEMA_PREFIX = "embedded_".freeze

ALL = [
Step.new(:edit_draft, :edit_draft, :update_draft),
Step.new(:review_links, :review_links, :redirect_to_next_step),
Step.new(:internal_note, :internal_note, :update_internal_note),
Step.new(:change_note, :change_note, :update_change_note),
Step.new(:schedule_publishing, :schedule_publishing, :validate_schedule),
Step.new(:review, :review, :validate_review_page),
Step.new(:confirmation, :confirmation, nil),
Step.new(:edit_draft, :edit_draft, :update_draft, true),
Step.new(:review_links, :review_links, :redirect_to_next_step, false),
Step.new(:internal_note, :internal_note, :update_internal_note, false),
Step.new(:change_note, :change_note, :update_change_note, false),
Step.new(:schedule_publishing, :schedule_publishing, :validate_schedule, false),
Step.new(:review, :review, :validate_review_page, true),
Step.new(:confirmation, :confirmation, nil, true),
].freeze

def is_subschema?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,9 @@ def confirmation
end

def back_path
step = is_reviewing_new_block? ? :edit_draft : previous_step.name
return nil unless step

content_block_manager.content_block_manager_content_block_workflow_path(
@content_block_edition,
step:,
step: previous_step.name,
)
end
included do
Expand All @@ -83,10 +80,6 @@ def back_path

private

def is_reviewing_new_block?
current_step.name == :review && @content_block_edition.document.is_new_block?
end

def embedded_objects(subschema_name)
@subschema = @schema.subschema(subschema_name)
@step_name = current_step.name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ module Workflow::Steps

def steps
@steps ||= if @schema.subschemas.any?
standard_steps = Workflow::Step::ALL.map(&:dup)
standard_steps = all_steps.map(&:dup)
extra_steps = @schema.subschemas.map do |subschema|
Workflow::Step.new(
"#{Workflow::Step::SUBSCHEMA_PREFIX}#{subschema.id}".to_sym,
"#{Workflow::Step::SUBSCHEMA_PREFIX}#{subschema.id}".to_sym,
:redirect_to_next_subschema_or_continue,
:redirect_to_next_step,
true,
)
end
standard_steps.insert(1, extra_steps).flatten!
else
Workflow::Step::ALL
all_steps
end
end

Expand All @@ -35,6 +36,14 @@ def next_step

private

def all_steps
if @content_block_edition.document.is_new_block?
Workflow::Step::ALL.select { |s| s.included_in_create_journey == true }
else
Workflow::Step::ALL
end
end

def initialize_edition_and_schema
@content_block_edition = ContentBlockManager::ContentBlock::Edition.find(params[:id])
@schema = ContentBlockManager::ContentBlock::Schema.find_by_block_type(@content_block_edition.document.block_type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,14 @@ def update_draft
)
@content_block_edition.save!

if @content_block_edition.document.is_new_block?
redirect_to content_block_manager.content_block_manager_content_block_workflow_path(
id: @content_block_edition.id,
step: :review,
)
else
redirect_to_next_step
end
redirect_to_next_step
rescue ActiveRecord::RecordInvalid
@schema = ContentBlockManager::ContentBlock::Schema.find_by_block_type(@content_block_edition.document.block_type)
@form = ContentBlockManager::ContentBlock::EditionForm::Edit.new(content_block_edition: @content_block_edition, schema: @schema)

render :edit_draft
end

def redirect_to_next_subschema_or_continue
@content_block_edition = ContentBlockManager::ContentBlock::Edition.find(params[:id])
if @content_block_edition.document.is_new_block?
if next_step.is_subschema?
redirect_to_next_step
else
redirect_to content_block_manager.content_block_manager_content_block_workflow_path(
id: @content_block_edition.id,
step: :review,
)
end
else
redirect_to_next_step
end
end

def validate_schedule
@content_block_edition = ContentBlockManager::ContentBlock::Edition.find(params[:id])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,15 @@ Feature: Create a content object
When I save and continue
And I review and confirm my answers are correct
Then I should be taken to the confirmation page for a new "pension"

Scenario: GDS editor clicks back and is taken back to rates
When I visit the Content Block Manager home page
And I click to create an object
And I click on the "pension" schema
When I complete the form with the following fields:
| title | description | organisation | instructions_to_publishers |
| my basic pension | this is basic | Ministry of Example | this is important |
And I click the back link
And I click save
Then I should be on the "embedded_rates" step

Original file line number Diff line number Diff line change
Expand Up @@ -509,3 +509,7 @@
I18n.t("activerecord.errors.models.content_block_manager/content_block/edition.invalid", attribute: attribute.humanize),
)
end

And(/^I click the back link$/) do
click_on "Back"
end
Original file line number Diff line number Diff line change
Expand Up @@ -31,47 +31,19 @@ class Workflow::ShowMethodsTest < ActionDispatch::IntegrationTest

describe "#back_path" do
let(:content_block_edition) { build_stubbed(:content_block_edition) }
let(:stub_document) { stub(:document, is_new_block?: is_new_block) }

before do
content_block_edition.stubs(:document).returns(stub_document)
end

describe "#back_path" do
describe "when editing an existing block" do
let(:is_new_block) { false }

it "returns the name of the next step" do
current_step_name = "my_step"
expected_step_name = "something"

current_step = mock("Workflow::Step", name: current_step_name)
previous_step = mock("Workflow::Step", name: expected_step_name)

test_class = ShowMethodsTestClass.new(current_step:, previous_step:, content_block_edition:)

assert_equal test_class.back_path, content_block_manager.content_block_manager_content_block_workflow_path(
content_block_edition,
step: expected_step_name,
)
end
end
end

describe "when editing an existing block" do
let(:is_new_block) { true }
it "returns the name of the previous step" do
expected_step_name = "something"

it "returns the edit draft step" do
current_step = mock("Workflow::Step", name: :review)
previous_step = mock("Workflow::Step")
current_step = mock("Workflow::Step")
previous_step = mock("Workflow::Step", name: expected_step_name)

test_class = ShowMethodsTestClass.new(current_step:, previous_step:, content_block_edition:)
test_class = ShowMethodsTestClass.new(current_step:, previous_step:, content_block_edition:)

assert_equal test_class.back_path, content_block_manager.content_block_manager_content_block_workflow_path(
content_block_edition,
step: :edit_draft,
)
end
assert_equal test_class.back_path, content_block_manager.content_block_manager_content_block_workflow_path(
content_block_edition,
step: expected_step_name,
)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,51 @@ class Workflow::StepsTest < ActionDispatch::IntegrationTest
end
end

describe "when the content block is new" do
let(:step) { "something" }

before do
content_block_document.expects(:is_new_block?).returns(true)
end

it "removes steps not included in the create journey" do
assert_equal workflow.steps, [
Workflow::Step.new(:edit_draft, :edit_draft, :update_draft, true),
Workflow::Step.new(:review, :review, :validate_review_page, true),
Workflow::Step.new(:confirmation, :confirmation, nil, true),
].flatten
end

describe "#next_step" do
[
%i[edit_draft review],
%i[review confirmation],
].each do |current_step, expected_step|
describe "when current_step is #{current_step}" do
let(:step) { current_step }

it "returns #{expected_step} step" do
assert_equal workflow.next_step.name, expected_step
end
end
end
end

describe "#previous_step" do
[
%i[review edit_draft],
].each do |current_step, expected_step|
describe "when current_step is #{current_step}" do
let(:step) { current_step }

it "returns #{expected_step} step" do
assert_equal workflow.previous_step.name, expected_step
end
end
end
end
end

describe "when a schema has subschemas" do
let(:subschemas) do
[
Expand All @@ -101,8 +146,8 @@ class Workflow::StepsTest < ActionDispatch::IntegrationTest
it "inserts the subschemas into the flow" do
assert_equal workflow.steps, [
Workflow::Step::ALL[0],
Workflow::Step.new(:embedded_something, :embedded_something, :redirect_to_next_subschema_or_continue),
Workflow::Step.new(:embedded_something_else, :embedded_something_else, :redirect_to_next_subschema_or_continue),
Workflow::Step.new(:embedded_something, :embedded_something, :redirect_to_next_step, true),
Workflow::Step.new(:embedded_something_else, :embedded_something_else, :redirect_to_next_step, true),
Workflow::Step::ALL[1..],
].flatten
end
Expand Down Expand Up @@ -148,5 +193,54 @@ class Workflow::StepsTest < ActionDispatch::IntegrationTest
end
end
end

describe "and the content block is new" do
before do
content_block_document.expects(:is_new_block?).returns(true)
end

it "removes steps not included in the create journey" do
assert_equal workflow.steps, [
Workflow::Step.new(:edit_draft, :edit_draft, :update_draft, true),
Workflow::Step.new(:embedded_something, :embedded_something, :redirect_to_next_step, true),
Workflow::Step.new(:embedded_something_else, :embedded_something_else, :redirect_to_next_step, true),
Workflow::Step.new(:review, :review, :validate_review_page, true),
Workflow::Step.new(:confirmation, :confirmation, nil, true),
].flatten
end

describe "#next_step" do
[
%i[edit_draft embedded_something],
%i[embedded_something embedded_something_else],
%i[embedded_something_else review],
%i[review confirmation],
].each do |current_step, expected_step|
describe "when current_step is #{current_step}" do
let(:step) { current_step }

it "returns #{expected_step} step" do
assert_equal workflow.next_step.name, expected_step
end
end
end
end

describe "#previous_step" do
[
%i[embedded_something edit_draft],
%i[embedded_something_else embedded_something],
%i[review embedded_something_else],
].each do |current_step, expected_step|
describe "when current_step is #{current_step}" do
let(:step) { current_step }

it "returns #{expected_step} step" do
assert_equal workflow.previous_step.name, expected_step
end
end
end
end
end
end
end