Browse Source

merge conflicts fixed

pull/22/head
Matthew Phelan 3 years ago
parent
commit
f13e4e39f6
  1. 4
      .env.example
  2. 1
      .github/workflows/pipeline.yml
  3. 48
      Gemfile.lock
  4. 2
      app/views/case_logs/_log_list.html.erb
  5. 8
      app/views/case_logs/index.html.erb
  6. 6
      app/views/form/questions/previous_housing_situation.html.erb
  7. 6
      app/views/form/questions/tenant_age.html.erb
  8. 8
      app/views/form/questions/tenant_code.html.erb
  9. 6
      app/views/form/questions/tenant_gender.html.erb
  10. 4
      app/views/layouts/application.html.erb
  11. 7
      db/migrate/20210921121533_add_postcode_to_case_logs.rb
  12. 1
      db/schema.rb
  13. 19
      doc/adr/adr-003.md
  14. 15
      spec/factories/case_log.rb
  15. 30
      spec/features/case_log_spec.rb
  16. 2
      spec/features/test_spec.rb
  17. 42
      spec/views/case_log_index_view_spec.rb

4
.env.example

@ -1,2 +1,2 @@
DB_USERNAME:postgres-user DB_USERNAME=postgres-user
DB_PASSWORD:postgres-password DB_PASSWORD=postgres-password

1
.github/workflows/pipeline.yml

@ -77,6 +77,7 @@ jobs:
name: Deploy name: Deploy
runs-on: ubuntu-latest runs-on: ubuntu-latest
environment: 'Beta - Production' environment: 'Beta - Production'
if: github.ref == 'refs/heads/main'
needs: needs:
- test - test
timeout-minutes: 30 timeout-minutes: 30

48
Gemfile.lock

