refactored validations for users

app/controllers/users_controller.rb

@@ -63,29 +63,21 @@ class UsersController < ApplicationController
   end
 
   def new
+    debugger
     @organisation_id = params["organisation_id"]
-    @resource = User.new
+    @user = User.new
   end
 
   def create
-    @resource = User.new
-    if user_params["email"].empty?
-      @resource.errors.add :email, I18n.t("validations.email.blank")
-    elsif !email_valid?(user_params["email"])
-      @resource.errors.add :email, I18n.t("validations.email.invalid")
-    elsif user_params[:role] && !current_user.assignable_roles.key?(user_params[:role].to_sym)
-      @resource.errors.add :role, I18n.t("validations.role.invalid")
-    end
-    if @resource.errors.present?
-      render :new, status: :unprocessable_entity
+    @user = User.new(user_params.merge(org_params).merge(password_params))
+    if @user.save
+      redirect_to created_user_redirect_path
     else
-      user = User.create(user_params.merge(org_params).merge(password_params))
-      if user.persisted?
-        redirect_to created_user_redirect_path
-      else
-        @resource.errors.add :email, I18n.t("validations.email.taken")
-        render :new, status: :unprocessable_entity
+      unless @user.errors[:organisation].empty?
+        @user.errors.add(:organisation_id, message: @user.errors[:organisation])
+        @user.errors.delete(:organisation)
       end
+      render :new, status: :unprocessable_entity
     end
   end
 

app/models/user.rb

@@ -1,6 +1,7 @@
 class User < ApplicationRecord
   # Include default devise modules. Others available are:
   # :omniauthable
+  include Helpers::Email
   devise :database_authenticatable, :recoverable, :rememberable, :validatable,
          :trackable, :lockable, :two_factor_authenticatable, :confirmable, :timeoutable
 
@@ -8,6 +9,9 @@ class User < ApplicationRecord
   has_many :owned_case_logs, through: :organisation
   has_many :managed_case_logs, through: :organisation
 
+  validate :validate_email
+  validates :name, :role, :email, presence: true
+
   has_paper_trail ignore: %w[last_sign_in_at
                              current_sign_in_at
                              current_sign_in_ip
@@ -149,4 +153,16 @@ class User < ApplicationRecord
   def valid_for_authentication?
     super && active?
   end
+
+private
+
+  def validate_email
+    unless email_valid?(email)
+      if User.exists?(["email LIKE ?", "%#{email}%"])
+        errors.add :email, I18n.t("validations.email.taken")
+      else
+        errors.add :email, I18n.t("validations.email.invalid")
+      end
+    end
+  end
 end

app/views/users/new.html.erb

@@ -4,7 +4,7 @@
   <%= govuk_back_link(href: :back) %>
 <% end %>
 
-<%= form_for(@resource, as: :user, html: { method: :post }) do |f| %>
+<%= form_for(@user, as: :user, html: { method: :post }) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <%= f.govuk_error_summary %>
@@ -15,22 +15,22 @@
 
       <%= f.govuk_text_field :name,
         autocomplete: "name",
-        label: { text: "Name (optional)", size: "m" } %>
+        label: { text: "Name", size: "m" } %>
 
       <%= f.govuk_email_field :email,
         label: { text: "Email address", size: "m" },
         autocomplete: "email",
         spellcheck: "false",
-        value: @resource.email %>
+        value: @user.email %>
 
       <% if current_user.support? %>
         <% null_option = [OpenStruct.new(id: "", name: "Select an option")] %>
         <% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %>
         <% answer_options = null_option + organisations %>
-        <% if @organisation_id %>
-          <% organisation = Organisation.find(@organisation_id) %>
-          <% answer_options = [OpenStruct.new(id: organisation.id, name: organisation.name)] %>
-        <% end %>
+
+        <% null_option = [OpenStruct.new(id: "", name: "Select an option")] %>
+        <% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %>
+        <% answer_options = null_option + organisations %>
 
           <%= f.govuk_collection_select :organisation_id,
             answer_options,

config/locales/en.yml

@@ -71,13 +71,25 @@ en:
           attributes:
             startdate:
               invalid: "Enter a date in the correct format, for example 1 9 2022"
-
             units:
               blank: "Enter total number of units at this location"
             type_of_unit:
               blank: "Select the most common type of unit at this location"
             mobility_type:
               blank: "Select the mobility standards for the majority of units in this location"
+        user:
+          attributes:
+            organisation_id:
+              blank: "Enter the existing organisation’s name"
+              invalid: "Enter the existing organisation’s name"
+            name:
+              blank: "Enter a name"
+            email:
+              invalid: "Enter an email address in the correct format, like name@example.com"
+              blank: "Enter an email address"
+            role:
+              invalid: "Role must be data accessor, data provider or data coordinator"
+              blank: "Select role"
 
   validations:
     organisation:

spec/requests/users_controller_spec.rb

@@ -935,6 +935,24 @@ RSpec.describe UsersController, type: :request do
           expect(page).to have_content(I18n.t("validations.role.invalid"))
         end
       end
+
+      context "when validating the required fields" do
+        let(:params) do
+          {
+            "user": {
+              name: "",
+              email: "",
+              role: "support",
+            },
+          }
+        end
+
+        it "shows an error" do
+          request
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("validations.role.invalid"))
+        end
+      end
     end
 
     describe "#new" do