Skip to content
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

Open microplan screen #1549

Merged
merged 8 commits into from
Oct 17, 2024
Merged

Open microplan screen #1549

merged 8 commits into from
Oct 17, 2024

Conversation

abishekTa-egov
Copy link
Collaborator

@abishekTa-egov abishekTa-egov commented Oct 17, 2024

API integrated open microplan screen and also My-Microplans in Other roles are searchable.
Boundary kpi localization issue

Summary by CodeRabbit

  • New Features

    • Updated CSS stylesheet to the latest version for improved styling.
    • Introduced a new MyMicroplans component for managing microplan information.
    • Added a new route for the MyMicroplans component in the app's navigation.
    • Enhanced search configurations with new default values and settings.
  • Improvements

    • Integrated translation capabilities in the BoundaryKpi component for better accessibility.
    • Expanded handling of various cases in the plan project creation and update process.
    • Updated tab labels from "ALL" to "MY_MICROPLANS" for a more user-friendly experience.
    • Improved search result display with additional fields and configurations.

Copy link
Contributor

coderabbitai bot commented Oct 17, 2024

📝 Walkthrough

Walkthrough

This pull request includes updates to multiple components and configurations within the micro UI project. Key changes involve updating a CSS stylesheet version, enhancing the BoundaryKpi component with translation capabilities, introducing a new component MyMicroplans, and modifying several configuration objects to improve search functionalities. Additionally, new constants and functions have been added to manage roles and links effectively, while the routing structure has been updated to incorporate the new component.

Changes

File Change Summary
health/micro-ui/web/micro-ui-internals/example/public/index.html Updated CSS stylesheet version from 1.0.82-campaign to 1.0.83-campaign.
health/micro-ui/web/public/index.html Updated CSS stylesheet version from 1.0.82-campaign to 1.0.83-campaign.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js Added useTranslation hook and utilized t function for translating displayed text.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanCard.js Introduced OTHERS constant and generateOtherLink function for role-based links.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js Modified TabSearchconfig with new default values and expanded sections for search results.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MyMicroplanSearchConfig.js Added new configuration for search input component with default values and settings.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js Introduced MyMicroplanSearchConfig and updated methods for customizations based on row status.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js Enhanced createUpdatePlanProject function by adding disease property and improving case handling.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js Added new MyMicroplans component for managing microplan information with hooks and translation.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js Integrated new route for MyMicroplans component within the existing routing structure.

Possibly related PRs

Suggested reviewers

  • nipunarora-eGov

🐰 In the garden, changes bloom,
A new stylesheet dispels the gloom.
With translations bright, we hop along,
Microplans now sing a vibrant song!
Roles and links, all neatly tied,
In our UI, let joy abide! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 26

🧹 Outside diff range comments (7)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)

Line range hint 1-38: Consider optimizing and cleaning up CSS imports.

While not directly related to the current change, I noticed that the file contains multiple CSS stylesheet imports, some of which are commented out. This could lead to potential issues:

  1. Version conflicts between different CSS imports.
  2. Unnecessary code that might confuse future developers.
  3. Possible performance issues due to loading multiple stylesheets.

Consider the following improvements:

  1. Remove or update commented-out stylesheet links if they're no longer needed.
  2. Consolidate CSS imports where possible to reduce the number of HTTP requests.
  3. Ensure that all imported stylesheets are necessary and up-to-date.
  4. Consider using a CSS bundling tool to combine and minify your stylesheets for better performance.

Here's an example of how you might clean up the CSS imports:

<head>
  <!-- ... other head elements ... -->
  
  <!-- Main DIGIT UI CSS -->
  <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
  
  <!-- Additional CSS only if necessary -->
  <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
  
  <!-- ... rest of the head ... -->
</head>

This cleanup will make the file more maintainable and potentially improve load times.

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js (6)

Line range hint 113-113: Missing break statements in switch cases

In the createUpdatePlanProject function's switch statement starting at line 113, break statements are missing after each case. This can lead to unintended fall-through behavior, causing multiple cases to execute.

Apply the following diff to add break statements after each case:

 case "CAMPAIGN_DETAILS":
     setCurrentKey((prev) => prev + 1);
     setCurrentStep((prev) => prev + 1);
     return {
         triggeredFrom,
     };
+    break;

 case "MICROPLAN_DETAILS":
     // existing code
     return {
         triggeredFrom,
     };
