Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a4e1f1c
Add per-row permission flags and action metadata to CaseContactDatatable
cliftonmcintosh Apr 7, 2026
4ece46c
Add ellipsis action menu to new case contacts table
cliftonmcintosh Apr 9, 2026
0d1a603
Require confirmation before deleting a case contact
cliftonmcintosh Apr 9, 2026
b60ee3a
Disable unauthorized menu items instead of hiding them
cliftonmcintosh Apr 9, 2026
484f603
Add system specs for action menu visibility
cliftonmcintosh Apr 9, 2026
6ca3198
Add system spec for Edit action
cliftonmcintosh Apr 9, 2026
ec25d2e
Add system specs for Delete action
cliftonmcintosh Apr 9, 2026
d1a3a48
Add system specs for Set Reminder and Resolve Reminder actions
cliftonmcintosh Apr 9, 2026
781fcc2
Add system specs for permission states on action menu items
cliftonmcintosh Apr 9, 2026
68043f4
use single quotes
cliftonmcintosh Apr 9, 2026
44f22e2
Extract fireSwalFollowupAlert from case_contact.js into dashboard.js
cliftonmcintosh Apr 9, 2026
7b719f0
Extract clickActionButton helper in dashboard click handler tests
cliftonmcintosh Apr 9, 2026
c4370f3
Fix flaky permission state specs with unique occurred_at dates
cliftonmcintosh Apr 9, 2026
3c48462
Merge branch 'main' into feature/6500-case-contacts-new-design-row-ac…
compwron Apr 14, 2026
7c5dfc1
Merge branch 'rubyforgood:main' into feature/6500-case-contacts-new-d…
cliftonmcintosh Apr 14, 2026
5fe23db
fix: add missing comma and simplify has_followup in CaseContactDatatable
cliftonmcintosh Apr 14, 2026
fd16040
fix: remove dataType 'json' from Delete and Resolve Reminder AJAX calls
cliftonmcintosh Apr 14, 2026
a43e4e4
fix: include CSRF token in Set Reminder AJAX request
cliftonmcintosh Apr 14, 2026
709890f
fix: preserve pagination position when reloading DataTable after row …
cliftonmcintosh Apr 14, 2026
f6223a3
fix: send Accept: application/json header and add respond_to to follo…
cliftonmcintosh Apr 14, 2026
dd273b1
fix: preload casa_org associations to eliminate N+1 queries in CaseCo…
cliftonmcintosh Apr 14, 2026
b4758d1
refactor: replace Capybara.reset_sessions! with shared_context for ad…
cliftonmcintosh Apr 14, 2026
db661da
Merge branch 'main' into feature/6500-case-contacts-new-design-row-ac…
cliftonmcintosh Apr 14, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions app/controllers/case_contacts/followups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -17,7 +20,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
Expand Down
10 changes: 8 additions & 2 deletions app/controllers/case_contacts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 16 additions & 1 deletion app/datatables/case_contact_datatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?)

Comment on lines +22 to +24
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case_contact.followups.find(&:requested?) forces loading the full followups association for each case contact and then scans it in Ruby. With .includes(:followups) this can pull a lot of rows into memory even though only the requested followup (if any) is needed. Consider fetching only requested followups (e.g., via a scoped query keyed by the page of case_contact IDs) and then looking up the ID per row, to reduce memory and object allocations.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_followup was re-scanning the already-loaded followups collection with any?(&:requested?) when requested_followup was already computed on the line above. Simplified to requested_followup.present?.

Regarding the suggestion to scope the followups query to only load requested followups: the current includes(:followups) does load all followups per record, but in practice each case contact is unlikely to accumulate enough followups for this to be significant. I left this as-is for now but it's a reasonable future improvement if it becomes a concern.

{
id: case_contact.id,
occurred_at: I18n.l(case_contact.occurred_at, format: :full, default: nil),
Expand All @@ -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.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),
followup_id: requested_followup&.id
}
end
end
Expand All @@ -49,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)
Copy link
Copy Markdown
Collaborator Author

@cliftonmcintosh cliftonmcintosh Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To handle an N+1 warning

