Refactor mentees.js for enhanced query processing and validation - BUG 4416#1597
Open
sumanvpacewisdom wants to merge 1 commit intodevelopfrom
Open
Refactor mentees.js for enhanced query processing and validation - BUG 4416#1597sumanvpacewisdom wants to merge 1 commit intodevelopfrom
sumanvpacewisdom wants to merge 1 commit intodevelopfrom
Conversation
…tion and filtering logic. This update ensures proper handling of tenant codes and improves the clarity of connected mentee retrieval.
WalkthroughThe mentees service refactors validation logic in the MenteesHelper.list method by moving tenant code and entity-type validation earlier in the flow. It consolidates duplicate validation checks, builds a filtered query upfront using validated data, and removes manual userId manipulation, reducing code redundancy. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/mentees.js`:
- Around line 1529-1534: The check for defaults.tenantCode returns a misleading
error key 'DEFAULT_ORG_CODE_NOT_SET'; update the responses.failureResponse call
in the block that validates defaults.tenantCode (where responses.failureResponse
is invoked with message: 'DEFAULT_ORG_CODE_NOT_SET') to use the correct
tenant-specific message key (e.g., 'DEFAULT_TENANT_CODE_NOT_SET' or the existing
tenant error constant from your error messages) while keeping the same
statusCode and responseCode; ensure you reference the tenant error constant if
available instead of a hard-coded string.
- Around line 1536-1547: The result of
entityTypeCache.getEntityTypesAndEntitiesWithCache (assigned to validationData)
can be an Error; before calling utils.validateAndBuildFilters you must guard
that validationData is not an Error. Add an explicit check after the
getEntityTypesAndEntitiesWithCache call (e.g., if (validationData instanceof
Error) { ... }) and handle it the same way other sections in this file do
(returning/propagating the error or logging and exiting) so that
utils.validateAndBuildFilters is only called with valid validation data.
- Around line 1528-1534: The list handler in src/services/mentees.js currently
calls getDefaults() and returns a hard failure if defaults.tenantCode is missing
even though defaults isn't used later; remove the unnecessary failure path by
either deleting the getDefaults() call entirely from the list flow or by making
the lookup non-blocking (e.g., catch errors and ignore missing tenantCode) so
the list function proceeds normally; update the code around getDefaults, the
conditional checking defaults.tenantCode, and any related
responses.failureResponse usage to ensure list continues without returning
DEFAULT_ORG_CODE_NOT_SET.
Comment on lines
+1528
to
+1534
| const defaults = await getDefaults() | ||
| if (!defaults.tenantCode) | ||
| return responses.failureResponse({ | ||
| message: 'DEFAULT_ORG_CODE_NOT_SET', | ||
| statusCode: httpStatusCode.bad_request, | ||
| responseCode: 'CLIENT_ERROR', | ||
| }) |
There was a problem hiding this comment.
Avoid a hard failure on an unused defaults lookup
defaults is fetched and validated, but not used afterwards in list. This adds an unrelated failure path for Line 1504 endpoint behavior and can block mentee listing unnecessarily.
Proposed fix
-const defaults = await getDefaults()
-if (!defaults.tenantCode)
- return responses.failureResponse({
- message: 'DEFAULT_ORG_CODE_NOT_SET',
- statusCode: httpStatusCode.bad_request,
- responseCode: 'CLIENT_ERROR',
- })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/mentees.js` around lines 1528 - 1534, The list handler in
src/services/mentees.js currently calls getDefaults() and returns a hard failure
if defaults.tenantCode is missing even though defaults isn't used later; remove
the unnecessary failure path by either deleting the getDefaults() call entirely
from the list flow or by making the lookup non-blocking (e.g., catch errors and
ignore missing tenantCode) so the list function proceeds normally; update the
code around getDefaults, the conditional checking defaults.tenantCode, and any
related responses.failureResponse usage to ensure list continues without
returning DEFAULT_ORG_CODE_NOT_SET.
Comment on lines
+1529
to
+1534
| if (!defaults.tenantCode) | ||
| return responses.failureResponse({ | ||
| message: 'DEFAULT_ORG_CODE_NOT_SET', | ||
| statusCode: httpStatusCode.bad_request, | ||
| responseCode: 'CLIENT_ERROR', | ||
| }) |
There was a problem hiding this comment.
Use the correct tenant error message key
Line 1529 validates defaults.tenantCode, but Lines 1531-1534 return DEFAULT_ORG_CODE_NOT_SET. This returns a misleading client error for tenant misconfiguration.
Proposed fix
if (!defaults.tenantCode)
return responses.failureResponse({
- message: 'DEFAULT_ORG_CODE_NOT_SET',
+ message: 'DEFAULT_TENANT_CODE_NOT_SET',
statusCode: httpStatusCode.bad_request,
responseCode: 'CLIENT_ERROR',
})📝 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.
Suggested change
| if (!defaults.tenantCode) | |
| return responses.failureResponse({ | |
| message: 'DEFAULT_ORG_CODE_NOT_SET', | |
| statusCode: httpStatusCode.bad_request, | |
| responseCode: 'CLIENT_ERROR', | |
| }) | |
| if (!defaults.tenantCode) | |
| return responses.failureResponse({ | |
| message: 'DEFAULT_TENANT_CODE_NOT_SET', | |
| statusCode: httpStatusCode.bad_request, | |
| responseCode: 'CLIENT_ERROR', | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/mentees.js` around lines 1529 - 1534, The check for
defaults.tenantCode returns a misleading error key 'DEFAULT_ORG_CODE_NOT_SET';
update the responses.failureResponse call in the block that validates
defaults.tenantCode (where responses.failureResponse is invoked with message:
'DEFAULT_ORG_CODE_NOT_SET') to use the correct tenant-specific message key
(e.g., 'DEFAULT_TENANT_CODE_NOT_SET' or the existing tenant error constant from
your error messages) while keeping the same statusCode and responseCode; ensure
you reference the tenant error constant if available instead of a hard-coded
string.
Comment on lines
+1536
to
+1547
| let validationData = await entityTypeCache.getEntityTypesAndEntitiesWithCache( | ||
| { | ||
| status: common.ACTIVE_STATUS, | ||
| model_names: { [Op.overlap]: [userExtensionModelName] }, | ||
| }, | ||
| tenantCode, | ||
| organizationCode, | ||
| userExtensionModelName | ||
| ) | ||
|
|
||
| let filteredQuery = utils.validateAndBuildFilters(query, validationData) | ||
|
|
There was a problem hiding this comment.
Guard entity-type cache failures before building filters
Line 1536 can return an Error object (pattern already handled elsewhere in this file), but Line 1546 immediately passes the result into validateAndBuildFilters. Add an explicit guard to prevent invalid filter construction.
Proposed fix
let validationData = await entityTypeCache.getEntityTypesAndEntitiesWithCache(
{
status: common.ACTIVE_STATUS,
model_names: { [Op.overlap]: [userExtensionModelName] },
},
tenantCode,
organizationCode,
userExtensionModelName
)
+if (validationData instanceof Error) {
+ throw validationData
+}
let filteredQuery = utils.validateAndBuildFilters(query, validationData)📝 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.
Suggested change
| let validationData = await entityTypeCache.getEntityTypesAndEntitiesWithCache( | |
| { | |
| status: common.ACTIVE_STATUS, | |
| model_names: { [Op.overlap]: [userExtensionModelName] }, | |
| }, | |
| tenantCode, | |
| organizationCode, | |
| userExtensionModelName | |
| ) | |
| let filteredQuery = utils.validateAndBuildFilters(query, validationData) | |
| let validationData = await entityTypeCache.getEntityTypesAndEntitiesWithCache( | |
| { | |
| status: common.ACTIVE_STATUS, | |
| model_names: { [Op.overlap]: [userExtensionModelName] }, | |
| }, | |
| tenantCode, | |
| organizationCode, | |
| userExtensionModelName | |
| ) | |
| if (validationData instanceof Error) { | |
| throw validationData | |
| } | |
| let filteredQuery = utils.validateAndBuildFilters(query, validationData) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/mentees.js` around lines 1536 - 1547, The result of
entityTypeCache.getEntityTypesAndEntitiesWithCache (assigned to validationData)
can be an Error; before calling utils.validateAndBuildFilters you must guard
that validationData is not an Error. Add an explicit check after the
getEntityTypesAndEntitiesWithCache call (e.g., if (validationData instanceof
Error) { ... }) and handle it the same way other sections in this file do
(returning/propagating the error or logging and exiting) so that
utils.validateAndBuildFilters is only called with valid validation data.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor mentees.js to enhance query processing by integrating validation and filtering logic. This update ensures proper handling of tenant codes and improves the clarity of connected mentee retrieval.
Release Notes
defaults.tenantCodeinMenteesHelper.list()to ensure tenant codes are properly validated before query processingentityTypeCachefor Mentee extension model validation withutils.validateAndBuildFiltersfor creating filtered queriesconnectedQuerywith validatedfilteredQuerywhen retrieving connected mentees viaconnectionQueries.getConnectionsDetails()Contributors