Skip to content

MPDX-9616 - Fix line item letters in PdsGoalCalculator#1795

Merged
zweatshirt merged 2 commits into
mainfrom
MPDX-9616
May 21, 2026
Merged

MPDX-9616 - Fix line item letters in PdsGoalCalculator#1795
zweatshirt merged 2 commits into
mainfrom
MPDX-9616

Conversation

@zweatshirt
Copy link
Copy Markdown
Contributor

@zweatshirt zweatshirt commented May 21, 2026

Description

  • Re-adds the letters for sub line items in the Summary of the PDS Goal calculator
  • Fixes issue where line item was displaying as 2C even if 2A and 2B weren't included. The solution is just to change this 2C item to be 2A.

https://jira.cru.org/browse/MPDX-9616

Testing

  • Go to Pds Goal Calculator
  • Check the summary line items and that the line items are in the correct order for both default and simple versions.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
  • I have run the Claude Code /pr-review command locally and fixed any relevant suggestions
  • I have requested a review from another person on the project
  • I have tested my changes in preview or in staging
  • I have cleaned up my commit history

@zweatshirt zweatshirt changed the title Fix line item letters in PdsGoalCalculator MPDX-9616 - Fix line item letters in PdsGoalCalculator May 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Bundle sizes [mpdx-react]

Compared against 1d25db1

No significant changes found

@zweatshirt zweatshirt marked this pull request as ready for review May 21, 2026 17:30
@zweatshirt zweatshirt self-assigned this May 21, 2026
@zweatshirt zweatshirt added the Preview Environment Add this label to create an Amplify Preview label May 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Preview branch generated at https://MPDX-9616.d3dytjb8adxkk5.amplifyapp.com

Copy link
Copy Markdown
Contributor Author

@zweatshirt zweatshirt left a comment

Choose a reason for hiding this comment

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

Multi-Agent Code Review — MPDX-9616

Verdict: ✅ CLEAN

No significant issues found. 4 specialized agents reviewed this PR (Architecture, Testing, Standards, UX). Two medium-priority findings from the initial pass were addressed in the latest commit (explicit formType on the mock, corrected test title). Only informational suggestions remain.


Risk Assessment

  • Score: 1/10 — LOW
  • Reviewer Level: Any
  • Dependency Impact: PdsSummaryTable has 1 consumer (SummaryReportStep.tsx). No breaking changes.

Suggestions (informational — do not require /dismiss)

  • PdsSummaryTable.tsx:128,137isSimple ? '2A' : '2C' is duplicated. A local variable like const singleSubItemLine = isSimple ? '2A' : '2C' would remove the duplication.
  • PdsSummaryTable.tsx:200 — Regex /^[1-5][A-Z]?$/ accepts any capital suffix; current data only uses A–C. /^[1-5][A-C]?$/ would be more precise, though either is correct for this closed dataset.
  • PdsSummaryTable.test.tsx — The PartTime + Simple path (Work Comp → '2A') is exercised by a pre-existing test but without a line-label assertion. Low priority given the identical logic to the FullTime + Simple path.

Agent Critical High Important Suggestions Confidence
Architecture 0 0 0 3 High
Testing 0 0 0 2 High
Standards 0 0 0 0 High
UX 0 0 0 0 High
Total 0 0 0 5

? [
{
line: '2C',
line: isSimple ? '2A' : '2C',
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.

[Suggestion] `isSimple ? '2A' : '2C'` is identical on lines 128 and 137. A local variable would remove the duplication:
const singleSubItemLine = isSimple ? '2A' : '2C';

Not blocking — readability note only.

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.

I find the current impl more readable, however, >= 3 occurrences I would definitely do this.

Comment thread src/components/HrTools/PdsGoalCalculator/SummaryReport/PdsSummaryTable.tsx Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

AI Review Auto-Approval

Risk Level: LOW (1/10)
Verdict: CLEAN (no issues found)

This PR was auto-approved because:

  • The multi-agent AI review determined it is low risk
  • No blocking issues were found

If you believe this PR needs human review, dismiss this approval and request a review manually.

@zweatshirt zweatshirt enabled auto-merge May 21, 2026 18:02
@zweatshirt zweatshirt merged commit 4a5f134 into main May 21, 2026
24 checks passed
@zweatshirt zweatshirt deleted the MPDX-9616 branch May 21, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Preview Environment Add this label to create an Amplify Preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant