Skip to content

fix: display division directors and team leaders on review page#5737

Open
weimiao67 wants to merge 4 commits into
mainfrom
OPS-5729/review_page_dd_teamleader
Open

fix: display division directors and team leaders on review page#5737
weimiao67 wants to merge 4 commits into
mainfrom
OPS-5729/review_page_dd_teamleader

Conversation

@weimiao67
Copy link
Copy Markdown
Contributor

@weimiao67 weimiao67 commented May 28, 2026

What changed

Fixed the AgreementMetaAccordion component to display actual Division Director(s) and Team Leader(s) data from the agreement object instead of hardcoded "TBD" values. The component now renders names from agreement.division_directors and agreement.team_leaders string arrays, with multi-column layout support when there are more than 3 entries.

Issue

#5729

How to test

  1. Navigate to an agreement review page
  2. In the "Review Agreement Details" accordion, verify that Division Director(s) and Team Leader(s) display actual names (derived from the budget line items' CAN portfolios)
  3. Verify that when no division directors or team leaders exist, "TBD" is displayed

A11y impact

  • No accessibility-impacting changes in this PR

Screenshots

Screenshot 2026-05-28 at 11 03 49 AM

Definition of Done Checklist

  • OESA: Code refactored for clarity
  • OESA: Dependency rules followed
  • Automated unit tests updated and passed
  • Automated integration tests updated and passed
  • Automated quality tests updated and passed
  • Automated load tests updated and passed
  • Automated a11y tests updated and passed
  • Automated security tests updated and passed
  • 90%+ Code coverage achieved
  • Form validations updated

@weimiao67 weimiao67 self-assigned this May 28, 2026
@weimiao67 weimiao67 marked this pull request as ready for review May 28, 2026 20:24

describe("agreement change accordion", () => {
it("check agreement meta-data", () => {
it.only("check agreement meta-data", () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@weimiao67 did you intend to leave this change in place? The is effectively going to skip 12 tests that come after this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, I forgot to remove it... fixed it in 9331690

Copy link
Copy Markdown
Contributor

@josbell josbell left a comment

Choose a reason for hiding this comment

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

Approved assuming that the .only will be resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants