Repository navigation
Materialized views fixes - #1550
Conversation
WalkthroughThe refresh flow was changed to support multi-tenant refreshes: the admin controller parses Changes
Sequence DiagramsequenceDiagram
participant Client as Client Request
participant AdminCtrl as Admin Controller
participant ViewsModule as Materialized Views Module
participant DB as Database
participant Scheduler as Scheduler Service
Client->>AdminCtrl: POST /triggerPeriodicViewRefresh\n(path param `{tenantCode|modelName}`)
AdminCtrl->>AdminCtrl: Parse & decode `{tenantCode|modelName}`
alt tenantCode provided
AdminCtrl->>ViewsModule: refreshMaterializedView(modelName, tenantCode)
ViewsModule->>ViewsModule: Validate params & model exists
ViewsModule->>DB: Execute REFRESH MATERIALIZED VIEW
ViewsModule-->>AdminCtrl: Return result (success/error + metadata)
else no tenantCode
AdminCtrl->>DB: Fetch distinct tenant codes
AdminCtrl->>AdminCtrl: Loop tenants
loop per tenant
AdminCtrl->>ViewsModule: refreshMaterializedView(modelName, tenantCode_i)
ViewsModule->>DB: Refresh view for tenant_i
ViewsModule-->>AdminCtrl: Per-tenant result
end
AdminCtrl->>Scheduler: Optionally POST job URLs with encoded tenant/model
end
AdminCtrl-->>Client: Consolidated response (tenantsProcessed, per-tenant results)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/services/admin.js (3)
856-859: Error message references wrong function name.Line 857 logs
'An error occurred in userDelete:'but this is in thetriggerPeriodicViewRefreshInternalmethod. This appears to be copy-paste error.Fix the error message
} catch (error) { - console.error('An error occurred in userDelete:', error) + console.error('An error occurred in triggerPeriodicViewRefreshInternal:', error) return error }
673-676: Undefined variablementeeIdin error log.At line 675, the error log references
menteeIdwhich is not defined in this scope. It should bementeeData.mentee_id.Fix the variable reference
} catch (cacheError) { - console.error(`Cache deletion failed for mentee ${menteeId}:`, cacheError) + console.error(`Cache deletion failed for mentee ${menteeData.mentee_id}:`, cacheError) }
1340-1347: Parameter order is incorrect in multiplegetUsersByUserIdscalls.The function signature at
src/database/queries/userExtension.js:299is:getUsersByUserIds(ids, options = {}, tenantCode, unscoped = false)At lines 1340-1347, 1441-1448, 1492-1499, 1550-1557, and 1607-1614, the parameters are reversed:
getUsersByUserIds(ids, options, false, tenantCode)This swaps
tenantCodeandunscoped, breaking tenant filtering and passing a string where a boolean is expected. Lines 238-244 and 284-290 use the correct order; fix the remaining call sites to match.
🤖 Fix all issues with AI agents
In `@src/controllers/v1/admin.js`:
- Around line 132-139: The controller expects
adminService.triggerPeriodicViewRefreshInternal to return a response object with
statusCode and result, but refreshMaterializedView currently calls
responses.successResponse() without passing the actual refresh payload and
returns raw errors on failure; update refreshMaterializedView to pass the
refresh result into responses.successResponse(...) (so the success response
includes the actual payload) and change its catch block to return a standardized
responses.errorResponse(error) (or construct an object with statusCode and
result/error) so triggerPeriodicViewRefreshInternal and the controller receive a
consistent response shape; verify triggerPeriodicViewRefreshInternal propagates
that response unchanged.
🧹 Nitpick comments (7)
src/generics/materializedViews.js (3)
428-433: Parameter validation returns data, but callers may not handle it consistently.The function now returns
{ success: false, message: '...' }for missing parameters instead of throwing. This is a behavioral change - ensure all callers check thesuccessproperty before using the result.Looking at line 476 in
refreshNextView, it callsrefreshMaterializedViewbut doesn't await or check its return value:refreshMaterializedView(modelNames[currentIndex], tenantCode)This means validation failures are silently ignored in the periodic refresh flow.
Consider awaiting and logging the result in refreshNextView
const refreshNextView = (currentIndex, modelNames, tenantCode) => { try { if (currentIndex < modelNames.length) { - refreshMaterializedView(modelNames[currentIndex], tenantCode) + refreshMaterializedView(modelNames[currentIndex], tenantCode) + .then(result => { + if (!result.success) { + logger.warn(`refreshNextView: Failed for ${modelNames[currentIndex]}: ${result.message}`) + } + }) + .catch(err => logger.error(`refreshNextView error: ${err.message}`)) currentIndex++ } else {
456-463: Inconsistent use ofmetadata?.rowCountvs directmetadataaccess.On line 462, you access
metadata?.rowCountwith optional chaining, but on line 461 you passmetadatadirectly. This is fine, but note that forREFRESH MATERIALIZED VIEW, Sequelize typically returnsundefinedfor the result and metadata may not containrowCount. Consider documenting this behavior or removing therowCountfield if it's always undefined for this query type.
464-470: Error object included in return value may cause serialization issues.Returning the raw
errobject (line 469) could cause issues if this result is serialized to JSON (e.g., circular references in error objects). Consider returning only serializable error information.Return serializable error info only
- return { success: false, message: err.message || 'Failed to refresh materialized view', error: err } + return { + success: false, + message: err.message || 'Failed to refresh materialized view', + errorCode: err.code, + errorName: err.name + }src/scripts/viewsScript.js (2)
74-77: URL validation could be more robust.The check
!apiUrl.includes('http')may pass invalid URLs like"httpxyz". Consider using URL parsing for stricter validation.Use URL constructor for validation
- if (!apiUrl || !apiUrl.includes('http')) { - console.error(`❌ Invalid scheduler service URL: ${apiUrl}`) - return - } + try { + new URL(apiUrl) // Validates URL format + } catch { + console.error(`❌ Invalid scheduler service URL: ${apiUrl}`) + return + }
177-198: Loop usesforEachwithletvariables from outer scope.The
forEachcallback modifiesjobsCreatedandglobalOffsetfrom the outer scope. This works correctly, but using afor...ofloop would be more consistent with the tenant iteration above (line 167) and clearer about the intent.src/services/admin.js (1)
1106-1114:notifyMentorsAboutMenteeDeletionpassestenantCodesbut parameter expectstenantCode.The
sendGenericNotificationfunction signature at line 38 shows it expectstenantCode(singular), but line 1113 passestenantCodes(plural array). The normalization logic at lines 47-48 handles arrays, but the parameter name is misleading.src/controllers/v1/admin.js (1)
91-107: Path parameter parsing logic is correct but fragile.The parsing correctly mirrors the encoding in
viewsScript.js. However, there's no validation that both parts are non-empty after splitting, which could lead to empty strings being passed to the service.Add validation for parsed values
if (req.params.id) { const parts = req.params.id.split('|') if (parts.length === 2) { - tenantCode = decodeURIComponent(parts[0]) - modelName = decodeURIComponent(parts[1]) + const decodedTenant = decodeURIComponent(parts[0]) + const decodedModel = decodeURIComponent(parts[1]) + if (decodedTenant && decodedModel) { + tenantCode = decodedTenant + modelName = decodedModel + } } }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/controllers/v1/admin.js`:
- Around line 122-157: After processing tenants, add a guard that if
results.length === 0 you return an appropriate "no tenants" response instead of
the success message; specifically, after the for loop in
src/controllers/v1/admin.js check results.length === 0 and return a response
using the existing responses helper and httpStatusCode (e.g., set statusCode to
httpStatusCode.notFound and message to 'NO_TENANTS_FOUND' or use the same
response helper used on lines ~115-119) so that when all tenant codes were
skipped you emit a descriptive NO_TENANTS_FOUND response instead of
MATERIALIZED_VIEW_REFRESH_INITIATED_SUCCESSFULLY; keep the
adminService.triggerPeriodicViewRefreshInternal and results payload unchanged
for non-empty results.
- Around line 100-106: The decodeURIComponent calls on req.params.id can throw
URIError for malformed percent-encodings; wrap the decodeURIComponent usage in a
try/catch (around the block that splits req.params.id and sets tenantCode and
modelName) and if a URIError is caught, return a 400 Bad Request with an
appropriate validation message instead of letting it bubble to the outer catch.
Specifically, catch errors thrown by decodeURIComponent when parsing parts from
req.params.id, and respond with a 400 response payload (or next a
BadRequestError) indicating invalid/ malformed id rather than letting
MATERIALIZED_VIEW_REFRESH_FAILED/500 be returned.
| if (req.params.id) { | ||
| const parts = req.params.id.split('|') | ||
| if (parts.length === 2) { | ||
| tenantCode = decodeURIComponent(parts[0]) | ||
| modelName = decodeURIComponent(parts[1]) | ||
| } | ||
| } |
There was a problem hiding this comment.
decodeURIComponent can throw URIError on malformed input, yielding a 500 instead of 400.
If req.params.id contains a malformed percent-encoding (e.g., %ZZ), decodeURIComponent throws a URIError. This bubbles to the outer catch (line 159) and returns a generic MATERIALIZED_VIEW_REFRESH_FAILED with a 500 status, which is misleading for what is essentially a bad request.
Suggested fix
if (req.params.id) {
- const parts = req.params.id.split('|')
- if (parts.length === 2) {
- tenantCode = decodeURIComponent(parts[0])
- modelName = decodeURIComponent(parts[1])
- }
+ try {
+ const parts = req.params.id.split('|')
+ if (parts.length === 2) {
+ tenantCode = decodeURIComponent(parts[0])
+ modelName = decodeURIComponent(parts[1])
+ }
+ } catch (decodeError) {
+ return responses.failureResponse({
+ statusCode: httpStatusCode.bad_request,
+ message: 'INVALID_PATH_PARAMETER',
+ responseCode: 'CLIENT_ERROR',
+ })
+ }
}🤖 Prompt for AI Agents
In `@src/controllers/v1/admin.js` around lines 100 - 106, The decodeURIComponent
calls on req.params.id can throw URIError for malformed percent-encodings; wrap
the decodeURIComponent usage in a try/catch (around the block that splits
req.params.id and sets tenantCode and modelName) and if a URIError is caught,
return a 400 Bad Request with an appropriate validation message instead of
letting it bubble to the outer catch. Specifically, catch errors thrown by
decodeURIComponent when parsing parts from req.params.id, and respond with a 400
response payload (or next a BadRequestError) indicating invalid/ malformed id
rather than letting MATERIALIZED_VIEW_REFRESH_FAILED/500 be returned.
| // Process each tenant | ||
| const results = [] | ||
| for (const tenant of tenants) { | ||
| const tenantCodeToProcess = tenant.code | ||
| // Skip invalid tenant codes | ||
| if (!tenantCodeToProcess || tenantCodeToProcess === 'undefined') { | ||
| continue | ||
| } | ||
| try { | ||
| const result = await adminService.triggerPeriodicViewRefreshInternal( | ||
| modelName, | ||
| tenantCodeToProcess | ||
| ) | ||
| results.push({ | ||
| tenantCode: tenantCodeToProcess, | ||
| modelName: modelName || 'all models', | ||
| success: result.statusCode === httpStatusCode.ok, | ||
| result: result.result, | ||
| }) | ||
| } catch (error) { | ||
| results.push({ | ||
| tenantCode: tenantCodeToProcess, | ||
| modelName: modelName || 'all models', | ||
| success: false, | ||
| error: error.message || 'Unknown error', | ||
| }) | ||
| } | ||
| } | ||
| return responses.successResponse({ | ||
| statusCode: httpStatusCode.ok, | ||
| message: 'NO_TENANTS_FOUND', | ||
| message: 'MATERIALIZED_VIEW_REFRESH_INITIATED_SUCCESSFULLY', | ||
| result: { | ||
| tenantsProcessed: results.length, | ||
| results: results, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Success response returned even when all tenants are skipped.
If every tenant returned by getDistinctTenantCodes() has an invalid code (falsy or 'undefined'), the loop skips all of them and results is empty. The response will be MATERIALIZED_VIEW_REFRESH_INITIATED_SUCCESSFULLY with tenantsProcessed: 0, which is the same shape as a success but semantically closer to NO_TENANTS_FOUND. Consider checking results.length === 0 after the loop and returning a more descriptive message, similar to lines 115-119.
🤖 Prompt for AI Agents
In `@src/controllers/v1/admin.js` around lines 122 - 157, After processing
tenants, add a guard that if results.length === 0 you return an appropriate "no
tenants" response instead of the success message; specifically, after the for
loop in src/controllers/v1/admin.js check results.length === 0 and return a
response using the existing responses helper and httpStatusCode (e.g., set
statusCode to httpStatusCode.notFound and message to 'NO_TENANTS_FOUND' or use
the same response helper used on lines ~115-119) so that when all tenant codes
were skipped you emit a descriptive NO_TENANTS_FOUND response instead of
MATERIALIZED_VIEW_REFRESH_INITIATED_SUCCESSFULLY; keep the
adminService.triggerPeriodicViewRefreshInternal and results payload unchanged
for non-empty results.
| const parts = req.params.id.split('|') | ||
| if (parts.length === 2) { | ||
| tenantCode = decodeURIComponent(parts[0]) | ||
| modelName = decodeURIComponent(parts[1]) | ||
| } |
There was a problem hiding this comment.
Is this correct? We need to encode the URI while creating the job, right? For example, /default|sessions becomes %2Fdefault%7Csessions, so the params.id will not contain |, which would otherwise cause it to break, right?
There was a problem hiding this comment.
@nevil-mathew so we will need to change it to query params
There was a problem hiding this comment.
@coderabbitai check if the changes are applied.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the current implementation of triggerPeriodicViewRefreshInternal in admin.js
echo "=== Current triggerPeriodicViewRefreshInternal implementation ==="
sed -n '/async triggerPeriodicViewRefreshInternal/,/^[[:space:]]*}/p' src/controllers/v1/admin.js | head -50
echo -e "\n=== Checking how scheduler jobs are created in viewsScript.js ==="
# Look for where the URL is constructed for the periodic refresh job
rg -A 10 -B 5 'triggerPeriodicViewRefreshInternal' src/scripts/viewsScript.js
echo -e "\n=== Checking route definition for this endpoint ==="
# Find route definitions that map to this controller method
rg -n 'triggerPeriodicViewRefreshInternal' src/routes/
Length of output: 2248
🏁 Script executed:
#!/bin/bash
# Check admin route definitions
echo "=== Admin route definitions ==="
cat src/routes/admin.js 2>/dev/null || cat src/routes/v1/admin.js 2>/dev/null || find src/routes -name "*admin*" -exec cat {} \;
echo -e "\n=== Search for the route pattern ==="
rg -n "triggerPeriodicViewRefreshInternal" src/routes/ -A 2 -B 2
echo -e "\n=== Check if there's a recent commit or change ==="
git log -1 --oneline src/controllers/v1/admin.js src/scripts/viewsScript.js
echo -e "\n=== Check git diff to see if there are uncommitted changes ==="
git diff src/controllers/v1/admin.js | head -100
Length of output: 214
🏁 Script executed:
#!/bin/bash
# Find all route files and look for the endpoint pattern
echo "=== Finding route files ==="
find src -name "*.js" -path "*/routes/*" -type f
echo -e "\n=== Looking for admin routes that might define this endpoint ==="
find src -name "*.js" -path "*/routes/*" -type f -exec grep -l "admin" {} \;
echo -e "\n=== Check one of these route files ==="
find src -name "*.js" -path "*/routes/*" -type f -exec grep -l "admin" {} \; | head -1 | xargs cat
echo -e "\n=== Search for route patterns with :id parameter ==="
rg -n "triggerPeriodicViewRefreshInternal|:id" src/routes/ -C 2
Length of output: 808
🏁 Script executed:
#!/bin/bash
# Let's see the exact current code around line 100-110 in admin.js
echo "=== Current code in admin.js (lines 95-120) ==="
sed -n '95,120p' src/controllers/v1/admin.js
echo -e "\n=== Check if there are any query param usage ==="
rg -n "req.query.tenant_code|req.query.model_name" src/controllers/v1/admin.js
echo -e "\n=== Let's check the viewsScript to see how URL is constructed ==="
rg -n "encodedParams|encodeURIComponent" src/scripts/viewsScript.js -B 2 -A 2
echo -e "\n=== Check if Express auto-decodes path parameters ==="
# Let's trace through what happens with the encoded value
echo "When URL is: /api/v1/admin/method/default%7Csessions"
echo "Express assigns to req.params.id: 'default|sessions' (auto-decoded)"
echo "So split('|') would work correctly"
Length of output: 1897
@nevil-mathew The code is still using path parameters (not query params as sumanvpacewisdom mentioned). However, the implementation is actually correct and works as intended. Here's what changed:
What was fixed:
The decodeURIComponent() calls were removed from lines 103-104:
// OLD (in diff):
tenantCode = decodeURIComponent(parts[0])
modelName = decodeURIComponent(parts[1])
// CURRENT:
tenantCode = parts[0]
modelName = parts[1]Why this works:
-
viewsScript.js encodes the parameters:
const encodedParams = encodeURIComponent(`${tenantCode}|${model}`) // Example: 'default|sessions' → 'default%7Csessions'
-
URL sent to scheduler:
/mentoring/v1/admin/triggerPeriodicViewRefreshInternal/default%7Csessions -
Express automatically decodes path parameters:
req.params.id = 'default|sessions' // Already decoded by Express!
-
Split works correctly:
parts = 'default|sessions'.split('|') // ['default', 'sessions'] ✅
The concern you raised about encoding breaking the pipe delimiter was valid, but Express's automatic decoding of path parameters handles this. The decodeURIComponent() calls were redundant (though harmless), and removing them was the correct fix.
Summary: The path parameter approach is working correctly. No change to query params was made.
🧬 Code Graph Analysis Results
<no_relevant_code_snippets>
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/controllers/v1/admin.js (2)
66-81:⚠️ Potential issue | 🟡 Minor
triggerPeriodicViewRefreshswallows errors — no response returned on failure.The catch block at line 79 only logs but doesn't return a failure response, so the caller gets
undefined. Other methods in this controller consistently returnresponses.failureResponse(...)on error.Suggested fix
} catch (err) { - console.log(err) + console.error('Error in triggerPeriodicViewRefresh:', err) + return responses.failureResponse({ + statusCode: httpStatusCode.internal_server_error, + message: 'MATERIALIZED_VIEW_REFRESH_FAILED', + responseCode: 'SERVER_ERROR', + }) }
50-65:⚠️ Potential issue | 🟡 Minor
triggerViewRebuildandtriggerViewRebuildInternalreturn raw error objects on failure.Lines 63 and 89 return
errordirectly instead of a formatted failure response. This produces an inconsistent response shape for the framework/caller and could leak stack traces.Suggested fix (triggerViewRebuild example)
} catch (error) { - return error + console.error('Error in triggerViewRebuild:', error) + return responses.failureResponse({ + statusCode: httpStatusCode.internal_server_error, + message: 'VIEW_REBUILD_FAILED', + responseCode: 'SERVER_ERROR', + }) }
🤖 Fix all issues with AI agents
In `@src/scripts/viewsScript.js`:
- Around line 148-153: The log accesses tenants.length before ensuring tenants
is non-null, which can throw if userExtensionQueries.getDistinctTenantCodes()
returns null/undefined; change the order: first check the result from
userExtensionQueries.getDistinctTenantCodes() (e.g., const tenants = await
userExtensionQueries.getDistinctTenantCodes(); if (!tenants || tenants.length
=== 0) { console.log(...); return }) and only after that call
console.log(`Starting periodic refresh for ${tenants.length} tenants`); update
references in this block (tenants and getDistinctTenantCodes) so the null guard
runs before any property access.
- Around line 210-212: In buildMaterializedViews, the code logs tenants.length
before checking if tenants is null/undefined, causing a possible
null-dereference; move the console.log(`Starting materialized view build for
${tenants.length} tenants`) to after the guard that verifies tenants is defined
(the same pattern used in triggerPeriodicViewRefresh) so the log executes only
when tenants is non-null, and reference the tenants variable and
buildMaterializedViews function when making the change.
🧹 Nitpick comments (4)
src/controllers/v1/admin.js (1)
113-120: Direct DB query in controller bypasses the service layer.
userExtensionQueries.getDistinctTenantCodes()is called directly from the controller. Other methods in this file delegate all business logic toadminService. Consider moving the multi-tenant iteration logic into the service to keep the controller thin and consistent with the rest of the file.src/scripts/viewsScript.js (3)
74-77: URL validation is fragile.
!apiUrl.includes('http')would pass for any string containing "http" anywhere (e.g., in a path segment). A more robust check:- if (!apiUrl || !apiUrl.includes('http')) { + if (!apiUrl || !(apiUrl.startsWith('http://') || apiUrl.startsWith('https://'))) {
78-96:request.postcallback errors are not propagated — the outer try/catch is ineffective.
request.postis callback-based and does not throw synchronously, so thetry/catchon line 78/94 will never catch callback errors. The current callback does handle errors internally via logging, so this isn't a runtime bug, but the surrounding try/catch gives a false sense of safety. Consider removing it or converting to a promise-based HTTP client.
1-4: Therequestlibrary is deprecated and should be migrated to a maintained alternative.The
requestpackage (line 3) has been deprecated since February 2020 and remains unmaintained. Consider migrating toaxios(already available in your dependencies),node-fetch, or the built-infetchfor Node 18+. This library is also used in 9 other files across the codebase, so this is a larger refactoring effort that should be tracked.
| const tenants = await userExtensionQueries.getDistinctTenantCodes() | ||
| console.log(`Starting periodic refresh for ${tenants.length} tenants`) | ||
| if (!tenants || tenants.length === 0) { | ||
| console.log('⚠️ No tenants found. Skipping periodic view refresh setup.') | ||
| return | ||
| } |
There was a problem hiding this comment.
tenants.length accessed before the null guard — potential TypeError.
Line 149 logs tenants.length, but the null/empty check is on line 150. If getDistinctTenantCodes() returns null or undefined, this will throw.
Suggested fix
const tenants = await userExtensionQueries.getDistinctTenantCodes()
- console.log(`Starting periodic refresh for ${tenants.length} tenants`)
if (!tenants || tenants.length === 0) {
console.log('⚠️ No tenants found. Skipping periodic view refresh setup.')
return
}
+ console.log(`Starting periodic refresh for ${tenants.length} tenants`)📝 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 tenants = await userExtensionQueries.getDistinctTenantCodes() | |
| console.log(`Starting periodic refresh for ${tenants.length} tenants`) | |
| if (!tenants || tenants.length === 0) { | |
| console.log('⚠️ No tenants found. Skipping periodic view refresh setup.') | |
| return | |
| } | |
| const tenants = await userExtensionQueries.getDistinctTenantCodes() | |
| if (!tenants || tenants.length === 0) { | |
| console.log('⚠️ No tenants found. Skipping periodic view refresh setup.') | |
| return | |
| } | |
| console.log(`Starting periodic refresh for ${tenants.length} tenants`) |
🤖 Prompt for AI Agents
In `@src/scripts/viewsScript.js` around lines 148 - 153, The log accesses
tenants.length before ensuring tenants is non-null, which can throw if
userExtensionQueries.getDistinctTenantCodes() returns null/undefined; change the
order: first check the result from userExtensionQueries.getDistinctTenantCodes()
(e.g., const tenants = await userExtensionQueries.getDistinctTenantCodes(); if
(!tenants || tenants.length === 0) { console.log(...); return }) and only after
that call console.log(`Starting periodic refresh for ${tenants.length}
tenants`); update references in this block (tenants and getDistinctTenantCodes)
so the null guard runs before any property access.
| const tenants = await userExtensionQueries.getDistinctTenantCodes() | ||
| console.log(`Starting materialized view build for ${tenants.length} tenants`) | ||
|
|
There was a problem hiding this comment.
Same null-dereference pattern in buildMaterializedViews.
Line 211 accesses tenants.length before the guard on line 213. Move the log after the null check, same as the fix for triggerPeriodicViewRefresh.
Suggested fix
const tenants = await userExtensionQueries.getDistinctTenantCodes()
- console.log(`Starting materialized view build for ${tenants.length} tenants`)
-
if (!tenants || tenants.length === 0) {
console.log('⚠️ No tenants found. Skipping materialized view build.')
return
}
+ console.log(`Starting materialized view build for ${tenants.length} tenants`)🤖 Prompt for AI Agents
In `@src/scripts/viewsScript.js` around lines 210 - 212, In
buildMaterializedViews, the code logs tenants.length before checking if tenants
is null/undefined, causing a possible null-dereference; move the
console.log(`Starting materialized view build for ${tenants.length} tenants`) to
after the guard that verifies tenants is defined (the same pattern used in
triggerPeriodicViewRefresh) so the log executes only when tenants is non-null,
and reference the tenants variable and buildMaterializedViews function when
making the change.
Release Notes
{tenantCode|modelName}to refresh a single tenant or, when omitted, discovers and refreshes all tenants.Contributor Statistics