From a4e1f1c637a4c2a71c6fa2596d3bfbd1eafb5e88 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Mon, 6 Apr 2026 19:35:10 -0500 Subject: [PATCH 01/20] Add per-row permission flags and action metadata to CaseContactDatatable Co-Authored-By: claude-sonnet-4-6 --- .../case_contacts_new_design_controller.rb | 2 +- app/datatables/case_contact_datatable.rb | 16 ++++- .../datatables/case_contact_datatable_spec.rb | 64 ++++++++++++++++++- .../case_contacts_new_design_spec.rb | 62 ++++++++++++++++++ 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/app/controllers/case_contacts/case_contacts_new_design_controller.rb b/app/controllers/case_contacts/case_contacts_new_design_controller.rb index 81e6a2b4a4..1900cf7334 100644 --- a/app/controllers/case_contacts/case_contacts_new_design_controller.rb +++ b/app/controllers/case_contacts/case_contacts_new_design_controller.rb @@ -10,7 +10,7 @@ def index def datatable authorize CaseContact case_contacts = policy_scope(current_organization.case_contacts) - datatable = CaseContactDatatable.new case_contacts, params + datatable = CaseContactDatatable.new(case_contacts, params, current_user) render json: datatable end diff --git a/app/datatables/case_contact_datatable.rb b/app/datatables/case_contact_datatable.rb index 7975c16407..b985aa8088 100644 --- a/app/datatables/case_contact_datatable.rb +++ b/app/datatables/case_contact_datatable.rb @@ -8,10 +8,20 @@ class CaseContactDatatable < ApplicationDatatable duration_minutes ].freeze + def initialize(base_relation, params, current_user) + super(base_relation, params) + @current_user = current_user + end + private + attr_reader :current_user + def data records.map do |case_contact| + policy = CaseContactPolicy.new(current_user, case_contact) + requested_followup = case_contact.followups.find(&:requested?) + { id: case_contact.id, occurred_at: I18n.l(case_contact.occurred_at, format: :full, default: nil), @@ -35,7 +45,11 @@ def data .map { |a| {question: a.contact_topic&.question, value: a.value} }, notes: case_contact.notes.presence, is_draft: !case_contact.active?, - has_followup: case_contact.followups.requested.exists? + has_followup: requested_followup.present?, + can_edit: policy.update?, + can_destroy: policy.destroy?, + edit_path: Rails.application.routes.url_helpers.edit_case_contact_path(case_contact), + followup_id: requested_followup&.id } end end diff --git a/spec/datatables/case_contact_datatable_spec.rb b/spec/datatables/case_contact_datatable_spec.rb index a68ec02be8..8ef0a960f2 100644 --- a/spec/datatables/case_contact_datatable_spec.rb +++ b/spec/datatables/case_contact_datatable_spec.rb @@ -30,7 +30,9 @@ let(:order_direction) { "desc" } let(:base_relation) { organization.case_contacts } - subject(:datatable) { described_class.new(base_relation, params) } + let(:current_user) { create(:casa_admin, casa_org: organization) } + + subject(:datatable) { described_class.new(base_relation, params, current_user) } describe "#data" do let!(:case_contact) do @@ -105,6 +107,66 @@ expect(contact_data[:is_draft]).to eq((!case_contact.active?).to_s) end + context "with action metadata" do + it "includes edit_path for the contact" do + contact_data = datatable.as_json[:data].first + expected_path = Rails.application.routes.url_helpers.edit_case_contact_path(case_contact) + expect(contact_data[:edit_path]).to eq(expected_path) + end + + it "includes followup_id as empty string when no requested followup exists" do + contact_data = datatable.as_json[:data].first + expect(contact_data[:followup_id]).to eq("") + end + + it "includes followup_id when a requested followup exists" do + followup = create(:followup, case_contact: case_contact, status: "requested") + contact_data = datatable.as_json[:data].first + expect(contact_data[:followup_id]).to eq(followup.id.to_s) + end + + it "does not include followup_id for a resolved followup" do + create(:followup, case_contact: case_contact, status: "resolved") + contact_data = datatable.as_json[:data].first + expect(contact_data[:followup_id]).to eq("") + end + end + + context "with permission flags" do + context "when current_user is an admin" do + it "sets can_edit to true" do + expect(datatable.as_json[:data].first[:can_edit]).to eq("true") + end + + it "sets can_destroy to true" do + expect(datatable.as_json[:data].first[:can_destroy]).to eq("true") + end + end + + context "when current_user is the volunteer who created the contact" do + let(:current_user) { volunteer } + + it "sets can_edit to true" do + expect(datatable.as_json[:data].first[:can_edit]).to eq("true") + end + + it "sets can_destroy to false for an active contact" do + expect(datatable.as_json[:data].first[:can_destroy]).to eq("false") + end + end + + context "when current_user is the volunteer who created a draft contact" do + let(:current_user) { volunteer } + let!(:case_contact) do + create(:case_contact, casa_case: casa_case, creator: volunteer, status: "started") + end + + it "sets can_destroy to true for their own draft" do + expect(datatable.as_json[:data].first[:can_destroy]).to eq("true") + end + end + end + context "when case_contact has no casa_case (draft)" do let!(:draft_contact) do build(:case_contact, diff --git a/spec/requests/case_contacts/case_contacts_new_design_spec.rb b/spec/requests/case_contacts/case_contacts_new_design_spec.rb index 16f085cc2d..6f4348ce78 100644 --- a/spec/requests/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/requests/case_contacts/case_contacts_new_design_spec.rb @@ -264,6 +264,68 @@ expect(record[:contact_topics]).to include(contact_topic.question) end end + + context "with permission flags and action metadata" do + let(:volunteer) { create(:volunteer, casa_org: organization) } + let!(:active_contact) { create(:case_contact, :active, casa_case: casa_case, creator: volunteer) } + let!(:draft_contact) { create(:case_contact, casa_case: casa_case, creator: volunteer, status: "started") } + + def post_datatable + post datatable_case_contacts_new_design_path, params: datatable_params, as: :json + end + + def row_for(contact_id) + json = JSON.parse(response.body, symbolize_names: true) + json[:data].find { |row| row[:id] == contact_id.to_s } + end + + context "when signed in as admin" do + it "includes can_edit as true" do + post_datatable + expect(row_for(active_contact.id)[:can_edit]).to eq("true") + end + + it "includes can_destroy as true" do + post_datatable + expect(row_for(active_contact.id)[:can_destroy]).to eq("true") + end + + it "includes edit_path for the contact" do + post_datatable + expect(row_for(active_contact.id)[:edit_path]).to eq(edit_case_contact_path(active_contact)) + end + + it "includes followup_id as empty when no followup exists" do + post_datatable + expect(row_for(active_contact.id)[:followup_id]).to eq("") + end + + it "includes followup_id when a requested followup exists" do + followup = create(:followup, case_contact: active_contact, status: :requested, creator: admin) + post_datatable + expect(row_for(active_contact.id)[:followup_id]).to eq(followup.id.to_s) + end + end + + context "when signed in as volunteer" do + before { sign_in volunteer } + + it "includes can_edit as true for their own contact" do + post_datatable + expect(row_for(active_contact.id)[:can_edit]).to eq("true") + end + + it "includes can_destroy as false for their own active contact" do + post_datatable + expect(row_for(active_contact.id)[:can_destroy]).to eq("false") + end + + it "includes can_destroy as true for their own draft contact" do + post_datatable + expect(row_for(draft_contact.id)[:can_destroy]).to eq("true") + end + end + end end end end From 4ece46c6775de7cd7bf3f4fa8e35e03231840dcc Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 09:40:10 -0500 Subject: [PATCH 02/20] Add ellipsis action menu to new case contacts table Each row in the new case contacts datatable now has a Bootstrap dropdown menu with Edit, Delete, and Set/Resolve Reminder actions. Items are shown or hidden based on per-row Pundit permissions already returned by the datatable JSON. Delete and Resolve Reminder use AJAX so the table reloads in place. Set Reminder opens a SweetAlert2 dialog for an optional note. Backend controllers now respond to JSON for destroy and followup resolve so jQuery does not follow the HTML redirect. Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts/followups_controller.rb | 5 +- app/controllers/case_contacts_controller.rb | 10 +- app/javascript/__tests__/dashboard.test.js | 220 +++++++++++++++++- app/javascript/src/dashboard.js | 81 ++++++- 4 files changed, 309 insertions(+), 7 deletions(-) diff --git a/app/controllers/case_contacts/followups_controller.rb b/app/controllers/case_contacts/followups_controller.rb index cd09e152ad..e0c453432f 100644 --- a/app/controllers/case_contacts/followups_controller.rb +++ b/app/controllers/case_contacts/followups_controller.rb @@ -17,7 +17,10 @@ def resolve @followup.resolved! create_notification - redirect_to casa_case_path(@followup.case_contact.casa_case) + respond_to do |format| + format.html { redirect_to casa_case_path(@followup.case_contact.casa_case) } + format.json { head :no_content } + end end private diff --git a/app/controllers/case_contacts_controller.rb b/app/controllers/case_contacts_controller.rb index 5f4275ea82..c32204fc1e 100644 --- a/app/controllers/case_contacts_controller.rb +++ b/app/controllers/case_contacts_controller.rb @@ -44,8 +44,14 @@ def destroy authorize @case_contact @case_contact.destroy - flash[:notice] = "Contact is successfully deleted." - redirect_to request.referer + + respond_to do |format| + format.html do + flash[:notice] = "Contact is successfully deleted." + redirect_to request.referer + end + format.json { head :no_content } + end end def restore diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index 9a289bd15d..db2db9c1e5 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -3,8 +3,14 @@ * @jest-environment jsdom */ +import Swal from 'sweetalert2' import { defineCaseContactsTable } from '../src/dashboard' +jest.mock('sweetalert2', () => ({ + __esModule: true, + default: { fire: jest.fn() } +})) + // Mock DataTable const mockDataTable = jest.fn() $.fn.DataTable = mockDataTable @@ -382,10 +388,218 @@ describe('defineCaseContactsTable', () => { expect(columns[10].searchable).toBe(false) }) - it('renders ellipsis icon', () => { - const rendered = columns[10].render(null, 'display', {}) + it('renders a button toggle with aria-label containing the contact date', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'true', can_destroy: 'true', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).toContain('class="fas fa-ellipsis-v"') + expect(rendered).toContain('aria-label="Actions for case contact on July 01, 2024"') + expect(rendered).toContain('type="button"') + }) + + it('renders the ellipsis icon as aria-hidden', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'true', can_destroy: 'true', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).toContain('aria-hidden="true"') + }) + + it('renders Edit item when can_edit is "true"', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'true', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).toContain('href="/case_contacts/1/edit"') + expect(rendered).toContain('Edit') + }) + + it('does not render Edit item when can_edit is "false"', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).not.toContain('href="/case_contacts/1/edit"') + expect(rendered).not.toContain('>Edit<') + }) + + it('renders Delete item when can_destroy is "true"', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'true', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).toContain('cc-delete-action') + expect(rendered).toContain('data-id="1"') + expect(rendered).toContain('Delete') + }) + + it('does not render Delete item when can_destroy is "false"', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).not.toContain('cc-delete-action') + expect(rendered).not.toContain('>Delete<') + }) + + it('renders Set Reminder when followup_id is empty', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).toContain('cc-set-reminder-action') + expect(rendered).toContain('Set Reminder') + expect(rendered).not.toContain('Resolve Reminder') + }) + + it('renders Resolve Reminder when followup_id is present', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '42' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).toContain('cc-resolve-reminder-action') + expect(rendered).toContain('data-followup-id="42"') + expect(rendered).toContain('Resolve Reminder') + expect(rendered).not.toContain('Set Reminder') + }) + + it('always renders the reminder menu item', () => { + const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '' } + const rendered = columns[10].render(null, 'display', row) + + expect(rendered).toMatch(/Set Reminder|Resolve Reminder/) + }) + }) + }) + + describe('click handlers', () => { + let mockAjaxReload + let mockTableInstance + + beforeEach(() => { + mockAjaxReload = jest.fn() + mockTableInstance = { ajax: { reload: mockAjaxReload } } + mockDataTable.mockReturnValue(mockTableInstance) + + // Add CSRF meta tag + document.head.innerHTML = '' + + defineCaseContactsTable() + }) + + afterEach(() => { + Swal.fire.mockReset() + }) + + describe('Delete action', () => { + it('sends DELETE request when cc-delete-action is clicked', () => { + const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) + + $('table#case_contacts tbody').append('') + $('.cc-delete-action').trigger('click') + + expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ + url: '/case_contacts/42', + type: 'DELETE', + dataType: 'json', + headers: { 'X-CSRF-Token': 'test-csrf-token' } + })) + + ajaxSpy.mockRestore() + }) + + it('reloads the DataTable after successful delete', () => { + jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) + + $('table#case_contacts tbody').append('') + $('.cc-delete-action').trigger('click') + + expect(mockAjaxReload).toHaveBeenCalled() + }) + }) + + describe('Set Reminder action', () => { + it('fires SweetAlert when cc-set-reminder-action is clicked', () => { + Swal.fire.mockResolvedValue({ isConfirmed: false }) + + $('table#case_contacts tbody').append('') + $('.cc-set-reminder-action').trigger('click') + + expect(Swal.fire).toHaveBeenCalled() + }) + + it('posts to the followups endpoint when confirmed without a note', async () => { + Swal.fire.mockResolvedValue({ value: '', isConfirmed: true }) + const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) + + $('table#case_contacts tbody').append('') + $('.cc-set-reminder-action').trigger('click') + + await Promise.resolve() + + expect(postSpy).toHaveBeenCalledWith('/case_contacts/5/followups', {}, expect.any(Function)) + + postSpy.mockRestore() + }) + + it('posts with note when confirmed with a note', async () => { + Swal.fire.mockResolvedValue({ value: 'My note', isConfirmed: true }) + const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) + + $('table#case_contacts tbody').append('') + $('.cc-set-reminder-action').trigger('click') + + await Promise.resolve() + + expect(postSpy).toHaveBeenCalledWith('/case_contacts/5/followups', { note: 'My note' }, expect.any(Function)) + + postSpy.mockRestore() + }) + + it('does not post when cancelled', async () => { + Swal.fire.mockResolvedValue({ isConfirmed: false }) + const postSpy = jest.spyOn($, 'post').mockImplementation() + + $('table#case_contacts tbody').append('') + $('.cc-set-reminder-action').trigger('click') + + await Promise.resolve() + + expect(postSpy).not.toHaveBeenCalled() + + postSpy.mockRestore() + }) + + it('reloads the DataTable after creating a reminder', async () => { + Swal.fire.mockResolvedValue({ value: '', isConfirmed: true }) + jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) + + $('table#case_contacts tbody').append('') + $('.cc-set-reminder-action').trigger('click') + + await Promise.resolve() + + expect(mockAjaxReload).toHaveBeenCalled() + }) + }) + + describe('Resolve Reminder action', () => { + it('sends PATCH request when cc-resolve-reminder-action is clicked', () => { + const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) + + $('table#case_contacts tbody').append('') + $('.cc-resolve-reminder-action').trigger('click') + + expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ + url: '/followups/42/resolve', + type: 'PATCH', + dataType: 'json', + headers: { 'X-CSRF-Token': 'test-csrf-token' } + })) + + ajaxSpy.mockRestore() + }) + + it('reloads the DataTable after resolving a reminder', () => { + jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) + + $('table#case_contacts tbody').append('') + $('.cc-resolve-reminder-action').trigger('click') - expect(rendered).toBe('') + expect(mockAjaxReload).toHaveBeenCalled() }) }) }) diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index a9115a0dcc..2e4a463c17 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -1,5 +1,6 @@ /* global alert */ /* global $ */ +import Swal from 'sweetalert2' const { Notifier } = require('./notifier') let pageNotifier @@ -150,7 +151,43 @@ const defineCaseContactsTable = function () { data: null, orderable: false, searchable: false, - render: () => '' + render: (_data, _type, row) => { + const buttonId = `cc-actions-btn-${row.id}` + const label = `Actions for case contact${row.occurred_at ? ' on ' + row.occurred_at : ''}` + + const editItem = row.can_edit === 'true' + ? `
  • Edit
  • ` + : '' + + const deleteItem = row.can_destroy === 'true' + ? `
  • ` + : '' + + const reminderItem = row.followup_id + ? `
  • ` + : `
  • ` + + return ` + + ` + } } ] }) @@ -167,6 +204,48 @@ const defineCaseContactsTable = function () { $(this).addClass('expanded').attr('aria-expanded', 'true') } }) + + const csrfToken = () => $('meta[name="csrf-token"]').attr('content') + + $('table#case_contacts').on('click', '.cc-delete-action', function () { + const id = $(this).data('id') + $.ajax({ + url: `/case_contacts/${id}`, + type: 'DELETE', + dataType: 'json', + headers: { 'X-CSRF-Token': csrfToken() }, + success: () => table.ajax.reload() + }) + }) + + $('table#case_contacts').on('click', '.cc-set-reminder-action', async function () { + const id = $(this).data('id') + const { value: text, isConfirmed } = await Swal.fire({ + input: 'textarea', + title: 'Optional: Add a note about what followup is needed.', + inputPlaceholder: 'Type your note here...', + inputAttributes: { 'aria-label': 'Type your note here' }, + showCancelButton: true, + showCloseButton: true, + confirmButtonText: 'Confirm', + confirmButtonColor: '#dc3545', + customClass: { inputLabel: 'mx-5' } + }) + if (!isConfirmed) return + const params = text ? { note: text } : {} + $.post(`/case_contacts/${id}/followups`, params, () => table.ajax.reload()) + }) + + $('table#case_contacts').on('click', '.cc-resolve-reminder-action', function () { + const followupId = $(this).data('followup-id') + $.ajax({ + url: `/followups/${followupId}/resolve`, + type: 'PATCH', + dataType: 'json', + headers: { 'X-CSRF-Token': csrfToken() }, + success: () => table.ajax.reload() + }) + }) } $(() => { // JQuery's callback for the DOM loading From 0d1a6038e29de23eadac332f05bccd6348bd0f2b Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 09:46:42 -0500 Subject: [PATCH 03/20] Require confirmation before deleting a case contact The delete action in the new case contacts table now shows a confirmation dialog before sending the DELETE request. Cancelling the dialog aborts the request. Tests updated to reflect async flow. Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/__tests__/dashboard.test.js | 33 ++++++++++++++++++++-- app/javascript/src/dashboard.js | 9 +++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index db2db9c1e5..31e441f67e 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -485,12 +485,24 @@ describe('defineCaseContactsTable', () => { }) describe('Delete action', () => { - it('sends DELETE request when cc-delete-action is clicked', () => { + it('shows a SweetAlert confirmation dialog when cc-delete-action is clicked', () => { + Swal.fire.mockResolvedValue({ isConfirmed: false }) + + $('table#case_contacts tbody').append('') + $('.cc-delete-action').trigger('click') + + expect(Swal.fire).toHaveBeenCalled() + }) + + it('sends DELETE request when confirmed', async () => { + Swal.fire.mockResolvedValue({ isConfirmed: true }) const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) $('table#case_contacts tbody').append('') $('.cc-delete-action').trigger('click') + await Promise.resolve() + expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ url: '/case_contacts/42', type: 'DELETE', @@ -501,12 +513,29 @@ describe('defineCaseContactsTable', () => { ajaxSpy.mockRestore() }) - it('reloads the DataTable after successful delete', () => { + it('does not send DELETE request when cancelled', async () => { + Swal.fire.mockResolvedValue({ isConfirmed: false }) + const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation() + + $('table#case_contacts tbody').append('') + $('.cc-delete-action').trigger('click') + + await Promise.resolve() + + expect(ajaxSpy).not.toHaveBeenCalled() + + ajaxSpy.mockRestore() + }) + + it('reloads the DataTable after successful delete', async () => { + Swal.fire.mockResolvedValue({ isConfirmed: true }) jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) $('table#case_contacts tbody').append('') $('.cc-delete-action').trigger('click') + await Promise.resolve() + expect(mockAjaxReload).toHaveBeenCalled() }) }) diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index 2e4a463c17..d839b6db75 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -207,8 +207,15 @@ const defineCaseContactsTable = function () { const csrfToken = () => $('meta[name="csrf-token"]').attr('content') - $('table#case_contacts').on('click', '.cc-delete-action', function () { + $('table#case_contacts').on('click', '.cc-delete-action', async function () { const id = $(this).data('id') + const { isConfirmed } = await Swal.fire({ + title: 'Delete this contact?', + showCancelButton: true, + confirmButtonText: 'Delete', + confirmButtonColor: '#dc3545' + }) + if (!isConfirmed) return $.ajax({ url: `/case_contacts/${id}`, type: 'DELETE', From b60ee3ab403909420da3448a134fd75dc03f5e05 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 12:32:12 -0500 Subject: [PATCH 04/20] Disable unauthorized menu items instead of hiding them Edit and Delete are now rendered as disabled dropdown items when the current user lacks permission, rather than being omitted from the menu. Disabled items retain aria-disabled="true" for screen reader accessibility. Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/__tests__/dashboard.test.js | 12 ++++++++---- app/javascript/src/dashboard.js | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index 31e441f67e..35160e054e 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -412,12 +412,14 @@ describe('defineCaseContactsTable', () => { expect(rendered).toContain('Edit') }) - it('does not render Edit item when can_edit is "false"', () => { + it('renders Edit as disabled when can_edit is "false"', () => { const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '' } const rendered = columns[10].render(null, 'display', row) + expect(rendered).toContain('Edit') + expect(rendered).toContain('disabled') + expect(rendered).toContain('aria-disabled="true"') expect(rendered).not.toContain('href="/case_contacts/1/edit"') - expect(rendered).not.toContain('>Edit<') }) it('renders Delete item when can_destroy is "true"', () => { @@ -429,12 +431,14 @@ describe('defineCaseContactsTable', () => { expect(rendered).toContain('Delete') }) - it('does not render Delete item when can_destroy is "false"', () => { + it('renders Delete as disabled when can_destroy is "false"', () => { const row = { id: '1', occurred_at: 'July 01, 2024', can_edit: 'false', can_destroy: 'false', edit_path: '/case_contacts/1/edit', followup_id: '' } const rendered = columns[10].render(null, 'display', row) + expect(rendered).toContain('Delete') + expect(rendered).toContain('disabled') + expect(rendered).toContain('aria-disabled="true"') expect(rendered).not.toContain('cc-delete-action') - expect(rendered).not.toContain('>Delete<') }) it('renders Set Reminder when followup_id is empty', () => { diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index d839b6db75..50f9176266 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -157,11 +157,11 @@ const defineCaseContactsTable = function () { const editItem = row.can_edit === 'true' ? `
  • Edit
  • ` - : '' + : `
  • Edit
  • ` const deleteItem = row.can_destroy === 'true' ? `
  • ` - : '' + : `
  • ` const reminderItem = row.followup_id ? `
  • ` From 484f60310a38d2dde1e27942e53d0380bdb111b1 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 13:06:16 -0500 Subject: [PATCH 05/20] Add system specs for action menu visibility Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts_new_design_spec.rb | 77 +++++++++++++++---- 1 file changed, 62 insertions(+), 15 deletions(-) diff --git a/spec/system/case_contacts/case_contacts_new_design_spec.rb b/spec/system/case_contacts/case_contacts_new_design_spec.rb index 3e05c44cd4..2d1c497528 100644 --- a/spec/system/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/system/case_contacts/case_contacts_new_design_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe "Case Contact Table Row Expansion", type: :system, js: true do +RSpec.describe "Case contacts new design", type: :system, js: true do let(:organization) { create(:casa_org) } let(:admin) { create(:casa_admin, casa_org: organization) } let(:casa_case) { create(:casa_case, casa_org: organization) } @@ -19,25 +19,72 @@ visit case_contacts_new_design_path end - it "shows the expanded content after clicking the chevron" do - find(".expand-toggle").click + describe "row expansion" do + it "shows the expanded content after clicking the chevron" do + find(".expand-toggle").click - expect(page).to have_content("What was discussed?") - expect(page).to have_content("Youth is doing well in school") - end + expect(page).to have_content("What was discussed?") + expect(page).to have_content("Youth is doing well in school") + end + + it "shows notes in the expanded content" do + find(".expand-toggle").click - it "shows notes in the expanded content" do - find(".expand-toggle").click + expect(page).to have_content("Additional Notes") + expect(page).to have_content("Important follow-up needed") + end - expect(page).to have_content("Additional Notes") - expect(page).to have_content("Important follow-up needed") + it "hides the expanded content after clicking the chevron again" do + find(".expand-toggle").click + expect(page).to have_content("Youth is doing well in school") + + find(".expand-toggle").click + expect(page).to have_no_content("Youth is doing well in school") + end end - it "hides the expanded content after clicking the chevron again" do - find(".expand-toggle").click - expect(page).to have_content("Youth is doing well in school") + describe "action menu" do + it "opens the dropdown when the ellipsis button is clicked" do + find(".cc-ellipsis-toggle").click + + expect(page).to have_css(".dropdown-menu.show") + end + + it "shows Edit in the menu" do + find(".cc-ellipsis-toggle").click + + expect(page).to have_text("Edit") + end + + it "shows Delete in the menu" do + find(".cc-ellipsis-toggle").click + + expect(page).to have_text("Delete") + end + + it "shows Set Reminder when no followup exists" do + find(".cc-ellipsis-toggle").click + + expect(page).to have_text("Set Reminder") + expect(page).to have_no_text("Resolve Reminder") + end + + it "shows Resolve Reminder when a requested followup exists" do + create(:followup, case_contact: case_contact, status: :requested, creator: admin) + visit case_contacts_new_design_path + + find(".cc-ellipsis-toggle").click + + expect(page).to have_text("Resolve Reminder") + expect(page).to have_no_text("Set Reminder") + end + + it "closes the dropdown when clicking outside" do + find(".cc-ellipsis-toggle").click + expect(page).to have_css(".dropdown-menu.show") - find(".expand-toggle").click - expect(page).to have_no_content("Youth is doing well in school") + find("h1").click + expect(page).to have_no_css(".dropdown-menu.show") + end end end From 6ca31982657f03fa43b546afbfef24b09f6ad8f4 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 13:08:57 -0500 Subject: [PATCH 06/20] Add system spec for Edit action Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts/case_contacts_new_design_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/system/case_contacts/case_contacts_new_design_spec.rb b/spec/system/case_contacts/case_contacts_new_design_spec.rb index 2d1c497528..945ca44ca9 100644 --- a/spec/system/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/system/case_contacts/case_contacts_new_design_spec.rb @@ -14,6 +14,7 @@ case_contact: case_contact, contact_topic: contact_topic, value: "Youth is doing well in school") + allow(Flipper).to receive(:enabled?).and_call_original allow(Flipper).to receive(:enabled?).with(:new_case_contact_table).and_return(true) sign_in admin visit case_contacts_new_design_path @@ -87,4 +88,13 @@ expect(page).to have_no_css(".dropdown-menu.show") end end + + describe "Edit action" do + it "navigates to the edit form when Edit is clicked" do + find(".cc-ellipsis-toggle").click + click_link "Edit" + + expect(page).to have_current_path(/case_contacts\/#{case_contact.id}\/form/) + end + end end From ec25d2e257f043b9edbc681edc03bdb4644c055c Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 13:11:30 -0500 Subject: [PATCH 07/20] Add system specs for Delete action Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts_new_design_spec.rb | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/system/case_contacts/case_contacts_new_design_spec.rb b/spec/system/case_contacts/case_contacts_new_design_spec.rb index 945ca44ca9..bc959e9d20 100644 --- a/spec/system/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/system/case_contacts/case_contacts_new_design_spec.rb @@ -97,4 +97,28 @@ expect(page).to have_current_path(/case_contacts\/#{case_contact.id}\/form/) end end + + describe "Delete action" do + let(:occurred_at_text) { I18n.l(case_contact.occurred_at, format: :full) } + + it "removes the row after confirming the delete dialog" do + expect(page).to have_text(occurred_at_text) + + find(".cc-ellipsis-toggle").click + find(".cc-delete-action").click + click_button "Delete" + + expect(page).to have_no_text(occurred_at_text) + end + + it "leaves the row in place when the delete dialog is cancelled" do + expect(page).to have_text(occurred_at_text) + + find(".cc-ellipsis-toggle").click + find(".cc-delete-action").click + click_button "Cancel" + + expect(page).to have_text(occurred_at_text) + end + end end From d1a3a48a14e7ac781fde26e95f8f87a5298e2fce Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 13:16:06 -0500 Subject: [PATCH 08/20] Add system specs for Set Reminder and Resolve Reminder actions Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts_new_design_spec.rb | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/spec/system/case_contacts/case_contacts_new_design_spec.rb b/spec/system/case_contacts/case_contacts_new_design_spec.rb index bc959e9d20..93b4cc769c 100644 --- a/spec/system/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/system/case_contacts/case_contacts_new_design_spec.rb @@ -121,4 +121,53 @@ expect(page).to have_text(occurred_at_text) end end + + describe "Set Reminder action" do + it "creates a followup and shows Resolve Reminder in the menu after confirming" do + find(".cc-ellipsis-toggle").click + find(".cc-set-reminder-action").click + click_button "Confirm" + + expect(page).to have_css("i.fas.fa-bell:not([style])") + + find(".cc-ellipsis-toggle").click + expect(page).to have_text("Resolve Reminder") + expect(page).to have_no_text("Set Reminder") + end + + it "does not create a followup when cancelled" do + find(".cc-ellipsis-toggle").click + find(".cc-set-reminder-action").click + click_button "Cancel" + + expect(case_contact.followups.reload).to be_empty + end + end + + describe "Resolve Reminder action" do + let!(:followup) { create(:followup, case_contact: case_contact, status: :requested, creator: admin) } + + before { visit case_contacts_new_design_path } + + it "resolves the followup and shows Set Reminder in the menu afterwards" do + find(".cc-ellipsis-toggle").click + find(".cc-resolve-reminder-action").click + + expect(page).to have_css("i.fas.fa-bell[style*='opacity']") + + find(".cc-ellipsis-toggle").click + expect(page).to have_text("Set Reminder") + expect(page).to have_no_text("Resolve Reminder") + end + + it "marks the followup as resolved" do + find(".cc-ellipsis-toggle").click + find(".cc-resolve-reminder-action").click + + # Wait for reload to confirm the AJAX completed before checking DB + expect(page).to have_css("i.fas.fa-bell[style*='opacity']") + + expect(followup.reload.status).to eq("resolved") + end + end end From 781fcc2d1368f5192ba39ed2e3e75fb743c0acf7 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 13:22:43 -0500 Subject: [PATCH 09/20] Add system specs for permission states on action menu items Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts_new_design_spec.rb | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/system/case_contacts/case_contacts_new_design_spec.rb b/spec/system/case_contacts/case_contacts_new_design_spec.rb index 93b4cc769c..eb598144bf 100644 --- a/spec/system/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/system/case_contacts/case_contacts_new_design_spec.rb @@ -170,4 +170,32 @@ expect(followup.reload.status).to eq("resolved") end end + + describe "permission states" do + let(:volunteer) { create(:volunteer, casa_org: organization) } + let(:casa_case_for_volunteer) { create(:casa_case, casa_org: organization) } + let!(:active_contact) { create(:case_contact, :active, casa_case: casa_case_for_volunteer, creator: volunteer) } + let!(:draft_contact) { create(:case_contact, casa_case: casa_case_for_volunteer, creator: volunteer, status: "started", occurred_at: 3.days.ago) } + + before do + sign_in volunteer + visit case_contacts_new_design_path + end + + it "shows Delete as disabled for an active contact" do + within("tbody tr", text: I18n.l(active_contact.occurred_at, format: :full), match: :first) do + find(".cc-ellipsis-toggle").click + end + + expect(page).to have_css("button.dropdown-item.disabled", text: "Delete") + end + + it "shows Delete as enabled for a draft contact" do + within("tbody tr", text: I18n.l(draft_contact.occurred_at, format: :full), match: :first) do + find(".cc-ellipsis-toggle").click + end + + expect(page).to have_css("button.cc-delete-action", text: "Delete") + end + end end From 68043f45107ce512fbeb839647fd63c7a2c37064 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 13:31:10 -0500 Subject: [PATCH 10/20] use single quotes --- app/javascript/src/dashboard.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index 50f9176266..84603ffd5b 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -157,11 +157,11 @@ const defineCaseContactsTable = function () { const editItem = row.can_edit === 'true' ? `
  • Edit
  • ` - : `
  • Edit
  • ` + : '
  • Edit
  • ' const deleteItem = row.can_destroy === 'true' ? `
  • ` - : `
  • ` + : '
  • ' const reminderItem = row.followup_id ? `
  • ` From 44f22e2e164b84ececa1fe89286e5e81732028ee Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 13:37:41 -0500 Subject: [PATCH 11/20] Extract fireSwalFollowupAlert from case_contact.js into dashboard.js Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/__tests__/dashboard.test.js | 21 +++++++++++++-------- app/javascript/src/case_contact.js | 3 ++- app/javascript/src/dashboard.js | 13 ++----------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index 35160e054e..2d7d0e288b 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -5,12 +5,16 @@ import Swal from 'sweetalert2' import { defineCaseContactsTable } from '../src/dashboard' - +import { fireSwalFollowupAlert } from '../src/case_contact' jest.mock('sweetalert2', () => ({ __esModule: true, default: { fire: jest.fn() } })) +jest.mock('../src/case_contact', () => ({ + fireSwalFollowupAlert: jest.fn() +})) + // Mock DataTable const mockDataTable = jest.fn() $.fn.DataTable = mockDataTable @@ -486,6 +490,7 @@ describe('defineCaseContactsTable', () => { afterEach(() => { Swal.fire.mockReset() + fireSwalFollowupAlert.mockReset() }) describe('Delete action', () => { @@ -545,17 +550,17 @@ describe('defineCaseContactsTable', () => { }) describe('Set Reminder action', () => { - it('fires SweetAlert when cc-set-reminder-action is clicked', () => { - Swal.fire.mockResolvedValue({ isConfirmed: false }) + it('calls fireSwalFollowupAlert when cc-set-reminder-action is clicked', () => { + fireSwalFollowupAlert.mockResolvedValue({ isConfirmed: false }) $('table#case_contacts tbody').append('') $('.cc-set-reminder-action').trigger('click') - expect(Swal.fire).toHaveBeenCalled() + expect(fireSwalFollowupAlert).toHaveBeenCalled() }) it('posts to the followups endpoint when confirmed without a note', async () => { - Swal.fire.mockResolvedValue({ value: '', isConfirmed: true }) + fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true }) const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) $('table#case_contacts tbody').append('') @@ -569,7 +574,7 @@ describe('defineCaseContactsTable', () => { }) it('posts with note when confirmed with a note', async () => { - Swal.fire.mockResolvedValue({ value: 'My note', isConfirmed: true }) + fireSwalFollowupAlert.mockResolvedValue({ value: 'My note', isConfirmed: true }) const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) $('table#case_contacts tbody').append('') @@ -583,7 +588,7 @@ describe('defineCaseContactsTable', () => { }) it('does not post when cancelled', async () => { - Swal.fire.mockResolvedValue({ isConfirmed: false }) + fireSwalFollowupAlert.mockResolvedValue({ isConfirmed: false }) const postSpy = jest.spyOn($, 'post').mockImplementation() $('table#case_contacts tbody').append('') @@ -597,7 +602,7 @@ describe('defineCaseContactsTable', () => { }) it('reloads the DataTable after creating a reminder', async () => { - Swal.fire.mockResolvedValue({ value: '', isConfirmed: true }) + fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true }) jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) $('table#case_contacts tbody').append('') diff --git a/app/javascript/src/case_contact.js b/app/javascript/src/case_contact.js index 77ee094d24..c2837c683d 100644 --- a/app/javascript/src/case_contact.js +++ b/app/javascript/src/case_contact.js @@ -54,5 +54,6 @@ $(() => { // JQuery's callback for the DOM loading }) export { - convertDateToSystemTimeZone + convertDateToSystemTimeZone, + fireSwalFollowupAlert } diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index 84603ffd5b..0877bd06dc 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -1,6 +1,7 @@ /* global alert */ /* global $ */ import Swal from 'sweetalert2' +import { fireSwalFollowupAlert } from './case_contact' const { Notifier } = require('./notifier') let pageNotifier @@ -227,17 +228,7 @@ const defineCaseContactsTable = function () { $('table#case_contacts').on('click', '.cc-set-reminder-action', async function () { const id = $(this).data('id') - const { value: text, isConfirmed } = await Swal.fire({ - input: 'textarea', - title: 'Optional: Add a note about what followup is needed.', - inputPlaceholder: 'Type your note here...', - inputAttributes: { 'aria-label': 'Type your note here' }, - showCancelButton: true, - showCloseButton: true, - confirmButtonText: 'Confirm', - confirmButtonColor: '#dc3545', - customClass: { inputLabel: 'mx-5' } - }) + const { value: text, isConfirmed } = await fireSwalFollowupAlert() if (!isConfirmed) return const params = text ? { note: text } : {} $.post(`/case_contacts/${id}/followups`, params, () => table.ajax.reload()) From 7b719f0bca5886b4cc89d6eb0cac66d16b1daf89 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 15:29:09 -0500 Subject: [PATCH 12/20] Extract clickActionButton helper in dashboard click handler tests Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/__tests__/dashboard.test.js | 41 ++++++++++------------ 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index 2d7d0e288b..2cb21771a5 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -477,6 +477,14 @@ describe('defineCaseContactsTable', () => { let mockAjaxReload let mockTableInstance + const clickActionButton = (action, attrs = {}) => { + const dataAttrs = Object.entries(attrs).map(([k, v]) => `data-${k}="${v}"`).join(' ') + $('table#case_contacts tbody').append( + `` + ) + $(`.cc-${action}-action`).trigger('click') + } + beforeEach(() => { mockAjaxReload = jest.fn() mockTableInstance = { ajax: { reload: mockAjaxReload } } @@ -497,8 +505,7 @@ describe('defineCaseContactsTable', () => { it('shows a SweetAlert confirmation dialog when cc-delete-action is clicked', () => { Swal.fire.mockResolvedValue({ isConfirmed: false }) - $('table#case_contacts tbody').append('') - $('.cc-delete-action').trigger('click') + clickActionButton('delete', { id: '42' }) expect(Swal.fire).toHaveBeenCalled() }) @@ -507,8 +514,7 @@ describe('defineCaseContactsTable', () => { Swal.fire.mockResolvedValue({ isConfirmed: true }) const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) - $('table#case_contacts tbody').append('') - $('.cc-delete-action').trigger('click') + clickActionButton('delete', { id: '42' }) await Promise.resolve() @@ -526,8 +532,7 @@ describe('defineCaseContactsTable', () => { Swal.fire.mockResolvedValue({ isConfirmed: false }) const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation() - $('table#case_contacts tbody').append('') - $('.cc-delete-action').trigger('click') + clickActionButton('delete', { id: '42' }) await Promise.resolve() @@ -540,8 +545,7 @@ describe('defineCaseContactsTable', () => { Swal.fire.mockResolvedValue({ isConfirmed: true }) jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) - $('table#case_contacts tbody').append('') - $('.cc-delete-action').trigger('click') + clickActionButton('delete', { id: '42' }) await Promise.resolve() @@ -553,8 +557,7 @@ describe('defineCaseContactsTable', () => { it('calls fireSwalFollowupAlert when cc-set-reminder-action is clicked', () => { fireSwalFollowupAlert.mockResolvedValue({ isConfirmed: false }) - $('table#case_contacts tbody').append('') - $('.cc-set-reminder-action').trigger('click') + clickActionButton('set-reminder', { id: '5' }) expect(fireSwalFollowupAlert).toHaveBeenCalled() }) @@ -563,8 +566,7 @@ describe('defineCaseContactsTable', () => { fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true }) const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) - $('table#case_contacts tbody').append('') - $('.cc-set-reminder-action').trigger('click') + clickActionButton('set-reminder', { id: '5' }) await Promise.resolve() @@ -577,8 +579,7 @@ describe('defineCaseContactsTable', () => { fireSwalFollowupAlert.mockResolvedValue({ value: 'My note', isConfirmed: true }) const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) - $('table#case_contacts tbody').append('') - $('.cc-set-reminder-action').trigger('click') + clickActionButton('set-reminder', { id: '5' }) await Promise.resolve() @@ -591,8 +592,7 @@ describe('defineCaseContactsTable', () => { fireSwalFollowupAlert.mockResolvedValue({ isConfirmed: false }) const postSpy = jest.spyOn($, 'post').mockImplementation() - $('table#case_contacts tbody').append('') - $('.cc-set-reminder-action').trigger('click') + clickActionButton('set-reminder', { id: '5' }) await Promise.resolve() @@ -605,8 +605,7 @@ describe('defineCaseContactsTable', () => { fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true }) jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) - $('table#case_contacts tbody').append('') - $('.cc-set-reminder-action').trigger('click') + clickActionButton('set-reminder', { id: '5' }) await Promise.resolve() @@ -618,8 +617,7 @@ describe('defineCaseContactsTable', () => { it('sends PATCH request when cc-resolve-reminder-action is clicked', () => { const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) - $('table#case_contacts tbody').append('') - $('.cc-resolve-reminder-action').trigger('click') + clickActionButton('resolve-reminder', { id: '5', 'followup-id': '42' }) expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ url: '/followups/42/resolve', @@ -634,8 +632,7 @@ describe('defineCaseContactsTable', () => { it('reloads the DataTable after resolving a reminder', () => { jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) - $('table#case_contacts tbody').append('') - $('.cc-resolve-reminder-action').trigger('click') + clickActionButton('resolve-reminder', { id: '5', 'followup-id': '42' }) expect(mockAjaxReload).toHaveBeenCalled() }) From c4370f3d72d3cbb0b50f7c6626719b8ac9aa1fa6 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Thu, 9 Apr 2026 15:39:51 -0500 Subject: [PATCH 13/20] Fix flaky permission state specs with unique occurred_at dates Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts_new_design_spec.rb | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/spec/system/case_contacts/case_contacts_new_design_spec.rb b/spec/system/case_contacts/case_contacts_new_design_spec.rb index eb598144bf..f39f539a1b 100644 --- a/spec/system/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/system/case_contacts/case_contacts_new_design_spec.rb @@ -174,28 +174,25 @@ describe "permission states" do let(:volunteer) { create(:volunteer, casa_org: organization) } let(:casa_case_for_volunteer) { create(:casa_case, casa_org: organization) } - let!(:active_contact) { create(:case_contact, :active, casa_case: casa_case_for_volunteer, creator: volunteer) } - let!(:draft_contact) { create(:case_contact, casa_case: casa_case_for_volunteer, creator: volunteer, status: "started", occurred_at: 3.days.ago) } + let!(:active_contact) { create(:case_contact, :active, casa_case: casa_case_for_volunteer, creator: volunteer, occurred_at: 5.days.ago) } + let!(:draft_contact) { create(:case_contact, casa_case: casa_case_for_volunteer, creator: volunteer, status: "started", occurred_at: 10.days.ago) } before do + Capybara.reset_sessions! sign_in volunteer visit case_contacts_new_design_path end it "shows Delete as disabled for an active contact" do - within("tbody tr", text: I18n.l(active_contact.occurred_at, format: :full), match: :first) do - find(".cc-ellipsis-toggle").click - end - - expect(page).to have_css("button.dropdown-item.disabled", text: "Delete") + find("#cc-actions-btn-#{active_contact.id}").click + expect(page).to have_css(".dropdown-menu[aria-labelledby='cc-actions-btn-#{active_contact.id}'].show") + expect(page).to have_css(".dropdown-menu[aria-labelledby='cc-actions-btn-#{active_contact.id}'] button.dropdown-item.disabled", text: "Delete") end it "shows Delete as enabled for a draft contact" do - within("tbody tr", text: I18n.l(draft_contact.occurred_at, format: :full), match: :first) do - find(".cc-ellipsis-toggle").click - end - - expect(page).to have_css("button.cc-delete-action", text: "Delete") + find("#cc-actions-btn-#{draft_contact.id}").click + expect(page).to have_css(".dropdown-menu[aria-labelledby='cc-actions-btn-#{draft_contact.id}'].show") + expect(page).to have_css(".dropdown-menu[aria-labelledby='cc-actions-btn-#{draft_contact.id}'] button.cc-delete-action", text: "Delete") end end end From 5fe23db1a596c5a3229c3035041cc9ff9a512d2b Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Mon, 13 Apr 2026 20:37:44 -0500 Subject: [PATCH 14/20] fix: add missing comma and simplify has_followup in CaseContactDatatable The missing comma after `has_followup` caused a Ruby syntax error, preventing the file from loading entirely. Also simplified `has_followup` to reuse the already-computed `requested_followup` value instead of scanning the collection a second time. Added a regression test asserting all action metadata keys are present in a single row. Co-Authored-By: Claude Sonnet 4.6 --- app/datatables/case_contact_datatable.rb | 2 +- spec/datatables/case_contact_datatable_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/datatables/case_contact_datatable.rb b/app/datatables/case_contact_datatable.rb index 1df72673dc..6536ccadf8 100644 --- a/app/datatables/case_contact_datatable.rb +++ b/app/datatables/case_contact_datatable.rb @@ -45,7 +45,7 @@ def data .map { |a| {question: a.contact_topic&.question, value: a.value} }, notes: case_contact.notes.presence, is_draft: !case_contact.active?, - has_followup: case_contact.followups.any?(&:requested?) + has_followup: requested_followup.present?, can_edit: policy.update?, can_destroy: policy.destroy?, edit_path: Rails.application.routes.url_helpers.edit_case_contact_path(case_contact), diff --git a/spec/datatables/case_contact_datatable_spec.rb b/spec/datatables/case_contact_datatable_spec.rb index 8ef0a960f2..7cb8b81476 100644 --- a/spec/datatables/case_contact_datatable_spec.rb +++ b/spec/datatables/case_contact_datatable_spec.rb @@ -108,6 +108,11 @@ end context "with action metadata" do + it "includes all expected action metadata keys" do + contact_data = datatable.as_json[:data].first + expect(contact_data).to include(:can_edit, :can_destroy, :edit_path, :followup_id, :has_followup) + end + it "includes edit_path for the contact" do contact_data = datatable.as_json[:data].first expected_path = Rails.application.routes.url_helpers.edit_case_contact_path(case_contact) From fd160402faec1de72a0e0d87ec5d30f97a9e6866 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Mon, 13 Apr 2026 20:42:43 -0500 Subject: [PATCH 15/20] fix: remove dataType 'json' from Delete and Resolve Reminder AJAX calls jQuery parses the response body when dataType is 'json', causing a parse error on the 204 No Content response and skipping the success callback. Removing dataType lets jQuery accept the empty response and reload the table. Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/__tests__/dashboard.test.js | 4 ++-- app/javascript/src/dashboard.js | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index 2cb21771a5..7011975885 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -521,9 +521,9 @@ describe('defineCaseContactsTable', () => { expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ url: '/case_contacts/42', type: 'DELETE', - dataType: 'json', headers: { 'X-CSRF-Token': 'test-csrf-token' } })) + expect(ajaxSpy.mock.calls[0][0]).not.toHaveProperty('dataType') ajaxSpy.mockRestore() }) @@ -622,9 +622,9 @@ describe('defineCaseContactsTable', () => { expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ url: '/followups/42/resolve', type: 'PATCH', - dataType: 'json', headers: { 'X-CSRF-Token': 'test-csrf-token' } })) + expect(ajaxSpy.mock.calls[0][0]).not.toHaveProperty('dataType') ajaxSpy.mockRestore() }) diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index 0877bd06dc..d231fec2d5 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -220,7 +220,6 @@ const defineCaseContactsTable = function () { $.ajax({ url: `/case_contacts/${id}`, type: 'DELETE', - dataType: 'json', headers: { 'X-CSRF-Token': csrfToken() }, success: () => table.ajax.reload() }) @@ -239,7 +238,6 @@ const defineCaseContactsTable = function () { $.ajax({ url: `/followups/${followupId}/resolve`, type: 'PATCH', - dataType: 'json', headers: { 'X-CSRF-Token': csrfToken() }, success: () => table.ajax.reload() }) From a43e4e49527e88c997a9c72b0cee5f9547b0058c Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Mon, 13 Apr 2026 20:45:52 -0500 Subject: [PATCH 16/20] fix: include CSRF token in Set Reminder AJAX request $.post does not set the X-CSRF-Token header, which can cause Rails to reject the request with an invalid authenticity token error. Replaced $.post with $.ajax to match the pattern used by Delete and Resolve Reminder. Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/__tests__/dashboard.test.js | 32 ++++++++++++++-------- app/javascript/src/dashboard.js | 8 +++++- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index 7011975885..e1192feff8 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -562,48 +562,58 @@ describe('defineCaseContactsTable', () => { expect(fireSwalFollowupAlert).toHaveBeenCalled() }) - it('posts to the followups endpoint when confirmed without a note', async () => { + it('posts to the followups endpoint with CSRF header when confirmed without a note', async () => { fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true }) - const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) + const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) clickActionButton('set-reminder', { id: '5' }) await Promise.resolve() - expect(postSpy).toHaveBeenCalledWith('/case_contacts/5/followups', {}, expect.any(Function)) + expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ + url: '/case_contacts/5/followups', + type: 'POST', + data: {}, + headers: { 'X-CSRF-Token': 'test-csrf-token' } + })) - postSpy.mockRestore() + ajaxSpy.mockRestore() }) it('posts with note when confirmed with a note', async () => { fireSwalFollowupAlert.mockResolvedValue({ value: 'My note', isConfirmed: true }) - const postSpy = jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) + const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) clickActionButton('set-reminder', { id: '5' }) await Promise.resolve() - expect(postSpy).toHaveBeenCalledWith('/case_contacts/5/followups', { note: 'My note' }, expect.any(Function)) + expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ + url: '/case_contacts/5/followups', + type: 'POST', + data: { note: 'My note' }, + headers: { 'X-CSRF-Token': 'test-csrf-token' } + })) - postSpy.mockRestore() + ajaxSpy.mockRestore() }) it('does not post when cancelled', async () => { fireSwalFollowupAlert.mockResolvedValue({ isConfirmed: false }) - const postSpy = jest.spyOn($, 'post').mockImplementation() + const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation() clickActionButton('set-reminder', { id: '5' }) await Promise.resolve() - expect(postSpy).not.toHaveBeenCalled() + expect(ajaxSpy).not.toHaveBeenCalled() - postSpy.mockRestore() + ajaxSpy.mockRestore() }) it('reloads the DataTable after creating a reminder', async () => { fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true }) - jest.spyOn($, 'post').mockImplementation((_url, _params, cb) => cb && cb()) + jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) clickActionButton('set-reminder', { id: '5' }) diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index d231fec2d5..c270390fde 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -230,7 +230,13 @@ const defineCaseContactsTable = function () { const { value: text, isConfirmed } = await fireSwalFollowupAlert() if (!isConfirmed) return const params = text ? { note: text } : {} - $.post(`/case_contacts/${id}/followups`, params, () => table.ajax.reload()) + $.ajax({ + url: `/case_contacts/${id}/followups`, + type: 'POST', + data: params, + headers: { 'X-CSRF-Token': csrfToken() }, + success: () => table.ajax.reload() + }) }) $('table#case_contacts').on('click', '.cc-resolve-reminder-action', function () { From 709890f04a58b4130f8ef09b492f9d11dba038c4 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Mon, 13 Apr 2026 20:48:13 -0500 Subject: [PATCH 17/20] fix: preserve pagination position when reloading DataTable after row actions table.ajax.reload() resets to page 1 by default. Passing (null, false) keeps the user on the current page after Delete, Set Reminder, and Resolve Reminder actions. Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/__tests__/dashboard.test.js | 12 ++++++------ app/javascript/src/dashboard.js | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index e1192feff8..385e1a6393 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -541,7 +541,7 @@ describe('defineCaseContactsTable', () => { ajaxSpy.mockRestore() }) - it('reloads the DataTable after successful delete', async () => { + it('reloads the DataTable without resetting pagination after successful delete', async () => { Swal.fire.mockResolvedValue({ isConfirmed: true }) jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) @@ -549,7 +549,7 @@ describe('defineCaseContactsTable', () => { await Promise.resolve() - expect(mockAjaxReload).toHaveBeenCalled() + expect(mockAjaxReload).toHaveBeenCalledWith(null, false) }) }) @@ -611,7 +611,7 @@ describe('defineCaseContactsTable', () => { ajaxSpy.mockRestore() }) - it('reloads the DataTable after creating a reminder', async () => { + it('reloads the DataTable without resetting pagination after creating a reminder', async () => { fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true }) jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) @@ -619,7 +619,7 @@ describe('defineCaseContactsTable', () => { await Promise.resolve() - expect(mockAjaxReload).toHaveBeenCalled() + expect(mockAjaxReload).toHaveBeenCalledWith(null, false) }) }) @@ -639,12 +639,12 @@ describe('defineCaseContactsTable', () => { ajaxSpy.mockRestore() }) - it('reloads the DataTable after resolving a reminder', () => { + it('reloads the DataTable without resetting pagination after resolving a reminder', () => { jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success()) clickActionButton('resolve-reminder', { id: '5', 'followup-id': '42' }) - expect(mockAjaxReload).toHaveBeenCalled() + expect(mockAjaxReload).toHaveBeenCalledWith(null, false) }) }) }) diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index c270390fde..23bd3c12d7 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -221,7 +221,7 @@ const defineCaseContactsTable = function () { url: `/case_contacts/${id}`, type: 'DELETE', headers: { 'X-CSRF-Token': csrfToken() }, - success: () => table.ajax.reload() + success: () => table.ajax.reload(null, false) }) }) @@ -235,7 +235,7 @@ const defineCaseContactsTable = function () { type: 'POST', data: params, headers: { 'X-CSRF-Token': csrfToken() }, - success: () => table.ajax.reload() + success: () => table.ajax.reload(null, false) }) }) @@ -245,7 +245,7 @@ const defineCaseContactsTable = function () { url: `/followups/${followupId}/resolve`, type: 'PATCH', headers: { 'X-CSRF-Token': csrfToken() }, - success: () => table.ajax.reload() + success: () => table.ajax.reload(null, false) }) }) } From f6223a3655e91f06ef8bbab94de039d9723ab359 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Tue, 14 Apr 2026 07:29:00 -0500 Subject: [PATCH 18/20] fix: send Accept: application/json header and add respond_to to followups#create Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts/followups_controller.rb | 5 +++- app/javascript/__tests__/dashboard.test.js | 8 ++--- app/javascript/src/dashboard.js | 6 ++-- spec/requests/case_contacts/followups_spec.rb | 30 +++++++++++++++++++ 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/app/controllers/case_contacts/followups_controller.rb b/app/controllers/case_contacts/followups_controller.rb index e0c453432f..9da822b590 100644 --- a/app/controllers/case_contacts/followups_controller.rb +++ b/app/controllers/case_contacts/followups_controller.rb @@ -7,7 +7,10 @@ def create note = simple_followup_params[:note] FollowupService.create_followup(case_contact, current_user, note) - redirect_to casa_case_path(case_contact.casa_case) + respond_to do |format| + format.html { redirect_to casa_case_path(case_contact.casa_case) } + format.json { head :no_content } + end end def resolve diff --git a/app/javascript/__tests__/dashboard.test.js b/app/javascript/__tests__/dashboard.test.js index 385e1a6393..7a19aa0f77 100644 --- a/app/javascript/__tests__/dashboard.test.js +++ b/app/javascript/__tests__/dashboard.test.js @@ -521,7 +521,7 @@ describe('defineCaseContactsTable', () => { expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ url: '/case_contacts/42', type: 'DELETE', - headers: { 'X-CSRF-Token': 'test-csrf-token' } + headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' } })) expect(ajaxSpy.mock.calls[0][0]).not.toHaveProperty('dataType') @@ -574,7 +574,7 @@ describe('defineCaseContactsTable', () => { url: '/case_contacts/5/followups', type: 'POST', data: {}, - headers: { 'X-CSRF-Token': 'test-csrf-token' } + headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' } })) ajaxSpy.mockRestore() @@ -592,7 +592,7 @@ describe('defineCaseContactsTable', () => { url: '/case_contacts/5/followups', type: 'POST', data: { note: 'My note' }, - headers: { 'X-CSRF-Token': 'test-csrf-token' } + headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' } })) ajaxSpy.mockRestore() @@ -632,7 +632,7 @@ describe('defineCaseContactsTable', () => { expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({ url: '/followups/42/resolve', type: 'PATCH', - headers: { 'X-CSRF-Token': 'test-csrf-token' } + headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' } })) expect(ajaxSpy.mock.calls[0][0]).not.toHaveProperty('dataType') diff --git a/app/javascript/src/dashboard.js b/app/javascript/src/dashboard.js index 23bd3c12d7..af0932ed6e 100644 --- a/app/javascript/src/dashboard.js +++ b/app/javascript/src/dashboard.js @@ -220,7 +220,7 @@ const defineCaseContactsTable = function () { $.ajax({ url: `/case_contacts/${id}`, type: 'DELETE', - headers: { 'X-CSRF-Token': csrfToken() }, + headers: { 'X-CSRF-Token': csrfToken(), Accept: 'application/json' }, success: () => table.ajax.reload(null, false) }) }) @@ -234,7 +234,7 @@ const defineCaseContactsTable = function () { url: `/case_contacts/${id}/followups`, type: 'POST', data: params, - headers: { 'X-CSRF-Token': csrfToken() }, + headers: { 'X-CSRF-Token': csrfToken(), Accept: 'application/json' }, success: () => table.ajax.reload(null, false) }) }) @@ -244,7 +244,7 @@ const defineCaseContactsTable = function () { $.ajax({ url: `/followups/${followupId}/resolve`, type: 'PATCH', - headers: { 'X-CSRF-Token': csrfToken() }, + headers: { 'X-CSRF-Token': csrfToken(), Accept: 'application/json' }, success: () => table.ajax.reload(null, false) }) }) diff --git a/spec/requests/case_contacts/followups_spec.rb b/spec/requests/case_contacts/followups_spec.rb index 49a6b486b7..6bf0594ced 100644 --- a/spec/requests/case_contacts/followups_spec.rb +++ b/spec/requests/case_contacts/followups_spec.rb @@ -27,6 +27,21 @@ expect(followup.note).to eq "Hello, world!" end + context "when requested as JSON" do + subject(:request) do + post case_contact_followups_path(case_contact), + params: params, + headers: {"Accept" => "application/json"} + + response + end + + it "returns 204 No Content" do + request + expect(response).to have_http_status(:no_content) + end + end + it "sends a Followup Notifier to case contact creator" do request followup = Followup.last @@ -66,6 +81,21 @@ expect { request }.to change { followup.reload.resolved? }.from(false).to(true) end + context "when requested as JSON" do + subject(:request) do + patch resolve_followup_path(followup), + headers: {"Accept" => "application/json"} + + response + end + + it "returns 204 No Content" do + followup + request + expect(response).to have_http_status(:no_content) + end + end + it "does not send Followup Notifier" do followup expect(FollowupResolvedNotifier).not_to receive(:with) From dd273b1859af1e6980d1548c7b7ec8ea63900e4c Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Tue, 14 Apr 2026 07:30:07 -0500 Subject: [PATCH 19/20] fix: preload casa_org associations to eliminate N+1 queries in CaseContactDatatable Per-row policy checks call same_org? which loads casa_org through casa_case for every record. Added preload for :casa_org and :creator_casa_org since includes cannot resolve has_one :through when left_joins is already in the chain. Co-Authored-By: Claude Sonnet 4.6 --- app/datatables/case_contact_datatable.rb | 1 + .../datatables/case_contact_datatable_spec.rb | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/app/datatables/case_contact_datatable.rb b/app/datatables/case_contact_datatable.rb index 6536ccadf8..4f5b28a8ff 100644 --- a/app/datatables/case_contact_datatable.rb +++ b/app/datatables/case_contact_datatable.rb @@ -63,6 +63,7 @@ def raw_records .joins("INNER JOIN users creators ON creators.id = case_contacts.creator_id") .left_joins(:casa_case) .includes(:casa_case, :contact_types, :contact_topics, :followups, :creator, contact_topic_answers: :contact_topic) + .preload(:casa_org, :creator_casa_org) .order(order_clause) .order(:id) end diff --git a/spec/datatables/case_contact_datatable_spec.rb b/spec/datatables/case_contact_datatable_spec.rb index 7cb8b81476..8a943c81f0 100644 --- a/spec/datatables/case_contact_datatable_spec.rb +++ b/spec/datatables/case_contact_datatable_spec.rb @@ -481,6 +481,32 @@ end end + describe "N+1 queries" do + let!(:contacts) do + 3.times.map do + create(:case_contact, + casa_case: create(:casa_case, casa_org: organization), + creator: create(:volunteer, casa_org: organization)) + end + end + + it "does not trigger N+1 queries for casa_org when computing per-row policy permissions" do + casa_org_queries = [] + subscription = ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload| + casa_org_queries << payload[:sql] if payload[:sql].match?(/SELECT.*casa_orgs/) + end + + datatable.as_json + + ActiveSupport::Notifications.unsubscribe(subscription) + + # With proper eager loading, casa_orgs is fetched in at most 2 batch queries + # (one for :casa_org through casa_case, one for :creator_casa_org through creator). + # An N+1 would fire 1 query per record (3+ here). + expect(casa_org_queries.length).to be <= 2 + end + end + describe "associations loading" do let!(:contacts) do 10.times.map do |i| From b4758d12de06f53d6bcf53082f5deeb7d941e6f6 Mon Sep 17 00:00:00 2001 From: Clifton McIntosh Date: Tue, 14 Apr 2026 15:14:10 -0500 Subject: [PATCH 20/20] refactor: replace Capybara.reset_sessions! with shared_context for admin sign-in Co-Authored-By: Claude Sonnet 4.6 --- .../case_contacts_new_design_spec.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/spec/system/case_contacts/case_contacts_new_design_spec.rb b/spec/system/case_contacts/case_contacts_new_design_spec.rb index f39f539a1b..af2364a531 100644 --- a/spec/system/case_contacts/case_contacts_new_design_spec.rb +++ b/spec/system/case_contacts/case_contacts_new_design_spec.rb @@ -16,11 +16,17 @@ value: "Youth is doing well in school") allow(Flipper).to receive(:enabled?).and_call_original allow(Flipper).to receive(:enabled?).with(:new_case_contact_table).and_return(true) - sign_in admin - visit case_contacts_new_design_path + end + + shared_context "signed in as admin" do + before do + sign_in admin + visit case_contacts_new_design_path + end end describe "row expansion" do + include_context "signed in as admin" it "shows the expanded content after clicking the chevron" do find(".expand-toggle").click @@ -45,6 +51,7 @@ end describe "action menu" do + include_context "signed in as admin" it "opens the dropdown when the ellipsis button is clicked" do find(".cc-ellipsis-toggle").click @@ -90,6 +97,7 @@ end describe "Edit action" do + include_context "signed in as admin" it "navigates to the edit form when Edit is clicked" do find(".cc-ellipsis-toggle").click click_link "Edit" @@ -99,6 +107,7 @@ end describe "Delete action" do + include_context "signed in as admin" let(:occurred_at_text) { I18n.l(case_contact.occurred_at, format: :full) } it "removes the row after confirming the delete dialog" do @@ -123,6 +132,7 @@ end describe "Set Reminder action" do + include_context "signed in as admin" it "creates a followup and shows Resolve Reminder in the menu after confirming" do find(".cc-ellipsis-toggle").click find(".cc-set-reminder-action").click @@ -145,6 +155,8 @@ end describe "Resolve Reminder action" do + include_context "signed in as admin" + let!(:followup) { create(:followup, case_contact: case_contact, status: :requested, creator: admin) } before { visit case_contacts_new_design_path } @@ -178,7 +190,6 @@ let!(:draft_contact) { create(:case_contact, casa_case: casa_case_for_volunteer, creator: volunteer, status: "started", occurred_at: 10.days.ago) } before do - Capybara.reset_sessions! sign_in volunteer visit case_contacts_new_design_path end