@ -26,7 +26,7 @@ GIT
GIT GIT
remote: https://github.com/rspec/rspec-rails.git remote: https://github.com/rspec/rspec-rails.git
revision: d2a9e0e1b18d7d0d95b98dfa6b31eadd8d1b3985 revision: 211d7d990e9762e229d8a86249b88c2a7604e8b0
branch: main branch: main
specs: specs:
rspec-rails (5.1.0.pre) rspec-rails (5.1.0.pre)
@ -111,7 +111,7 @@ GEM
public_suffix (>= 2.0.2, < 5.0) public_suffix (>= 2.0.2, < 5.0)
ast (2.4.2) ast (2.4.2)
bindex (0.8.1) bindex (0.8.1)
bootsnap (1.8.1) bootsnap (1.9.1)
msgpack (~> 1.0) msgpack (~> 1.0)
builder (3.2.4) builder (3.2.4)
byebug (11.1.3) byebug (11.1.3)
@ -143,11 +143,11 @@ GEM
ffi (1.15.4) ffi (1.15.4)
globalid (0.5.2) globalid (0.5.2)
activesupport (>= 5.0) activesupport (>= 5.0)
govuk-components (2.1.0) govuk-components (2.1.1)
activemodel (>= 6.0) activemodel (>= 6.0)
railties (>= 6.0) railties (>= 6.0)
view_component (~> 2.39.0) view_component (~> 2.39.0)
govuk_design_system_formbuilder (2.7.3) govuk_design_system_formbuilder (2.7.4)
actionview (>= 6.0) actionview (>= 6.0)
activemodel (>= 6.0) activemodel (>= 6.0)
activesupport (>= 6.0) activesupport (>= 6.0)
@ -169,7 +169,7 @@ GEM
nokogiri (>= 1.5.9) nokogiri (>= 1.5.9)
mail (2.7.1) mail (2.7.1)
mini_mime (>= 0.1.1) mini_mime (>= 0.1.1)
marcel (1.0.1) marcel (1.0.2)
method_source (1.0.0) method_source (1.0.0)
mini_mime (1.1.1) mini_mime (1.1.1)
minitest (5.14.4) minitest (5.14.4)
@ -183,7 +183,7 @@ GEM
childprocess (>= 0.6.3, < 5) childprocess (>= 0.6.3, < 5)
iniparse (~> 1.4) iniparse (~> 1.4)
rexml (~> 3.2) rexml (~> 3.2)
parallel (1.20.1) parallel (1.21.0)
parser (3.0.2.0) parser (3.0.2.0)
ast (~> 2.4.1) ast (~> 2.4.1)
pg (1.2.3) pg (1.2.3)
@ -194,7 +194,7 @@ GEM
byebug (~> 11.0) byebug (~> 11.0)
pry (~> 0.13.0) pry (~> 0.13.0)
public_suffix (4.0.6) public_suffix (4.0.6)
puma (5.4.0) puma (5.5.0)
nio4r (~> 2.0) nio4r (~> 2.0)
racc (1.5.2) racc (1.5.2)
rack (2.2.3) rack (2.2.3)
@ -237,33 +237,33 @@ GEM
ffi (~> 1.0) ffi (~> 1.0)
regexp_parser (2.1.1) regexp_parser (2.1.1)
rexml (3.2.5) rexml (3.2.5)
rubocop (1.15.0) rubocop (1.21.0)
parallel (~> 1.10) parallel (~> 1.10)
parser (>= 3.0.0.0) parser (>= 3.0.0.0)
rainbow (>= 2.2.2, < 4.0) rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.8, < 3.0) regexp_parser (>= 1.8, < 3.0)
rexml rexml
rubocop-ast (>= 1.5.0, < 2.0) rubocop-ast (>= 1.9.1, < 2.0)
ruby-progressbar (~> 1.7) ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 3.0) unicode-display_width (>= 1.4.0, < 3.0)
rubocop-ast (1.6.0) rubocop-ast (1.11.0)
parser (>= 3.0.1.1) parser (>= 3.0.1.1)
rubocop-govuk (4.0.0) rubocop-govuk (4.1.0)
rubocop (~> 1.15.0) rubocop (= 1.21.0)
rubocop-ast (~> 1.6.0) rubocop-ast (= 1.11.0)
rubocop-rails (~> 2.10.0) rubocop-rails (= 2.12.2)
rubocop-rake (= 0.5.1) rubocop-rake (= 0.6.0)
rubocop-rspec (~> 2.3.0) rubocop-rspec (= 2.4.0)
rubocop-performance (1.11.5) rubocop-performance (1.11.5)
rubocop (>= 1.7.0, < 2.0) rubocop (>= 1.7.0, < 2.0)
rubocop-ast (>= 0.4.0) rubocop-ast (>= 0.4.0)
rubocop-rails (2.10.1) rubocop-rails (2.12.2)
activesupport (>= 4.2.0) activesupport (>= 4.2.0)
rack (>= 1.1) rack (>= 1.1)
rubocop (>= 1.7.0, < 2.0) rubocop (>= 1.7.0, < 2.0)
rubocop-rake (0.5.1) rubocop-rake (0.6.0)
rubocop rubocop (~> 1.0)
rubocop-rspec (2.3.0) rubocop-rspec (2.4.0)
rubocop (~> 1.0) rubocop (~> 1.0)
rubocop-ast (>= 1.1.0) rubocop-ast (>= 1.1.0)
ruby-progressbar (1.11.0) ruby-progressbar (1.11.0)
@ -294,14 +294,14 @@ GEM
actionpack (>= 4.0) actionpack (>= 4.0)
activesupport (>= 4.0) activesupport (>= 4.0)
sprockets (>= 3.0.0) sprockets (>= 3.0.0)
stimulus-rails (0.4.2) stimulus-rails (0.5.4)
rails (>= 6.0.0) rails (>= 6.0.0)
thor (1.1.0) thor (1.1.0)
turbo-rails (0.7.11) turbo-rails (0.7.14)
rails (>= 6.0.0) rails (>= 6.0.0)
tzinfo (2.0.4) tzinfo (2.0.4)
concurrent-ruby (~> 1.0) concurrent-ruby (~> 1.0)
unicode-display_width (2.0.0) unicode-display_width (2.1.0)
view_component (2.39.0) view_component (2.39.0)
activesupport (>= 5.0.0, < 8.0) activesupport (>= 5.0.0, < 8.0)
method_source (~> 1.0) method_source (~> 1.0)
@ -310,7 +310,7 @@ GEM
activemodel (>= 6.0.0) activemodel (>= 6.0.0)
bindex (>= 0.4.0) bindex (>= 0.4.0)
railties (>= 6.0.0) railties (>= 6.0.0)
webpacker (5.4.2) webpacker (5.4.3)
activesupport (>= 5.2) activesupport (>= 5.2)
rack-proxy (>= 0.6.1) rack-proxy (>= 0.6.1)
railties (>= 5.2) railties (>= 5.2)

