From 11011817ed2c521bf9b5893a8fdcc94eea60cacb Mon Sep 17 00:00:00 2001
From: Daniel Baark <5101747+baarkerlounger@users.noreply.github.com>
Date: Wed, 1 Dec 2021 13:58:29 +0000
Subject: [PATCH] Devise doesn't play nice with Turbo yet (#119)

* Devise doesn't play nice with Turbo yet

* Move errors above header

* Add specific field error messages if email or password omitted

* Add email validation

* Update app/controllers/users/sessions_controller.rb

Co-authored-by: Paul Robert Lloyd <paulrobertlloyd@users.noreply.github.com>

* Update app/controllers/users/sessions_controller.rb

Co-authored-by: Paul Robert Lloyd <paulrobertlloyd@users.noreply.github.com>

* Update app/controllers/users/sessions_controller.rb

Co-authored-by: Paul Robert Lloyd <paulrobertlloyd@users.noreply.github.com>

* Update spec/features/user_spec.rb

Co-authored-by: Paul Robert Lloyd <paulrobertlloyd@users.noreply.github.com>

* Remove default you need to sign in or sign up message

Co-authored-by: Paul Robert Lloyd <paulrobertlloyd@users.noreply.github.com>
---
 Gemfile                                      |  3 +-
 Gemfile.lock                                 | 20 ++++++++----
 app/controllers/users/sessions_controller.rb | 24 ++++++++++++++
 app/helpers/devise_helper.rb                 | 10 ++++++
 app/views/devise/sessions/new.html.erb       |  3 ++
 app/views/layouts/application.html.erb       |  4 +--
 config/routes.rb                             |  2 +-
 spec/features/user_spec.rb                   | 34 ++++++++++++++++++++
 8 files changed, 89 insertions(+), 11 deletions(-)
 create mode 100644 app/controllers/users/sessions_controller.rb
 create mode 100644 app/helpers/devise_helper.rb

diff --git a/Gemfile b/Gemfile
index e274a06c1..4d8e8ea3c 100644
--- a/Gemfile
+++ b/Gemfile
@@ -32,7 +32,8 @@ gem "roo"
 # Json Schema
 gem "json-schema"
 # Authentication
-gem "devise"
+# Point at branch until devise is compatible with Turbo, see https://github.com/heartcombo/devise/pull/5340
+gem "devise", github: "ghiculescu/devise", branch: "error-code-422"
 gem "turbo-rails", "~> 0.8"
 gem "uk_postcode"
 gem "view_component"
diff --git a/Gemfile.lock b/Gemfile.lock
index b986e007b..100e839eb 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,3 +1,15 @@
+GIT
+  remote: https://github.com/ghiculescu/devise.git
+  revision: 3b2d9ae3d47be5c9228c4446119b04b0e98917c1
+  branch: error-code-422
+  specs:
+    devise (4.8.0)
+      bcrypt (~> 3.0)
+      orm_adapter (~> 0.1)
+      railties (>= 4.1.0)
+      responders
+      warden (~> 1.2.3)
+
 GIT
   remote: https://github.com/rspec/rspec-core.git
   revision: e36aa2a9ebe68acee3ce05190fc2124947b45925
@@ -148,12 +160,6 @@ GEM
     concurrent-ruby (1.1.9)
     crass (1.0.6)
     deep_merge (1.2.1)
-    devise (4.8.0)
-      bcrypt (~> 3.0)
-      orm_adapter (~> 0.1)
-      railties (>= 4.1.0)
-      responders
-      warden (~> 1.2.3)
     diff-lcs (1.4.4)
     discard (1.2.0)
       activerecord (>= 4.2, < 7)
@@ -408,7 +414,7 @@ DEPENDENCIES
   capybara
   capybara-lockstep
   chartkick
-  devise
+  devise!
   discard
   dotenv-rails
   factory_bot_rails
diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb
new file mode 100644
index 000000000..e3915e25f
--- /dev/null
+++ b/app/controllers/users/sessions_controller.rb
@@ -0,0 +1,24 @@
+class Users::SessionsController < Devise::SessionsController
+  def create
+    self.resource = resource_class.new
+    if params.dig("user", "email").empty?
+      resource.errors.add :email, "Enter an email address"
+    elsif !email_valid?(params.dig("user", "email"))
+      resource.errors.add :email, "Enter an email address in the correct format, like name@example.com"
+    end
+    if params.dig("user", "password").empty?
+      resource.errors.add :password, "Enter a password"
+    end
+    if resource.errors.present?
+      render :new, status: :unprocessable_entity
+    else
+      super
+    end
+  end
+
+private
+
+  def email_valid?(email)
+    email =~ URI::MailTo::EMAIL_REGEXP
+  end
+end
diff --git a/app/helpers/devise_helper.rb b/app/helpers/devise_helper.rb
new file mode 100644
index 000000000..a07964c6c
--- /dev/null
+++ b/app/helpers/devise_helper.rb
@@ -0,0 +1,10 @@
+module DeviseHelper
+  def flash_to_model_errors(resource)
+    if flash.alert
+      if flash.alert != I18n.t("devise.failure.unauthenticated")
+        resource.errors.add :base, flash.alert
+      end
+      flash.discard
+    end
+  end
+end
diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb
index 5ad69b2fe..485216d4e 100644
--- a/app/views/devise/sessions/new.html.erb
+++ b/app/views/devise/sessions/new.html.erb
@@ -1,6 +1,9 @@
 <%= form_for(resource, as: resource_name, url: session_path(resource_name)) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
+      <% flash_to_model_errors(resource) %>
+      <%= f.govuk_error_summary %>
+
       <h1 class="govuk-heading-l">Sign in to your account to submit CORE data</h1>
 
       <%= f.govuk_email_field :email,
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index 8eda1a8bb..b866643f8 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -57,12 +57,12 @@
       </div>
 
       <main class="govuk-main-wrapper" id="main-content" role="main">
-        <% flash.each do |type, msg| %>
+        <% if flash.notice %>
           <%= govuk_notification_banner(
             title_text: 'Success',
             success: true, title_heading_level: 3,
             title_id: "swanky-notifications") do |notification_banner|
-              notification_banner.heading(text: msg)
+              notification_banner.heading(text: flash.notice)
             end
           %>
         <% end %>
diff --git a/config/routes.rb b/config/routes.rb
index f30e8868c..3ca1e142d 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1,6 +1,6 @@
 Rails.application.routes.draw do
   devise_for :admin_users, ActiveAdmin::Devise.config
-  devise_for :users, controllers: { passwords: "users/passwords" }, skip: [:registrations]
+  devise_for :users, controllers: { passwords: "users/passwords", sessions: "users/sessions" }, skip: [:registrations]
   devise_scope :user do
     get "confirmations/reset", to: "users/passwords#reset_confirmation"
     get "users/edit" => "devise/registrations#edit", :as => "edit_user_registration"
diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb
index e41551845..5954fd6b8 100644
--- a/spec/features/user_spec.rb
+++ b/spec/features/user_spec.rb
@@ -7,12 +7,18 @@ RSpec.describe "User Features" do
       expect(page).to have_current_path("/users/sign_in")
     end
 
+    it "does not see the default devise error message" do
+      visit("/case_logs")
+      expect(page).to have_no_content("You need to sign in or sign up before continuing.")
+    end
+
     it " is redirected to case logs after signing in" do
       visit("/case_logs")
       fill_in("user[email]", with: user.email)
       fill_in("user[password]", with: "pAssword1")
       click_button("Sign in")
       expect(page).to have_current_path("/case_logs")
+      expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
     end
   end
 
@@ -80,6 +86,34 @@ RSpec.describe "User Features" do
     end
   end
 
+  context "Trying to log in with incorrect credentials" do
+    it "shows a gov uk error summary and no flash message" do
+      visit("/case_logs")
+      fill_in("user[email]", with: user.email)
+      fill_in("user[password]", with: "nonsense")
+      click_button("Sign in")
+      expect(page).to have_selector("#error-summary-title")
+      expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success")
+    end
+
+    it "show specific field error messages if a field was omitted" do
+      visit("/case_logs")
+      click_button("Sign in")
+      expect(page).to have_selector("#error-summary-title")
+      expect(page).to have_selector("#user-email-field-error")
+      expect(page).to have_selector("#user-password-field-error")
+    end
+
+    it "show specific field error messages if an invalid email address is entered" do
+      visit("/case_logs")
+      fill_in("user[email]", with: "thisisn'tanemail")
+      click_button("Sign in")
+      expect(page).to have_selector("#error-summary-title")
+      expect(page).to have_selector("#user-email-field-error")
+      expect(page).to have_content(/Enter an email address in the correct format, like name@example.com/)
+    end
+  end
+
   context "Your Account " do
     before(:each) do
       visit("/case_logs")