Skip to content

New Case Contacts Table: expandable case contact rows#6823

Open
cliftonmcintosh wants to merge 7 commits intorubyforgood:mainfrom
cliftonmcintosh:feature/6498-expandable-case-contact-rows
Open

New Case Contacts Table: expandable case contact rows#6823
cliftonmcintosh wants to merge 7 commits intorubyforgood:mainfrom
cliftonmcintosh:feature/6498-expandable-case-contact-rows

Conversation

@cliftonmcintosh
Copy link
Copy Markdown
Collaborator

@cliftonmcintosh cliftonmcintosh commented Apr 2, 2026

What github issue is this PR for, if any?

Resolves #6498

What changed, and why?

Implements expand/collapse functionality for rows in the new case contacts table.

app/datatables/case_contact_datatable.rb

  • Added contact_topic_answers and notes to the JSON response so the browser has the data it needs to render expanded content
  • Added contact_topic_answers: :contact_topic to the includes in raw_records to avoid N+1 queries

app/javascript/src/dashboard.js

  • Updated the chevron column render to add the expand-toggle CSS class and a pointer cursor
  • Captured the DataTable instance in a variable so the click handler can reference it
  • Added a delegated click handler using the DataTables Child Rows API (row.child()) to show/hide expanded content on chevron click
  • Added buildExpandedContent() to build the expanded row HTML from the row's JSON data, matching the structure of the Figma design (topic heading followed by answer paragraph)

app/assets/stylesheets/pages/case_contacts.scss

  • Added expand-toggle styles with a 0.3s CSS rotation animation for the chevron (matching the existing sidebar pattern)
  • Added expanded-content and expanded-topic styles to match the Figma layout

The DataTables Child Rows API is used rather than Bootstrap collapse or manual DOM insertion because DataTables replaces the entire <tbody> via AJAX on initialization. When this happens, any server-rendered or manually inserted rows are destroyed. row.child() inserts rows that DataTables manages itself, so they persist correctly.

Topics column pill styling. While working on this issue I noticed the Topics column was rendering as a raw pipe-delimited string. I did not see an issue that covers the Topics column, so I updated the render for each topic to be a badge-pill chip (matching the existing Draft badge style) with a +N More overflow indicator when there are more than two topics. I am happy to pull this into a separate issue and PR if that is preferred.

How is this tested?

Request specs — added to spec/requests/case_contacts/case_contacts_new_design_spec.rb:

  • contact_topic_answers is present and is an array in the datatable JSON response
  • Topic question is included in each answer
  • notes is included in the response
  • Blank answer values are omitted
  • Notes is blank when the field is empty
  • contact_topics is returned as an array of strings (not a pipe-delimited string)
  • The topic question is included in the array

System specs — added spec/system/case_contacts/case_contacts_new_design_spec.rb:

  • Clicking the chevron shows contact topic answer content in the expanded row
  • Clicking the chevron shows notes in the expanded row
  • Clicking the chevron a second time hides the expanded content

Jest — updated app/javascript/__tests__/dashboard.test.js:

  • Updated chevron render expectation to reflect the expand-toggle class
  • Topics column: each topic renders as a badge-pill chip
  • Topics column: renders empty string when there are no topics
  • Topics column: shows only the first two topics when there are more than two
  • Topics column: shows a +N More badge for overflow topics
  • Topics column: no overflow badge when there are two or fewer topics

Screenshots

Screen recording updated after commit fb31759.

Screen recording of collapsible sections
Screen.Recording.2026-04-05.at.07.40.38.mov

AI Disclosure

This PR was implemented with the assistance of Claude Code (Anthropic, claude-sonnet-4-6). The AI was used to:

  • Analyze the codebase and Figma designs to inform the implementation approach
  • Draft and iterate on code changes across the datatable, JavaScript, and stylesheet files
  • Write and run RSpec request specs, system specs, and Jest tests
  • Investigate the DataTables Child Rows API as the correct architectural approach for this use case

All generated code was reviewed and committed by the author.