2
app/views/case_logs/_log_list.html.erb

@ -15,8 +15,10 @@
<%= link_to log.id, case_log_path(log) %> <%= link_to log.id, case_log_path(log) %>
</th> </th>
<td class="govuk-table__cell govuk-table__cell"> <td class="govuk-table__cell govuk-table__cell">
<%= log.postcode %>
</td> </td>
<td class="govuk-table__cell govuk-table__cell"> <td class="govuk-table__cell govuk-table__cell">
<%= log.tenant_code %>
</td> </td>
<td id="last-changed" class="govuk-table__cell"> <td id="last-changed" class="govuk-table__cell">
<%= log.updated_at.strftime("%d %b %Y") %> <%= log.updated_at.strftime("%d %b %Y") %>

8
app/views/case_logs/index.html.erb

@ -6,9 +6,13 @@
<%= link_to "Create new log", case_logs_path, method: :post, class: "govuk-button" %> <%= 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 %>
<p><a href="#" class="govuk-link">See all completed logs (<%= @submitted_case_logs.count %>)</a></p> <p><a href="#" class="govuk-link">See all completed logs (<%= @submitted_case_logs.count %>)</a></p>
</div> </div>

6
app/views/form/questions/previous_housing_situation.html.erb

@ -25,8 +25,10 @@
] %> ] %>
<% previous_question = Form.previous_question("previous_housing_situation") %> <% previous_question = Form.previous_question("previous_housing_situation") %>
<%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> <% content_for :before_content do %>
Back <%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %>
Back
<% end %>
<% end %> <% end %>
<%= turbo_frame_tag "case_log_form", target: "_top" do %> <%= turbo_frame_tag "case_log_form", target: "_top" do %>

6
app/views/form/questions/tenant_age.html.erb

@ -1,6 +1,8 @@
<% previous_question = Form.previous_question("tenant_age") %> <% previous_question = Form.previous_question("tenant_age") %>
<%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> <% content_for :before_content do %>
Back <%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %>
Back
<% end %>
<% end %> <% end %>
<%= turbo_frame_tag "case_log_form", target: "_top" do %> <%= turbo_frame_tag "case_log_form", target: "_top" do %>

8
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 %> <%= 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| %> <%= form_with action: '/case_logs', method: "next_question", builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %>
<%= f.govuk_text_field :tenant_code, <%= f.govuk_text_field :tenant_code,
hint: { text: "More detail" }, hint: { text: "More detail" },

6
app/views/form/questions/tenant_gender.html.erb

@ -6,8 +6,10 @@
] %> ] %>
<% previous_question = Form.previous_question("tenant_gender") %> <% previous_question = Form.previous_question("tenant_gender") %>
<%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %> <% content_for :before_content do %>
Back <%= govuk_back_link href: "/case_logs/#{case_log_id}/#{previous_question}" do %>
Back
<% end %>
<% end %> <% end %>
<%= turbo_frame_tag "case_log_form", target: "_top" do %> <%= turbo_frame_tag "case_log_form", target: "_top" do %>

4
app/views/layouts/application.html.erb

