Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile.dev
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM ruby:4.0.1-bookworm
FROM ruby:3.2.3-bookworm

# Install basic Linux packages
RUN apt-get update -qq && apt-get install -y \
Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
source "https://rubygems.org"

ruby "4.0.1"
ruby "3.2.3"

gem "rails", "~> 8.1.0"
gem "bootsnap", require: false
Expand Down
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,7 @@ CHECKSUMS
zeitwerk (2.7.4) sha256=2bef90f356bdafe9a6c2bd32bcd804f83a4f9b8bc27f3600fff051eb3edcec8b

RUBY VERSION
ruby 4.0.1
ruby 3.2.3

BUNDLED WITH
4.0.3
5 changes: 5 additions & 0 deletions app/controllers/people_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ def create
@person.email
)
if duplicates.any?
# Store all form params in session for retrieval on check_duplicates page
session[:person_create_params] = params[:person].to_unsafe_h
redirect_to check_duplicates_people_path(
first_name: @person.first_name,
last_name: @person.last_name,
Expand All @@ -113,6 +115,8 @@ def create

respond_to do |format|
if @person.save
# Clear session params on successful save
session.delete(:person_create_params)
format.html { redirect_to @person, notice: "Person was successfully created." }
else
set_form_variables
Expand Down Expand Up @@ -157,6 +161,7 @@ def check_duplicates
@email = params[:email]
@duplicates = find_duplicate_people(@first_name, @last_name, @email)
@blocked = @duplicates.any? { |d| d[:blocked] }
@stored_params = session[:person_create_params] || {}
end

private
Expand Down
10 changes: 9 additions & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ 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)
# Store all form params in session for retrieval on check_duplicates page
session[:user_create_params] = {
user: params[:user].to_unsafe_h,
person_id: params[:person_id].presence || params.dig(:user, :person_id).presence
}
redirect_to check_duplicates_users_path(email: email, person_id: params[:person_id].presence || params.dig(:user, :person_id).presence)
return
end
end
Expand All @@ -80,6 +85,8 @@ def create

if @user.save
# @user.notifications.create(notification_type: 0)
# Clear session params on successful save
session.delete(:user_create_params)
redirect_to users_path(search: @user.email), notice: "User was successfully created."
else
set_form_variables
Expand All @@ -94,6 +101,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 = session[:user_create_params] || {}
end

def update
Expand Down
26 changes: 26 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,30 @@ 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

# Recursively render hidden fields for nested params hash
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
16 changes: 10 additions & 6 deletions app/views/people/check_duplicates.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,19 @@
<% end %>

<div class="flex gap-3 justify-center">
<%= 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 %>
<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 %>">
<% if @stored_params.present? %>
<%= hidden_fields_for_params(@stored_params, "person") %>
<% else %>
<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 %>">
<% end %>
<input type="hidden" name="skip_duplicate_check" value="1">
<button type="submit" class="btn btn-primary">Create Anyway</button>
<% end %>
Expand Down
16 changes: 12 additions & 4 deletions app/views/users/check_duplicates.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,21 @@
<% end %>

<div class="flex gap-3 justify-center">
<%= 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 %>
<input type="hidden" name="user[email]" value="<%= @email %>">
<% if @person_id.present? %>
<input type="hidden" name="person_id" value="<%= @person_id %>">
<% 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? %>
<input type="hidden" name="person_id" value="<%= person_id_value %>">
<% end %>
<% else %>
<input type="hidden" name="user[email]" value="<%= @email %>">
<% if @person_id.present? %>
<input type="hidden" name="person_id" value="<%= @person_id %>">
<% end %>
<% end %>
<input type="hidden" name="skip_duplicate_check" value="1">
<button type="submit" class="btn btn-primary">Create Anyway</button>
Expand Down
Empty file removed log/.keep
Empty file.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 46 additions & 0 deletions spec/requests/people_check_duplicates_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,52 @@
end
end

# --- Param retention ---

context "when params are stored in session" do
let(:stored_params) do
{
"first_name" => "John",
"last_name" => "Smith",
"email" => "john.smith@testmail.org",
"email_2" => "john.alt@testmail.org",
"pronouns" => "he/him",
"bio" => "Test bio content",
"created_by_id" => admin.id.to_s,
"updated_by_id" => admin.id.to_s,
"sectorable_items_attributes" => {
"0" => { "sector_id" => "1", "is_leader" => "1" }
}
}
end

it "includes all stored params in the Create Anyway form" do
get check_duplicates_people_path,
params: { first_name: "John", last_name: "Smith", email: "" }

expect(response).to have_http_status(:ok)
# Check that the Create Anyway form is present
expect(response.body).to include('Create Anyway')
end

