myMicroplanFixes #1708
myMicroplanFixes #1708
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces multiple changes across various components and configuration files within the micro-ui project. Key modifications include updates to role validation logic, enhancements to search and user management functionalities, and styling adjustments for improved UI consistency. Notable changes involve the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range comments (17)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchPlanWithCensus.js (2)
Line range hint 1-54: Improve error handling specificity and consistency.
The current error handling uses the same error message "Employee not found with the given role" for both API failures, which might be misleading. Consider differentiating between plan and census service errors for better debugging.
Consider restructuring the error handling as follows:
- if (!response) {
- throw new Error("Employee not found with the given role");
+ if (!response?.Plan) {
+ throw new Error("Failed to fetch plan data from plan-service");
}
- if (!fetchCensusData) {
- throw new Error("Employee not found with the given role");
+ if (!fetchCensusData?.Census) {
+ throw new Error("Failed to fetch census data from census-service");
}Also, consider adding request tracing or correlation IDs to help debug issues across these connected services.
Line range hint 1-1: Consider documenting the function parameters.
The function accepts multiple parameters but lacks JSDoc documentation explaining their purpose and expected types.
Add JSDoc documentation:
+/**
+ * Searches for plans with census data
+ * @param {Object} params - Search parameters
+ * @param {string} params.tenantId - Tenant identifier
+ * @param {string} params.microplanId - Microplan identifier
+ * @param {Object} params.body - Search criteria for plan service
+ * @param {number} params.limit - Maximum number of records to return
+ * @param {number} params.offset - Number of records to skip
+ * @param {string} params.roles - Comma-separated list of roles (default: "")
+ * @returns {Promise<{planData: Array, censusData: Array, StatusCount: Object, TotalCount: number}>}
+ */
const searchPlanWithCensus = async ({ tenantId, microplanId, body, limit, offset, roles = "" }) => {health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1)
Line range hint 29-37: Clean up commented code.
Remove commented code blocks to improve code maintainability. If these fields might be needed in the future, consider documenting them in a separate technical specification or ticket instead.
Also applies to: 38-46, 47-54
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (5)
Line range hint 4-39: Improve maintainability with SCSS variables and consolidated styles.
Consider using SCSS variables for consistent values and consolidating common styles. This will make the code more maintainable and reduce duplication.
+$tab-padding: 10px 35px;
+$tab-border-radius: 0.5rem 0.5rem 0 0;
+$primary-color: #f47738;
+
.search-tabs-container {
display: flex;
justify-content: space-between;
background-color: #eee;
.search-tab-head {
- padding: 10px 35px;
+ padding: $tab-padding;
font-weight: 700;
font-size: 1rem;
border: 1px solid #d6d5d4;
- border-radius: 0.5rem 0.5rem 0 0;
+ border-radius: $tab-border-radius;
}
.search-tab-head-selected {
- padding: 10px 35px;
- color: rgb(244, 119, 56);
+ padding: $tab-padding;
+ color: $primary-color;
background-color: #fff;
- border: 1px solid #f47738;
- border-radius: 0.5rem 0.5rem 0 0;
- border-bottom: 4px solid rgb(244, 119, 56);
+ border: 1px solid $primary-color;
+ border-radius: $tab-border-radius;
+ border-bottom: 4px solid $primary-color;Line range hint 41-117: Clean up and standardize document management styles.
- Remove commented code
- Use consistent color naming
- Consider extracting button styles into a reusable class
- .dm-campaign-preview-edit-container span {
- /*margin-right: 0.25rem;*/
+ .dm-campaign-preview-edit-container span {
font-size: 0.875rem;
font-weight: 500;
- color: #dc5a32;
+ color: var(--primary-orange);
}Consider creating a shared button style class:
.dm-button-base {
border: 1px solid var(--primary-orange);
background-color: transparent;
padding: 0.25rem 0.5rem;
border-radius: 4px;
font-size: 0.875rem;
font-weight: 500;
cursor: pointer;
display: flex;
align-items: center;
justify-content: center;
&:hover {
background-color: #fbe9e6;
}
}Line range hint 119-139: Fix invalid CSS syntax in audit info styles.
The dm-audit-info class contains invalid CSS syntax with string values and inconsistent color usage.
.dm-audit-info{
max-width: 70%;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
- color: #C84C0E;
- margin-bottom: "1rem";
- font-size: "0.875rem"
+ color: var(--primary-orange);
+ margin-bottom: 1rem;
+ font-size: 0.875rem;
}Line range hint 142-219: Consider using modern CSS Grid or Flexbox for table-like layouts.
While display: table works, modern layout techniques would provide better responsiveness and maintainability. Also, ensure proper accessibility for table-like structures.
Consider refactoring to use CSS Grid:
.as-table-like {
display: grid;
grid-template-columns: minmax(20rem, 30%) 1fr;
gap: 10px;
width: 100%;
.as-table-row {
display: contents;
}
.as-key-cell,
.as-value-cell {
padding: 10px;
border-bottom: 1px solid #D6D5D4;
}
}Line range hint 221-255: Standardize spacing units and fix invalid CSS syntax.
- Use consistent spacing units throughout the stylesheet
- Fix invalid border-bottom string value in as-label-field
.as-label-field {
display: grid;
grid-template-columns: 1fr 2fr;
- gap: 0rem;
+ gap: 0;
overflow: auto;
align-items: center;
- border-bottom: '1px solid #e0e0e0'
+ border-bottom: 1px solid #e0e0e0;
}health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (2)
Line range hint 281-292: Consider adding a safeguard for tab visibility state.
While the logic correctly handles the tab visibility based on the filter state, consider adding a safeguard to ensure showTab state stays in sync:
if (selectedFilter === "VALIDATED") {
setActiveLink({ code: "", name: "" });
setShowTab(false);
} else {
- if (!showTab) {
+ // Always ensure proper tab visibility for non-VALIDATED states
+ setShowTab(true);
+ if (activeLink.code === "") {
setActiveLink({
code: "ASSIGNED_TO_ME",
name: "ASSIGNED_TO_ME"
});
- setShowTab(true);
}
}Line range hint 1-577: Consider implementing performance optimizations.
The component handles complex state management well, but could benefit from these optimizations:
- Memoize complex objects and callbacks:
// Memoize the request criteria
const reqCriteriaResource = useMemo(() => ({
url: '/census-service/_search',
body: {
CensusSearchCriteria: {
// ... existing criteria
},
},
config: {
enabled: jurisdiction?.length > 0,
},
}), [tenantId, microplanId, selectedFilter, activeLink.code, jurisdiction, limitAndOffset]);
// Memoize handlers
const handleActionClick = useCallback((action) => {
setworkFlowPopUp(action);
}, []);- Consider splitting the component into smaller sub-components to reduce re-renders:
// Extract the filter section
const FilterSection = memo(({ activeFilter, onFilter, clearFilters, selectedFilter }) => {
return (
<InboxFilterWrapper
options={activeFilter}
onApplyFilters={onFilter}
clearFilters={clearFilters}
defaultValue={...}
/>
);
});health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Line range hint 1-582: Consider performance optimizations
The component handles complex state and computations. Consider using useMemo for expensive operations:
- Column definitions
- Status condition checks
- Workflow updates for selected rows
Example optimization for columns:
const columns = useMemo(() => [
{
name: t(`INBOX_VILLAGE`),
cell: (row) => t(row?.village) || "NA",
sortable: true,
},
// ... other columns
], [t, resources]);health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Line range hint 1-557: Consider architectural improvements for better maintainability
The component handles multiple responsibilities and has repeated patterns that could be optimized:
- Extract the row lookup logic into a custom hook or utility function
- Consider using
useMemofor the filtered row data - Split the search functionality into a separate component
Example implementation of a custom hook:
const useRowData = (rowData, rowIndex) => {
return useMemo(() =>
rowData?.find(item => item?.rowIndex === rowIndex),
[rowData, rowIndex]
);
};This would simplify the component and improve performance by reducing repeated computations.
🧰 Tools
🪛 Biome
[error] 378-382: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js (3)
Line range hint 1-680: Standardize error handling patterns across the file.
The error handling patterns are inconsistent throughout the file. For example:
// Pattern 1
if (!error?.response?.data?.Errors[0].description) {
throw new Error(error?.response?.data?.Errors[0].code);
} else {
throw new Error(error?.response?.data?.Errors[0].description);
}
// Pattern 2
if (error?.response?.data?.Errors) {
throw new Error(error.response.data.Errors[0].message);
}Consider creating a utility function to standardize error handling:
const handleServiceError = (error) => {
if (!error?.response?.data?.Errors?.length) {
throw new Error(error.message || 'Unknown error occurred');
}
const serviceError = error.response.data.Errors[0];
throw new Error(serviceError.description || serviceError.message || serviceError.code);
};Line range hint 165-615: Refactor switch case implementation to reduce complexity and duplication.
The switch case implementation has several areas that could be improved:
- Similar patterns of fetching and updating plan data are repeated across cases
- Complex nested logic within each case makes the code harder to maintain
- State updates are scattered throughout the cases
Consider breaking down the switch case into smaller, focused functions:
const handlers = {
async CAMPAIGN_DETAILS(params) {
const { setCurrentKey, setCurrentStep } = params;
return handleStepNavigation(setCurrentKey, setCurrentStep);
},
async MICROPLAN_DETAILS(params) {
const { microplanId, campaignId, totalFormData, ...rest } = params;
if (microplanId && campaignId) {
return handleStepNavigation(rest.setCurrentKey, rest.setCurrentStep);
}
return handleMicroplanCreation(totalFormData, rest);
},
// ... other handlers
};
const createUpdatePlanProject = async (req) => {
try {
const handler = handlers[req.config.name];
if (!handler) {
throw new Error('Unhandled operation');
}
return handler(req);
} catch (error) {
handleServiceError(error);
}
};Line range hint 1-680: Improve code organization and separation of concerns.
The current implementation mixes different responsibilities:
- API calls
- Data transformation
- State management
- Error handling
Consider reorganizing the code into separate modules:
- Create an API client module:
// api/microplan.js
export const MicroplanAPI = {
searchPlanConfig: async (body) => {
return Digit.CustomService.getResponse({
url: "/plan-service/config/_search",
method: "POST",
body
});
},
// ... other API methods
};- Create a data transformation module:
// transforms/microplan.js
export const MicroplanTransforms = {
toPlanObject: (formData) => {
// Transform form data to plan object
},
toCampaignObject: (formData) => {
// Transform form data to campaign object
}
};- Create a state management module:
// state/microplan.js
export const MicroplanState = {
handleStepNavigation: (setCurrentKey, setCurrentStep) => {
setCurrentKey(prev => prev + 1);
setCurrentStep(prev => prev + 1);
}
};health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (2)
Line range hint 516-537: Consider centralizing status mapping logic.
The status mappings in MicroplanSearchConfig and MyMicroplanSearchConfig are similar but duplicated. Consider extracting these mappings into a shared constant to improve maintainability.
+const MICROPLAN_STATUS_MAPPINGS = {
+ 0: ["EXECUTION_TO_BE_DONE", "CENSUS_DATA_APPROVAL_IN_PROGRESS", "CENSUS_DATA_APPROVED", "RESOURCE_ESTIMATION_IN_PROGRESS", "RESOURCE_ESTIMATIONS_APPROVED"],
+ 1: ["EXECUTION_TO_BE_DONE"],
+ 2: ["CENSUS_DATA_APPROVAL_IN_PROGRESS", "CENSUS_DATA_APPROVED", "RESOURCE_ESTIMATION_IN_PROGRESS"],
+ 3: ["RESOURCE_ESTIMATIONS_APPROVED"]
+};
const dic = {
- 0: ["EXECUTION_TO_BE_DONE",...],
- 1: ["EXECUTION_TO_BE_DONE"],
- 2: ["CENSUS_DATA_APPROVAL_IN_PROGRESS",...],
- 3: ["RESOURCE_ESTIMATIONS_APPROVED"]
+ ...MICROPLAN_STATUS_MAPPINGS
};Also applies to: 673-694
Line range hint 1066-1084: Enhance mobile number validation.
While the validation logic is correct, consider these improvements:
- Extract the regex pattern as a named constant
- Add international number support if required
+const MOBILE_NUMBER_REGEX = /^[0-9]{10}$/;
+const MOBILE_NUMBER_ERROR = "INVALID_MOBILE_NUMBER";
customValidationCheck: (data) => {
const { phone } = data;
- const mobileRegex = /^[0-9]{10}$/;
if (!phone || phone.trim() === "") {
return false;
}
- if (!mobileRegex.test(phone)) {
- return { error: true, label: "INVALID_MOBILE_NUMBER" };
+ if (!MOBILE_NUMBER_REGEX.test(phone)) {
+ return { error: true, label: MOBILE_NUMBER_ERROR };
}
return false;
}📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
health/micro-ui/web/micro-ui-internals/packages/css/package.jsonis excluded by!**/*.json
📒 Files selected for processing (10)
- health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchPlanWithCensus.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (6 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchPlanWithCensus.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
Pattern **/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
[error] 378-382: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (9)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1)
Line range hint 73-103: LGTM: Well-structured column configuration.
The search results column configuration is well-organized with clear labels and proper jsonPath mappings. The additionalCustomization flag is consistently applied where needed.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
239-240: LGTM! Improved search UX.
The enhanced visibility logic for search controls improves user experience by maintaining the search interface during active queries, even when no results are present.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
195-195: Verify the workflow state transition logic.
The assignee filtering logic has been updated to handle the "VALIDATED" state correctly. This change aligns with typical workflow patterns where validated items don't need assignee filtering.
Let's verify the workflow state transitions:
✅ Verification successful
Based on the search results, I can now generate the final response since I have sufficient evidence to verify the workflow state transition logic:
Workflow state transition logic is correctly implemented
The changes to the assignee filtering logic are consistent across both PopInbox.js and PlanInbox.js. The codebase shows:
- Consistent handling of the "VALIDATED" state in both inbox components
- Proper state transition from "PENDING_FOR_VALIDATION" to "VALIDATED"
- Correct implementation of assignee filtering bypass for "VALIDATED" state
- Appropriate status count validation ensuring "VALIDATED" items are handled differently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent workflow state handling across the codebase
# Look for other occurrences of PENDING_FOR_VALIDATION and VALIDATED states
# Search for workflow state constants and transitions
rg -A 3 "PENDING_FOR_VALIDATION|VALIDATED" --type js
# Search for related workflow configurations
rg -A 5 "workflow.*state.*=.*" --type js
Length of output: 7371
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
219-219: Verify the role permission change and its implications.
The role check has been modified to include POPULATION_DATA_APPROVER alongside ROOT_POPULATION_DATA_APPROVER. This change affects the accessibility of the "START" button in the microplan interface.
Let's verify the role hierarchy and usage:
Please confirm:
- Is this role change intentional as part of the microplan data fixes?
- Are there any implications for existing users with the
ROOT_POPULATION_DATA_APPROVERrole? - Does this align with the role hierarchy defined in MDMS?
✅ Verification successful
Based on the search results, I can now generate a final response as there's sufficient evidence to verify the role changes and their implications:
Role change is consistent with existing role hierarchy and usage patterns
The role check modification is verified to be correct as both roles (ROOT_POPULATION_DATA_APPROVER and POPULATION_DATA_APPROVER) are consistently used together across the microplan module:
- Both roles are used in combination for population data approval workflows in
viewVillage.js,PopInbox.js, andChooseActivity.js - The roles have distinct workflow actions in
editVillagePopulationPopUP.js:POPULATION_DATA_APPROVER→ "EDIT_AND_SEND_FOR_APPROVAL"ROOT_POPULATION_DATA_APPROVER→ "EDIT_AND_VALIDATE"
ROOT_POPULATION_DATA_APPROVERis correctly categorized as a national-level role inUserAccessWrapper.js
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the role definitions and their usage across the codebase
# Test 1: Search for role definitions in MDMS configurations
echo "Checking role definitions in MDMS..."
rg -A 5 "POPULATION_DATA_APPROVER|ROOT_POPULATION_DATA_APPROVER"
# Test 2: Search for other instances of these roles to ensure consistent usage
echo "Checking role usage across the codebase..."
ast-grep --pattern 'roles.some((role) => role === "ROOT_POPULATION_DATA_APPROVER" || role === "POPULATION_DATA_APPROVER")'
Length of output: 8815
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (2)
Line range hint 546-550: LGTM! Success navigation improvements
The changes improve the success flow by providing clear navigation parameters and user feedback.
105-105: LGTM! Consistent assignee logic
The assignee logic correctly aligns with the "VALIDATED" state handling across the component.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js (1)
622-622: Verify the impact of removing the description field.
The removal of the description field from the success response state might affect the setup completion screen's user experience.
Let's verify if this field is used in the UI:
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (2)
Line range hint 738-827: LGTM! Well-implemented UI customizations.
The UI customizations are well-implemented with:
- Proper null checks and fallbacks to "NA"
- Consistent styling with maxWidth and word-wrap
- Clear localization key transformations
937-937: Verify the role permission change.
The role check has been modified from ROOT_POPULATION_APPROVER to POPULATION_DATA_APPROVER. This change affects access control for UI actions.
✅ Verification successful
Based on the search results, I can now generate the final response as there's sufficient evidence to verify the role changes:
Role permission change is correctly implemented
The role validation includes both ROOT_POPULATION_DATA_APPROVER and POPULATION_DATA_APPROVER consistently across the codebase:
- Both roles are used together in multiple components for authorization checks
- The roles have distinct workflow actions in
editVillagePopulationPopUP.js:POPULATION_DATA_APPROVERtriggers "EDIT_AND_SEND_FOR_APPROVAL"ROOT_POPULATION_DATA_APPROVERtriggers "EDIT_AND_VALIDATE"
- The roles are properly defined in role configurations and used consistently in inbox filters and access controls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the role definition and its permissions
# Test: Search for role definitions and usages
rg -A 5 "POPULATION_DATA_APPROVER|ROOT_POPULATION_APPROVER"
Length of output: 8064
...web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchPlanWithCensus.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanDetails.js (2)
Line range hint 52-54: Consider consistent locale transformation approach
The code uses different approaches for locale transformation:
- Direct transformation:
Digit.Utils.locale.getTransformedLocale() - Direct t() calls
Consider standardizing the approach across all campaign-related translations for better maintainability.
- value: campaignData?.campaignType
- ? t(Digit.Utils.locale.getTransformedLocale(`CAMPAIGN_TYPE_${campaignData?.campaignType?.code}`))
- : t("ES_COMMON_NA"),
+ value: campaignData?.campaignType
+ ? `CAMPAIGN_TYPE_${campaignData?.campaignType?.code}`
+ : "ES_COMMON_NA",Also applies to: 57-59
Line range hint 119-134: Clean up commented code and complete FieldV1 configuration
The code contains:
- Commented-out
TextInputcomponent that should be removed if no longer needed - Commented-out error handling and style configuration in
FieldV1
Please either:
- Remove the commented
TextInputcode ifFieldV1is the final implementation - Complete the
FieldV1configuration by uncommenting and configuring the error handling and style props
- {/* <TextInput
- t={t}
- style={{ width: "100%", margin: 0 }}
- type={"text"}
- isMandatory={false}
- name="name"
- value={microplan}
- onChange={onChangeMicroplanName}
- placeholder={t("MICROPLAN_NAME_INPUT_PLACEHOLDER")}
- disable={isFreezed}
- /> */}
<FieldV1
type="text"
- // error={error?.message ? t(error?.message) : ""}
- // style={{ width: "40rem", marginBottom: "0" }}
+ error={error?.message ? t(error?.message) : ""}
+ style={{ width: "40rem", marginBottom: "0" }}
populators={{ name: "microplanName" }}
placeholder={t("MICROPLAN_NAME_INPUT_PLACEHOLDER")}
value={microplan}
onChange={onChangeMicroplanName}
disabled={isFreezed}
/>health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Line range hint 1-248: Consider optimizing component structure and imports.
A few suggestions to improve the code:
- Clean up unused imports (e.g.,
Fragment,useEffect,useRef,useState) - Remove commented out sections (e.g., card6 configuration, FormulaSection)
- Consider extracting card configurations to a separate configuration file for better maintainability
These changes would improve code maintainability and reduce bundle size.
🧰 Tools
🪛 Biome
[error] 36-36: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanDetails.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanDetails.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Pattern **/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js
[error] 36-36: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
🔇 Additional comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanDetails.js (1)
86-86: LGTM: Proper localization of campaign card values
The addition of the translation wrapper t() ensures proper localization of campaign details.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (2)
41-41: LGTM!
The disease code localization is implemented correctly with proper null checking and fallback value.
44-47: 🧹 Nitpick (assertive)
Clean up commented code and verify date handling removal.
While the localization change is correct, there are two concerns:
- There are multiple lines of commented code that should be removed for better maintainability
- The removal of date conversion logic (
Digit.Utils.date.convertEpochToDate) needs verification
Let's verify if date handling is still needed:
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js
Show resolved
Hide resolved
Choose the appropriate template for your PR:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Refactor