-
Notifications
You must be signed in to change notification settings - Fork 849
WEB-682 Add validation to prevent negative values in code value position field #3112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Position Field Validation src/app/system/codes/view-code/view-code.component.html, src/app/system/codes/view-code/view-code.component.ts |
Added min="0" attribute to position input element and expanded form validators from Validators.required to [Validators.required, Validators.min(0)]. Changed initial position value from 0 to empty string in form initialization. |
Estimated code review effort
🎯 1 (Trivial) | ⏱️ ~4 minutes
Possibly related PRs
- openMF/web-app#2983: Both PRs add
min="0"to number inputs and introduceValidators.min(0)for corresponding Angular form controls to prevent negative values. - openMF/web-app#2833: Both PRs apply a minimum-value constraint to a numeric form field by adding min attribute and
Validators.minrule to prevent non-positive/negative inputs.
Suggested reviewers
- alberto-art3ch
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main change: adding validation to prevent negative values in the code value position field, which aligns with the changes in both the HTML (min="0" attribute) and TypeScript (Validators.min(0)) implementations. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/system/codes/view-code/view-code.component.html (1)
70-78:⚠️ Potential issue | 🟠 MajorMissing
mat-errorfor the newminvalidation, and existing error message references wrong field.Two issues in this block:
The
min="0"HTML attribute is added, andValidators.min(0)is set in the TS, but there's nomat-errorfor theminvalidation error. Users who enter a negative value will see the field marked invalid with no error message explaining why.Line 75 uses
'labels.inputs.name'instead of'labels.inputs.Position'— this is a copy-paste bug from the name field's error block (pre-existing, but directly relevant to this change area).Proposed fix
`@if` (codeValues.at(i).controls.position.hasError('required')) { <mat-error> - {{ 'labels.inputs.name' | translate }} {{ 'labels.commons.is' | translate }} + {{ 'labels.inputs.Position' | translate }} {{ 'labels.commons.is' | translate }} <strong>{{ 'labels.commons.required' | translate }}</strong> </mat-error> } + `@if` (codeValues.at(i).controls.position.hasError('min')) { + <mat-error> + {{ 'labels.inputs.Position' | translate }} + <strong>{{ 'labels.commons.must be 0 or greater' | translate }}</strong> + </mat-error> + }
alberto-art3ch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@alberto-art3ch Thank You for the review |
Changes Made :-
-Added minimum value validation (min=0) to prevent negative numbers in the Code Value Position field.
WEB-682
Before :-
After :-
Summary by CodeRabbit