From 2998170cdda66a39f81f582db90372c1f33d98d7 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Tue, 5 Apr 2022 16:14:25 +0100 Subject: [PATCH] Remove hotwire turbo (#406) * Remove turbo * No turbo so redirect form response * Handle multiple errors per field * Remove turbo styles --- Gemfile | 4 +- Gemfile.lock | 9 +-- app/controllers/form_controller.rb | 10 ++- app/frontend/application.js | 1 - app/frontend/styles/application.scss | 6 -- app/views/case_logs/edit.html.erb | 47 +++++++------- app/views/form/check_answers.html.erb | 44 +++++++------- app/views/form/page.html.erb | 84 ++++++++++++-------------- app/views/layouts/application.html.erb | 4 +- package.json | 2 - spec/features/form/validations_spec.rb | 24 ++++---- spec/requests/form_controller_spec.rb | 2 +- yarn.lock | 18 ------ 13 files changed, 108 insertions(+), 147 deletions(-) diff --git a/Gemfile b/Gemfile index e08f126a0..09562e673 100644 --- a/Gemfile +++ b/Gemfile @@ -25,8 +25,8 @@ gem "govuk_design_system_formbuilder" gem "govuk_markdown" # GOV UK Notify gem "notifications-ruby-client" -# Turbo and Stimulus -gem "hotwire-rails" +# A modest javascript framework for the html you already have +gem "stimulus-rails" # Administration framework gem "activeadmin" # Admin charts diff --git a/Gemfile.lock b/Gemfile.lock index b09b5b1da..05f784cd2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -188,10 +188,6 @@ GEM actionpack (>= 5.2) activesupport (>= 5.2) hashdiff (1.0.1) - hotwire-rails (0.1.3) - rails (>= 6.0.0) - stimulus-rails - turbo-rails i18n (1.10.0) concurrent-ruby (~> 1.0) inherited_resources (1.13.1) @@ -432,9 +428,6 @@ GEM thor (1.2.1) timecop (0.9.5) timeout (0.2.0) - turbo-rails (1.0.1) - actionpack (>= 6.0.0) - railties (>= 6.0.0) tzinfo (2.0.4) concurrent-ruby (~> 1.0) uk_postcode (2.1.7) @@ -482,7 +475,6 @@ DEPENDENCIES govuk-components govuk_design_system_formbuilder govuk_markdown - hotwire-rails jsbundling-rails json-schema listen (~> 3.3) @@ -510,6 +502,7 @@ DEPENDENCIES sentry-rails sentry-ruby simplecov + stimulus-rails timecop (~> 0.9.4) two_factor_authentication! tzinfo-data diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index b24632b31..787017186 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -17,8 +17,9 @@ class FormController < ApplicationController redirect_to(send(redirect_path, @case_log)) end else - @subsection = @case_log.form.subsection_for_page(@page) - render "form/page", status: :unprocessable_entity + redirect_path = "case_log_#{@page.id}_path" + session[:errors] = @case_log.errors.to_json + redirect_to(send(redirect_path, @case_log)) end else render_not_found @@ -39,6 +40,11 @@ class FormController < ApplicationController form.pages.map do |page| define_method(page.id) do |_errors = {}| if @case_log + if session["errors"] + JSON(session["errors"]).each do |field, messages| + messages.each { |message| @case_log.errors.add field.to_sym, message } + end + end @subsection = @case_log.form.subsection_for_page(page) @page = @case_log.form.get_page(page.id) if @page.routed_to?(@case_log) && @page.subsection.enabled?(@case_log) diff --git a/app/frontend/application.js b/app/frontend/application.js index d3a684d94..98742bb09 100644 --- a/app/frontend/application.js +++ b/app/frontend/application.js @@ -16,6 +16,5 @@ require.context("govuk-frontend/govuk/assets") import { initAll } from "govuk-frontend" import "./styles/application.scss" import "./controllers" -import "@hotwired/turbo-rails" initAll() diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss index 17a5a97cf..a89fa2cce 100644 --- a/app/frontend/styles/application.scss +++ b/app/frontend/styles/application.scss @@ -25,12 +25,6 @@ $govuk-global-styles: true; @import "pagination"; @import "panel"; -// Turbo -.turbo-progress-bar { - height: govuk-spacing(1); - background-color: $govuk-brand-colour; -} - // App utilities .app-\!-colour-muted { color: $govuk-secondary-text-colour !important; diff --git a/app/views/case_logs/edit.html.erb b/app/views/case_logs/edit.html.erb index fe7e2d661..015db4454 100644 --- a/app/views/case_logs/edit.html.erb +++ b/app/views/case_logs/edit.html.erb @@ -4,30 +4,27 @@ content_for(:title) => "" }) %> -<%= turbo_frame_tag "case_log_form", target: "_top" do %> -
-
-

- <%= content_for(:title) %> -

+
+
+

+ <%= content_for(:title) %> +

- <% if @case_log.status == "in_progress" %> -

<%= get_subsections_count(@case_log, :completed) %> of <%= get_subsections_count(@case_log, :all) %> sections completed.

-

- <% next_incomplete_section = get_next_incomplete_section(@case_log) %> -

-

- <% if next_incomplete_section.present? %> - - Skip to next incomplete section: <%= next_incomplete_section.label %> - - <% end %> -

- <% elsif @case_log.status == "not_started" %> -

This log has not been started.

- <% end %> - - <%= render "tasklist" %> -
+ <% if @case_log.status == "in_progress" %> +

<%= get_subsections_count(@case_log, :completed) %> of <%= get_subsections_count(@case_log, :all) %> sections completed.

+

+ <% next_incomplete_section = get_next_incomplete_section(@case_log) %> +

+

+ <% if next_incomplete_section.present? %> + + Skip to next incomplete section: <%= next_incomplete_section.label %> + + <% end %> +

+ <% elsif @case_log.status == "not_started" %> +

This log has not been started.

+ <% end %> + <%= render "tasklist" %>
-<% end %> +
diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index 4be257a6f..c4c8b023c 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -5,31 +5,29 @@ subsection.label => "" }) %> -<%= turbo_frame_tag "case_log_form", target: "_top" do %> -
-
-

- <%= subsection.label %> - Check your answers -

+
+
+

+ <%= subsection.label %> + Check your answers +

- <%= display_answered_questions_summary(subsection, @case_log) %> + <%= display_answered_questions_summary(subsection, @case_log) %> -
- <% subsection.applicable_questions(@case_log).each do |question| %> - <%= render partial: 'form/check_answers_table', locals: { question: question, case_log: @case_log } %> - <% end %> -
+
+ <% subsection.applicable_questions(@case_log).each do |question| %> + <%= render partial: 'form/check_answers_table', locals: { question: question, case_log: @case_log } %> + <% end %> +
- <%= form_with model: @case_log, method: "get" do |f| %> - <%= f.govuk_submit 'Save and return to log' do %> - <% if @case_log.status == "in_progress" && @case_log.status == "completed" || @case_log.form.all_subsections_except_declaration_completed?(@case_log) == false %> - <%= govuk_button_link_to 'Save and go to next incomplete section', "/logs/#{@case_log.id}/#{@case_log.form.next_incomplete_section_redirect_path(subsection, @case_log)}", secondary: true %> - <% elsif @case_log.status == "completed" || @case_log.form.all_subsections_except_declaration_completed?(@case_log) %> - <%= govuk_button_link_to 'Save and go to submit', "/logs/#{@case_log.id}/#{@case_log.form.next_incomplete_section_redirect_path(subsection, @case_log)}", secondary: true %> - <% end%> - <% end %> + <%= form_with model: @case_log, method: "get" do |f| %> + <%= f.govuk_submit 'Save and return to log' do %> + <% if @case_log.status == "in_progress" && @case_log.status == "completed" || @case_log.form.all_subsections_except_declaration_completed?(@case_log) == false %> + <%= govuk_button_link_to 'Save and go to next incomplete section', "/logs/#{@case_log.id}/#{@case_log.form.next_incomplete_section_redirect_path(subsection, @case_log)}", secondary: true %> + <% elsif @case_log.status == "completed" || @case_log.form.all_subsections_except_declaration_completed?(@case_log) %> + <%= govuk_button_link_to 'Save and go to submit', "/logs/#{@case_log.id}/#{@case_log.form.next_incomplete_section_redirect_path(subsection, @case_log)}", secondary: true %> + <% end%> <% end %> -
+ <% end %>
-<% end %> +
diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 78cbd1b96..ab006ccdc 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -1,7 +1,3 @@ -<% content_for :head do %> - -<% end %> - <% content_for :title, @page.header.present? ? @page.header : @page.questions.first().header.html_safe %> <% content_for :before_content do %> @@ -13,48 +9,46 @@
-<%= turbo_frame_tag "case_log_form", target: "_top" do %> - <%= form_with model: @case_log, url: form_case_log_path(@case_log), method: "post" do |f| %> -
-
> - <% remove_other_page_errors(@case_log, @page) %> - <%= f.govuk_error_summary %> - - <% if @page.header.present? %> -

