Skip to content

Commit 14db3ac

Browse files
committed
Fix multiple issues with dynamic question rendering and answer cleanup
**Issue 1: Questions not rendering on initial page load** - Changed back to <%= form.fields_for %> (with =) to properly render questions - Initial page load now shows existing answers correctly **Issue 2: Escaped </div> appearing in dynamic content** - Added html.gsub to strip escaped HTML tags from AJAX response - Regex removes any &lt;/tag&gt; patterns that fields_for generates - Cleanly handles both server-rendered and AJAX-loaded content **Issue 3: Answers not deleted when options removed** - Added @person.options.reload in update_answers method - Ensures we calculate applicable_questions with fresh PersonOption data - Previously used stale association data, causing orphaned answers - Added controller test to verify answer deletion on option removal **Test Results:** - All 950 unit tests passing (0 failures, 0 errors) - All 122 system tests passing (0 failures, 0 errors) - New test: should delete answers when option is removed
1 parent 03ccca2 commit 14db3ac

File tree

3 files changed

+50
-5
lines changed

3 files changed

+50
-5
lines changed

app/controllers/people_controller.rb

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,14 @@ def get_questions
136136
end
137137

138138
@person.instance_variable_set(:@calculated_questions, Question.where(id: question_ids.to_a).ordered)
139-
140-
# Create a form builder-like object for the partial
141139
@form = ActionView::Helpers::FormBuilder.new(:person, @person, view_context, {})
142140

143-
render partial: 'people/questions_content', locals: { person: @person, form: @form }, layout: false
141+
html = render_to_string(partial: 'people/questions_content', locals: { person: @person, form: @form }, formats: [:html])
142+
143+
# Remove any escaped HTML tags that fields_for might generate as part of its return value
144+
html = html.gsub(/&lt;\/\w+&gt;/, '')
145+
146+
render html: html.html_safe
144147
end
145148

146149
def certificates
@@ -1182,6 +1185,16 @@ def update_options
11821185
end
11831186

11841187
def update_answers
1188+
# Reload options association to get the updated PersonOption records
1189+
@person.options.reload
1190+
1191+
# Calculate which questions are currently applicable based on package and options
1192+
applicable_question_ids = @person.applicable_questions.pluck(:id)
1193+
1194+
# Delete answers for questions that are no longer applicable
1195+
@person.answers.where.not(question_id: applicable_question_ids).destroy_all
1196+
1197+
# Process submitted answers
11851198
answers_data = person_params[:answers_attributes] || {}
11861199

11871200
# Handle both array and hash-like formats (fields_for creates hash with string keys)

app/views/people/_questions_content.html.erb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
<% applicable_questions.each do |question| %>
66
<% existing_answer = person.answers.find_by(question_id: question.id) %>
7-
<% answer_index = existing_answer&.id || "new_#{question.id}" %>
87

98
<div class="my-4">
109
<%= form.fields_for :answers, existing_answer || person.answers.build(question: question) do |answer_form| %>

test/controllers/people_controller_test.rb

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,9 +304,42 @@ class PeopleControllerTest < ActionDispatch::IntegrationTest
304304

305305
# Person should appear in the list since they have access through package
306306
assert_select "a", text: /Package, Person/
307-
307+
308308
# Person should be struck through since they don't have a PersonOption record (not seated at table)
309309
# The line-through class is on the td element, not the a element
310310
assert_select "td.line-through a", text: /Package, Person/
311311
end
312+
313+
test "should delete answers when option is removed" do
314+
person = people(:Kathryn)
315+
lunch_option = billables(:two)
316+
question = questions(:meal_choice)
317+
318+
# Kathryn already has answers in fixtures, verify they exist
319+
initial_answer_count = person.answers.count
320+
assert initial_answer_count > 0, "Person should have existing answers"
321+
assert person.answers.exists?(question: question), "Person should have answer for meal choice"
322+
323+
# Verify Kathryn has the lunch option via PersonOption
324+
assert person.options.exists?(option_id: lunch_option.id), "Person should have lunch option"
325+
326+
# Update person, removing all options (by passing empty options hash)
327+
patch person_url(person), params: {
328+
person: {
329+
name: person.name,
330+
studio_id: person.studio_id,
331+
type: person.type,
332+
level_id: person.level_id,
333+
age_id: person.age_id,
334+
role: person.role,
335+
options: {} # Empty options means all options are removed
336+
}
337+
}
338+
339+
assert_response :redirect
340+
341+
# Verify the answer was deleted because the option is no longer selected
342+
person.reload
343+
assert_equal 0, person.answers.count, "All answers should be deleted when options are removed"
344+
end
312345
end

0 commit comments

Comments
 (0)