diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index b8d7128bd..145377ea7 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -105,11 +105,13 @@ def create @person.email ) if duplicates.any? - redirect_to check_duplicates_people_path( - first_name: @person.first_name, - last_name: @person.last_name, - email: @person.email - ) + @first_name = @person.first_name + @last_name = @person.last_name + @email = @person.email + @duplicates = duplicates + @blocked = duplicates.any? { |d| d[:blocked] } + @stored_params = params[:person].to_unsafe_h + render :check_duplicates return end end @@ -162,6 +164,7 @@ def check_duplicates @email = params[:email] @duplicates = find_duplicate_people(@first_name, @last_name, @email) @blocked = @duplicates.any? { |d| d[:blocked] } + @stored_params = {} end private diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 966402b7f..c276dfddb 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -78,7 +78,15 @@ def create person_id = params[:person_id].presence || params.dig(:user, :person_id).presence || @user.person_id duplicates = find_duplicate_users(email, exclude_person_id: person_id) if duplicates.any? - redirect_to check_duplicates_users_path(email: email, person_id: person_id) + @email = email + @person_id = person_id + @duplicates = duplicates + @blocked = duplicates.any? { |d| d[:blocked] } + @stored_params = { + "user" => params[:user].to_unsafe_h, + "person_id" => params[:person_id].presence || params.dig(:user, :person_id).presence + } + render :check_duplicates return end end @@ -112,6 +120,7 @@ def check_duplicates @person_id = params[:person_id] @duplicates = find_duplicate_users(@email, exclude_person_id: @person_id) @blocked = @duplicates.any? { |d| d[:blocked] } + @stored_params = {} end def update diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f477fb141..69650b4ba 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -215,4 +215,29 @@ def us_time_zone_fundamentals ] ActiveSupport::TimeZone.us_zones.select { |z| zone_names.include?(z.name) }.sort_by { |z| zone_names.index(z.name) }.map { |z| [ z.to_s, z.name ] } end + + def hidden_fields_for_params(params_hash, prefix = nil) + return "".html_safe if params_hash.blank? + + fields = [] + params_hash.each do |key, value| + field_name = prefix ? "#{prefix}[#{key}]" : key.to_s + + if value.is_a?(Hash) + fields << hidden_fields_for_params(value, field_name) + elsif value.is_a?(Array) + value.each_with_index do |item, index| + if item.is_a?(Hash) + fields << hidden_fields_for_params(item, "#{field_name}[#{index}]") + else + fields << tag(:input, type: "hidden", name: "#{field_name}[]", value: item) + end + end + else + fields << tag(:input, type: "hidden", name: field_name, value: value) + end + end + + safe_join(fields) + end end diff --git a/app/views/people/check_duplicates.html.erb b/app/views/people/check_duplicates.html.erb index 1c465e743..df25a326a 100644 --- a/app/views/people/check_duplicates.html.erb +++ b/app/views/people/check_duplicates.html.erb @@ -85,15 +85,19 @@ <% end %>
- <%= link_to "Go back", "javascript:history.back()", class: "btn btn-secondary-outline" %> + <%= link_to "Go back", new_person_path(person: @stored_params), class: "btn btn-secondary-outline" %> <% unless @blocked %> <%= form_with url: people_path, method: :post, data: { turbo: false } do %> - - - - - + <% if @stored_params.present? %> + <%= hidden_fields_for_params(@stored_params, "person") %> + <% else %> + + + + + + <% end %> <% end %> diff --git a/app/views/users/check_duplicates.html.erb b/app/views/users/check_duplicates.html.erb index 77cc8b657..a50e787b9 100644 --- a/app/views/users/check_duplicates.html.erb +++ b/app/views/users/check_duplicates.html.erb @@ -67,13 +67,21 @@ <% end %>
- <%= link_to "Go back", "javascript:history.back()", class: "btn btn-secondary-outline" %> + <%= link_to "Go back", new_user_path(@stored_params), class: "btn btn-secondary-outline" %> <% unless @blocked %> <%= form_with url: users_path, method: :post, data: { turbo: false } do %> - - <% if @person_id.present? %> - + <% if @stored_params.present? && @stored_params["user"].present? %> + <%= hidden_fields_for_params(@stored_params.fetch("user", {}), "user") %> + <% person_id_value = @stored_params["person_id"].presence || @person_id %> + <% if person_id_value.present? %> + + <% end %> + <% else %> + + <% if @person_id.present? %> + + <% end %> <% end %> diff --git a/spec/requests/people_create_spec.rb b/spec/requests/people_create_spec.rb index 7623eb185..aeec4b65c 100644 --- a/spec/requests/people_create_spec.rb +++ b/spec/requests/people_create_spec.rb @@ -59,6 +59,71 @@ end end + describe "duplicate check on create" do + let!(:existing_person) { create(:person, first_name: "Jane", last_name: "Doe", email: "jane.doe@example.com") } + + it "renders check_duplicates when a duplicate is found" do + post people_path, params: { + person: { + first_name: "Jane", + last_name: "Doe", + email: "jane.new@testmail.org", + created_by_id: admin.id, + updated_by_id: admin.id + } + } + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Possible duplicate person") + expect(response.body).to include("Jane Doe") + end + + it "does not create the person when duplicates are found" do + expect { + post people_path, params: { + person: { + first_name: "Jane", + last_name: "Doe", + email: "jane.new@testmail.org", + created_by_id: admin.id, + updated_by_id: admin.id + } + } + }.not_to change(Person, :count) + end + + it "includes form params as hidden fields for Create anyway" do + post people_path, params: { + person: { + first_name: "Jane", + last_name: "Doe", + email: "jane.new@testmail.org", + email_2: "jane.alt@testmail.org", + created_by_id: admin.id, + updated_by_id: admin.id + } + } + + expect(response.body).to include("jane.alt@testmail.org") + expect(response.body).to include("skip_duplicate_check") + end + + it "creates the person when skip_duplicate_check is set" do + expect { + post people_path, params: { + skip_duplicate_check: "1", + person: { + first_name: "Jane", + last_name: "Doe", + email: "jane.new@testmail.org", + created_by_id: admin.id, + updated_by_id: admin.id + } + } + }.to change(Person, :count).by(1) + end + end + describe "user_attributes are applied on create" do it "updates the user's time_zone from the person form" do user = create(:user, time_zone: "Hawaii") diff --git a/spec/requests/users_create_spec.rb b/spec/requests/users_create_spec.rb new file mode 100644 index 000000000..83587f7d2 --- /dev/null +++ b/spec/requests/users_create_spec.rb @@ -0,0 +1,69 @@ +require "rails_helper" + +RSpec.describe "POST /users", type: :request do + let(:admin) { create(:user, :admin) } + + before { sign_in admin } + + describe "duplicate check on create" do + let!(:existing_person) do + Person.create!( + first_name: "Solo", last_name: "Person", + email: "solo@testmail.org", + created_by: admin, updated_by: admin + ) + end + + it "renders check_duplicates when a duplicate is found" do + post users_path, params: { + user: { + email: "solo@testmail.org", + first_name: "New", + last_name: "User" + } + } + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Possible duplicate user") + expect(response.body).to include("solo@testmail.org") + end + + it "does not create the user when duplicates are found" do + expect { + post users_path, params: { + user: { + email: "solo@testmail.org", + first_name: "New", + last_name: "User" + } + } + }.not_to change(User, :count) + end + + it "includes form params as hidden fields for Create anyway" do + post users_path, params: { + user: { + email: "solo@testmail.org", + first_name: "New", + last_name: "User" + } + } + + expect(response.body).to include("solo@testmail.org") + expect(response.body).to include("skip_duplicate_check") + end + + it "creates the user when skip_duplicate_check is set" do + expect { + post users_path, params: { + skip_duplicate_check: "1", + user: { + email: "solo@testmail.org", + first_name: "New", + last_name: "User" + } + } + }.to change(User, :count).by(1) + end + end +end