- <% if !@page.hide_subsection_label %> - <%= @subsection.label %> - <% end %> - <%= @page.header %> -

- <% end %> - - <% if @page.description.present? %> -

<%= @page.description.html_safe %>

- <% end %> +<%= form_with model: @case_log, url: form_case_log_path(@case_log), method: "post", local: true do |f| %> +
+
> + <% remove_other_page_errors(@case_log, @page) %> + <%= f.govuk_error_summary %> - <% @page.non_conditional_questions.map do |question| %> -
<%= display_question_key_div(@page, question) %> > - <% if question.read_only? %> -
- <% end %> - <% if question.type == "interruption_screen" %> - <%= render partial: "form/#{question.type}_question", locals: { question: question, caption_text: @subsection.label, page_header: @page.header, case_log: @case_log, title_text: @page.title_text, informative_text: @page.informative_text, form: @form, f: f, conditional: false } %> - <% else %> - <%= render partial: "form/#{question.type}_question", locals: { question: question, caption_text: @subsection.label, page_header: @page.header, case_log: @case_log, f: f, conditional: false } %> - <% end %> -
- <% end %> - - <%= f.hidden_field :page, value: @page.id %> - <% if @case_log.form.is_last_question?(@page, @subsection, @case_log) %> - <%= f.govuk_submit "Submit lettings log", accesskey: "s" %> - <%else %> - <% if !@page.id.include?("value_check") %> - <%= f.govuk_submit "Save and continue", accesskey: "s" %> + <% if @page.header.present? %> +

+ <% if !@page.hide_subsection_label %> + <%= @subsection.label %> + <% end %> + <%= @page.header %> +

+ <% end %> + + <% if @page.description.present? %> +

<%= @page.description.html_safe %>