+    break;

 // Add `break` statements for all other cases similarly

Line range hint 166-168: Fix the typo when invalidating operations

In the ASSUMPTIONS_FORM case, the invalidatedOperations array is incorrectly mapping over planObject.assumptions instead of planObject.operations.

Apply this diff to correct the mapping:

 const invalidatedOperations = planObject.operations.length > 0 ? planObject.assumptions.map(row => {
     return {
         ...row,
         active: false
     }
 }) : []
+const invalidatedOperations = planObject.operations.length > 0 ? planObject.operations.map(row => {
+    return {
+        ...row,
+        active: false
+    }
+}) : []

Line range hint 166-168: Refactor invalidation logic for assumptions and operations

The logic for invalidating assumptions and operations is identical. Consider creating a helper function to DRY up the code.

Implement a helper function to invalidate entities:

const invalidateEntities = (entities) => {
    return entities.length > 0 ? entities.map(row => ({ ...row, active: false })) : [];
};

const invalidatedAssumptions = invalidateEntities(planObject.assumptions);
const invalidatedOperations = invalidateEntities(planObject.operations);

Line range hint 150-152: Optimize deep comparison using _.isEqual

Deep equality checks with _.isEqual can be expensive for large objects. If performance is a concern, consider optimizing this check or implementing a shallow comparison if appropriate.


Line range hint 204-208: Reduce code duplication in HYPOTHESIS and SUB_HYPOTHESIS cases

The HYPOTHESIS and SUB_HYPOTHESIS cases contain similar logic for fetching the plan and updating assumptions. Refactor this common logic into a separate function to improve maintainability.


Line range hint 225-225: Ensure proper control flow in the default case

In the switch statement, the default case at line 225 lacks a break statement. While not strictly necessary after a return, adding a break can improve readability and prevent potential issues if the code changes.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between f774087 and f8e914c.

⛔ Files ignored due to path filters (1)
  • health/micro-ui/web/micro-ui-internals/packages/css/package.json is excluded by !**/*.json
📒 Files selected for processing (9)
  • health/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
  • health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js (1 hunks)
  • health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanCard.js (3 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/MyMicroplanSearchConfig.js (1 hunks)
  • health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (8 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/pages/employee/MyMicroplans.js (1 hunks)
  • health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js (1)

Pattern **/*.js: check

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanCard.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/MyMicroplanSearchConfig.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/pages/employee/MyMicroplans.js (1)

Pattern **/*.js: check

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (1)

Pattern **/*.js: check

🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js

[error] 17-17: 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)


[error] 26-26: 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)


[error] 39-45: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.

Unsafe fix: Use a SelfClosingElement instead

(lint/style/useSelfClosingElements)

🔇 Additional comments (17)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/BoundaryKpi.js (3)

4-4: LGTM: Correct import for internationalization.

The addition of the useTranslation import from 'react-i18next' is correct and aligns with the PR objective of addressing localization issues.


7-7: LGTM: Proper usage of useTranslation hook.

The useTranslation hook is correctly implemented, destructuring the t function for use within the component. This supports the localization objective of the PR.


Line range hint 1-24: Overall assessment: Well-implemented localization changes.

The changes in this file successfully implement localization for the BoundaryKpi component, addressing the PR objective related to boundary KPI localization. The implementation follows React and i18n best practices, improving the component's internationalization capabilities without compromising its core functionality.

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanCard.js (1)

61-61: LGTM: Component structure and exports remain consistent.

The overall structure of the MicroplanCard component and its default export remain unchanged, which is good for maintaining consistency in the codebase.

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js (1)

50-50: LGTM! Component export is correct.

The default export of the MyMicroplans component is properly implemented.

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/index.js (2)

16-16: LGTM: Import statement for MyMicroplans component.

The import statement is correctly placed and follows React naming conventions.


16-16: Summary: Changes align with PR objectives.

The addition of the MyMicroplans component import and the corresponding route successfully integrates the new feature into the existing structure. These changes align with the PR objective of enhancing the My-Microplans feature and making it searchable for users in other roles.

To further improve the code:

  1. Consider adding a space after the arrow function's opening parenthesis in the new route for consistency.
  2. Remove the redundant comment "my-microplans" to maintain code cleanliness.

Overall, the implementation is sound and achieves the intended goal.

Also applies to: 181-184

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (3)

99-99: Verify the empty jsonPath for the "Actions" column.

At line 99~, the jsonPath for the "Actions" column is an empty string. Ensure that this is intentional and that the rendering logic handles this case appropriately.


110-110: Confirm the use of customHookName.

At line 110~, customHookName is commented out, but it is added uncommented at line 113~. Verify whether the hook should be active. If it's not ready for use, consider keeping it commented until it is.

Also applies to: 113-113


117-123: ⚠️ Potential issue

Check the placement and structure of additional label entries.

The label objects from lines 117~ to 120~ are placed outside the initial tab configuration object. Ensure they are correctly structured within the TabSearchconfig array and associated with appropriate configurations, such as type, apiDetails, and sections.

Ensure that these entries are correctly nested. For example:

-    {label: "DRAFTED_SETUP"},
-    {label: "EXECUTION_TO_BE_DONE",},
-    {label:"EXECUTION_IN_PROGRESS"},
-    {label:"MICROPLAN_EXECUTED"},
+    {
+      label: "DRAFTED_SETUP",
+      type: "search",
+      // ...additional configurations
+    },
+    {
+      label: "EXECUTION_TO_BE_DONE",
+      type: "search",
+      // ...additional configurations
+    },
+    {
+      label: "EXECUTION_IN_PROGRESS",
+      type: "search",
+      // ...additional configurations
+    },
+    {
+      label: "MICROPLAN_EXECUTED",
+      type: "search",
+      // ...additional configurations
+    },
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MyMicroplanSearchConfig.js (3)

74-74: 🧹 Nitpick (assertive)

Correct the typo in 'Camapaign Type'.

The label Camapaign Type contains a spelling error. It should be Campaign Type.

Apply this diff to fix the typo:

 {
-  label:"Camapaign Type",
+  label:"Campaign Type",
   jsonPath:"additionalDetails.campaignType"
 },

Likely invalid or redundant comment.


94-97: 🧹 Nitpick (assertive)

Remove redundant commented code to improve readability.

The customHookName is commented out at line 94 and then declared again at line 97. Removing the commented line reduces clutter and improves code clarity.

Apply this diff to remove the commented line:

- // customHookName: "microplanv1.useSavedMicroplans",  //! Note

Likely invalid or redundant comment.


82-85: ⚠️ Potential issue

Specify a valid 'jsonPath' for the 'Actions' column.

The 'Actions' column has an empty jsonPath, which may cause issues when rendering data. Ensure that jsonPath is properly set to retrieve the necessary data for this column.

Apply this diff to specify the correct jsonPath:

 {
   label:"Actions",
-  jsonPath:"",
+  jsonPath:"actions",
   additionalCustomization:true
 },

Replace "actions" with the appropriate path based on your data structure.

Likely invalid or redundant comment.

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (2)

5-7: Imports are properly added

The additional components (DeleteIconv2, DownloadIcon, FileIcon, Button, Card, CardSubHeader, EditIcon, ArrowForward) are correctly imported from @egovernments/digit-ui-react-components to support the new UI elements.


182-204: Approved: Custom rendering for microplan names

The custom rendering for the "Name of the Microplan" column appropriately handles long text and 'NA' values, improving readability.

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/createUpdatePlanProject.js (2)

Line range hint 252-254: Confirm the correct export syntax for createUpdatePlanProject

At the end of the file, ensure that the export syntax for createUpdatePlanProject aligns with the module system being used (CommonJS or ES Modules).

If using ES Modules, the export is correct:

export default createUpdatePlanProject;

If using CommonJS, adjust the export accordingly:

module.exports = createUpdatePlanProject;

69-69: Ensure consistent initialization of disease property

At line 69, the disease property is added to additionalDetails in the campaignObject. Verify that this property is consistently set wherever campaignObject is created or updated to prevent potential undefined errors.

Run the following script to check all instances where additionalDetails is set:

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range comments (1)
health/micro-ui/web/public/index.html (1)

Line range hint 1-37: Consider reviewing the overall CSS inclusion strategy.

While the current change is appropriate, I noticed that this file includes multiple CSS stylesheets from different versions and packages. This approach might lead to potential conflicts or inconsistencies in styling.

