Fix: Cached Data Shown After Logout and Re-login#4040
Fix: Cached Data Shown After Logout and Re-login#4040priyankeshh wants to merge 1 commit intobeckn:mainfrom
Conversation
WalkthroughThe changes introduce a unified logout utility that clears Redux state, cookies, localStorage (with selective preservation), and sessionStorage, and redirects to the login page. Redux store logic is updated to reset state on logout. Axios cache control is enforced globally via a new utility and interceptors, and all API calls are refactored to use this utility. The header component now consistently uses the new logout flow for both the home icon and menu actions. These updates collectively ensure cached data from previous sessions is cleared on logout. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TopHeader
participant ReduxStore
participant logoutUser
participant Cookies
participant localStorage
participant sessionStorage
participant Router
User->>TopHeader: Clicks Logout/Home
TopHeader->>logoutUser: logoutUser(dispatch, router)
logoutUser->>ReduxStore: Dispatch logout action
logoutUser->>Cookies: Remove auth/user cookies
logoutUser->>localStorage: Preserve userPhone, clear others
logoutUser->>sessionStorage: Clear all
logoutUser->>Router: Redirect to login page
sequenceDiagram
participant App
participant setupAxiosInterceptors
participant axios
App->>setupAxiosInterceptors: On app startup
setupAxiosInterceptors->>axios: Add request interceptor for cache control headers
Assessment against linked issues
Poem
✨ Finishing Touches
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. 🪧 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.
Actionable comments posted: 4
🧹 Nitpick comments (6)
apps/retail/utilities/logout.ts (2)
11-11: Use a more specific type for dispatch parameter.The type
Dispatch<any>is too generic and can lead to type safety issues. Consider using a more specific type such asDispatch<AnyAction>or a custom type defining the actions your application supports.-export const logoutUser = (dispatch: Dispatch<any>, router: NextRouter) => { +export const logoutUser = (dispatch: Dispatch<AnyAction>, router: NextRouter) => {Don't forget to add the import:
import { Dispatch, AnyAction } from '@reduxjs/toolkit'🧰 Tools
🪛 ESLint
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
39-40: Consider adding error handling for router operations.The router.push operation could potentially fail, especially in case of network issues or invalid routes. Consider adding error handling.
- // Redirect to login page - router.push('/') + // Redirect to login page + router.push('/') + .catch(error => { + console.error('Failed to redirect to login page:', error) + // Consider fallback action like window.location.href = '/' + })apps/retail/utilities/api-utils.ts (1)
9-25: Extract cache control headers into a constant to avoid duplication.The cache control headers are duplicated between this function and the setupAxiosInterceptors function, which could lead to maintenance issues.
+const CACHE_CONTROL_HEADERS = { + 'Cache-Control': 'no-cache, no-store, must-revalidate', + Pragma: 'no-cache', + Expires: '0' +} + export const fetchWithCacheControl = async (url: string, options: AxiosRequestConfig = {}) => { - const defaultHeaders = { - 'Cache-Control': 'no-cache, no-store, must-revalidate', - Pragma: 'no-cache', - Expires: '0' - } + const defaultHeaders = CACHE_CONTROL_HEADERS const requestOptions: AxiosRequestConfig = { ...options, headers: { ...defaultHeaders, ...options.headers } } return axios(url, requestOptions) }Also update the interceptor function to use this constant:
export const setupAxiosInterceptors = () => { axios.interceptors.request.use(config => { // Add cache control headers to all requests config.headers = { ...config.headers, - 'Cache-Control': 'no-cache, no-store, must-revalidate', - Pragma: 'no-cache', - Expires: '0' + ...CACHE_CONTROL_HEADERS } return config }) }apps/retail/pages/search.tsx (1)
92-94: Enhance error handling for failed API requests.The error handling is minimal and doesn't provide any feedback to the user if the search request fails.
.catch(e => { setIsLoading(false) + console.error('Search request failed:', e) + // Consider adding user-friendly error notification + // For example: + // toast.error('Unable to load search results. Please try again.') })🧰 Tools
🪛 ESLint
[error] 92-92: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/retail/pages/_app.tsx (2)
1-1: Remove unused useEffect import.The useEffect hook is imported but never used in this file.
-import React, { useEffect } from 'react' +import React from 'react'🧰 Tools
🪛 ESLint
[error] 1-1: 'useEffect' is defined but never used.
(@typescript-eslint/no-unused-vars)
24-25: Consider moving interceptor setup into a component lifecycle.The
setupAxiosInterceptorsis called at the module level, which might lead to issues if the setup depends on component state or props. Consider moving it into a useEffect hook inside the MyApp component.-// Set up axios interceptors for cache control -setupAxiosInterceptors() function MyApp({ Component, pageProps }: AppProps) { + useEffect(() => { + // Set up axios interceptors for cache control + setupAxiosInterceptors() + }, []) + return ( <BecknProvider theme={{Note: If you implement this change, keep the
useEffectimport from React.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/retail/public/images/logout.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
apps/retail/components/header/index.tsx(3 hunks)apps/retail/pages/_app.tsx(2 hunks)apps/retail/pages/search.tsx(2 hunks)apps/retail/store/index.ts(2 hunks)apps/retail/utilities/api-utils.ts(1 hunks)apps/retail/utilities/logout.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/retail/pages/search.tsx (1)
apps/retail/utilities/api-utils.ts (1)
fetchWithCacheControl(9-25)
apps/retail/components/header/index.tsx (1)
apps/retail/utilities/logout.ts (1)
logoutUser(11-41)
apps/retail/pages/_app.tsx (1)
apps/retail/utilities/api-utils.ts (1)
setupAxiosInterceptors(30-42)
🪛 ESLint
apps/retail/utilities/logout.ts
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
apps/retail/pages/_app.tsx
[error] 1-1: 'useEffect' is defined but never used.
(@typescript-eslint/no-unused-vars)
apps/retail/store/index.ts
[error] 37-37: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 37-37: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (8)
apps/retail/utilities/logout.ts (1)
19-26: Well-structured approach for preserving specific localStorage items.The implementation elegantly preserves certain localStorage values during logout, which maintains user preferences while clearing sensitive data.
apps/retail/pages/search.tsx (2)
81-85: Good implementation of cache-controlled API requests.The switch to using
fetchWithCacheControlensures consistent cache control headers, aligning with the PR objective to prevent cached data issues after logout and re-login.
107-115:Details
✅ Verification successful
Verify if cached search results are properly cleared during logout.
This useEffect hook loads cached search results from localStorage on component mount. Ensure that the
logoutUserfunction clears 'searchItems' from localStorage to prevent stale data being shown after logout and re-login.
🏁 Script executed:
#!/bin/bash # Check if the 'searchItems' key is cleared during logout # First, check if the logoutUser function clears localStorage entirely echo "Checking if localStorage is cleared during logout:" cat apps/retail/utilities/logout.ts | grep -A 5 "Clear all localStorage" # Next, verify if there's any specific handling for 'searchItems' echo -e "\nChecking for specific handling of 'searchItems':" grep -r "searchItems" --include="*.ts" --include="*.tsx" apps/retail/Length of output: 843
🏁 Script executed:
#!/bin/bash echo "Inspecting preservedData in logout utility:" grep -R "preservedData" -n apps/retail/utilities/logout.ts || echo "No preservedData declaration found" echo -e "\nShowing logout.ts for context (first 200 lines):" sed -n '1,200p' apps/retail/utilities/logout.tsLength of output: 1621
Confirm
searchItemsIs Cleared on Logout
ThelogoutUserfunction inapps/retail/utilities/logout.tscallslocalStorage.clear()and only restores keys listed inkeysToPreserve(which does not include"searchItems"). Therefore, the cached search results are removed on logout as intended—no further changes needed.apps/retail/pages/_app.tsx (1)
24-25: Good implementation of global cache control.Setting up axios interceptors at the application level ensures consistent cache control for all API requests, which helps prevent cached data issues after logout and re-login.
apps/retail/components/header/index.tsx (1)
13-14: Add appropriate imports for unified logout handlingGood addition of Redux dispatch and the centralized logout utility. This sets the foundation for consistent logout behavior across the application.
apps/retail/store/index.ts (3)
1-1: Good import of combineReducers for improved store organizationAdding combineReducers helps organize the Redux store structure and enables the state reset pattern on logout.
16-34: Well-structured combined reducer implementationGood job restructuring the store to use combineReducers. This provides a cleaner structure and facilitates the logout state reset functionality.
47-49: Good integration with configureStoreThe rootReducer is properly integrated with configureStore, ensuring that the state reset on logout works throughout the application.
| export const setupAxiosInterceptors = () => { | ||
| axios.interceptors.request.use(config => { | ||
| // Add cache control headers to all requests | ||
| config.headers = { | ||
| ...config.headers, | ||
| 'Cache-Control': 'no-cache, no-store, must-revalidate', | ||
| Pragma: 'no-cache', | ||
| Expires: '0' | ||
| } | ||
|
|
||
| return config | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent adding multiple identical interceptors.
If setupAxiosInterceptors is called multiple times, it will add the same interceptor multiple times. Consider adding a check to prevent this.
+let isInterceptorSetup = false;
export const setupAxiosInterceptors = () => {
+ if (isInterceptorSetup) return;
+
axios.interceptors.request.use(config => {
// Add cache control headers to all requests
config.headers = {
...config.headers,
'Cache-Control': 'no-cache, no-store, must-revalidate',
Pragma: 'no-cache',
Expires: '0'
}
return config
})
+
+ isInterceptorSetup = true;
}📝 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 const setupAxiosInterceptors = () => { | |
| axios.interceptors.request.use(config => { | |
| // Add cache control headers to all requests | |
| config.headers = { | |
| ...config.headers, | |
| 'Cache-Control': 'no-cache, no-store, must-revalidate', | |
| Pragma: 'no-cache', | |
| Expires: '0' | |
| } | |
| return config | |
| }) | |
| } | |
| let isInterceptorSetup = false; | |
| export const setupAxiosInterceptors = () => { | |
| if (isInterceptorSetup) return; | |
| axios.interceptors.request.use(config => { | |
| // Add cache control headers to all requests | |
| config.headers = { | |
| ...config.headers, | |
| 'Cache-Control': 'no-cache, no-store, must-revalidate', | |
| Pragma: 'no-cache', | |
| Expires: '0' | |
| } | |
| return config | |
| }) | |
| isInterceptorSetup = true; | |
| } |
| onClick={() => { | ||
| const user = localStorage.getItem('userPhone') as string | ||
| localStorage.clear() | ||
| localStorage.setItem('userPhone', user) | ||
| const dispatch = useDispatch() | ||
| logoutUser(dispatch, router) | ||
| router.push(`/homePage`) |
There was a problem hiding this comment.
Incorrect usage of useDispatch hook in event handler
The useDispatch hook is being called inside an event handler, which violates React's rules of hooks. Hooks should only be called at the top level of your component.
- onClick={() => {
- const dispatch = useDispatch()
- logoutUser(dispatch, router)
- router.push(`/homePage`)
- }}
+ const dispatch = useDispatch()
+ onClick={() => {
+ logoutUser(dispatch, router)
+ // Remove this line as logoutUser already redirects to login page
+ // router.push(`/homePage`)
+ }}Committable suggestion skipped: line range outside the PR's diff.
| <Box | ||
| onClick={() => { | ||
| const dispatch = useDispatch() | ||
| logoutUser(dispatch, router) | ||
| setMenuModalOpen(false) | ||
| }} | ||
| className={styles.top_header_modal} | ||
| > | ||
| <Image | ||
| src="/images/logout.svg" | ||
| alt="Logout icon" | ||
| /> | ||
| Logout | ||
| </Box> |
There was a problem hiding this comment.
Incorrect usage of useDispatch hook in Logout menu item
Similar to the home icon issue, useDispatch is being called inside an event handler. Additionally, the menu item's behavior conflicts with the logout utility's redirect.
+ const dispatch = useDispatch()
<Box
onClick={() => {
- const dispatch = useDispatch()
logoutUser(dispatch, router)
setMenuModalOpen(false)
}}
className={styles.top_header_modal}
>Consider removing setMenuModalOpen(false) as it's unnecessary - the logout function will redirect the user away from this page anyway.
Committable suggestion skipped: line range outside the PR's diff.
Fixes #4027
What
Implemented a comprehensive solution to fix the issue where previously searched or viewed results were still visible after logging out and logging back in.
Why
This resolves the issue described in [reference to ticket/issue] where data was being retrieved from the cache instead of re-fetching fresh data for the new session.
How
Testing
Summary by CodeRabbit
New Features
Improvements
Refactor