cliftonmcintosh and others added 5 commits April 1, 2026 14:00
Co-Authored-By: claude-sonnet-4-6 <noreply@anthropic.com>
Co-Authored-By: claude-sonnet-4-6 <noreply@anthropic.com>
Co-Authored-By: claude-sonnet-4-6 <noreply@anthropic.com>
Co-Authored-By: claude-sonnet-4-6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added javascript for use by Github Labeler to mark pull requests that update Javascript code ruby Pull requests that update Ruby code Tests! 🎉💖👏 labels Apr 2, 2026
@cliftonmcintosh cliftonmcintosh self-assigned this Apr 2, 2026
@cliftonmcintosh cliftonmcintosh changed the title Feature/6498 expandable case contact rows New Case Contact Table: expandable case contact rows Apr 2, 2026
@cliftonmcintosh cliftonmcintosh changed the title New Case Contact Table: expandable case contact rows New Case Contacts Table: expandable case contact rows Apr 2, 2026
@compwron
Copy link
Copy Markdown
Collaborator

compwron commented Apr 5, 2026

Please fix lint errors :)

compwron
compwron previously approved these changes Apr 5, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds expand/collapse behavior to the new Case Contacts DataTable so users can view per-row topic answers and notes inline, while also improving how topics are rendered in the Topics column.

Changes:

  • Extend the case contacts datatable JSON payload to include contact_topic_answers and notes, and preload associations to avoid N+1 queries.
  • Add DataTables child-row expand/collapse behavior in dashboard.js, plus topic “pill” rendering with overflow handling.
  • Add SCSS for chevron rotation and expanded-row content styling; add request/system/Jest coverage for the new behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
spec/system/case_contacts/case_contacts_new_design_spec.rb System coverage for expanding/collapsing and displaying answers/notes
spec/requests/case_contacts/case_contacts_new_design_spec.rb Request coverage for new JSON fields (contact_topic_answers, notes, contact_topics array)
app/javascript/src/dashboard.js UI behavior for toggling child rows and rendering topic pills
app/javascript/tests/dashboard.test.js Jest updates for chevron markup + topic pill rendering rules
app/datatables/case_contact_datatable.rb Add expanded-content fields to JSON + includes for preloading
app/assets/stylesheets/pages/case_contacts.scss Styling for chevron rotation + expanded content layout

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

orderable: false,
searchable: false,
render: () => '<i class="fa-solid fa-chevron-down"></i>'
render: () => '<i class="fa-solid fa-chevron-down expand-toggle" style="cursor: pointer;"></i>'
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The chevron toggle is rendered as a plain and only responds to mouse clicks. This is not keyboard-accessible and doesn’t expose state to assistive tech. Consider rendering a (or adding role="button" + tabindex="0") and toggling aria-expanded (and handling Enter/Space) when the row is expanded/collapsed.

Suggested change
render: () => '<i class="fa-solid fa-chevron-down expand-toggle" style="cursor: pointer;"></i>'
render: () => '<button type="button" class="expand-toggle" aria-expanded="false" aria-label="Expand row details" style="cursor: pointer; background: none; border: 0; padding: 0;"><i class="fa-solid fa-chevron-down" aria-hidden="true"></i></button>'

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.

Updated

orderable: false,
searchable: false,
render: () => '<i class="fa-solid fa-chevron-down"></i>'
render: () => '<i class="fa-solid fa-chevron-down expand-toggle" style="cursor: pointer;"></i>'
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The chevron icon includes an inline cursor style even though .expand-toggle has a dedicated stylesheet block. Moving cursor styling to CSS (e.g., .expand-toggle { cursor: pointer; }) will keep presentation concerns out of JS and make future test expectations less brittle.

Suggested change
render: () => '<i class="fa-solid fa-chevron-down expand-toggle" style="cursor: pointer;"></i>'
render: () => '<i class="fa-solid fa-chevron-down expand-toggle"></i>'

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.

Updated

