Fix question/answer feature bugs - all tests passing

**Rails 8 params.expect() Issue:**
- Changed PeopleController person_params from params.expect() to params.require().permit()
  (same fix as BillablesController - nested attributes with dynamic keys don't work with expect)
- Fixed update_answers to handle ActionController::Parameters correctly (not a Hash)

**Dynamic Question Loading:**
- Created person_questions_controller.js Stimulus controller for AJAX updates
- Added PeopleController#get_questions action for dynamic question fetching
- Split questions partial into container (_questions.html.erb) and content (_questions_content.html.erb)
- Modified Person model to support pre-calculated questions for AJAX
- Connected package select and option checkboxes to trigger dynamic updates
- Only update on user interaction, not on initial page load (prevent form conflicts)

**Question Removal Fix:**
- Changed from rendering _destroy checkbox to creating _destroy field dynamically via JavaScript
- Prevents ActiveRecord from seeing questions as 'changed' when they're not
- Avoids triggering validation errors on unrelated questions
- JavaScript now creates hidden _destroy field only when Remove Question is clicked

**Form Field Fixes:**
- Fixed textarea label association so Capybara can find fields by label text
- Updated questions_controller.js removeQuestion to create field dynamically

**Test Improvements:**
- Added proper waits for dynamic content loading
- Used specific selectors for clicking Remove Question link
- Added assertions to verify JavaScript behavior (element hiding, field creation)
- All 13 question tests now passing (100% success rate, 39 assertions)

**Core Functionality Working:**
✅ Add questions to billable options
✅ Remove questions from options
✅ Questions appear dynamically when options/packages selected
✅ Answer radio button questions
✅ Answer textarea questions
✅ Save answers to database
✅ Summary views and PDF generation

Author: rubys

app/controllers/people_controller.rb

@@ -107,6 +107,42 @@ def invoice
     end
   end
 
+  def get_questions
+    person_id = params[:person_id]
+    package_id = params[:package_id]
+    option_ids = params[:option_ids] || []
+
+    # Create a temporary person object to calculate applicable questions
+    @person = if person_id.present? && person_id.to_i != 0
+                Person.find(person_id)
+              else
+                Person.new
+              end
+    @person.package_id = package_id if package_id.present?
+
+    # Calculate applicable questions based on package and selected options
+    question_ids = Set.new
+
+    if package_id.present? && (package = Billable.find_by(id: package_id))
+      package.package_includes.each do |pi|
+        question_ids.merge(pi.option.questions.pluck(:id))
+      end
+    end
+
+    option_ids.each do |option_id|
+      if option = Billable.find_by(id: option_id, type: 'Option')
+        question_ids.merge(option.questions.pluck(:id))
+      end
+    end
+
+    @person.instance_variable_set(:@calculated_questions, Question.where(id: question_ids.to_a).ordered)
+
+    # Create a form builder-like object for the partial
+    @form = ActionView::Helpers::FormBuilder.new(:person, @person, view_context, {})
+
+    render partial: 'people/questions_content', locals: { person: @person, form: @form }, layout: false
+  end
+
   def certificates
     if request.get?
       @studios = [['-- all studios --', nil]] + Studio.by_name.pluck(:name, :id)
@@ -882,8 +918,9 @@ def set_person
 
     # Only allow a list of trusted parameters through.
     def person_params
-      params.expect(person: [:name, :studio_id, :type, :back, :level_id, :age_id, :category, :role, :exclude_id, :package_id, :independent, :invoice_to_id, :available, :table_id, { options: {} },
-        { answers_attributes: [:id, :question_id, :answer_value] }])
+      params.require(:person).permit(:name, :studio_id, :type, :back, :level_id, :age_id, :category, :role, :exclude_id, :package_id, :independent, :invoice_to_id, :available, :table_id,
+        options: {},
+        answers_attributes: [:id, :question_id, :answer_value])
     end
 
     def filtered_params(person)
@@ -1145,9 +1182,13 @@ def update_options
     end
 
     def update_answers
-      answers_data = person_params[:answers_attributes] || []
+      answers_data = person_params[:answers_attributes] || {}
+
+      # Handle both array and hash-like formats (fields_for creates hash with string keys)
+      # ActionController::Parameters acts like a hash but isn't one
+      answers_collection = answers_data.respond_to?(:values) ? answers_data.values : answers_data
 
-      answers_data.each do |answer_attrs|
+      answers_collection.each do |answer_attrs|
         question_id = answer_attrs[:question_id]
         answer_value = answer_attrs[:answer_value]
 

app/javascript/controllers/person_questions_controller.js

@@ -0,0 +1,67 @@
+import { Controller } from "@hotwired/stimulus"
+
+// Connects to data-controller="person-questions"
+export default class extends Controller {
+  static targets = ["questionsContainer", "optionCheckbox", "packageSelect"]
+  static values = {
+    personId: Number,
+    url: String
+  }
+
+  connect() {
+    // Don't update on initial load - let the server-rendered content show
+    // Only update when package or options change
+  }
+
+  // Called when package dropdown changes
+  packageChanged() {
+    this.updateQuestions()
+  }
+
+  // Called when any option checkbox changes
+  optionChanged() {
+    this.updateQuestions()
+  }
+
+  updateQuestions() {
+    // Get selected package
+    const packageId = this.hasPackageSelectTarget ?
+      this.packageSelectTarget.value : null
+
+    // Get selected options (checkboxes that are checked and not disabled)
+    const selectedOptions = []
+    this.optionCheckboxTargets.forEach(checkbox => {
+      if (checkbox.checked && !checkbox.disabled) {
+        // Extract option ID from name like "person[options][123]"
+        const match = checkbox.name.match(/\[options\]\[(\d+)\]/)
+        if (match) {
+          selectedOptions.push(match[1])
+        }
+      }
+    })
+
+    // Fetch questions for these selections
+    this.fetchQuestions(packageId, selectedOptions)
+  }
+
+  fetchQuestions(packageId, optionIds) {
+    const params = new URLSearchParams()
+    if (packageId) params.append('package_id', packageId)
+    optionIds.forEach(id => params.append('option_ids[]', id))
+    params.append('person_id', this.personIdValue)
+
+    fetch(`${this.urlValue}?${params.toString()}`, {
+      headers: {
+        'Accept': 'text/html',
+        'X-Requested-With': 'XMLHttpRequest'
+      }
+    })
+    .then(response => response.text())
+    .then(html => {
+      this.questionsContainerTarget.innerHTML = html
+    })
+    .catch(error => {
+      console.error('Error fetching questions:', error)
+    })
+  }
+}

app/javascript/controllers/questions_controller.js

@@ -62,12 +62,17 @@ export default class extends Controller {
 
   removeQuestion(event) {
     event.preventDefault()
-    const questionDiv = event.target.closest('[data-questions-target="question"]')
-    const destroyField = questionDiv.querySelector('[data-questions-target="destroyField"]')
+    const link = event.target.closest('a')
+    const questionDiv = link.closest('[data-questions-target="question"]')
+    const questionName = link.dataset.questionName
 
-    if (destroyField) {
-      // Question already exists in database, mark for destruction
-      destroyField.value = "1"
+    if (questionName) {
+      // Question already exists in database, create _destroy field and mark for destruction
+      const destroyField = document.createElement('input')
+      destroyField.type = 'hidden'
+      destroyField.name = `${questionName}[_destroy]`
+      destroyField.value = '1'
+      questionDiv.appendChild(destroyField)
       questionDiv.style.display = "none"
     } else {
       // New question, just remove from DOM

app/models/person.rb

@@ -182,6 +182,9 @@ def eligible_heats(start_times)
 
   # Get all questions for this person based on their package and selected options
   def applicable_questions
+    # Return pre-calculated questions if available (used for dynamic updates)
+    return @calculated_questions if defined?(@calculated_questions)
+
     question_ids = Set.new
 
     # Questions from package

app/views/billables/_form.html.erb

@@ -101,10 +101,8 @@
           <div class="mt-3">
             <%= link_to "Remove Question", "#",
               class: "text-red-600 hover:text-red-800",
-              data: { action: "click->questions#removeQuestion" } %>
-            <% if question_form.object.persisted? %>
-              <%= question_form.check_box :_destroy, class: "hidden", data: { questions_target: "destroyField" } %>
-            <% end %>
+              data: { action: "click->questions#removeQuestion",
+                      question_name: (question_form.object.persisted? ? question_form.object_name : nil) } %>
           </div>
         </div>
       <% end %>

app/views/people/_form.html.erb

@@ -1,4 +1,9 @@
-<%= form_with(model: person, class: "contents", id: 'person-form', data: {controller: "person", id: JSON.dump(person.id) }) do |form| %>
+<%= form_with(model: person, class: "contents", id: 'person-form', data: {
+  controller: "person person-questions",
+  person_questions_person_id_value: person.id,
+  person_questions_url_value: get_questions_people_path,
+  id: JSON.dump(person.id)
+}) do |form| %>
   <% if person.errors.any? %>
     <div id="error_explanation" class="bg-red-50 text-red-500 px-3 py-2 font-medium rounded-lg mt-3">
       <h2><%= pluralize(person.errors.count, "error") %> prohibited this person from being saved:</h2>

app/views/people/_options.html.erb

@@ -30,7 +30,8 @@
     <%= check_box_tag "person[options][#{option.id}]", '1', true, class: "option-checkbox entry-count h-10 opacity-70", disabled: true %>
     <% elsif value <= 1 %>
     <input name="person[options][<%= option.id %>]" type="hidden" value="0" autocomplete="off">
-    <%= check_box_tag "#{field}", '1', value > 0, class: "option-checkbox entry-count h-10" %>
+    <%= check_box_tag "#{field}", '1', value > 0, class: "option-checkbox entry-count h-10",
+        data: { "person-questions-target": "optionCheckbox", action: "change->person-questions#optionChanged" } %>
     <% else %>
     <input type="text" id="<%= field %>" name="<%= field %>" class="entry-count" value=<%= value %>>
     <% end %>

app/views/people/_package.html.erb

@@ -2,7 +2,12 @@
 <% unless @packages.empty? %>
   <%= label_tag :person_package_id, 'Package' %>
   <%= select_tag 'person[package_id]', options_for_select(@packages, @person.package_id || ''), {
-    data: {"person-target" => "package", 'action' => 'person#setPackage', 'url' => package_people_path},
+    data: {
+      "person-target" => "package",
+      "person-questions-target" => "packageSelect",
+      'action' => 'person#setPackage person-questions#packageChanged',
+      'url' => package_people_path
+    },
     class: "mx-auto block shadow rounded-md border border-gray-200 outline-none px-3 py-2 mt-2 w-full"} %>
 <% end %>
 </div>

app/views/people/_questions.html.erb

@@ -1,37 +1,3 @@
-<div class="my-5" id="questions-section">
-  <% applicable_questions = @person.applicable_questions %>
-  <% if applicable_questions.any? %>
-    <h3 class="font-bold text-xl mb-4">Questions</h3>
-
-    <% applicable_questions.each do |question| %>
-      <% existing_answer = @person.answers.find_by(question_id: question.id) %>
-      <% answer_index = existing_answer&.id || "new_#{question.id}" %>
-
-      <div class="my-4">
-        <%= form.fields_for :answers, existing_answer || @person.answers.build(question: question) do |answer_form| %>
-          <%= answer_form.hidden_field :question_id, value: question.id %>
-
-          <label class="block font-medium mb-2"><%= question.question_text %></label>
-
-          <% if question.question_type == 'radio' %>
-            <div class="space-y-2">
-              <% question.choices.each do |choice| %>
-                <label class="flex items-center">
-                  <%= answer_form.radio_button :answer_value, choice,
-                    checked: existing_answer&.answer_value == choice,
-                    class: "mr-2" %>
-                  <%= choice %>
-                </label>
-              <% end %>
-            </div>
-          <% else %>
-            <%= answer_form.text_area :answer_value,
-              rows: 3,
-              placeholder: "Enter your answer...",
-              class: "block shadow rounded-md border border-gray-200 outline-none px-3 py-2 mt-2 w-full" %>
-          <% end %>
-        <% end %>
-      </div>
-    <% end %>
-  <% end %>
+<div class="my-5" id="questions-section" data-person-questions-target="questionsContainer">
+  <%= render partial: 'people/questions_content', locals: { person: @person, form: form } %>
 </div>

app/views/people/_questions_content.html.erb

@@ -0,0 +1,35 @@
+<% applicable_questions = person.applicable_questions %>
+<% if applicable_questions.any? %>
+  <h3 class="font-bold text-xl mb-4">Questions</h3>
+
+  <% applicable_questions.each do |question| %>
+    <% existing_answer = person.answers.find_by(question_id: question.id) %>
+    <% answer_index = existing_answer&.id || "new_#{question.id}" %>
+
+    <div class="my-4">
+      <%= form.fields_for :answers, existing_answer || person.answers.build(question: question) do |answer_form| %>
+        <%= answer_form.hidden_field :question_id, value: question.id %>
+
+        <% if question.question_type == 'radio' %>
+          <div class="space-y-2">
+            <label class="block font-medium mb-2"><%= question.question_text %></label>
+            <% question.choices.each do |choice| %>
+              <label class="flex items-center">
+                <%= answer_form.radio_button :answer_value, choice,
+                  checked: existing_answer&.answer_value == choice,
+                  class: "mr-2" %>
+                <%= choice %>
+              </label>
+            <% end %>
+          </div>
+        <% else %>
+          <%= answer_form.label :answer_value, question.question_text, class: "block font-medium mb-2" %>
+          <%= answer_form.text_area :answer_value,
+            rows: 3,
+            placeholder: "Enter your answer...",
+            class: "block shadow rounded-md border border-gray-200 outline-none px-3 py-2 mt-2 w-full" %>
+        <% end %>
+      <% end %>
+    </div>
+  <% end %>
+<% end %>