diff --git a/.env.example b/.env.example index c3c956334..94596a0da 100644 --- a/.env.example +++ b/.env.example @@ -1,2 +1,2 @@ -DB_USERNAME:postgres-user -DB_PASSWORD:postgres-password +DB_USERNAME=postgres-user +DB_PASSWORD=postgres-password diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index e164be8dc..ee38336ce 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -77,6 +77,7 @@ jobs: name: Deploy runs-on: ubuntu-latest environment: 'Beta - Production' + if: github.ref == 'refs/heads/main' needs: - test timeout-minutes: 30 diff --git a/Gemfile.lock b/Gemfile.lock index 46a7f1ad1..56d0f8624 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,7 +26,7 @@ GIT GIT remote: https://github.com/rspec/rspec-rails.git - revision: d2a9e0e1b18d7d0d95b98dfa6b31eadd8d1b3985 + revision: 211d7d990e9762e229d8a86249b88c2a7604e8b0 branch: main specs: rspec-rails (5.1.0.pre) @@ -111,7 +111,7 @@ GEM public_suffix (>= 2.0.2, < 5.0) ast (2.4.2) bindex (0.8.1) - bootsnap (1.8.1) + bootsnap (1.9.1) msgpack (~> 1.0) builder (3.2.4) byebug (11.1.3) @@ -143,11 +143,11 @@ GEM ffi (1.15.4) globalid (0.5.2) activesupport (>= 5.0) - govuk-components (2.1.0) + govuk-components (2.1.1) activemodel (>= 6.0) railties (>= 6.0) view_component (~> 2.39.0) - govuk_design_system_formbuilder (2.7.3) + govuk_design_system_formbuilder (2.7.4) actionview (>= 6.0) activemodel (>= 6.0) activesupport (>= 6.0) @@ -169,7 +169,7 @@ GEM nokogiri (>= 1.5.9) mail (2.7.1) mini_mime (>= 0.1.1) - marcel (1.0.1) + marcel (1.0.2) method_source (1.0.0) mini_mime (1.1.1) minitest (5.14.4) @@ -183,7 +183,7 @@ GEM childprocess (>= 0.6.3, < 5) iniparse (~> 1.4) rexml (~> 3.2) - parallel (1.20.1) + parallel (1.21.0) parser (3.0.2.0) ast (~> 2.4.1) pg (1.2.3) @@ -194,7 +194,7 @@ GEM byebug (~> 11.0) pry (~> 0.13.0) public_suffix (4.0.6) - puma (5.4.0) + puma (5.5.0) nio4r (~> 2.0) racc (1.5.2) rack (2.2.3) @@ -237,33 +237,33 @@ GEM ffi (~> 1.0) regexp_parser (2.1.1) rexml (3.2.5) - rubocop (1.15.0) + rubocop (1.21.0) parallel (~> 1.10) parser (>= 3.0.0.0) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml - rubocop-ast (>= 1.5.0, < 2.0) + rubocop-ast (>= 1.9.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.6.0) + rubocop-ast (1.11.0) parser (>= 3.0.1.1) - rubocop-govuk (4.0.0) - rubocop (~> 1.15.0) - rubocop-ast (~> 1.6.0) - rubocop-rails (~> 2.10.0) - rubocop-rake (= 0.5.1) - rubocop-rspec (~> 2.3.0) + rubocop-govuk (4.1.0) + rubocop (= 1.21.0) + rubocop-ast (= 1.11.0) + rubocop-rails (= 2.12.2) + rubocop-rake (= 0.6.0) + rubocop-rspec (= 2.4.0) rubocop-performance (1.11.5) rubocop (>= 1.7.0, < 2.0) rubocop-ast (>= 0.4.0) - rubocop-rails (2.10.1) + rubocop-rails (2.12.2) activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 1.7.0, < 2.0) - rubocop-rake (0.5.1) - rubocop - rubocop-rspec (2.3.0) + rubocop-rake (0.6.0) + rubocop (~> 1.0) + rubocop-rspec (2.4.0) rubocop (~> 1.0) rubocop-ast (>= 1.1.0) ruby-progressbar (1.11.0) @@ -294,14 +294,14 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - stimulus-rails (0.4.2) + stimulus-rails (0.5.4) rails (>= 6.0.0) thor (1.1.0) - turbo-rails (0.7.11) + turbo-rails (0.7.14) rails (>= 6.0.0) tzinfo (2.0.4) concurrent-ruby (~> 1.0) - unicode-display_width (2.0.0) + unicode-display_width (2.1.0) view_component (2.39.0) activesupport (>= 5.0.0, < 8.0) method_source (~> 1.0) @@ -310,7 +310,7 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - webpacker (5.4.2) + webpacker (5.4.3) activesupport (>= 5.2) rack-proxy (>= 0.6.1) railties (>= 5.2) diff --git a/app/views/case_logs/_log_list.html.erb b/app/views/case_logs/_log_list.html.erb index efc434dd8..37d0dca14 100644 --- a/app/views/case_logs/_log_list.html.erb +++ b/app/views/case_logs/_log_list.html.erb @@ -15,8 +15,10 @@ <%= link_to log.id, case_log_path(log) %> + <%= log.postcode %> + <%= log.tenant_code %> <%= log.updated_at.strftime("%d %b %Y") %> diff --git a/app/views/case_logs/index.html.erb b/app/views/case_logs/index.html.erb index 061638259..35ae34399 100644 --- a/app/views/case_logs/index.html.erb +++ b/app/views/case_logs/index.html.erb @@ -6,9 +6,13 @@ <%= link_to "Create new log", case_logs_path, method: :post, class: "govuk-button" %> - <%= render partial: "log_list", locals: { case_logs: @in_progress_case_logs, title: "Logs you need to complete", date_title: "Last Changed" } %> + <% if @in_progress_case_logs.present? %> + <%= render partial: "log_list", locals: { case_logs: @in_progress_case_logs, title: "Logs you need to complete", date_title: "Last Changed" } %> + <% end %> - <%= render partial: "log_list", locals: { case_logs: @submitted_case_logs, title: "Logs you've submitted", date_title: "Date Submitted" } %> + <% if @submitted_case_logs.present? %> + <%= render partial: "log_list", locals: { case_logs: @submitted_case_logs, title: "Logs you've submitted", date_title: "Date Submitted" } %> + <% end %>

