Feature/cach fix tenant#1610
Feature/cach fix tenant#1610borkarsaish65 wants to merge 4 commits intoELEVATE-Project:tenant_phase2_tempfrom
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughRemoved caching mechanisms across three service files. Entity and modules services no longer use cache-first retrieval strategies for list operations, and sessions service changed to cache raw data instead of processed responses. Cache invalidation logic was also removed from entity operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/sessions.js (1)
1762-1772:⚠️ Potential issue | 🔴 CriticalCache hit returns unprocessed session data—API responses differ depending on cache state.
The
details()function has an inconsistency: when a session is retrieved from cache (line 1462), it is returned directly without callingprocessDbResponse. However, when the same session is fetched from the database (line 1547), it is processed byprocessDbResponsebefore being returned (line 1772).This means the cache stores raw
sessionDetails(with entity-type fields in their raw format), and the cache hit path returns this raw data to clients, while the cache miss path returns processed data with transformed entity-type fields. API clients will receive different response formats depending on whether the cache was hit or missed.Solution: Either call
processDbResponseon the cached response before returning it (line 1539), or store the processed result in the cache instead of the raw data (line 1768).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/sessions.js` around lines 1762 - 1772, The cached session path in details() returns raw sessionDetails while the DB path runs utils.processDbResponse(sessionDetails, validationData) before returning, causing inconsistent API responses; fix by ensuring cached responses are processed the same way—either call utils.processDbResponse(...) on the cached session before returning it in details(), or change the cache write (cacheHelper.sessions.set with cacheCopy) to store the processed result (result of utils.processDbResponse) instead of raw sessionDetails so both cache hits and misses return the same processed payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/services/sessions.js`:
- Around line 1762-1772: The cached session path in details() returns raw
sessionDetails while the DB path runs utils.processDbResponse(sessionDetails,
validationData) before returning, causing inconsistent API responses; fix by
ensuring cached responses are processed the same way—either call
utils.processDbResponse(...) on the cached session before returning it in
details(), or change the cache write (cacheHelper.sessions.set with cacheCopy)
to store the processed result (result of utils.processDbResponse) instead of raw
sessionDetails so both cache hits and misses return the same processed payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f9c6fd3-548a-4f8e-9d8c-ea06b78d18cb
📒 Files selected for processing (3)
src/services/entity.jssrc/services/modules.jssrc/services/sessions.js
| if (utils.isNumeric(id)) { | ||
| try { | ||
| const cacheCopy = { ...processDbResponse } | ||
| const cacheCopy = { ...sessionDetails } |
There was a problem hiding this comment.
Why are we storing the raw response in the cache? Has the cache hit flow been updated to handle this new logic? What’s the reason behind this change?
There was a problem hiding this comment.
The change was needed because processDbResponse() transforms type into an object { label: "PUBLIC", value: "PUBLIC" }. When the processed result was cached, it was breaking the session update and delete API flows — the comparison sessionDetail.type != bodyData.type was always true (object vs string), triggering CANNOT_EDIT_MENTOR_AND_TYPE incorrectly.
Storing raw data fixes that comparison. The cache-hit path in the details endpoint has been verified and works correctly with the raw data.
Release Notes
entity_listwas deleted and the method now always fetches results directlymodules_listresults; pagination/search behavior is now independent of cached dataprocessDbResponsecomputation moved to occur after the caching blockAuthor Contributions