21:01:42 web.1  | N+1 queries detected:
21:01:42 web.1  |   SELECT "casa_orgs".* FROM "casa_orgs" INNER JOIN "casa_cases" ON "casa_orgs"."id" = "casa_cases"."casa_org_id" WHERE "casa_cases"."id" = $1 LIMIT $2
21:01:42 web.1  |   SELECT "casa_orgs".* FROM "casa_orgs" INNER JOIN "casa_cases" ON "casa_orgs"."id" = "casa_cases"."casa_org_id" WHERE "casa_cases"."id" = $1 LIMIT $2
21:01:42 web.1  |   SELECT "casa_orgs".* FROM "casa_orgs" INNER JOIN "casa_cases" ON "casa_orgs"."id" = "casa_cases"."casa_org_id" WHERE "casa_cases"."id" = $1 LIMIT $2
21:01:42 web.1  |   SELECT "casa_orgs".* FROM "casa_orgs" INNER JOIN "casa_cases" ON "casa_orgs"."id" = "casa_cases"."casa_org_id" WHERE "casa_cases"."id" = $1 LIMIT $2
21:01:42 web.1  |   SELECT "casa_orgs".* FROM "casa_orgs" INNER JOIN "casa_cases" ON "casa_orgs"."id" = "casa_cases"."casa_org_id" WHERE "casa_cases"."id" = $1 LIMIT $2
21:01:42 web.1  |   SELECT "casa_orgs".* FROM "casa_orgs" INNER JOIN "casa_cases" ON "casa_orgs"."id" = "casa_cases"."casa_org_id" WHERE "casa_cases"."id" = $1 LIMIT $2
21:01:42 web.1  | Call stack:
21:01:42 web.1  |   app/policies/case_contact_policy.rb:57:in 'CaseContactPolicy#same_org?'
21:01:42 web.1  |   app/policies/application_policy.rb:72:in 'ApplicationPolicy#is_supervisor_same_org?'
21:01:42 web.1  |   app/policies/application_policy.rb:89:in 'ApplicationPolicy#admin_or_supervisor_same_org?'
21:01:42 web.1  |   app/policies/case_contact_policy.rb:45:in 'CaseContactPolicy#creator_or_supervisor_or_admin?'
21:01:42 web.1  |   app/policies/case_contact_policy.rb:11:in 'CaseContactPolicy#update?'
21:01:42 web.1  |   app/datatables/case_contact_datatable.rb:49:in 'block in CaseContactDatatable#data'
21:01:42 web.1  |   app/datatables/case_contact_datatable.rb:21:in 'Enumerable#map'
21:01:42 web.1  |   app/datatables/case_contact_datatable.rb:21:in 'CaseContactDatatable#data'
21:01:42 web.1  |   app/datatables/application_datatable.rb:15:in 'ApplicationDatatable#as_json'
21:01:42 web.1  |   app/controllers/case_contacts/case_contacts_new_design_controller.rb:15:in 'CaseContacts::CaseContactsNewDesignController#datatable'

.order(order_clause)
.order(:id)
end
Expand Down
265 changes: 262 additions & 3 deletions app/javascript/__tests__/dashboard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,17 @@
* @jest-environment jsdom
*/

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()
Expand Down Expand Up @@ -382,10 +392,259 @@ 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('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"')
})

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('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')
})

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

const clickActionButton = (action, attrs = {}) => {
const dataAttrs = Object.entries(attrs).map(([k, v]) => `data-${k}="${v}"`).join(' ')
$('table#case_contacts tbody').append(
`<tr><td><button class="cc-${action}-action" ${dataAttrs}>${action}</button></td></tr>`
)
$(`.cc-${action}-action`).trigger('click')
}

beforeEach(() => {
mockAjaxReload = jest.fn()
mockTableInstance = { ajax: { reload: mockAjaxReload } }
mockDataTable.mockReturnValue(mockTableInstance)

// Add CSRF meta tag
document.head.innerHTML = '<meta name="csrf-token" content="test-csrf-token">'

defineCaseContactsTable()
})

afterEach(() => {
Swal.fire.mockReset()
fireSwalFollowupAlert.mockReset()
})

describe('Delete action', () => {
it('shows a SweetAlert confirmation dialog when cc-delete-action is clicked', () => {
Swal.fire.mockResolvedValue({ isConfirmed: false })

clickActionButton('delete', { id: '42' })

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())

clickActionButton('delete', { id: '42' })

await Promise.resolve()

expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({
url: '/case_contacts/42',
type: 'DELETE',
headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' }
}))
expect(ajaxSpy.mock.calls[0][0]).not.toHaveProperty('dataType')

ajaxSpy.mockRestore()
})

it('does not send DELETE request when cancelled', async () => {
Swal.fire.mockResolvedValue({ isConfirmed: false })
const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation()

clickActionButton('delete', { id: '42' })

await Promise.resolve()

expect(ajaxSpy).not.toHaveBeenCalled()

ajaxSpy.mockRestore()
})

it('reloads the DataTable without resetting pagination after successful delete', async () => {
Swal.fire.mockResolvedValue({ isConfirmed: true })
jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success())

clickActionButton('delete', { id: '42' })

await Promise.resolve()

expect(mockAjaxReload).toHaveBeenCalledWith(null, false)
})
})

describe('Set Reminder action', () => {
it('calls fireSwalFollowupAlert when cc-set-reminder-action is clicked', () => {
fireSwalFollowupAlert.mockResolvedValue({ isConfirmed: false })

clickActionButton('set-reminder', { id: '5' })

expect(fireSwalFollowupAlert).toHaveBeenCalled()
})

it('posts to the followups endpoint with CSRF header when confirmed without a note', async () => {
fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true })
const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success())

clickActionButton('set-reminder', { id: '5' })

await Promise.resolve()

expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({
url: '/case_contacts/5/followups',
type: 'POST',
data: {},
headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' }
}))

ajaxSpy.mockRestore()
})

it('posts with note when confirmed with a note', async () => {
fireSwalFollowupAlert.mockResolvedValue({ value: 'My note', isConfirmed: true })
const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success())

clickActionButton('set-reminder', { id: '5' })

await Promise.resolve()

expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({
url: '/case_contacts/5/followups',
type: 'POST',
data: { note: 'My note' },
headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' }
}))

ajaxSpy.mockRestore()
})

it('does not post when cancelled', async () => {
fireSwalFollowupAlert.mockResolvedValue({ isConfirmed: false })
const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation()

clickActionButton('set-reminder', { id: '5' })

await Promise.resolve()

expect(ajaxSpy).not.toHaveBeenCalled()

ajaxSpy.mockRestore()
})

it('reloads the DataTable without resetting pagination after creating a reminder', async () => {
fireSwalFollowupAlert.mockResolvedValue({ value: '', isConfirmed: true })
jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success())

clickActionButton('set-reminder', { id: '5' })

await Promise.resolve()

expect(mockAjaxReload).toHaveBeenCalledWith(null, false)
})
})

describe('Resolve Reminder action', () => {
it('sends PATCH request when cc-resolve-reminder-action is clicked', () => {
const ajaxSpy = jest.spyOn($, 'ajax').mockImplementation(({ success }) => success && success())

clickActionButton('resolve-reminder', { id: '5', 'followup-id': '42' })

expect(ajaxSpy).toHaveBeenCalledWith(expect.objectContaining({
url: '/followups/42/resolve',
type: 'PATCH',
headers: { 'X-CSRF-Token': 'test-csrf-token', Accept: 'application/json' }
}))
expect(ajaxSpy.mock.calls[0][0]).not.toHaveProperty('dataType')

ajaxSpy.mockRestore()
})

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(rendered).toBe('<i class="fas fa-ellipsis-v"></i>')
expect(mockAjaxReload).toHaveBeenCalledWith(null, false)
})
})
})
Expand Down
3 changes: 2 additions & 1 deletion app/javascript/src/case_contact.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,6 @@ $(() => { // JQuery's callback for the DOM loading
})

export {
convertDateToSystemTimeZone
convertDateToSystemTimeZone,
fireSwalFollowupAlert
}
Loading
Loading