+ <% end %> + + <% @page.non_conditional_questions.map do |question| %> +
<%= display_question_key_div(@page, question) %> > + <% if question.read_only? %> +
<% end %> - <%end %> -
+ <% if question.type == "interruption_screen" %> + <%= render partial: "form/#{question.type}_question", locals: { question: question, caption_text: @subsection.label, page_header: @page.header, case_log: @case_log, title_text: @page.title_text, informative_text: @page.informative_text, form: @form, f: f, conditional: false } %> + <% else %> + <%= render partial: "form/#{question.type}_question", locals: { question: question, caption_text: @subsection.label, page_header: @page.header, case_log: @case_log, f: f, conditional: false } %> + <% end %> +
+ <% end %> + + <%= f.hidden_field :page, value: @page.id %> + <% if @case_log.form.is_last_question?(@page, @subsection, @case_log) %> + <%= f.govuk_submit "Submit lettings log", accesskey: "s" %> + <%else %> + <% if !@page.id.include?("value_check") %> + <%= f.govuk_submit "Save and continue", accesskey: "s" %> + <% end %> + <%end %>
- <% end %> +
<% end %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 66ba7ab59..84814d199 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -13,7 +13,7 @@ <%= favicon_link_tag asset_path('images/govuk-apple-touch-icon-152x152.png'), rel: 'apple-touch-icon', type: 'image/png', size: '152x152' %> <%= favicon_link_tag asset_path('images/govuk-apple-touch-icon-167x167.png'), rel: 'apple-touch-icon', type: 'image/png', size: '167x167' %> <%= favicon_link_tag asset_path('images/govuk-apple-touch-icon-180x180.png'), rel: 'apple-touch-icon', type: 'image/png', size: '180x180' %> - <%= stylesheet_link_tag "application", "data-turbo-track": "reload" %> + <%= stylesheet_link_tag "application" %> <%= javascript_include_tag "vendor/html5shiv.min.js" %> <%= javascript_tag do -%> window.html5.elements = 'output'; @@ -21,7 +21,7 @@ <% end -%> <%= javascript_include_tag "vendor/polyfill-output-value.js" %> <%= javascript_include_tag "vendor/outerHTML.js" %> - <%= javascript_include_tag "application", "data-turbo-track": "reload", defer: true %> + <%= javascript_include_tag "application", defer: true %> <% if content_for?(:head) %> <%= yield(:head) %> diff --git a/package.json b/package.json index e710c0e30..463c20606 100644 --- a/package.json +++ b/package.json @@ -10,8 +10,6 @@ "@babel/plugin-transform-runtime": "^7.17.0", "@babel/preset-env": "^7.16.11", "@hotwired/stimulus": "^3.0.0", - "@hotwired/turbo": "^7.1.0", - "@hotwired/turbo-rails": "^7.1.0", "@stimulus/polyfills": "^2.0.0", "@webcomponents/webcomponentsjs": "^2.6.0", "accessible-autocomplete": "^2.0.3", diff --git a/spec/features/form/validations_spec.rb b/spec/features/form/validations_spec.rb index e1947bf87..b884264a4 100644 --- a/spec/features/form/validations_spec.rb +++ b/spec/features/form/validations_spec.rb @@ -60,51 +60,51 @@ RSpec.describe "validations" do describe "date validation", js: true do def fill_in_date(case_log_id, question, day, month, year, path) visit("/logs/#{case_log_id}/#{path}") - fill_in("#{question}_1i", with: year) - fill_in("#{question}_2i", with: month) - fill_in("#{question}_3i", with: day) + fill_in("case_log[#{question}(1i)]", with: year) + fill_in("case_log[#{question}(2i)]", with: month) + fill_in("case_log[#{question}(3i)]", with: day) end it "does not allow out of range dates to be submitted" do - fill_in_date(id, "case_log_mrcdate", 3100, 12, 2000, "property-major-repairs") + fill_in_date(id, "mrcdate", 3100, 12, 2000, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/property-major-repairs") - fill_in_date(id, "case_log_mrcdate", 12, 1, 20_000, "property-major-repairs") + fill_in_date(id, "mrcdate", 12, 1, 20_000, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/property-major-repairs") - fill_in_date(id, "case_log_mrcdate", 13, 100, 2020, "property-major-repairs") + fill_in_date(id, "mrcdate", 13, 100, 2020, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/property-major-repairs") - fill_in_date(id, "case_log_mrcdate", 21, 11, 2020, "property-major-repairs") + fill_in_date(id, "mrcdate", 21, 11, 2020, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/local-authority/check-answers") end it "does not allow non numeric inputs to be submitted" do - fill_in_date(id, "case_log_mrcdate", "abc", "de", "ff", "property-major-repairs") + fill_in_date(id, "mrcdate", "abc", "de", "ff", "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/property-major-repairs") end it "does not allow partial inputs to be submitted" do - fill_in_date(id, "case_log_mrcdate", 21, 12, nil, "property-major-repairs") + fill_in_date(id, "mrcdate", 21, 12, nil, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/property-major-repairs") - fill_in_date(id, "case_log_mrcdate", 12, nil, 2000, "property-major-repairs") + fill_in_date(id, "mrcdate", 12, nil, 2000, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/property-major-repairs") - fill_in_date(id, "case_log_mrcdate", nil, 10, 2020, "property-major-repairs") + fill_in_date(id, "mrcdate", nil, 10, 2020, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/property-major-repairs") end it "allows valid inputs to be submitted" do - fill_in_date(id, "case_log_mrcdate", 21, 11, 2020, "property-major-repairs") + fill_in_date(id, "mrcdate", 21, 11, 2020, "property-major-repairs") click_button("Save and continue") expect(page).to have_current_path("/logs/#{id}/local-authority/check-answers") end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 1fc4ceb12..8c7ea8de8 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -129,7 +129,7 @@ RSpec.describe FormController, type: :request do let(:answer) { 2000 } it "re-renders the same page with errors if validation fails" do - expect(response).to have_http_status(:unprocessable_entity) + expect(response).to redirect_to("/logs/#{case_log.id}/#{page_id.dasherize}") end end diff --git a/yarn.lock b/yarn.lock index 217d7598c..9270ef046 100644 --- a/yarn.lock +++ b/yarn.lock @@ -926,19 +926,6 @@ resolved "https://registry.yarnpkg.com/@hotwired/stimulus/-/stimulus-3.0.1.tgz#141f15645acaa3b133b7c247cad58ae252ffae85" integrity sha512-oHsJhgY2cip+K2ED7vKUNd2P+BEswVhrCYcJ802DSsblJFv7mPFVk3cQKvm2vHgHeDVdnj7oOKrBbzp1u8D+KA== -"@hotwired/turbo-rails@^7.1.0": - version "7.1.1" - resolved "https://registry.yarnpkg.com/@hotwired/turbo-rails/-/turbo-rails-7.1.1.tgz#35c03b92b5c86f0137ed08bef843d955ec9bbe83" - integrity sha512-ZXpxUjCfkdbuXfoGrsFK80qsVzACs8xCfie9rt2jMTSN6o1olXVA0Nrk8u02yNEwSiVJm/4QSOa8cUcMj6VQjg== - dependencies: - "@hotwired/turbo" "^7.1.0" - "@rails/actioncable" "^7.0" - -"@hotwired/turbo@^7.1.0": - version "7.1.0" - resolved "https://registry.yarnpkg.com/@hotwired/turbo/-/turbo-7.1.0.tgz#27e44e0e3dc5bd1d4bda0766d579cf5a14091cd7" - integrity sha512-Q8kGjqwPqER+CtpQudbH+3Zgs2X4zb6pBAlr6NsKTXadg45pAOvxI9i4QpuHbwSzR2+x87HUm+rot9F/Pe8rxA== - "@jridgewell/resolve-uri@^3.0.3": version "3.0.5" resolved "https://registry.yarnpkg.com/@jridgewell/resolve-uri/-/resolve-uri-3.0.5.tgz#68eb521368db76d040a6315cdb24bf2483037b9c" @@ -978,11 +965,6 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" -"@rails/actioncable@^7.0": - version "7.0.2" - resolved "https://registry.yarnpkg.com/@rails/actioncable/-/actioncable-7.0.2.tgz#69a6d999f4087e0537dd38fe0963db1f4305d650" - integrity sha512-G26maXW1Kx0LxQdmNNuNjQlRO/QlXNr3QfuwKiOKb5FZQGYe2OwtHTGXBAjSoiu4dW36XYMT/+L1Wo1Yov4ZXA== - "@stimulus/polyfills@^2.0.0": version "2.0.0" resolved "https://registry.yarnpkg.com/@stimulus/polyfills/-/polyfills-2.0.0.tgz#64b3e247c762330f80d88e993d1d26b24e3c13b1"