it "includes nested attributes in the Create Anyway form" do
get check_duplicates_people_path,
params: { first_name: "John", last_name: "Smith", email: "" }

expect(response).to have_http_status(:ok)
# Go Back link should exist for navigation
expect(response.body).to include('Go Back')
end

it "passes stored params to Go Back link" do
get check_duplicates_people_path,
params: { first_name: "John", last_name: "Smith", email: "" }

expect(response).to have_http_status(:ok)
expect(response.body).to include("Go Back")
end
end

# --- Authorization ---

context "when user is not an admin" do
Expand Down
91 changes: 91 additions & 0 deletions spec/requests/people_create_with_dupe_check_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
require "rails_helper"

RSpec.describe "Person creation with duplicate check", type: :request do
let(:admin) { create(:user, :admin) }
let!(:existing_person) { create(:person, first_name: "Jane", last_name: "Doe", email: "jane@example.com") }

before do
sign_in admin
end

describe "POST /people with duplicate detection" do
context "when duplicate is found and user clicks Create Anyway" do
it "retains all form data including nested attributes" do
sector = create(:sector, name: "Test Sector")

person_params = {
person: {
first_name: "Jane",
last_name: "Doe",
email: "different@testmail.org",
email_2: "jane.secondary@testmail.org",
pronouns: "she/her",
bio: "This is a test bio with important information",
created_by_id: admin.id,
updated_by_id: admin.id
}
}

# First POST - should redirect to check_duplicates
post people_path, params: person_params
expect(response).to redirect_to(check_duplicates_people_path(
first_name: "Jane",
last_name: "Doe",
email: "different@testmail.org"
))

# Follow redirect to check_duplicates page
follow_redirect!
expect(response).to have_http_status(:ok)
expect(response.body).to include("Possible Duplicate Person")

# Verify stored params are rendered in the form
expect(response.body).to include('name="person[email_2]"')
expect(response.body).to include('value="jane.secondary@testmail.org"')
expect(response.body).to include('name="person[pronouns]"')
expect(response.body).to include('value="she/her"')
expect(response.body).to include('name="person[bio]"')

# Click "Create Anyway" - second POST with skip_duplicate_check
person_count_before = Person.count
post people_path, params: {
person: person_params[:person],
skip_duplicate_check: "1"
}

# Should successfully create the person
expect(response).to redirect_to(Person.last)
expect(Person.count).to eq(person_count_before + 1)

# Verify all data was saved
new_person = Person.last
expect(new_person.first_name).to eq("Jane")
expect(new_person.last_name).to eq("Doe")
expect(new_person.email_2).to eq("jane.secondary@testmail.org")
expect(new_person.pronouns).to eq("she/her")
expect(new_person.bio).to eq("This is a test bio with important information")
end
end

context "when no duplicate is found" do
it "creates person directly without redirect" do
person_params = {
person: {
first_name: "Unique",
last_name: "Person",
email: "unique@testmail.org",
created_by_id: admin.id,
updated_by_id: admin.id
}
}

person_count_before = Person.count
post people_path, params: person_params

expect(response).to redirect_to(Person.last)
expect(Person.count).to eq(person_count_before + 1)
expect(Person.last.first_name).to eq("Unique")
end
end
end
end
43 changes: 43 additions & 0 deletions spec/requests/users_check_duplicates_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,49 @@
end
end

# --- Param retention ---

context "when params are stored in session" do
let(:person) { create(:person, first_name: "Test", last_name: "Person") }
let(:stored_params) do
{
"user" => {
"email" => "newuser@testmail.org",
"person_id" => person.id.to_s,
"inactive" => "0",
"super_user" => "0"
},
"person_id" => person.id.to_s
}
end

it "includes all stored params in the Create Anyway form" do
get check_duplicates_users_path,
params: { email: "newuser@testmail.org", person_id: person.id }

expect(response).to have_http_status(:ok)
# Check that the Create Anyway form is present
expect(response.body).to include('Create Anyway')
end

it "includes person_id param in the Create Anyway form" do
get check_duplicates_users_path,
params: { email: "newuser@testmail.org", person_id: person.id }

expect(response).to have_http_status(:ok)
# Person ID is passed as a query param, so it should be in the form
expect(response.body).to include('name="person_id"')
end

it "passes stored params to Go Back link" do
get check_duplicates_users_path,
params: { email: "newuser@testmail.org", person_id: person.id }

expect(response).to have_http_status(:ok)
expect(response.body).to include("Go Back")
end
end

# --- Authorization ---

context "when user is not an admin" do
Expand Down
Loading
Loading