@ -1,7 +1,7 @@
<!DOCTYPE html> <!DOCTYPE html>
<html lang="en" class="govuk-template "> <html lang="en" class="govuk-template ">
<head> <head>
<title>MHCLG CORE Data Collection</title> <title>DLUHC CORE Data Collection</title>
<%= csrf_meta_tags %> <%= csrf_meta_tags %>
<%= csp_meta_tag %> <%= csp_meta_tag %>
<%= tag :meta, name: 'viewport', content: 'width=device-width, initial-scale=1' %> <%= tag :meta, name: 'viewport', content: 'width=device-width, initial-scale=1' %>
@ -49,7 +49,7 @@
</div> </div>
<div class="govuk-header__content"> <div class="govuk-header__content">
<%= 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" %>
</div> </div>
</div> </div>
</header> </header>

7
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

1
db/schema.rb

@ -57,6 +57,7 @@ ActiveRecord::Schema.define(version: 2021_09_22_103729) do
t.integer "person_8_age" t.integer "person_8_age"
t.string "person_8_gender" t.string "person_8_gender"
t.string "person_8_economic" t.string "person_8_economic"
t.string "postcode"
end end
end end

19
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.

15
spec/factories/case_log.rb

@ -1,6 +1,17 @@
FactoryBot.define do FactoryBot.define do
factory :case_log do factory :case_log do
id { 342_351 } sequence(:id) { |i| i }
status { 0 } 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
end end

30
spec/features/case_log_spec.rb

@ -1,6 +1,6 @@
require "rails_helper" require "rails_helper"
RSpec.describe "Test Features" do 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(:id) { case_log.id }
let(:status) { case_log.status } let(:status) { case_log.status }
pages = ['tenant_code', 'tenant_age', 'tenant_gender', 'tenant_ethnic_group', 'tenant_nationality', 'economic_status', 'other_household_members'] 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 it "redirects to the task list for the new log" do
visit("/case_logs") visit("/case_logs")
click_link("Create new log") 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}") expect(page).to have_content("Tasklist for log #{id}")
end end
end end
describe "Viewing a log" do describe "Viewing a log" do
it "displays a tasklist header" 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("Tasklist for log #{id}")
expect(page).to have_content("This submission is #{status}") expect(page).to have_content("This submission is #{status}")
end end
@ -29,7 +29,7 @@ RSpec.describe "Test Features" do
expect(page).to have_field("tenant-age-field") expect(page).to have_field("tenant-age-field")
click_button("Save and continue") click_button("Save and continue")
expect(page).to have_field("tenant-gender-0-field") 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") expect(page).to have_field("tenant-age-field")
end end
@ -39,19 +39,19 @@ RSpec.describe "Test Features" do
expect(page).to have_field("tenant-age-field") expect(page).to have_field("tenant-age-field")
end end
end end
end
describe "Back link directs correctly" do describe "Back link directs correctly" do
it "go back to tasklist page from tenant code" do it "go back to tasklist page from tenant code" do
visit("/case_logs/#{id}/tenant_code") visit("/case_logs/#{id}/tenant_code")
click_link(text: 'Back') click_link(text: "Back")
expect(page).to have_content("Tasklist for log #{id}") expect(page).to have_content("Tasklist for log #{id}")
end end
it "go back to tenant code page from tenant age page" do it "go back to tenant code page from tenant age page" do
visit("/case_logs/#{id}/tenant_age") visit("/case_logs/#{id}/tenant_age")
click_link(text: 'Back') click_link(text: "Back")
expect(page).to have_field("tenant-code-field") expect(page).to have_field("tenant-code-field")
end
end end
end end

2
spec/features/test_spec.rb

@ -2,7 +2,7 @@ require "rails_helper"
RSpec.describe "Test Features" do RSpec.describe "Test Features" do
it "Displays the name of the app" do it "Displays the name of the app" do
visit("/") 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 end
it "Links to the About page" do it "Links to the About page" do

42
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(/<table class="govuk-table">/)
expect(rendered).to match(/Logs you need to complete/)
expect(rendered).not_to match(/Logs you&#39;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(/<table class="govuk-table">/)
expect(rendered).to match(/Logs you&#39;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(/<table class="govuk-table">/)
expect(rendered).to match(/Logs you&#39;ve submitted/)
expect(rendered).to match(/Logs you need to complete/)
end
end
end
Loading…
Cancel
Save