Consider the following suggestions:

  1. Review if all these CSS inclusions are necessary. Can some be consolidated or removed?
  2. Evaluate if using a more consistent versioning strategy across stylesheets would be beneficial.
  3. Consider using a build process to bundle and minimize CSS files, which could improve load times and reduce the risk of conflicts.

These suggestions aim to improve maintainability and performance in the long run. If you'd like assistance in implementing any of these suggestions, please let me know.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between f8e914c and e638256.

📒 Files selected for processing (1)
  • health/micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
health/micro-ui/web/public/index.html (1)

15-15: CSS version update looks good, but verify compatibility.

The update of the CSS version from 1.0.82-campaign to 1.0.83-campaign seems appropriate. This change aligns with the PR objectives of enhancing functionality and addressing issues.

To ensure this change doesn't introduce any unexpected visual or functional regressions, please run the following verification steps:

  1. Test the application thoroughly, especially the areas mentioned in the PR objectives (open microplan screen, My-Microplans feature, and boundary KPIs).
  2. Check for any visual inconsistencies or layout issues across different pages and components.
  3. Verify that the new CSS version is compatible with all browsers and devices supported by your application.

If you encounter any issues, consider rolling back to the previous version or addressing the problems in a follow-up PR.

coderabbitai[bot]
coderabbitai bot previously requested changes Oct 17, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)

Line range hint 235-251: Fix initialization of roleschosen to avoid type inconsistency

In the preProcess function, roleschosen is initialized as an empty array [] when filterForm is undefined, but later you expect it to be an object. This could lead to unexpected behavior. To ensure consistency, initialize roleschosen as an empty object {}.

Apply this diff to fix the issue:

-let { roleschosen } = data?.state?.filterForm || []
+let { roleschosen } = data?.state?.filterForm || {}

if (!roleschosen) {
    roleschosen = {}
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between e638256 and 47a8abb.

📒 Files selected for processing (2)
  • health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanCard.js (3 hunks)
  • health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanCard.js (1)

Pattern **/*.js: check

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)

Pattern **/*.js: check

🔇 Additional comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/MicroplanCard.js (1)

61-61: Export statement looks good

The modification to the export statement appears to be a minor change, possibly just formatting. The logic remains the same, exporting the MicroplanCard component as default.

health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)

251-256: Confirm the correct parameter name 'names' in data.params

In the preProcess function, data.params.names is assigned the value of name. Please verify if the API expects the parameter to be names or name to ensure consistency and correct functionality.

Comment on lines 10 to 21
const OTHERS={
OTHERS:[
"PLAN_ESTIMATION_APPROVER",
"ROOT_PLAN_ESTIMATION_APPROVER",
"POPULATION_DATA_APPROVER",
"ROOT_POPULATION_DATA_APPROVER",
"FACILITY_CATCHMENT_MAPPER",
"ROOT_FACILITY_CATCHMENT_MAPPER",
"MICROPLAN_VIEWER"
]

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve constant naming and structure for consistency

The new OTHERS constant could be improved for better clarity and consistency:

  1. The name OTHERS is not descriptive. Consider a more specific name like ADDITIONAL_ROLES.
  2. The structure is inconsistent with the existing ROLES constant.

To address these issues and incorporate the suggestion from a previous review, consider merging this into the ROLES object:

const ROLES = {
  MICROPLAN: ["MICROPLAN_ADMIN"],
  ADDITIONAL: [
    "PLAN_ESTIMATION_APPROVER",
    "ROOT_PLAN_ESTIMATION_APPROVER",
    "POPULATION_DATA_APPROVER",
    "ROOT_POPULATION_DATA_APPROVER",
    "FACILITY_CATCHMENT_MAPPER",
    "ROOT_FACILITY_CATCHMENT_MAPPER",
    "MICROPLAN_VIEWER"
  ]
};

This approach centralizes role definitions and maintains consistency.

Comment on lines 35 to 41
const generateOtherLink = (labelKey, pathSuffix) => {
return {
label: t(labelKey),
link: `/${window?.contextPath}/employee/microplan/${pathSuffix}`,
roles: ROLES.OTHERS,
};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Remove or modify the redundant generateOtherLink function

The generateOtherLink function is nearly identical to generateLink, introducing unnecessary code duplication. Additionally, it uses ROLES.OTHERS, which is incorrect. Consider the following options:

  1. If no differentiation is needed, remove generateOtherLink and use generateLink for all links.

  2. If different behavior is intended for "other" links, update the function accordingly:

const generateAdditionalLink = (labelKey, pathSuffix) => {
  return {
    label: t(labelKey),
    link: `/${window?.contextPath}/employee/microplan/${pathSuffix}`,
    roles: ROLES.ADDITIONAL, // Use the newly defined ADDITIONAL roles
  };
};

This approach aligns with the suggested changes to the ROLES constant and provides a clear distinction between different types of links.

Comment on lines +43 to +48
let links = [
generateLink("SETUP_MICROPLAN","setup-microplan"),
generateLink("SEARCH_MICROPLANS","microplan-search"),
generateLink("USER_MANAGEMENT","user-management"),
generateOtherLink("MY_MICROPLANS","my-microplans")
];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use consistent link generation in the links array

The "MY_MICROPLANS" link is added using generateOtherLink, which is inconsistent with other links. To maintain consistency and reduce potential issues, update this section to use a single link generation function:

let links = [
  generateLink("SETUP_MICROPLAN", "setup-microplan"),
  generateLink("SEARCH_MICROPLANS", "microplan-search"),
  generateLink("USER_MANAGEMENT", "user-management"),
  generateLink("MY_MICROPLANS", "my-microplans")
];

If different role requirements are needed for "MY_MICROPLANS", ensure that the link generation function (whether generateLink or a modified version) can handle different role sets appropriately.

Comment on lines +146 to +180
row.status === "DRAFT" ? (<Button
label={t("WBH_EDIT")}
variation="secondary"
icon={<EditIcon styles={{ height: "1.25rem", width: "2.5rem" }} />}
type="button"
className="dm-workbench-download-template-btn dm-hover"
onButtonClick={(e) => {

}}
/>) : (row.status === "EXECUTION_TO_BE_DONE") ? (
<Button
label={t("START")}
variation="secondary"
icon={<ArrowForward styles={{ height: "1.25rem", width: "2.5rem" }} />}
type="button"
className="dm-workbench-download-template-btn dm-hover"
onButtonClick={(e) => {

}}
/>
) : (row.status === "RESOURCE_ESTIMATIONS_APPROVED") ? (
<Button
label={t("WBH_DOWNLOAD")}
variation="secondary"
icon={<DownloadIcon styles={{ height: "1.25rem", width: "2.5rem" }} />}
type="button"
className="dm-workbench-download-template-btn dm-hover"
onButtonClick={(e) => {

}}
/>
) : null


);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor nested ternary operators for better readability

The nested ternary operators used to render different buttons based on row.status can be difficult to read and maintain. Consider refactoring this logic into a more readable format, such as using if-else statements or mapping statuses to components.

For example, you can refactor the code as follows:

let actionButton = null;

if (row.status === "DRAFT") {
  actionButton = (
    <Button
      label={t("WBH_EDIT")}
      variation="secondary"
      icon={<EditIcon styles={{ height: "1.25rem", width: "2.5rem" }} />}
      type="button"
      className="dm-workbench-download-template-btn dm-hover"
      onButtonClick={(e) => {
        // Implement action
      }}
    />
  );
} else if (row.status === "EXECUTION_TO_BE_DONE") {
  actionButton = (
    <Button
      label={t("START")}
      variation="secondary"
      icon={<ArrowForward styles={{ height: "1.25rem", width: "2.5rem" }} />}
      type="button"
      className="dm-workbench-download-template-btn dm-hover"
      onButtonClick={(e) => {
        // Implement action
      }}
    />
  );
} else if (row.status === "RESOURCE_ESTIMATIONS_APPROVED") {
  actionButton = (
    <Button
      label={t("WBH_DOWNLOAD")}
      variation="secondary"
      icon={<DownloadIcon styles={{ height: "1.25rem", width: "2.5rem" }} />}
      type="button"
      className="dm-workbench-download-template-btn dm-hover"
      onButtonClick={(e) => {
        // Implement action
      }}
    />
  );
}

return actionButton;

@nipunarora-eGov nipunarora-eGov merged commit 2b392a2 into console Oct 17, 2024
3 checks passed
@nipunarora-eGov nipunarora-eGov deleted the open-summary-screen branch October 17, 2024 14:26
@abishekTa-egov abishekTa-egov restored the open-summary-screen branch October 17, 2024 14:46
This was referenced Oct 17, 2024
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