Comment on lines +158 to +168
$('table#case_contacts tbody').on('click', '.expand-toggle', function () {
const tr = $(this).closest('tr')
const row = table.row(tr)

if (row.child.isShown()) {
row.child.hide()
$(this).removeClass('expanded')
} else {
row.child(buildExpandedContent(row.data())).show()
$(this).addClass('expanded')
}
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

This click handler toggles the expanded visual state but doesn’t update any accessibility attributes (e.g., aria-expanded) or support keyboard activation. If the toggle becomes a button (or role=button), update the handler to keep aria-expanded in sync and respond to Enter/Space.

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.

Updated

Comment on lines +34 to +35
.reject { |a| a.value.blank? }
.map { |a| {question: a.contact_topic&.question, value: a.value} },
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

contact_topic_answers uses safe navigation for a.contact_topic, which can result in { question: nil/blank } entries being returned. That will render an empty heading in the expanded row. Consider filtering out answers with missing/blank questions (or providing a fallback label) so the expanded UI always has a meaningful topic heading.

Suggested change
.reject { |a| a.value.blank? }
.map { |a| {question: a.contact_topic&.question, value: a.value} },
.map { |a| {question: a.contact_topic&.question, value: a.value} }
.reject { |a| a[:value].blank? || a[:question].blank? },

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

@cliftonmcintosh cliftonmcintosh Apr 5, 2026

Choose a reason for hiding this comment

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

I investigated this and, from what I can tell, the scenario where question could be blank is unreachable in practice, for two reasons:

  1. There is a database-level foreign key constraint on contact_topic_answers.contact_topic_id — deleting a ContactTopic while answers still reference it raises a PG::ForeignKeyViolation. I confirmed this by attempting to write a spec that creates an answer, then destroys its topic, and the spec failed with that error.

  2. The ContactTopicAnswer model validates that contact_topic is present whenever value is present. Combined with our existing .reject { |a| a.value.blank? }, every answer that reaches the .map is guaranteed to have a non-nil contact_topic.

The &. safe navigation operator on a.contact_topic&.question is good defensive code and I've left it in place, but adding a second .reject to guard against a state the database constraints prevent would add noise without real protection. I've left the code as-is.

@cliftonmcintosh
Copy link
Copy Markdown
Collaborator Author

cliftonmcintosh commented Apr 5, 2026

Please fix lint errors :)

@compwron thanks for the review.

It looks to me like these are test errors in code unrelated to the changes in this PR. Please let me know if you see anything here that you think might have been impacted by the changes in this PR.

Errors from npm lint
    npm run test
  shell: /usr/bin/bash -e {0}

> casa@0.1.0 test
> jest

PASS app/javascript/__tests__/dashboard.test.js
PASS app/javascript/__tests__/type_checker.test.js
FAIL app/javascript/__tests__/validated_form.test.js
  ● NonDrivingContactMediumWarning › warningHighlightUI › when passed a truthy value, it makes the parent container for the checkboxes yellow and draws a yellow border around the miles driven input

    expect(received).toBe(expected) // Object.is equality

    Expected: "2px solid #ffc107"
    Received: "2px solid rgb(255, 193, 7)"

      152 |
      153 |           expect(checkboxContainer.css('background-color')).toBe('rgb(255, 248, 225)')
    > 154 |           expect(milesDrivenInput.css('border')).toBe('2px solid #ffc107')
          |                                                  ^
      155 |           done()
      156 |         } catch (error) {
      157 |           done(error)

      at Document.toBe (app/javascript/__tests__/validated_form.test.js:154:50)
      at mightThrow (node_modules/jquery/dist/jquery.js:3489:29)
      at process (node_modules/jquery/dist/jquery.js:3557:12)
      at Timeout.task [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:579:19)

  ● NonDrivingContactMediumWarning › warningHighlightUI › when passed a falsy value, it removes the error highlighting

    expect(received).toBe(expected) // Object.is equality

    Expected: "2px solid #ffc107"
    Received: "2px solid rgb(255, 193, 7)"

      169 |
      170 |           expect(checkboxContainer.css('background-color')).toBe('rgb(255, 248, 225)')
    > 171 |           expect(milesDrivenInput.css('border')).toBe('2px solid #ffc107')
          |                                                  ^
      172 |
      173 |           component.warningHighlightUI()
      174 |

      at Document.toBe (app/javascript/__tests__/validated_form.test.js:171:50)
      at mightThrow (node_modules/jquery/dist/jquery.js:3489:29)
      at process (node_modules/jquery/dist/jquery.js:3557:12)
      at Timeout.task [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:579:19)

PASS app/javascript/__tests__/add_to_calendar_button.test.js
PASS app/javascript/__tests__/read_more.test.js
PASS app/javascript/__tests__/case_emancipations.test.js
PASS app/javascript/__tests__/require_communication_preference.test.js
PASS app/javascript/__tests__/password_confirmation.test.js
PASS app/javascript/__tests__/case_button_states.test.js
PASS app/javascript/__tests__/case_contact.test.js
FAIL app/javascript/__tests__/two_minute_warning_session_timeout.test.js
  ● session_timeout_poller › timer runs every second

    TypeError: Cannot redefine property: location
        at Object.defineProperty (<anonymous>)

      14 |     mockReload = jest.fn()
      15 |     global.alert = mockAlert
    > 16 |     Object.defineProperty(window, 'location', {
         |            ^
      17 |       writable: true,
      18 |       value: { reload: mockReload }
      19 |     })

      at Object.defineProperty (app/javascript/__tests__/two_minute_warning_session_timeout.test.js:16:12)

PASS app/javascript/__tests__/time_zone.test.js
PASS app/javascript/__tests__/casa_case.test.js
PASS app/javascript/__tests__/notifier.test.js (7.43 s)

Test Suites: 2 failed, 12 passed, 14 total
Tests:       3 failed, 175 passed, 178 total
Snapshots:   0 total
Time:        8.067 s
Ran all test suites.
Error: Process completed with exit code 1.

My assessment of the Jest failures in CI

With the assistance of Claude Code, from what I can tell, neither of the jest failures is related to the changes in this PR. Both are pre-existing test compatibility issues with newer versions of Node and jsdom. The production code in both cases appears to be correct.

validated_form.test.js (2 failures)

The tests assert '2px solid #ffc107' but newer versions of jsdom normalize CSS color values to rgb() notation when reading them back via jQuery's .css(). A suggested fix would be to update the two assertions to expect '2px solid rgb(255, 193, 7)' instead of the hex string. This is a small, low-risk change confined to the test file.

two_minute_warning_session_timeout.test.js (1 failure)

The test uses Object.defineProperty(window, 'location', { writable: true, value: ... }) to mock window.location. Newer versions of jsdom define location as non-configurable, so this now throws TypeError: Cannot redefine property: location. A suggested fix would be to replace that pattern with delete window.location followed by a plain assignment, which is the modern jsdom-compatible approach.

I've created an issue and opened a PR for fixing this. I am happy to cherry-pick those commits into this PR if that is preferred. Please let me know.

…panded

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cliftonmcintosh
Copy link
Copy Markdown
Collaborator Author

cliftonmcintosh commented Apr 5, 2026

While running the app locally I noticed two N+1 warnings in the Rails log. I think both of them predate the changes in this PR, but I want to flag them for visibility.

expand for log snippet
18:09:23 web.1  | N+1 queries detected:
18:09:23 web.1  |   SELECT "casa_cases"."id", "casa_cases"."case_number", "casa_cases"."created_at", "casa_cases"."updated_at", "casa_cases"."casa_org_id", "casa_cases"."birth_month_year_youth", "casa_cases"."active", "casa_cases"."court_report_submitted_at", "casa_cases"."court_report_status", "casa_cases"."slug", "casa_cases"."date_in_care" FROM "casa_cases" WHERE "casa_cases"."id" = $1 LIMIT $2
18:09:23 web.1  |   SELECT "casa_cases"."id", "casa_cases"."case_number", "casa_cases"."created_at", "casa_cases"."updated_at", "casa_cases"."casa_org_id", "casa_cases"."birth_month_year_youth", "casa_cases"."active", "casa_cases"."court_report_submitted_at", "casa_cases"."court_report_status", "casa_cases"."slug", "casa_cases"."date_in_care" FROM "casa_cases" WHERE "casa_cases"."id" = $1 LIMIT $2
18:09:23 web.1  |   SELECT "casa_cases"."id", "casa_cases"."case_number", "casa_cases"."created_at", "casa_cases"."updated_at", "casa_cases"."casa_org_id", "casa_cases"."birth_month_year_youth", "casa_cases"."active", "casa_cases"."court_report_submitted_at", "casa_cases"."court_report_status", "casa_cases"."slug", "casa_cases"."date_in_care" FROM "casa_cases" WHERE "casa_cases"."id" = $1 LIMIT $2
18:09:23 web.1  |   SELECT "casa_cases"."id", "casa_cases"."case_number", "casa_cases"."created_at", "casa_cases"."updated_at", "casa_cases"."casa_org_id", "casa_cases"."birth_month_year_youth", "casa_cases"."active", "casa_cases"."court_report_submitted_at", "casa_cases"."court_report_status", "casa_cases"."slug", "casa_cases"."date_in_care" FROM "casa_cases" WHERE "casa_cases"."id" = $1 LIMIT $2
18:09:23 web.1  |   SELECT "casa_cases"."id", "casa_cases"."case_number", "casa_cases"."created_at", "casa_cases"."updated_at", "casa_cases"."casa_org_id", "casa_cases"."birth_month_year_youth", "casa_cases"."active", "casa_cases"."court_report_submitted_at", "casa_cases"."court_report_status", "casa_cases"."slug", "casa_cases"."date_in_care" FROM "casa_cases" WHERE "casa_cases"."id" = $1 LIMIT $2
18:09:23 web.1  |   SELECT "casa_cases"."id", "casa_cases"."case_number", "casa_cases"."created_at", "casa_cases"."updated_at", "casa_cases"."casa_org_id", "casa_cases"."birth_month_year_youth", "casa_cases"."active", "casa_cases"."court_report_submitted_at", "casa_cases"."court_report_status", "casa_cases"."slug", "casa_cases"."date_in_care" FROM "casa_cases" WHERE "casa_cases"."id" = $1 LIMIT $2
18:09:23 web.1  |   SELECT "casa_cases"."id", "casa_cases"."case_number", "casa_cases"."created_at", "casa_cases"."updated_at", "casa_cases"."casa_org_id", "casa_cases"."birth_month_year_youth", "casa_cases"."active", "casa_cases"."court_report_submitted_at", "casa_cases"."court_report_status", "casa_cases"."slug", "casa_cases"."date_in_care" FROM "casa_cases" WHERE "casa_cases"."id" = $1 LIMIT $2
18:09:23 web.1  | Call stack:
18:09:23 web.1  |   app/datatables/case_contact_datatable.rb:20:in 'block in CaseContactDatatable#data'
18:09:23 web.1  |   app/datatables/case_contact_datatable.rb:14:in 'Enumerable#map'
18:09:23 web.1  |   app/datatables/case_contact_datatable.rb:14:in 'CaseContactDatatable#data'
18:09:23 web.1  |   app/datatables/application_datatable.rb:15:in 'ApplicationDatatable#as_json'
18:09:23 web.1  |   app/controllers/case_contacts/case_contacts_new_design_controller.rb:15:in 'CaseContacts::CaseContactsNewDesignController#datatable'
18:09:23 web.1  |
18:09:23 web.1  | N+1 queries detected:
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  |   SELECT 1 AS one FROM "followups" WHERE "followups"."case_contact_id" = $1 AND "followups"."status" = $2 LIMIT $3
18:09:23 web.1  | Call stack:
18:09:23 web.1  |   app/datatables/case_contact_datatable.rb:38:in 'block in CaseContactDatatable#data'
18:09:23 web.1  |   app/datatables/case_contact_datatable.rb:14:in 'Enumerable#map'
18:09:23 web.1  |   app/datatables/case_contact_datatable.rb:14:in 'CaseContactDatatable#data'
18:09:23 web.1  |   app/datatables/application_datatable.rb:15:in 'ApplicationDatatable#as_json'
18:09:23 web.1  |   app/controllers/case_contacts/case_contacts_new_design_controller.rb:15:in 'CaseContacts::CaseContactsNewDesignController#datatable'

N+1 #1

casa_case (datatable line 20)

case_contact.casa_case&.case_number triggers a query per row because :casa_case is missing from the includes. The existing .left_joins(:casa_case) joins for SQL filtering purposes but does not eager load the association.

Fix: add :casa_case to the includes chain in raw_records.

N+1 #2

followups.requested.exists? (datatable line 38)

:followups is in the includes, so the association records are loaded in memory. However, calling .requested.exists? fires a new SQL query regardless because exists? always hits the database.

Fix: replace with case_contact.followups.any?(&:requested?) — since requested is an enum value, Rails generates a requested? predicate method on each record, so this operates on the already-loaded association without an additional query.

I am happy to fix both in this PR if that is preferred, or leave them for a separate issue.

@cliftonmcintosh cliftonmcintosh requested a review from compwron April 5, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript for use by Github Labeler to mark pull requests that update Javascript code ruby Pull requests that update Ruby code Tests! 🎉💖👏

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New Case Contact Table: implement expansion functionality

3 participants