[MOSIP-44045] - Bulk Upload Error Message Fix #833
Conversation
Signed-off-by: GOKULRAJ136 <[email protected]>
WalkthroughRemoved the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component as CreateComponent
participant AuditSvc as AuditService
participant Dialog as ConfirmationDialog
participant SaveSvc as SaveService
User->>Component: click "Submit"
Component->>Component: if form.invalid\nmarkAllControlsTouched()\nreturn
alt form valid
Component->>AuditSvc: create pre-save audit entry
Component->>Dialog: open confirmation dialog with data
Dialog-->>User: show confirm/cancel
alt user confirms
Dialog-->>Component: confirmation result
Component->>SaveSvc: save data
Component->>AuditSvc: record success audit
Component-->>User: success result
else user cancels
Dialog-->>Component: cancel result
Component-->>User: cancelled
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In
@admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.ts:
- Around line 145-148: The early-return for if (this.uploadForm.invalid)
prevents the fileName-specific UI feedback from running; before returning,
invoke the same fileName validation handling that's in the else block (lines
~172-178) — either call/extract a helper (e.g., handleFileNameError()) or
replicate its steps: get the fileName control (this.uploadForm.get('fileName')),
if invalid set this.fileNameError = true and apply the red-border/invalid CSS to
the fileName input element (or call the existing method that does this) so the
visual red border and flag remain set, then return.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.ts (1)
169-185: Dead code: Else block is now unreachable.The else block at lines 169-185 is now unreachable due to the early return at line 148. This code can never execute, making it dead code that should be removed once the fileName handling logic is properly preserved.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: uitest-admin / build-dockers
- GitHub Check: build-admin-ui / npm-build
- GitHub Check: Analyze (java-kotlin)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.ts
Show resolved
Hide resolved
Signed-off-by: GOKULRAJ136 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.ts (1)
133-133: Remove commented-out code.This commented-out line is no longer needed since the red border handling has been replaced with the standard Angular Material validation approach.
🔎 Proposed fix
this.uploadForm.get('files').setValue(file); this.uploadForm.get('fileName').setValue(file.name); - //document.getElementById("fileName").classList.remove('addredborder'); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.htmladmin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: uitest-admin / build-dockers
- GitHub Check: uitest-admin / build-docker-warning
- GitHub Check: build-admin-ui / npm-build
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (2)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.html (1)
66-67: LGTM! Standard Angular Material validation pattern implemented correctly.The addition of
mat-errorfor the fileName control follows Angular Material best practices. When the form is submitted with an invalid file input, themarkAllAsTouched()call in the TypeScript will trigger this error message to display.admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.ts (1)
143-165: Validation refactor is cleaner and follows Angular Material standards.The early-return guard with
markAllAsTouched()correctly triggers validation for all form controls, including the fileName field. The correspondingmat-errorin the template (line 67) will display the validation message when the fileName control is invalid and touched.Regarding the previous review concern about missing red border visual feedback: this refactor intentionally moves from custom CSS error handling (
fileNameErrorflag and manual border styling) to standard Angular Material validation patterns. The mat-error provides clear validation feedback, and the commented-out code at line 133 confirms the old approach is being deprecated.Note: If the custom red border styling is still required for UX reasons, it can be applied via CSS by targeting the
.fileuploadcontainerwhen the fileName control is invalid:.fileuploadcontainer.ng-invalid.ng-touched { border: 1px solid red; }
Signed-off-by: GOKULRAJ136 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.css (1)
85-85: Height increase accommodates error message display.The 8px height increase (44px → 52px) aligns with the new validation message display mentioned in the PR objectives. The change appropriately provides space for the
mat-errorelement.Consider using min-height for better flexibility
For improved responsiveness and accessibility (e.g., larger user font sizes or longer translated text), consider using
min-heightinstead of a fixedheight:-.fileuploadcontainer label { +.fileuploadcontainer label { color: #FF4081; width: 148px; - height: 52px; + min-height: 52px; border-radius: 5px;This allows the label to grow if content requires more space while maintaining the minimum height for consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.css
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: uitest-admin / build-docker-warning
- GitHub Check: uitest-admin / build-dockers
- GitHub Check: build-admin-ui / npm-build
- GitHub Check: Analyze (java-kotlin)
Signed-off-by: GOKULRAJ136 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @admin-ui/src/assets/i18n/kan.json:
- Around line 791-801: validationMessages contains mixed scripts and
inconsistent terminology: update validationMessages.file.required from the
Korean/Kannada mix ("파일್ ...") to proper Kannada (e.g., use "ಫೈಲ್ ಅಗತ್ಯ" or
similar consistent phrasing) and change validationMessages.tableName.required to
use the same Kannada term for table as elsewhere (e.g., replace with "ಟೇಬಲ್
ಹೆಸರು ಅಗತ್ಯ" to match bulkUpload.tableName wording).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.htmladmin-ui/src/assets/i18n/ara.jsonadmin-ui/src/assets/i18n/eng.jsonadmin-ui/src/assets/i18n/es.jsonadmin-ui/src/assets/i18n/fra.jsonadmin-ui/src/assets/i18n/hin.jsonadmin-ui/src/assets/i18n/kan.jsonadmin-ui/src/assets/i18n/tam.json
🚧 Files skipped from review as they are similar to previous changes (1)
- admin-ui/src/app/features/bulkupload/masterdataupload/create/create.component.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: uitest-admin / build-dockers
- GitHub Check: build-admin-ui / npm-build
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (6)
admin-ui/src/assets/i18n/es.json (1)
708-718: LGTM: bulkUpload required-field validation messages added consistently.
Keys match the expected structure (bulkUpload.validationMessages.{operation|tableName|file}.required) and should surface correctly in the UI.admin-ui/src/assets/i18n/ara.json (1)
665-675: LGTM: Arabic bulkUpload validation messages added with the correct key structure.admin-ui/src/assets/i18n/eng.json (1)
666-676: LGTM: English bulkUpload validation messages are clear and consistent.admin-ui/src/assets/i18n/hin.json (1)
784-794: LGTM! Validation messages properly added for bulk upload form.The new
validationMessagesblock correctly adds required-field validation messages for the three form controls (operation, tableName, and file). The JSON structure is valid, and the keys align with the centralized form validation approach described in the PR.admin-ui/src/assets/i18n/fra.json (1)
924-934: LGTM! Validation messages properly added for bulk upload form.The new
validationMessagesblock correctly adds required-field validation messages in French for the three form controls. The JSON structure is valid, keys are consistent with other locale files, and the implementation supports the centralized form validation approach.admin-ui/src/assets/i18n/tam.json (1)
792-802: LGTM! Validation messages properly added for bulk upload form.The new
validationMessagesblock correctly adds required-field validation messages in Tamil for the three form controls. The JSON structure is valid, keys are consistent with other locale files, and the implementation supports the centralized form validation approach described in the PR.
Signed-off-by: GOKULRAJ136 <[email protected]>
1 participant
Summary by CodeRabbit
Bug Fixes
User Interface
Localization
✏️ Tip: You can customize this high-level summary in your review settings.