-
Notifications
You must be signed in to change notification settings - Fork 19
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
Pagination added to microplan search and my microplan #1706
base: console
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@abishekTa-egov has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes several modifications across multiple files related to the microplan user interface. Key changes involve updating the label in the Changes
Possibly related PRs
Suggested reviewers
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 (
|
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: 5
🧹 Outside diff range comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchSavedPlansWithCampaign.js (1)
Line range hint 77-82
: Enhance error handling with context and logging
The error handling could be more informative and maintainable:
- Add operation context to the generic error message
- Consider adding error logging for debugging
if (error?.response?.data?.Errors) {
+ console.error("Plan search failed:", error.response.data);
throw new Error(error.response.data.Errors[0].message);
}
- throw new Error("An unknown error occurred");
+ console.error("Unexpected error in plan search:", error);
+ throw new Error("Failed to search saved plans with campaign");
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (2)
Line range hint 15-106
: Add pagination configuration to support the new pagination feature.
The PR objective mentions adding pagination, but the configuration lacks pagination-specific parameters that are typically required for table pagination (e.g., page size, default page number).
Consider adding pagination configuration like this:
apiDetails: {
serviceName: "/plan-service/config/_search",
requestParam: {},
- requestBody: {},
+ requestBody: {
+ PlanConfigurationSearchCriteria: {
+ pagination: {
+ limit: 10,
+ offset: 0,
+ sortBy: "name",
+ order: "DESC"
+ }
+ }
+ },
masterName: "commonUiConfig",
Also, update the searchResult configuration to handle pagination:
searchResult: {
uiConfig: {
+ pagination: {
+ enabled: true,
+ defaultPageSize: 10,
+ pageSizeOptions: [10, 20, 50]
+ },
columns: [
Line range hint 77-82
: Consider adding tooltips for status columns.
The status column might benefit from tooltips explaining what each status means, especially for states like "EXECUTION_TO_BE_DONE" vs "EXECUTION_IN_PROGRESS".
{label: "DRAFTED_SETUP"},
-{label: "EXECUTION_TO_BE_DONE",},
-{label:"EXECUTION_IN_PROGRESS"},
-{label:"MICROPLAN_EXECUTED"},
+{
+ label: "EXECUTION_TO_BE_DONE",
+ tooltip: "Plans that are ready for execution"
+},
+{
+ label: "EXECUTION_IN_PROGRESS",
+ tooltip: "Plans currently being executed"
+},
+{
+ label: "MICROPLAN_EXECUTED",
+ tooltip: "Plans that have completed execution"
+},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- 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 (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchSavedPlans.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchSavedPlansWithCampaign.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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/searchSavedPlans.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchSavedPlansWithCampaign.js (1)
Pattern **/*.js
: check
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchSavedPlans.js (1)
Line range hint 10-53
: Verify pagination implementation completeness.
The function now returns totalCount, but there are several aspects to verify:
- Ensure the body parameter includes pagination parameters (offset/limit)
- Verify if the project-factory API call needs pagination
- Confirm that mergePlanAndCampaign function correctly handles paginated data
Let's verify the implementation:
✅ Verification successful
Let me gather more specific information about the pagination implementation in related files.
Based on the gathered information, I can now provide a final response:
Pagination implementation is properly handled
The pagination implementation is complete and correct:
- The
plan-service/config/_search
API receives pagination parameters throughPlanConfigurationSearchCriteria.pagination
in the request body - The
SearchSavedPlans
function correctly:- Accepts pagination parameters through the
body
argument - Returns
TotalCount
from the plan service response - Properly merges paginated data from both APIs using
mergePlanAndCampaign
- Accepts pagination parameters through the
- The UI configuration in
MicroplanSearchConfig.js
andMyMicroplanSearchConfig.js
properly maps pagination parameters throughtableFormJsonPath: "requestBody.PlanConfigurationSearchCriteria.pagination"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for pagination parameters in API calls and related components
# Check for pagination parameter handling in related files
echo "Checking for pagination parameter handling..."
rg -l 'offset|limit|page' --type js
# Check for other pagination-related changes
echo "Checking for pagination UI components..."
rg -l 'Pagination|paginate|pageSize' --type js
# Look for similar implementations
echo "Checking for similar pagination patterns..."
ast-grep --pattern 'TotalCount|totalCount'
Length of output: 19425
Script:
#!/bin/bash
# Check the implementation of pagination in related microplan files
echo "Checking microplan pagination implementation..."
rg -A 10 -B 10 'plan-service/config/_search' --type js
# Check the usage of the SearchSavedPlans hook
echo "Checking usage of SearchSavedPlans..."
rg -A 5 -B 5 'SearchSavedPlans' --type js
# Check the mergePlanAndCampaign implementation
echo "Checking mergePlanAndCampaign implementation..."
ast-grep --pattern 'mergePlanAndCampaign'
Length of output: 48235
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/services/searchSavedPlansWithCampaign.js (1)
Line range hint 38-46
: Verify pagination parameter handling
While the pagination implementation looks functional, consider adding these safeguards:
- Validate limit/offset values
- Enforce maximum page size
- Handle edge cases where offset exceeds total count
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1)
15-15
: LGTM: Label update is consistent with the codebase.
The label change from "MY_MICROPLANS" to "MY_MICROPLANS_HEADER" looks good.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Line range hint 192-195
: Pagination implementation looks good.
The preProcess method correctly handles pagination parameters (limit and offset) for the search criteria, which aligns with the PR objective of adding pagination to microplan search.
🧰 Tools
🪛 Biome
[error] 85-85: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
Added pagination to microplan search and my microplan
Summary by CodeRabbit
New Features
Improvements
Bug Fixes