Fix duplicate check losing form params and nested attributes

app/controllers/people_controller.rb

@@ -75,6 +75,13 @@ def new
     set_form_variables
   end
 
+  def retry_new
+    @person = Person.new(person_params.except(:user_attributes))
+    authorize! @person
+    set_form_variables
+    render :new
+  end
+
   def edit
     @person = Person.includes(
       :user,
@@ -99,17 +106,20 @@ def create
     end
 
     unless params[:skip_duplicate_check].present?
-      duplicates = find_duplicate_people(
+      @duplicates = find_duplicate_people(
         @person.first_name,
         @person.last_name,
         @person.email
       )
-      if duplicates.any?
-        redirect_to check_duplicates_people_path(
-          first_name: @person.first_name,
-          last_name: @person.last_name,
-          email: @person.email
-        )
+      if @duplicates.any?
+        @first_name = @person.first_name
+        @last_name = @person.last_name
+        @email = @person.email
+        @blocked = @duplicates.any? { |d| d[:blocked] }
+        raw_params = params[:person]&.to_unsafe_h || {}
+        @had_avatar = raw_params[:avatar].is_a?(ActionDispatch::Http::UploadedFile)
+        @stored_params = raw_params
+        render :check_duplicates
         return
       end
     end
@@ -162,6 +172,7 @@ def check_duplicates
     @email = params[:email]
     @duplicates = find_duplicate_people(@first_name, @last_name, @email)
     @blocked = @duplicates.any? { |d| d[:blocked] }
+    @stored_params = { first_name: @first_name, last_name: @last_name, email: @email }
   end
 
   private

app/controllers/users_controller.rb

@@ -61,6 +61,13 @@ def new
     set_form_variables
   end
 
+  def retry_new
+    @user = User.new(user_params)
+    authorize! @user
+    set_form_variables
+    render :new
+  end
+
   def edit
     @user = User.includes(comments: [ :created_by, :updated_by ]).find(params[:id])
     authorize! @user
@@ -73,12 +80,14 @@ def create
 
     # Check for duplicate email before saving
     unless params[:skip_duplicate_check].present?
-      email = @user.email
-      if email.present? && !email.downcase.end_with?("@example.com")
-        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 = @user.email
+      if @email.present? && !@email.downcase.end_with?("@example.com")
+        @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?
+          @blocked = @duplicates.any? { |d| d[:blocked] }
+          @stored_params = params[:user]&.to_unsafe_h || {}
+          render :check_duplicates
           return
         end
       end
@@ -112,6 +121,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 = { email: @email }
   end
 
   def update

app/frontend/javascript/controllers/cocoon_controller.js

@@ -173,7 +173,6 @@ function globalClickHandler(e) {
 if (!globalListenerInstalled) {
   document.addEventListener("click", globalClickHandler, true);
   globalListenerInstalled = true;
-  console.log("[cocoon] click handler installed");
 }
 
 export default class extends Controller {

app/helpers/application_helper.rb

@@ -203,6 +203,33 @@ def select_caret_onchange
     "if(this.value){this.classList.remove('select-placeholder')}else{this.classList.add('select-placeholder')}"
   end
 
+  def hidden_fields_for_params(hash, prefix = nil)
+    return "".html_safe if hash.blank?
+
+    fields = []
+    hash.each do |key, value|
+      field_name = prefix ? "#{prefix}[#{key}]" : key.to_s
+      case value
+      when ActionDispatch::Http::UploadedFile
+        next
+      when Hash
+        fields << hidden_fields_for_params(value, field_name)
+      when Array
+        value.each_with_index do |item, i|
+          if item.is_a?(Hash)
+            fields << hidden_fields_for_params(item, "#{field_name}[#{i}]")
+          else
+            fields << tag.input(type: "hidden", name: "#{field_name}[]", value: item)
+          end
+        end
+      else
+        next if key.to_s == "id" && value.blank?
+        fields << tag.input(type: "hidden", name: field_name, value: value)
+      end
+    end
+    safe_join(fields)
+  end
+
   def us_time_zone_fundamentals
     zone_names = [
       "Eastern Time (US & Canada)",

app/views/contact_methods/_contact_method_fields.html.erb

@@ -1,6 +1,5 @@
-<%= f.hidden_field :kind, value: :phone %>
-
 <div class="nested-fields rounded-lg bg-white border border-gray-200 p-6 mb-4">
+  <%= f.hidden_field :kind, value: :phone %>
   <div class="grid grid-cols-1 md:grid-cols-5 gap-4 mb-6">
     <!-- Contact Type -->
     <div>

app/views/people/_form.html.erb

@@ -173,6 +173,7 @@
         <%= link_to_add_association "➕ Add phone",
                                     f,
                                     :contact_methods,
+                                    partial: "contact_methods/contact_method_fields",
                                     class: "btn btn-secondary-outline" %>
       </div>
     </div>

app/views/people/check_duplicates.html.erb

@@ -84,20 +84,36 @@
         </p>
       <% end %>
 
-      <div class="flex gap-3 justify-center">
-        <%= link_to "Go back", "javascript:history.back()", class: "btn btn-secondary-outline" %>
+      <% if @had_avatar %>
+        <p class="text-sm text-gray-500 bg-gray-50 border border-gray-200 rounded-lg px-4 py-3 mb-6">
+          The photo you uploaded will need to be re-added after creating this person.
+        </p>
+      <% end %>
 
-        <% unless @blocked %>
-          <%= form_with url: people_path, method: :post, data: { turbo: false } do %>
-            <input type="hidden" name="person[first_name]" value="<%= @first_name %>">
-            <input type="hidden" name="person[last_name]" value="<%= @last_name %>">
-            <input type="hidden" name="person[email]" value="<%= @email %>">
-            <input type="hidden" name="person[created_by_id]" value="<%= current_user.id %>">
-            <input type="hidden" name="person[updated_by_id]" value="<%= current_user.id %>">
-            <input type="hidden" name="skip_duplicate_check" value="1">
-            <button type="submit" class="btn btn-primary">Create anyway</button>
+      <div class="flex flex-col items-center gap-3">
+        <div class="flex gap-3">
+          <%= form_with url: retry_new_people_path, method: :post, data: { turbo: false } do %>
+            <%= hidden_fields_for_params(@stored_params, "person") %>
+            <button type="submit" class="btn btn-secondary-outline">Go back</button>
           <% end %>
-        <% end %>
+
+          <% unless @blocked %>
+            <%= form_with url: people_path, method: :post, data: { turbo: false } do %>
+              <%= hidden_fields_for_params(@stored_params, "person") %>
+              <input type="hidden" name="person[created_by_id]" value="<%= current_user.id %>">
+              <input type="hidden" name="person[updated_by_id]" value="<%= current_user.id %>">
+              <input type="hidden" name="skip_duplicate_check" value="1">
+              <button type="submit" class="btn btn-primary">Create anyway</button>
+            <% end %>
+          <% end %>
+        </div>
+
+        <% shareable_url = check_duplicates_people_url(first_name: @first_name, last_name: @last_name, email: @email) %>
+        <%= mail_to "",
+              "Ask a colleague",
+              subject: "Possible duplicate person: #{@first_name} #{@last_name}",
+              body: "I'm trying to add a new person and found a possible duplicate. Can you take a look?\n\n#{shareable_url}",
+              class: "text-sm text-gray-500 hover:text-blue-600 underline" %>
       </div>
     </div>
   </div>

app/views/users/check_duplicates.html.erb

@@ -66,19 +66,34 @@
         </p>
       <% end %>
 
-      <div class="flex gap-3 justify-center">
-        <%= link_to "Go back", "javascript:history.back()", class: "btn btn-secondary-outline" %>
-
-        <% unless @blocked %>
-          <%= form_with url: users_path, method: :post, data: { turbo: false } do %>
-            <input type="hidden" name="user[email]" value="<%= @email %>">
+      <div class="flex flex-col items-center gap-3">
+        <div class="flex gap-3">
+          <%= form_with url: retry_new_users_path, method: :post, data: { turbo: false } do %>
+            <%= hidden_fields_for_params(@stored_params, "user") %>
             <% if @person_id.present? %>
               <input type="hidden" name="person_id" value="<%= @person_id %>">
             <% end %>
-            <input type="hidden" name="skip_duplicate_check" value="1">
-            <button type="submit" class="btn btn-primary">Create anyway</button>
+            <button type="submit" class="btn btn-secondary-outline">Go back</button>
           <% end %>
-        <% end %>
+
+          <% unless @blocked %>
+            <%= form_with url: users_path, method: :post, data: { turbo: false } do %>
+              <%= hidden_fields_for_params(@stored_params, "user") %>
+              <% if @person_id.present? %>
+                <input type="hidden" name="person_id" value="<%= @person_id %>">
+              <% end %>
+              <input type="hidden" name="skip_duplicate_check" value="1">
+              <button type="submit" class="btn btn-primary">Create anyway</button>
+            <% end %>
+          <% end %>
+        </div>
+
+        <% shareable_url = check_duplicates_users_url(email: @email, person_id: @person_id) %>
+        <%= mail_to "",
+              "Ask a colleague",
+              subject: "Possible duplicate user: #{@email}",
+              body: "I'm trying to add a new user and found a possible duplicate. Can you take a look?\n\n#{shareable_url}",
+              class: "text-sm text-gray-500 hover:text-blue-600 underline" %>
       </div>
     </div>
   </div>

config/routes.rb

@@ -35,6 +35,7 @@
   resources :users, only: [ :new, :index, :show, :edit, :update, :create, :destroy ] do
     collection do
       get :check_duplicates
+      post :retry_new
     end
     member do
       post :send_reset_password_instructions
@@ -111,6 +112,7 @@
   resources :people do
     collection do
       get :check_duplicates
+      post :retry_new
     end
     resources :comments, only: [ :index, :create ]
   end

spec/requests/people_create_with_dupe_check_spec.rb

@@ -0,0 +1,120 @@
+require "rails_helper"
+
+RSpec.describe "POST /people with duplicate check", type: :request do
+  let(:admin) { create(:user, :admin) }
+  let!(:existing_person) { create(:person, first_name: "Jane", last_name: "Doe", email: "jane.doe@example.com") }
+
+  before { sign_in admin }
+
+  context "when duplicates are found" do
+    it "renders the check_duplicates page instead of redirecting" do
+      post people_path, params: {
+        person: { first_name: "Jane", last_name: "Doe", email: "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 "preserves all form params in hidden fields for Create anyway" do
+      post people_path, params: {
+        person: {
+          first_name: "Jane",
+          last_name: "Doe",
+          email: "new@testmail.org",
+          bio: "Test bio content",
+          created_by_id: admin.id,
+          updated_by_id: admin.id
+        }
+      }
+
+      expect(response).to have_http_status(:ok)
+      expect(response.body).to include("Test bio content")
+      expect(response.body).to include("skip_duplicate_check")
+    end
+
+    it "creates the person with all params when Create anyway is submitted" do
+      expect {
+        post people_path, params: {
+          skip_duplicate_check: "1",
+          person: {
+            first_name: "Jane",
+            last_name: "Doe",
+            email: "new@testmail.org",
+            bio: "Full bio preserved",
+            created_by_id: admin.id,
+            updated_by_id: admin.id
+          }
+        }
+      }.to change(Person, :count).by(1)
+
+      created = Person.last
+      expect(created.first_name).to eq("Jane")
+      expect(created.last_name).to eq("Doe")
+      expect(created.bio).to eq("Full bio preserved")
+    end
+
+    it "creates nested attributes when Create anyway is submitted" do
+      sector = create(:sector, published: true)
+
+      expect {
+        post people_path, params: {
+          skip_duplicate_check: "1",
+          person: {
+            first_name: "Jane",
+            last_name: "Doe",
+            email: "new@testmail.org",
+            created_by_id: admin.id,
+            updated_by_id: admin.id,
+            sectorable_items_attributes: { "0" => { sector_id: sector.id, is_leader: "1", _destroy: "false" } }
+          }
+        }
+      }.to change(Person, :count).by(1)
+
+      created = Person.last
+      expect(created.sectorable_items.count).to eq(1)
+      expect(created.sectorable_items.first.sector).to eq(sector)
+      expect(created.sectorable_items.first.is_leader).to be true
+    end
+  end
+
+  describe "POST /people/retry_new" do
+    let!(:sector) { create(:sector, published: true) }
+
+    it "re-renders the new form with nested attributes preserved" do
+      post retry_new_people_path, params: {
+        person: {
+          first_name: "Jane",
+          last_name: "Doe",
+          email: "new@testmail.org",
+          bio: "Test bio",
+          created_by_id: admin.id,
+          updated_by_id: admin.id,
+          sectorable_items_attributes: { "0" => { sector_id: sector.id, is_leader: "1", _destroy: "false" } }
+        }
+      }
+
+      expect(response).to have_http_status(:ok)
+      expect(response.body).to include("Jane")
+      expect(response.body).to include("Doe")
+      expect(response.body).to include(sector.name)
+    end
+  end
+
+  context "when no duplicates are found" do
+    it "creates the person normally" do
+      expect {
+        post people_path, params: {
+          person: {
+            first_name: "Unique",
+            last_name: "Name",
+            email: "unique@testmail.org",
+            created_by_id: admin.id,
+            updated_by_id: admin.id
+          }
+        }
+      }.to change(Person, :count).by(1)
+    end
+  end
+end