See all completed logs (<%= @submitted_case_logs.count %>)

diff --git a/app/views/form/questions/previous_housing_situation.html.erb b/app/views/form/questions/previous_housing_situation.html.erb index 9b6e3713d..70851a4b2 100644 --- a/app/views/form/questions/previous_housing_situation.html.erb +++ b/app/views/form/questions/previous_housing_situation.html.erb @@ -25,8 +25,10 @@ ] %> <% previous_question = Form.previous_question("previous_housing_situation") %> -<%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> - Back +<% content_for :before_content do %> + <%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> + Back + <% end %> <% end %> <%= turbo_frame_tag "case_log_form", target: "_top" do %> diff --git a/app/views/form/questions/tenant_age.html.erb b/app/views/form/questions/tenant_age.html.erb index 12236c363..aabcd39d9 100644 --- a/app/views/form/questions/tenant_age.html.erb +++ b/app/views/form/questions/tenant_age.html.erb @@ -1,6 +1,8 @@ <% previous_question = Form.previous_question("tenant_age") %> -<%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> - Back +<% content_for :before_content do %> + <%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> + Back + <% end %> <% end %> <%= turbo_frame_tag "case_log_form", target: "_top" do %> diff --git a/app/views/form/questions/tenant_code.html.erb b/app/views/form/questions/tenant_code.html.erb index eebb74aaa..d13eed35e 100644 --- a/app/views/form/questions/tenant_code.html.erb +++ b/app/views/form/questions/tenant_code.html.erb @@ -1,5 +1,11 @@ +<% previous_question = Form.previous_question("tenant_code") %> +<% content_for :before_content do %> + <%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> + Back + <% end %> +<% end %> + <%= turbo_frame_tag "case_log_form", target: "_top" do %> - <%= render GovukComponent::BackLinkComponent.new(href: "/case_logs/#{case_log_id}", text: 'Back') do %><% end %> <%= form_with action: '/case_logs', method: "next_question", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %> <%= f.govuk_text_field :tenant_code, hint: { text: "More detail" }, diff --git a/app/views/form/questions/tenant_gender.html.erb b/app/views/form/questions/tenant_gender.html.erb index 73fdb86f0..e01b7aa41 100644 --- a/app/views/form/questions/tenant_gender.html.erb +++ b/app/views/form/questions/tenant_gender.html.erb @@ -6,8 +6,10 @@ ] %> <% previous_question = Form.previous_question("tenant_gender") %> -<%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> - Back +<% content_for :before_content do %> + <%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> + Back + <% end %> <% end %> <%= turbo_frame_tag "case_log_form", target: "_top" do %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index fbe24ec3c..343307b55 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -1,7 +1,7 @@ - MHCLG CORE Data Collection + DLUHC CORE Data Collection <%= csrf_meta_tags %> <%= csp_meta_tag %> <%= tag :meta, name: 'viewport', content: 'width=device-width, initial-scale=1' %> @@ -49,7 +49,7 @@
- <%= link_to "Share Lettings and Sales for Social Housing in England Data with MHCLG", "/", class: "govuk-header__link govuk-header__link--service-name" %> + <%= link_to "Share Lettings and Sales for Social Housing in England Data with DLUHC", "/", class: "govuk-header__link govuk-header__link--service-name" %>
diff --git a/db/migrate/20210921121533_add_postcode_to_case_logs.rb b/db/migrate/20210921121533_add_postcode_to_case_logs.rb new file mode 100644 index 000000000..15f385c8f --- /dev/null +++ b/db/migrate/20210921121533_add_postcode_to_case_logs.rb @@ -0,0 +1,7 @@ +class AddPostcodeToCaseLogs < ActiveRecord::Migration[6.1] + def change + change_table :case_logs, bulk: true do |t| + t.column :postcode, :string + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 8ef9e2c72..7d92915dc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -57,6 +57,7 @@ ActiveRecord::Schema.define(version: 2021_09_22_103729) do t.integer "person_8_age" t.string "person_8_gender" t.string "person_8_economic" + t.string "postcode" end end diff --git a/doc/adr/adr-003.md b/doc/adr/adr-003.md new file mode 100644 index 000000000..2b363481b --- /dev/null +++ b/doc/adr/adr-003.md @@ -0,0 +1,19 @@ +### ADR - 003: Form Submission Flow + +Turbo Frames (https://github.com/hotwired/turbo-rails) for form pages/questions with data saved (but not necessarily fully validated) to Active Record model on each submit. + + +#### Impact on Performance + +Using Turbo Frames allows us to swap out just the question part of the page without needing full page refreshes as you go through the form and provides a "Single Page Application like" user experience. Each question still gets a unique URL that can be navigated to directly with the Case Log ID and the overall user experience is that form navigation feels faster. + +#### Impact on interrupted sessions + +We currently have a single Active Record model for Case Logs that contains all the question fields. Every time a question is submitted the answer will be saved in the Active Record model instance before the next frame is rendered. This model will need to be able to handle partial records and partial validation anyway since not all API users will have all the required data. Validation can occur based on the data already saved and/or once the form is finally submitted. Front end validation will still happen additionally as you go through the form to help make sure users don't get a long list of errors at the end. Using session data here and updating the model only once the form is completed would not seem to have any advantages over this approach. + +This means that when a user navigates away from the form or closes the tab etc, they can use the URL to navigate directly back to where they left off, or follow the form flow through again, and in both cases their submitted answers will still be there. + + +#### Impact on API + +The API will still expect to take a JSON describing the case log, instantiate the model with the given fields, and run validations as if it had been submitted. diff --git a/spec/factories/case_log.rb b/spec/factories/case_log.rb index 9380466df..e2f132c53 100644 --- a/spec/factories/case_log.rb +++ b/spec/factories/case_log.rb @@ -1,6 +1,17 @@ FactoryBot.define do factory :case_log do - id { 342_351 } - status { 0 } + sequence(:id) { |i| i } + trait :in_progress do + status { 0 } + tenant_code { "TH356" } + postcode { "SW2 6HI" } + end + trait :submitted do + status { 1 } + tenant_code { "BZ737" } + postcode { "NW1 7TY" } + end + created_at { Time.zone.now } + updated_at { Time.zone.now } end end diff --git a/spec/features/case_log_spec.rb b/spec/features/case_log_spec.rb index da82e50f0..f02326646 100644 --- a/spec/features/case_log_spec.rb +++ b/spec/features/case_log_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" RSpec.describe "Test Features" do - let!(:case_log) { FactoryBot.create(:case_log) } + let!(:case_log) { FactoryBot.create(:case_log, :in_progress) } let(:id) { case_log.id } let(:status) { case_log.status } pages = ['tenant_code', 'tenant_age', 'tenant_gender', 'tenant_ethnic_group', 'tenant_nationality', 'economic_status', 'other_household_members'] @@ -9,14 +9,14 @@ RSpec.describe "Test Features" do it "redirects to the task list for the new log" do visit("/case_logs") click_link("Create new log") - id = CaseLog.first.id + id = CaseLog.order(created_at: :desc).first.id expect(page).to have_content("Tasklist for log #{id}") end end describe "Viewing a log" do it "displays a tasklist header" do - visit("/case_logs/342351") + visit("/case_logs/#{id}") expect(page).to have_content("Tasklist for log #{id}") expect(page).to have_content("This submission is #{status}") end @@ -29,7 +29,7 @@ RSpec.describe "Test Features" do expect(page).to have_field("tenant-age-field") click_button("Save and continue") expect(page).to have_field("tenant-gender-0-field") - visit page.driver.request.env['HTTP_REFERER'] + visit page.driver.request.env["HTTP_REFERER"] expect(page).to have_field("tenant-age-field") end @@ -39,19 +39,19 @@ RSpec.describe "Test Features" do expect(page).to have_field("tenant-age-field") end end - end - describe "Back link directs correctly" do - it "go back to tasklist page from tenant code" do - visit("/case_logs/#{id}/tenant_code") - click_link(text: 'Back') - expect(page).to have_content("Tasklist for log #{id}") - end + describe "Back link directs correctly" do + it "go back to tasklist page from tenant code" do + visit("/case_logs/#{id}/tenant_code") + click_link(text: "Back") + expect(page).to have_content("Tasklist for log #{id}") + end - it "go back to tenant code page from tenant age page" do - visit("/case_logs/#{id}/tenant_age") - click_link(text: 'Back') - expect(page).to have_field("tenant-code-field") + it "go back to tenant code page from tenant age page" do + visit("/case_logs/#{id}/tenant_age") + click_link(text: "Back") + expect(page).to have_field("tenant-code-field") + end end end diff --git a/spec/features/test_spec.rb b/spec/features/test_spec.rb index 5fa8db2cc..a17336378 100644 --- a/spec/features/test_spec.rb +++ b/spec/features/test_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe "Test Features" do it "Displays the name of the app" do visit("/") - expect(page).to have_content("Share Lettings and Sales for Social Housing in England Data with MHCLG") + expect(page).to have_content("Share Lettings and Sales for Social Housing in England Data with DLUHC") end it "Links to the About page" do diff --git a/spec/views/case_log_index_view_spec.rb b/spec/views/case_log_index_view_spec.rb new file mode 100644 index 000000000..f99c82419 --- /dev/null +++ b/spec/views/case_log_index_view_spec.rb @@ -0,0 +1,42 @@ +require "rails_helper" +RSpec.describe "case_logs/index" do + let(:in_progress_log) { FactoryBot.build(:case_log, :in_progress) } + let(:submitted_log) { FactoryBot.build(:case_log, :submitted) } + + context "given an in progress log list" do + it "renders a table for in progress logs only" do + assign(:in_progress_case_logs, [in_progress_log]) + assign(:submitted_case_logs, []) + render + expect(rendered).to match(//) + expect(rendered).to match(/Logs you need to complete/) + expect(rendered).not_to match(/Logs you've submitted/) + expect(rendered).to match(in_progress_log.tenant_code) + expect(rendered).to match(in_progress_log.postcode) + end + end + + context "given a submitted log list" do + it "renders a table for in progress logs only" do + assign(:in_progress_case_logs, []) + assign(:submitted_case_logs, [submitted_log]) + render + expect(rendered).to match(/
/) + expect(rendered).to match(/Logs you've submitted/) + expect(rendered).not_to match(/Logs you need to complete/) + expect(rendered).to match(submitted_log.tenant_code) + expect(rendered).to match(submitted_log.postcode) + end + end + + context "given a submitted log list and an in_progress log list" do + it "renders two tables, one for each status" do + assign(:in_progress_case_logs, [in_progress_log]) + assign(:submitted_case_logs, [submitted_log]) + render + expect(rendered).to match(/
/) + expect(rendered).to match(/Logs you've submitted/) + expect(rendered).to match(/Logs you need to complete/) + end + end +end