Skip to content

fix(sltb): assessed levy positive amounts already on main - #175

Closed
ginaxu1 wants to merge 4 commits into
mainfrom
fix/sltb-officer-required-asterisks
Closed

ginaxu1 wants to merge 4 commits into
mainfrom
fix/sltb-officer-required-asterisks

Conversation

@ginaxu1

@ginaxu1 ginaxu1 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Related: https://github.com/LSFLK/lsf-govtech-tnsw/issues/180

Summary

Test plan

  • Levy: -5 / 0 → error; 1000 / 1000.50 / 10.2 → accept
  • With ui-packages#112: approve shows red *; signed keys blocked on levy input

These fields are only required when Review Outcome is approve (allOf),
so JSONForms never sets ControlProps.required and the red * is missing.
Mark them in the title so officers see they are mandatory when shown.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 06b6d4d6-83a6-4eda-97e6-d41ebcd2cde7
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Blendsheet reference and levy are required only when Review Outcome is
approve (allOf), so JSONForms never sets ControlProps.required. Drop the
title "*" hack and set uiSchema options.required so TextControl/NumberControl
can show the same red asterisk as other required fields.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ginaxu1 ginaxu1 changed the title fix(sltb): show required asterisk on blendsheet ref and levy fix(sltb): red required asterisk on blendsheet ref and levy when approving Oct 15, 2026
Aravinda-HWK
Aravinda-HWK previously approved these changes Oct 15, 2026
Rely on ui-packages useIsRequired for the approve-path asterisk.
Keep then.required for reference_number and levy_amount, and constrain
levy_amount with exclusiveMinimum 0 and multipleOf 0.01.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ginaxu1 ginaxu1 changed the title fix(sltb): red required asterisk on blendsheet ref and levy when approving fix(sltb): required asterisk and positive levy on blend sheet approve Oct 15, 2026
@ginaxu1

ginaxu1 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

Updated to match OpenNSW/ui-packages#112

  • Removed options.required from the blendsheet ref and levy controls. The asterisk now comes from then.required via the renderer's AJV-based useIsRequired.
  • levy_amount already had exclusiveMinimum: 0; added multipleOf: 0.01 so values like 1000 and 1000.50 are valid and non-positive amounts are rejected.

@ginaxu1 ginaxu1 changed the title fix(sltb): required asterisk and positive levy on blend sheet approve fix(sltb): constrain assessed levy to positive amounts with cents Oct 15, 2026
@ginaxu1
ginaxu1 requested a review from Aravinda-HWK October 15, 2026 05:27
multipleOf: 0.01 falsely rejects valid positive amounts such as 10.2
because of IEEE float remainder checks in AJV. Positive-only validation
stays as exclusiveMinimum: 0 (already on main from #159/#165).

Co-authored-by: Cursor <cursoragent@cursor.com>
@ginaxu1 ginaxu1 changed the title fix(sltb): constrain assessed levy to positive amounts with cents fix(sltb): assessed levy positive amounts already on main Oct 15, 2026
@ginaxu1

ginaxu1 commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Not needed, remaining UX is #172

@ginaxu1 ginaxu1 closed this Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants