Microplancheck2 #2
Conversation
WalkthroughThe changes in this pull request focus on enhancing the asynchronous functionality and validation logic within the campaign-related files. Key modifications include the introduction of new utility functions, such as Changes
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
Documentation and Community
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range comments (12)
health-services/project-factory/src/server/utils/targetUtils.ts (5)
Line range hint
9-19: Document the purpose of removing the first element in microplan pathThe logic for handling microplan vs non-microplan requests is clear, but the purpose of
headerColumnsAfterHierarchy.shift()isn't immediately obvious.Consider adding a comment explaining why the first element is removed in the microplan path:
else { headerColumnsAfterHierarchy = await getConfigurableColumnHeadersBasedOnCampaignType(request); + // Remove the first column as it's not needed for microplan requests headerColumnsAfterHierarchy.shift(); }
Line range hint
23-55: Enhance type safety and naming conventionsThe function has several areas that could be improved:
- The parameter name
dataaappears to be a typo- Type safety could be enhanced with proper interfaces
- Complex condition combining logic could be extracted
Consider applying these improvements:
+ interface DeliveryCondition { + attribute: { code: string }; + operator: { code: string }; + value?: number; + fromValue?: number; + toValue?: number; + } - function modifyDeliveryConditions(dataa: any[]): any { + function modifyDeliveryConditions(deliveryRules: Array<{ conditions: DeliveryCondition[] }>): Set<string> {
Line range hint
58-69: Extract magic number to configurationThe hard limit of 18 columns should be moved to configuration for better maintainability.
Consider extracting the magic number:
+ const MAX_TARGET_COLUMNS = config.maxTargetColumns || 18; if (targetColumnsBasedOnDeliveryConditions.length > 18) { - targetColumnsBasedOnDeliveryConditions.splice(18); + targetColumnsBasedOnDeliveryConditions.splice(MAX_TARGET_COLUMNS); targetColumnsBasedOnDeliveryConditions.push(getLocalizedName("OTHER_TARGETS", localizationMap)); }
Line range hint
87-109: Add error handling and improve readabilityThe function has several areas for improvement:
- Missing error handling for the external API call
- Deep nesting of conditions reduces readability
Consider this refactoring:
async function updateTargetColumnsIfDeliveryConditionsDifferForSMC(request: any) { + try { const existingCampaignDetails = request?.body?.ExistingCampaignDetails; - if (existingCampaignDetails) { - if (isDynamicTargetTemplateForProjectType(request?.body?.CampaignDetails?.projectType) && - config?.isCallGenerateWhenDeliveryConditionsDiffer && - !_.isEqual(existingCampaignDetails?.deliveryRules, request?.body?.CampaignDetails?.deliveryRules)) { + if (!existingCampaignDetails) return; + + const shouldUpdateTargets = + isDynamicTargetTemplateForProjectType(request?.body?.CampaignDetails?.projectType) && + config?.isCallGenerateWhenDeliveryConditionsDiffer && + !_.isEqual(existingCampaignDetails?.deliveryRules, request?.body?.CampaignDetails?.deliveryRules); + + if (shouldUpdateTargets) { // ... rest of the function } + } catch (error) { + console.error('Failed to update target columns:', error); + throw error; + } }
Line range hint
111-116: Add type safety for configuration valueThe function could benefit from better type handling for the configuration value.
Consider this improvement:
+ type ProjectType = string; - function isDynamicTargetTemplateForProjectType(projectType: string) { + function isDynamicTargetTemplateForProjectType(projectType: ProjectType): boolean { - const projectTypesFromConfig = config?.enableDynamicTemplateFor; + const projectTypesFromConfig: string = config?.enableDynamicTemplateFor ?? ''; const projectTypesArray = projectTypesFromConfig ? projectTypesFromConfig.split(',') : []; return projectTypesArray.includes(projectType); }health-services/project-factory/src/server/validators/microplanValidators.ts (1)
Line range hint
466-484: Handle UndefinedcampaignIdinisMicropplanCampaignIdFunctionIn
isMicropplanCampaignId(campaignId: string), thecampaignIdparameter may beundefinedif neitherrequest.query.campaignIdnorrequest.body.ResourceDetails.campaignIdis defined. This could lead to unexpected behavior when executing the database query. Consider adding a null or undefined check forcampaignIdbefore proceeding.Apply this diff to add a null check for
campaignId:export async function isMicropplanCampaignId(campaignId: string) { + if (!campaignId) { + return false; // or handle the undefined campaignId appropriately + } if (campaignId == "microplan") return true; const query = ` SELECT (additionaldetails->>'source' = 'microplan') AS is_microplan FROM ${config?.DB_CONFIG.DB_CAMPAIGN_DETAILS_TABLE_NAME} WHERE id = $1; `; try { const { rowCount, rows } = await executeQuery(query, [campaignId]); return rowCount > 0 ? rows[0].is_microplan : false; } catch (error: any) { logger.error(`Error checking if campaign ${campaignId} is a microplan request: ${error.message}`); throw error; } }health-services/project-factory/src/server/utils/microplanUtils.ts (2)
Line range hint
538-540: Simplify Boolean AssignmentThe boolean assignment using the ternary operator is unnecessary. You can directly assign the result of the
awaitexpression toisLockSheetNeeded.Apply this diff to simplify the code:
const isLockSheetNeeded = - await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId) - ? true - : false; + await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId);
Line range hint
598-600: Simplify Boolean AssignmentThe boolean assignment using the ternary operator is unnecessary. You can directly assign the result of the
awaitexpression toisLockSheetNeeded.Apply this diff to simplify the code:
const isLockSheetNeeded = - await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId) - ? true - : false; + await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId);health-services/project-factory/src/server/utils/genericUtils.ts (2)
Line range hint
1235-1244: Consider enhancing error handling for microplan data retrievalWhile the conditional logic for microplan handling is well-implemented, consider adding explicit error handling for cases where the microplan check fails.
const isSourceMicroplan = await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId); +try { const type = request?.body?.ResourceDetails?.type; if (isSourceMicroplan) { if (type == 'user') { return await getUserDataFromMicroplanSheet(request, fileStoreId, tenantId, createAndSearchConfig, localizationMap); } else { return await getDataFromSheetFromNormalCampaign(type, fileStoreId, tenantId, createAndSearchConfig, optionalSheetName, localizationMap); } } +} catch (error) { + logger.error(`Error retrieving microplan data: ${error.message}`); + throw error; +}
Line range hint
1434-1439: LGTM: Clear microplan source handling with good loggingThe microplan source handling is well-implemented. Consider enhancing the logging to include more context.
const isSourceMicroplan = await isMicroplanRequest(request); - if (isSourceMicroplan) { - logger.info(`Source is Microplan.`); + if (isSourceMicroplan) { + logger.info(`Source is Microplan for campaign: ${request?.query?.campaignId}`);health-services/project-factory/src/server/api/campaignApis.ts (2)
Line range hint
243-252: Assign the result of 'isMicroplanCampaignId' to 'isSourceMicroplan' variableIn the function
updateErrorsForUser, the result ofawait isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId);is not assigned to any variable. To utilize this value, assign it toisSourceMicroplan.Apply this diff to fix the issue:
- await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId); + const isSourceMicroplan = await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId);
Line range hint
2975-2983: Avoid using 'await' inside object literals directlyUsing
awaitdirectly inside an object literal is invalid syntax. You need to compute the value before constructing the object.Refactor the code to compute the
sourcevalue before theadditionalDetailsobject:+ const isMicroplan = await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId); additionalDetails: { ...request?.body?.ResourceDetails?.additionalDetails, sheetErrors: request?.body?.additionalDetailsErrors, - source: - await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId) - ? "microplan" - : null, + source: isMicroplan ? "microplan" : null, [name]: [fileStoreId], },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (7)
health-services/project-factory/src/server/api/campaignApis.ts(16 hunks)health-services/project-factory/src/server/utils/campaignUtils.ts(15 hunks)health-services/project-factory/src/server/utils/genericUtils.ts(6 hunks)health-services/project-factory/src/server/utils/microplanUtils.ts(3 hunks)health-services/project-factory/src/server/utils/targetUtils.ts(1 hunks)health-services/project-factory/src/server/validators/campaignValidators.ts(8 hunks)health-services/project-factory/src/server/validators/microplanValidators.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
health-services/project-factory/src/server/utils/campaignUtils.ts
[error] 538-540: 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] 598-600: 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-services/project-factory/src/server/validators/campaignValidators.ts
[error] 484-484: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🔇 Additional comments (4)
health-services/project-factory/src/server/utils/targetUtils.ts (2)
Line range hint 71-85: LGTM! Well-structured localization handling
The function effectively uses template literals and properly handles localization for all string components.
2-2: Verify the microplan utility function refactoring
The migration from checkIfSourceIsMicroplan to isMicroplanRequest appears to be part of a broader refactoring effort to consolidate microplan-related functionality.
Also applies to: 6-6
✅ Verification successful
Based on the search results, I can now generate the final response:
Migration from checkIfSourceIsMicroplan to isMicroplanRequest is complete and consistent
The refactoring appears to be thorough and complete:
- No traces of the old
checkIfSourceIsMicroplanfunction found in the codebase - The new
isMicroplanRequestfunction is properly implemented inmicroplanUtils.tsand consistently used across multiple files:campaignValidators.tstargetUtils.tsgenericUtils.tscampaignUtils.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the complete removal of old function and consistent usage of new function
rg "checkIfSourceIsMicroplan"
rg "isMicroplanRequest" -A 2
Length of output: 5244
health-services/project-factory/src/server/utils/genericUtils.ts (2)
21-21: LGTM: Import statement is well-structured
The import statement correctly imports necessary microplan-related utility functions.
547-548: LGTM: Proper async/await usage for microplan check
The modification correctly implements the microplan campaign ID check using async/await pattern.
| export async function validateFacilityBoundaryForLowestLevel(request: any, boundaries: any, rowData: any, errors: any = [], localizationMap?: { [key: string]: string }) { | ||
| if (request?.body?.ResourceDetails?.type == "facility" && await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId)) { |
There was a problem hiding this comment.
Add Null Check for campaignId Before Using
The function validateFacilityBoundaryForLowestLevel accesses request?.body?.ResourceDetails?.campaignId without verifying if campaignId is defined. If campaignId is undefined, passing it to isMicropplanCampaignId() might cause unexpected behavior or errors. Consider adding a check to ensure campaignId is defined before using it.
Apply this diff to add a null check for campaignId:
export async function validateFacilityBoundaryForLowestLevel(request: any, boundaries: any, rowData: any, errors: any = [], localizationMap?: { [key: string]: string }) {
+ const campaignId = request?.body?.ResourceDetails?.campaignId;
+ if (request?.body?.ResourceDetails?.type == "facility" && campaignId && await isMicropplanCampaignId(campaignId)) {
- if (request?.body?.ResourceDetails?.type == "facility" && await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId)) {
const hierarchy = request?.body?.hierarchyType?.boundaryHierarchy
const lastLevel = hierarchy?.[hierarchy.length - 1]?.boundaryType
for (const data of rowData?.boundaryCodes) {
const boundaryFromBoundariesType = boundaries.find((boundary: any) => boundary.code == data)?.type
if (boundaryFromBoundariesType != lastLevel) {
errors.push({ status: "INVALID", rowNumber: rowData?.rowNumber, errorDetails: `${data} is not a ${lastLevel} level boundary` })
}
}
}
}📝 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.
| export async function validateFacilityBoundaryForLowestLevel(request: any, boundaries: any, rowData: any, errors: any = [], localizationMap?: { [key: string]: string }) { | |
| if (request?.body?.ResourceDetails?.type == "facility" && await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId)) { | |
| export async function validateFacilityBoundaryForLowestLevel(request: any, boundaries: any, rowData: any, errors: any = [], localizationMap?: { [key: string]: string }) { | |
| const campaignId = request?.body?.ResourceDetails?.campaignId; | |
| if (request?.body?.ResourceDetails?.type == "facility" && campaignId && await isMicropplanCampaignId(campaignId)) { | |
| const hierarchy = request?.body?.hierarchyType?.boundaryHierarchy | |
| const lastLevel = hierarchy?.[hierarchy.length - 1]?.boundaryType | |
| for (const data of rowData?.boundaryCodes) { | |
| const boundaryFromBoundariesType = boundaries.find((boundary: any) => boundary.code == data)?.type | |
| if (boundaryFromBoundariesType != lastLevel) { | |
| errors.push({ status: "INVALID", rowNumber: rowData?.rowNumber, errorDetails: `${data} is not a ${lastLevel} level boundary` }) | |
| } | |
| } | |
| } | |
| } |
| const isSourceMicroplan = await isMicroplanRequest(request); | ||
| campaignType = (isSourceMicroplan) ? `${config?.prefixForMicroplanCampaigns}-${campaignType}` : campaignType; | ||
| const mdmsResponse = await callMdmsTypeSchema(request, request?.query?.tenantId || request?.body?.ResourceDetails?.tenantId, false, request?.query?.type || request?.body?.ResourceDetails?.type, campaignType) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider moving microplan prefix to configuration
While the prefix handling for microplan campaigns is correct, consider moving the prefix to configuration for better maintainability.
const isSourceMicroplan = await isMicroplanRequest(request);
- campaignType = (isSourceMicroplan) ? `${config?.prefixForMicroplanCampaigns}-${campaignType}` : campaignType;
+ campaignType = (isSourceMicroplan) ? `${config?.microplan?.campaignTypePrefix || 'MP'}-${campaignType}` : campaignType;Committable suggestion skipped: line range outside the PR's diff.
| @@ -48,6 +47,7 @@ import { checkAndGiveIfParentCampaignAvailable } from "../utils/onGoingCampaignU | |||
| import { validateMicroplanFacility } from "../validators/microplanValidators"; | |||
| import { | |||
| createPlanFacilityForMicroplan, | |||
| isMicropplanCampaignId, | |||
There was a problem hiding this comment.
Correct the typo in function name 'isMicropplanCampaignId'
The function name isMicropplanCampaignId is misspelled; it should be isMicroplanCampaignId with a single 'p' in 'microplan'. Please correct the import statement and ensure consistency throughout the codebase.
Apply this diff to fix the typo:
-import {
- createPlanFacilityForMicroplan,
- isMicropplanCampaignId,
- updateFacilityDetailsForMicroplan,
-} from "../utils/microplanUtils";
+import {
+ createPlanFacilityForMicroplan,
+ isMicroplanCampaignId,
+ updateFacilityDetailsForMicroplan,
+} from "../utils/microplanUtils";Committable suggestion skipped: line range outside the PR's diff.
| @@ -24,7 +24,7 @@ import { generateTargetColumnsBasedOnDeliveryConditions, isDynamicTargetTemplate | |||
| import { getBoundariesFromCampaignSearchResponse, validateBoundariesIfParentPresent } from "../utils/onGoingCampaignUpdateUtils"; | |||
| import { validateFacilityBoundaryForLowestLevel, validateLatLongForMicroplanCampaigns, validatePhoneNumberSheetWise, validateTargetsForMicroplanCampaigns, validateUniqueSheetWise, validateUserForMicroplan } from "./microplanValidators"; | |||
| import { produceModifiedMessages } from "../kafka/Producer"; | |||
| import { planConfigSearch, planFacilitySearch } from "../utils/microplanUtils"; | |||
| import { isMicroplanRequest, isMicropplanCampaignId, planConfigSearch, planFacilitySearch } from "../utils/microplanUtils"; | |||
There was a problem hiding this comment.
Correct the typo in function name 'isMicroplanCampaignId'
The function name isMicropplanCampaignId is misspelled; it should be isMicroplanCampaignId with a single 'p' in 'microplan'. Please correct the import statement and ensure consistency throughout the codebase.
Apply this diff to fix the typo:
-import { isMicroplanRequest, isMicropplanCampaignId, planConfigSearch, planFacilitySearch } from "../utils/microplanUtils";
+import { isMicroplanRequest, isMicroplanCampaignId, planConfigSearch, planFacilitySearch } from "../utils/microplanUtils";📝 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.
| import { isMicroplanRequest, isMicropplanCampaignId, planConfigSearch, planFacilitySearch } from "../utils/microplanUtils"; | |
| import { isMicroplanRequest, isMicroplanCampaignId, planConfigSearch, planFacilitySearch } from "../utils/microplanUtils"; |
| @@ -481,7 +481,7 @@ function validateStorageCapacity(obj: any, index: any) { | |||
|
|
|||
|
|
|||
| async function validateCampaignId(request: any) { | |||
| const { campaignId, tenantId, type, additionalDetails } = request?.body?.ResourceDetails; | |||
| const { campaignId, tenantId, type } = request?.body?.ResourceDetails; | |||
There was a problem hiding this comment.
Fix unsafe optional chaining when destructuring 'request.body.ResourceDetails'
Destructuring { campaignId, tenantId, type } from request?.body?.ResourceDetails is unsafe because if request.body.ResourceDetails is undefined, it will cause a TypeError. Optional chaining cannot be used in destructuring assignments.
Apply this diff to fix the issue:
- const { campaignId, tenantId, type } = request?.body?.ResourceDetails;
+ if (!request.body || !request.body.ResourceDetails) {
+ throwError("COMMON", 400, "VALIDATION_ERROR", "ResourceDetails is missing");
+ }
+ const { campaignId, tenantId, type } = request.body.ResourceDetails;Alternatively, provide a default value to prevent the error:
- const { campaignId, tenantId, type } = request?.body?.ResourceDetails;
+ const { campaignId, tenantId, type } = request.body.ResourceDetails || {};Ensure that request.body.ResourceDetails exists before destructuring.
📝 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.
| const { campaignId, tenantId, type } = request?.body?.ResourceDetails; | |
| if (!request.body || !request.body.ResourceDetails) { | |
| throwError("COMMON", 400, "VALIDATION_ERROR", "ResourceDetails is missing"); | |
| } | |
| const { campaignId, tenantId, type } = request.body.ResourceDetails; |
| const { campaignId, tenantId, type } = request?.body?.ResourceDetails; | |
| const { campaignId, tenantId, type } = request.body.ResourceDetails || {}; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 484-484: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
| @@ -93,7 +93,7 @@ import { | |||
| modifyNewSheetData, | |||
| unhideColumnsOfProcessedFile, | |||
| } from "./onGoingCampaignUpdateUtils"; | |||
| import { changeCreateDataForMicroplan, lockSheet } from "./microplanUtils"; | |||
| import { changeCreateDataForMicroplan, isMicroplanRequest, isMicropplanCampaignId, lockSheet } from "./microplanUtils"; | |||
There was a problem hiding this comment.
Correct the typo in function name 'isMicroplanCampaignId'
The function name isMicropplanCampaignId is misspelled; it should be isMicroplanCampaignId with a single 'p' in 'microplan'. Please correct the import statement and ensure consistency throughout the codebase.
Apply this diff to fix the typo:
-import { changeCreateDataForMicroplan, isMicroplanRequest, isMicropplanCampaignId, lockSheet } from "./microplanUtils";
+import { changeCreateDataForMicroplan, isMicroplanRequest, isMicroplanCampaignId, lockSheet } from "./microplanUtils";📝 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.
| import { changeCreateDataForMicroplan, isMicroplanRequest, isMicropplanCampaignId, lockSheet } from "./microplanUtils"; | |
| import { changeCreateDataForMicroplan, isMicroplanRequest, isMicroplanCampaignId, lockSheet } from "./microplanUtils"; |
| await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId) | ||
| ? true | ||
| : false; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify boolean assignment without redundant ternary operator
The ternary operator is unnecessary here since await isMicroplanCampaignId(...) already returns a boolean. Assign the result directly to isLockSheetNeeded.
Apply this diff to simplify the code:
- const isLockSheetNeeded =
- await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId)
- ? true
- : false;
+ const isLockSheetNeeded = await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId);📝 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.
| await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId) | |
| ? true | |
| : false; | |
| const isLockSheetNeeded = await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 538-540: 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)
| { | ||
| ...request?.body?.ResourceDetails?.additionalDetails, | ||
| sheetErrors: request?.body?.additionalDetailsErrors, | ||
| source: | ||
| await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId) | ||
| ? "microplan" | ||
| : null, | ||
| [name]: [fileStoreId], | ||
| } , |
There was a problem hiding this comment.
Avoid using 'await' inside object literals directly
Using await directly inside an object literal is invalid syntax. Compute the awaited value before constructing the object.
Refactor the code as follows:
+ const isMicroplan = await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId);
request.body.ResourceDetails = {
...request.body.ResourceDetails,
processedFileStoreId: fileStoreId,
status:
request.body.ResourceDetails.status != resourceDataStatuses.invalid
? resourceDataStatuses.completed
: resourceDataStatuses.invalid,
auditDetails: {
...request.body.ResourceDetails.auditDetails,
lastModifiedBy: request.body.RequestInfo.userInfo.uuid,
lastModifiedTime: Date.now(),
},
additionalDetails: {
...request.body.ResourceDetails.additionalDetails,
sheetErrors: request.body.additionalDetailsErrors,
- source:
- await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId)
- ? "microplan"
- : null,
+ source: isMicroplan ? "microplan" : null,
[name]: [fileStoreId],
},
};Committable suggestion skipped: line range outside the PR's diff.
| await isMicropplanCampaignId(request?.body?.ResourceDetails?.campaignId) | ||
| ? true | ||
| : false; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify boolean assignment without redundant ternary operator
Again, the ternary operator is unnecessary and can be removed.
Apply this diff to simplify the code:
- const isLockSheetNeeded =
- await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId)
- ? true
- : false;
+ const isLockSheetNeeded = await isMicroplanCampaignId(request?.body?.ResourceDetails?.campaignId);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 598-600: 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)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
These changes improve the overall robustness and responsiveness of the campaign management system.