From c868e75bf2959485733c36970d5981c8584e385f Mon Sep 17 00:00:00 2001
From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com>
Date: Fri, 27 May 2022 11:23:16 +0100
Subject: [PATCH] Bug Fixes (#618)

* Don't trigger confirmation emails for inactive users

* Show organisation type
---
 app/helpers/organisation_helper.rb                  |  5 -----
 app/models/organisation.rb                          |  8 +++++++-
 app/models/user.rb                                  |  6 ++++--
 app/views/organisations/_organisation_list.html.erb |  2 +-
 spec/models/user_spec.rb                            | 12 ++++++++++++
 5 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/app/helpers/organisation_helper.rb b/app/helpers/organisation_helper.rb
index c600f61da..68fa98242 100644
--- a/app/helpers/organisation_helper.rb
+++ b/app/helpers/organisation_helper.rb
@@ -8,9 +8,4 @@ module OrganisationHelper
       current_organisation.name
     end
   end
-
-  DISPLAY_PROVIDER_TYPE = { "LA": "Local authority", "PRP": "Private registered provider" }.freeze
-  def display_provider_type(provider_type)
-    DISPLAY_PROVIDER_TYPE[provider_type.to_sym]
-  end
 end
diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index 6d22c303a..3d66b0a85 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -62,12 +62,18 @@ class Organisation < ApplicationRecord
     data_protection_confirmed? ? "Accepted" : "Not accepted"
   end
 
+  DISPLAY_PROVIDER_TYPE = { "LA": "Local authority", "PRP": "Private registered provider" }.freeze
+
+  def display_provider_type
+    DISPLAY_PROVIDER_TYPE[provider_type.to_sym]
+  end
+
   def display_attributes
     [
       { name: "name", value: name, editable: true },
       { name: "address", value: address_string, editable: true },
       { name: "telephone_number", value: phone, editable: true },
-      { name: "type", value: "Org type", editable: false },
+      { name: "type", value: display_provider_type, editable: false },
       { name: "local_authorities_operated_in", value: local_authority_names, editable: false, format: :bullet },
       { name: "rent_periods", value: rent_period_labels, editable: false, format: :bullet },
       { name: "holds_own_stock", value: holds_own_stock.to_s.humanize, editable: false },
diff --git a/app/models/user.rb b/app/models/user.rb
index 97cc43c62..5eec7bed8 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -90,8 +90,10 @@ class User < ApplicationRecord
     old_user_id.present?
   end
 
-  def skip_confirmation!
-    !active?
+  def send_confirmation_instructions
+    return unless active?
+
+    super
   end
 
   def need_two_factor_authentication?(_request)
diff --git a/app/views/organisations/_organisation_list.html.erb b/app/views/organisations/_organisation_list.html.erb
index 591cedba7..78f60685d 100644
--- a/app/views/organisations/_organisation_list.html.erb
+++ b/app/views/organisations/_organisation_list.html.erb
@@ -28,7 +28,7 @@
           <%= govuk_link_to(organisation.name, "organisations/#{organisation.id}/logs") %>
         <% end %>
         <% row.cell(text: organisation.housing_registration_no) %>
-        <% row.cell(text: display_provider_type(organisation.provider_type)) %>
+        <% row.cell(text: organisation.display_provider_type) %>
       <% end %>
     <% end %>
   <% end %>
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index df3f82911..646e65d2a 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -85,6 +85,18 @@ RSpec.describe User, type: :model do
       )
     end
 
+    it "does not send a confirmation email to inactive users" do
+      expect(DeviseNotifyMailer).not_to receive(:confirmation_instructions)
+      described_class.create!(
+        name: "unconfirmed_user",
+        email: "unconfirmed_user@example.com",
+        password: "password123",
+        organisation: other_organisation,
+        role: "data_provider",
+        active: false,
+      )
+    end
+
     context "when the user is a data provider" do
       it "cannot assign roles" do
         expect(user.assignable_roles).to eq({})