-
Notifications
You must be signed in to change notification settings - Fork 46
Pgr hrms dtpk #2868
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
base: develop
Are you sure you want to change the base?
Pgr hrms dtpk #2868
Conversation
📝 WalkthroughWalkthroughThe updates include documentation changes to the changelog and README for the HRMS module, a configuration adjustment by commenting out a banner component, a new style applied to the EmployeeDOB field, and enhancements to the createEmployee page. The latter adds jurisdiction validation and a new ToastOverlay component for improved toast notification handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateEmployeePage
participant DigitUserService
participant ToastOverlay
User->>CreateEmployeePage: Submit Employee Form
CreateEmployeePage->>DigitUserService: getUser()
DigitUserService-->>CreateEmployeePage: userInfo (tenantId)
CreateEmployeePage->>CreateEmployeePage: hasMatchingJurisdiction(jurisdictions, userInfo.tenantId)
alt No matching jurisdiction
CreateEmployeePage->>ToastOverlay: Show error toast (ERR_BASE_TENANT_MANDATORY)
ToastOverlay-->>User: Display error message
else Valid jurisdiction
CreateEmployeePage->>CreateEmployeePage: Proceed with submission
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
micro-ui/web/micro-ui-internals/example/package.json
is excluded by!**/*.json
micro-ui/web/micro-ui-internals/packages/modules/hrms/package.json
is excluded by!**/*.json
micro-ui/web/package.json
is excluded by!**/*.json
micro-ui/web/sandbox/package.json
is excluded by!**/*.json
📒 Files selected for processing (5)
micro-ui/web/micro-ui-internals/packages/modules/hrms/CHANGELOG.md
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/hrms/README.md
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/config/config.js
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/EmployeeDOB.js
(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/createEmployee.js
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/EmployeeDOB.js
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/config/config.js
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/createEmployee.js
🪛 markdownlint-cli2 (0.17.2)
micro-ui/web/micro-ui-internals/packages/modules/hrms/CHANGELOG.md
1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
2-2: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
2-2: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/config/config.js (1)
65-74
: Banner component commented out - verify intentionality.The HRBanner component with employee ID message has been commented out. This will remove the informational banner from the employee creation form.
Please confirm this is an intentional UI change. If this is temporary for testing, consider using feature flags instead of commenting out code.
micro-ui/web/micro-ui-internals/packages/modules/hrms/README.md (1)
55-55
: LGTM! Changelog entry is well-formatted.The new changelog entry follows the established format and clearly describes the PRR validation feature addition.
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/EmployeeDOB.js (1)
21-24
: LGTM! Style object is well-defined.The fieldStyle object with maxWidth and paddingRight is properly structured for responsive design.
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/createEmployee.js (5)
11-11
: LGTM! User info retrieval for validation.Adding userInfo retrieval enables proper tenant validation in the jurisdiction check.
168-172
: LGTM! Utility function is well-implemented.The hasMatchingJurisdiction function correctly validates jurisdiction matching with proper null checks and array validation.
326-326
: LGTM! Improved toast rendering with component.The ToastOverlay component provides better encapsulation and consistent positioning for toast notifications.
212-213
: Confirm jurisdiction key consistency across formsIt looks like the duplicate-jurisdiction counting was switched from
sum.tenantId
tosum.boundary
in createEmployee.js, but EditEmployee’s form still usessum.tenantId
. This will lead to inconsistent behavior when detecting duplicates.Please verify which key is correct and update accordingly:
- micro-ui/.../createEmployee.js (lines 212–213): now uses
sum.boundary
- micro-ui/.../EditEmployee/EditForm.js (line 183): still uses
sum.tenantId
If
boundary
is intended, change the EditForm logic to usesum.boundary
; if not, revert createEmployee back tosum.tenantId
.
188-191
: Missing translation key: “ERR_BASE_TENANT_MANDATORY”The search across all
.json
translation files returned no matches for this error key. Please verify:
- That the key
ERR_BASE_TENANT_MANDATORY
is defined in your translation files (or correct file type/extension).- That this key aligns with the agreed business terminology and requirements for jurisdiction errors.
Files/locations to check:
- micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/createEmployee.js (lines 188–191)
## [1.8.15] [25-July-2025] | ||
- DatePicker HRMS Maxwidth Added, | ||
|
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.
🧹 Nitpick (assertive)
Fix markdown formatting issues.
The changelog entry has several formatting issues that should be addressed:
+
## [1.8.15] [25-July-2025]
-- DatePicker HRMS Maxwidth Added,
+- DatePicker HRMS Maxwidth Added
+
Also consider using a consistent date format with other entries (some use DD-MMM-YYYY, others use DD-Month-YYYY).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
## [1.8.15] [25-July-2025] | |
- DatePicker HRMS Maxwidth Added, | |
| |
## [1.8.15] [25-July-2025] | |
- DatePicker HRMS Maxwidth Added | |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
2-2: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
2-2: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/modules/hrms/CHANGELOG.md at lines 1
to 3, fix the markdown formatting by removing the trailing comma after the entry
description and ensure consistent spacing and punctuation. Also, standardize the
date format to match other entries, preferably using a consistent style like
DD-MMM-YYYY or DD-Month-YYYY throughout the file.
<div className="field" style={{ | ||
fieldStyle | ||
}}> |
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.
Fix style application syntax error.
The style object is not being applied correctly. The current syntax creates a nested object instead of applying the fieldStyle directly.
- <div className="field" style={{
- fieldStyle
- }}>
+ <div className="field" style={fieldStyle}>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div className="field" style={{ | |
fieldStyle | |
}}> | |
<div className="field" style={fieldStyle}> |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/EmployeeDOB.js
around lines 39 to 41, the style attribute is incorrectly applying the
fieldStyle object by nesting it inside another object. To fix this, remove the
extra curly braces so that the style attribute directly receives the fieldStyle
object without nesting.
const ToastOverlay = ({ showToast, t, onClose }) => { | ||
if (!showToast) return null; | ||
return ( | ||
<div style={{ | ||
position: "fixed", | ||
bottom: "100px", | ||
left: "50%", | ||
transform: "translateX(-50%)", | ||
zIndex: 9999, | ||
maxWidth: "90%", | ||
}}> | ||
<Toast | ||
type={showToast.key} | ||
label={t(showToast.label)} | ||
onClose={onClose} | ||
/> | ||
</div> | ||
); | ||
}; |
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.
🧹 Nitpick (assertive)
Consider extracting inline styles to a CSS class or styled component.
The ToastOverlay component improves code organization, but the inline styles could be better managed.
Consider creating a CSS class or using styled-components:
.toast-overlay {
position: fixed;
bottom: 100px;
left: 50%;
transform: translateX(-50%);
z-index: 9999;
max-width: 90%;
}
- <div style={{
- position: "fixed",
- bottom: "100px",
- left: "50%",
- transform: "translateX(-50%)",
- zIndex: 9999,
- maxWidth: "90%",
- }}>
+ <div className="toast-overlay">
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/hrms/src/pages/createEmployee.js
around lines 148 to 166, the ToastOverlay component uses inline styles for
positioning and layout. To improve code organization and maintainability,
extract these inline styles into a CSS class or a styled component. Create a CSS
class with the given style properties and apply it to the wrapping div instead
of inline styles, or define a styled component with these styles and use it in
place of the div.
Choose the appropriate template for your PR:
Feature/Bugfix Request
JIRA ID
Module
Description
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Refactor