Feat/added persistant storage (session-storage) Inputs #49
Conversation
📝 WalkthroughWalkthroughAdds TTL-backed sessionStorage/localStorage draft persistence with PII sanitization for single, batch, and link flows; restores drafts on mount, debounced saves after initial load, integrates URL-driven token/client overrides and token verification, and clears drafts on successful submission. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Browser UI
participant Storage as Storage Util
participant Token as Token Verifier
participant Backend as Submit Service
UI->>Storage: load sanitized draft (key)
Storage-->>UI: return draft or null
UI->>UI: restore fields (dates, user/client, items, token state)
alt tokenAddress or custom token present
UI->>Token: verify token address / resolve decimals
Token-->>UI: verification result
end
UI->>Storage: debounce save sanitized draft
Storage-->>Storage: persist {data, expiresAt, createdAt}
UI->>Backend: submit invoice(s) (payload includes user/client, items, token)
Backend-->>UI: success
UI->>Storage: clear relevant storage key
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/page/CreateInvoice.jsx (2)
166-204: Normalize token state to avoid “selectedToken + customToken” stale mixes.In the URL override path you may set
selectedTokenwithout clearingcustomTokenAddress/verifiedToken(and vice versa). That can rehydrate later into a confusing state.Proposed diff
if ( token.address?.toLowerCase() === urlTokenAddress.toLowerCase() ) { setSelectedToken(token); setUseCustomToken(false); + setCustomTokenAddress(""); + setVerifiedToken(null); + setTokenVerificationState("idle"); return; } ... setUseCustomToken(true); setCustomTokenAddress(urlTokenAddress); + setSelectedToken(null); verifyToken(urlTokenAddress);Also applies to: 238-295
483-490: GuardchainIdexplicitly before env lookup for clearer failures.Right now an undefined
chainIdbecomesVITE_CONTRACT_ADDRESS_undefinedand surfaces as “Unsupported network” without explaining what’s missing.Proposed diff
- const chainId = account?.chainId; + const chainId = account?.chainId; + if (!chainId) { + throw new Error("Wallet network not detected"); + } const contractAddress = import.meta.env[ `VITE_CONTRACT_ADDRESS_${chainId}` ];frontend/src/page/CreateInvoicesBatch.jsx (1)
563-570: Add explicitchainIdguard before env lookup.Same as CreateInvoice—fail with a clearer message when
chainIdisn’t available.
🤖 Fix all issues with AI agents
In @frontend/src/page/CreateInvoice.jsx:
- Around line 49-54: The code is persisting PII (names, emails, addresses) to
localStorage via saveToStorage/getFromStorage/StorageKeys in CreateInvoice.jsx
(also appearing in the other ranges); change the storage strategy to avoid
long-lived localStorage by either switching these calls to sessionStorage,
adding a TTL wrapper that expires entries on read, and/or stop persisting email
fields altogether. Update the localStorage helper functions (getFromStorage,
saveToStorage, clearStorage) or add new session/ttl variants so reads check
expiry and deletes expired entries, change callers in CreateInvoice.jsx (the
places using StorageKeys around lines 49, 111-165, 238-295) to use the new safe
API, and ensure any persisted invoice data is minimized (no emails) or encrypted
before persisting.
In @frontend/src/page/CreateInvoicesBatch.jsx:
- Around line 57-62: The component currently persists large batches of client
PII using getFromStorage/saveToStorage (StorageKeys) which can hit localStorage
quotas and expose sensitive data; change persistence to use session-scoped
storage with TTL (implement new saveToSession/getFromSession helpers or extend
existing utils to accept a storage type and expiresAt), avoid storing email
fields and other direct PII (strip email/name/country from the saved batch
payload before calling saveToStorage/saveToSession), and enforce a cap on
persisted batch size (e.g., truncate to N items) when preparing data in
CreateInvoicesBatch.jsx and where the component calls
saveToStorage/getFromStorage/clearStorage so the persisted object is smaller and
ephemeral.
- Around line 142-177: When restoring token state in the useEffect, make the
branches mutually exclusive: if savedData.useCustomToken is true prefer the
custom path (call setUseCustomToken(true),
setCustomTokenAddress(savedData.customTokenAddress),
setVerifiedToken(savedData.verifiedToken ?? null) and
setTokenVerificationState(savedData.verifiedToken ? "success" : "idle")), and
explicitly clear the selected-token fields (setSelectedToken(null)); otherwise,
prefer the saved selectedToken (setSelectedToken(savedData.selectedToken) and
setUseCustomToken(false)) and explicitly clear custom-token fields
(setCustomTokenAddress("") , setVerifiedToken(null),
setTokenVerificationState("idle")) so mixed/ambiguous saved payloads cannot
produce hybrid state.
🧹 Nitpick comments (3)
frontend/src/utils/localStorage.js (2)
5-8: Freeze exported keys to prevent accidental mutation.Right now
StorageKeyscan be mutated by consumers.Proposed diff
const STORAGE_KEYS = { CREATE_INVOICE: 'chainvoice_create_invoice', CREATE_INVOICES_BATCH: 'chainvoice_create_invoices_batch', }; +Object.freeze(STORAGE_KEYS); /** * Storage keys */ export const StorageKeys = STORAGE_KEYS;Also applies to: 56-57
27-39: Make quota detection more robust + consider definingclearStoragebeforesaveToStorage.
QuotaExceededErrorisn’t consistent across browsers; also callingsaveToStorageduring a circular import can theoretically hitclearStoragebefore initialization.Proposed diff
+/** + * Clear specific key from localStorage + */ +export const clearStorage = (key) => { + try { + if (typeof window === 'undefined') return; + window.localStorage.removeItem(key); + } catch (error) { + console.error(`Error clearing localStorage (${key}):`, error); + } +}; + /** * Safe localStorage setter */ export const saveToStorage = (key, value) => { try { if (typeof window === 'undefined') return; window.localStorage.setItem(key, JSON.stringify(value)); } catch (error) { console.error(`Error saving to localStorage (${key}):`, error); // Handle quota exceeded error - if (error.name === 'QuotaExceededError') { + const isQuotaExceeded = + error?.name === 'QuotaExceededError' || + error?.name === 'NS_ERROR_DOM_QUOTA_REACHED' || + error?.code === 22 || + error?.code === 1014; + if (isQuotaExceeded) { console.warn('localStorage quota exceeded. Clearing old data...'); clearStorage(key); } } }; - -/** - * Clear specific key from localStorage - */ -export const clearStorage = (key) => { - try { - if (typeof window === 'undefined') return; - window.localStorage.removeItem(key); - } catch (error) { - console.error(`Error clearing localStorage (${key}):`, error); - } -};Also applies to: 44-51
frontend/src/page/CreateInvoice.jsx (1)
516-538: Remove unusedFormData(you’re fully controlled now).
const formData = new FormData(e.target);is dead code.Proposed diff
const handleSubmit = async (e) => { e.preventDefault(); - const formData = new FormData(e.target); - const data = { userAddress: account?.address?.toString(), userFname,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/localStorage.js
🔇 Additional comments (3)
frontend/src/page/CreateInvoice.jsx (2)
503-506: Clearing persisted draft only on confirmed success looks correct.
661-747: Controlled inputs conversion looks solid.Good: avoids uncontrolled→controlled warnings via
|| ""on item fields and keeps state as source of truth.Also applies to: 769-855, 1109-1169, 1229-1271
frontend/src/page/CreateInvoicesBatch.jsx (1)
585-587: Clearing persisted batch draft after confirmation is correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/src/page/CreateInvoicesBatch.jsx (2)
142-179: Add error handling for data restoration.The restoration logic lacks error handling for corrupted localStorage data. Invalid date strings, malformed JSON, or incompatible schema changes could cause runtime errors.
🛡️ Proposed fix with error handling
// Load data from localStorage on mount useEffect(() => { + try { const savedData = getFromStorage(StorageKeys.CREATE_INVOICES_BATCH); if (savedData) { // Restore dates if (savedData.dueDate) { - setDueDate(new Date(savedData.dueDate)); + const dueDate = new Date(savedData.dueDate); + if (!isNaN(dueDate.getTime())) { + setDueDate(dueDate); + } } if (savedData.issueDate) { - setIssueDate(new Date(savedData.issueDate)); + const issueDate = new Date(savedData.issueDate); + if (!isNaN(issueDate.getTime())) { + setIssueDate(issueDate); + } } // Restore user info if (savedData.userInfo) { setUserInfo(savedData.userInfo); } // Restore invoice rows if (savedData.invoiceRows && savedData.invoiceRows.length > 0) { setInvoiceRows(savedData.invoiceRows); } // Restore token selection if (savedData.selectedToken) { setSelectedToken(savedData.selectedToken); setUseCustomToken(false); } if (savedData.useCustomToken && savedData.customTokenAddress) { setUseCustomToken(true); setCustomTokenAddress(savedData.customTokenAddress); if (savedData.verifiedToken) { setVerifiedToken(savedData.verifiedToken); setTokenVerificationState("success"); } } } + } catch (error) { + console.error("Failed to restore form data:", error); + // Clear corrupted data + clearStorage(StorageKeys.CREATE_INVOICES_BATCH); + } setIsInitialLoad(false); }, []);
200-233: Optimize dependency array to avoid unnecessary effect evaluations.Using
JSON.stringify(userInfo)andJSON.stringify(invoiceRows)in the dependency array creates new string values on every render, causing React to re-evaluate the effect even when the underlying data hasn't changed. While the debounce prevents excessive saves, the effect closure and timeout are recreated unnecessarily.Consider using a custom deep-comparison hook or separate the persistence logic:
♻️ Alternative approach with useRef
+ const prevDataRef = useRef(); + // Save form data to localStorage (debounced) useEffect(() => { if (isInitialLoad) return; + const currentData = { + dueDate: dueDate?.toISOString(), + issueDate: issueDate?.toISOString(), + userInfo, + invoiceRows, + selectedToken, + customTokenAddress, + useCustomToken, + verifiedToken, + }; + + // Skip if data hasn't actually changed + if (JSON.stringify(currentData) === JSON.stringify(prevDataRef.current)) { + return; + } + prevDataRef.current = currentData; + const saveData = () => { - const dataToSave = { - dueDate: dueDate?.toISOString(), - issueDate: issueDate?.toISOString(), - userInfo, - invoiceRows, - selectedToken, - customTokenAddress, - useCustomToken, - verifiedToken, - }; - - saveToStorage(StorageKeys.CREATE_INVOICES_BATCH, dataToSave); + saveToStorage(StorageKeys.CREATE_INVOICES_BATCH, currentData); }; // Debounce save operations const timeoutId = setTimeout(saveData, 500); return () => clearTimeout(timeoutId); }, [ dueDate, issueDate, - // Use JSON.stringify to detect deep changes in nested objects - JSON.stringify(userInfo), - JSON.stringify(invoiceRows), + userInfo, + invoiceRows, selectedToken, customTokenAddress, useCustomToken, verifiedToken, isInitialLoad, ]);Note: This still has the issue of object reference changes, but moves the deep comparison inside the effect where it only runs when shallow dependencies change.
frontend/src/page/CreateInvoice.jsx (3)
112-164: Add error handling and validate restored dates.Same as batch file: restoration lacks error handling for corrupted data. Additionally, the logic correctly prioritizes URL parameters over saved client data.
🛡️ Proposed fix with error handling
// Load data from localStorage on mount useEffect(() => { + try { const savedData = getFromStorage(StorageKeys.CREATE_INVOICE); if (savedData) { // Restore dates if (savedData.dueDate) { - setDueDate(new Date(savedData.dueDate)); + const dueDate = new Date(savedData.dueDate); + if (!isNaN(dueDate.getTime())) { + setDueDate(dueDate); + } } if (savedData.issueDate) { - setIssueDate(new Date(savedData.issueDate)); + const issueDate = new Date(savedData.issueDate); + if (!isNaN(issueDate.getTime())) { + setIssueDate(issueDate); + } } // Restore user info if (savedData.userFname) setUserFname(savedData.userFname); if (savedData.userLname) setUserLname(savedData.userLname); if (savedData.userEmail) setUserEmail(savedData.userEmail); if (savedData.userCountry) setUserCountry(savedData.userCountry); if (savedData.userCity) setUserCity(savedData.userCity); if (savedData.userPostalcode) setUserPostalcode(savedData.userPostalcode); // Restore client info (only if no URL params) const urlClientAddress = searchParams.get("clientAddress"); if (!urlClientAddress) { if (savedData.clientAddress) setClientAddress(savedData.clientAddress); if (savedData.clientFname) setClientFname(savedData.clientFname); if (savedData.clientLname) setClientLname(savedData.clientLname); if (savedData.clientEmail) setClientEmail(savedData.clientEmail); if (savedData.clientCountry) setClientCountry(savedData.clientCountry); if (savedData.clientCity) setClientCity(savedData.clientCity); if (savedData.clientPostalcode) setClientPostalcode(savedData.clientPostalcode); } // Restore token selection if (savedData.selectedToken) { setSelectedToken(savedData.selectedToken); setUseCustomToken(false); } if (savedData.useCustomToken && savedData.customTokenAddress) { setUseCustomToken(true); setCustomTokenAddress(savedData.customTokenAddress); if (savedData.verifiedToken) { setVerifiedToken(savedData.verifiedToken); setTokenVerificationState("success"); } } // Restore item data if (savedData.itemData && savedData.itemData.length > 0) { setItemData(savedData.itemData); } } + } catch (error) { + console.error("Failed to restore form data:", error); + clearStorage(StorageKeys.CREATE_INVOICE); + } setIsInitialLoad(false); }, []);
179-201: Redundant localStorage read for token matching.Lines 186-196 re-read from localStorage to check if
savedData.selectedTokenmatches the URL token, butselectedTokenis already available in state from the restoration effect (lines 145-148). This is redundant and could use the state directly.♻️ Proposed refactor using existing state
if (urlTokenAddress) { if (isCustomFromURL) { setUseCustomToken(true); setCustomTokenAddress(urlTokenAddress); verifyToken(urlTokenAddress); } else { - // Try to find token in saved selection first - const savedData = getFromStorage(StorageKeys.CREATE_INVOICE); - if (savedData?.selectedToken) { - const token = savedData.selectedToken; - if ( - token.address?.toLowerCase() === urlTokenAddress.toLowerCase() - ) { - setSelectedToken(token); - setUseCustomToken(false); - return; - } + // Check if current selectedToken matches URL token + if ( + selectedToken?.address?.toLowerCase() === urlTokenAddress.toLowerCase() + ) { + // Already set from localStorage, no need to update + setUseCustomToken(false); + return; } // If not found, try custom token verification setUseCustomToken(true); setCustomTokenAddress(urlTokenAddress); verifyToken(urlTokenAddress); } }
238-295: Optimize dependency array to avoid unnecessary effect evaluations.Same issue as batch file:
JSON.stringify(itemData)in the dependency array creates new strings on every render, causing unnecessary effect re-evaluation. The large dependency array with many individual fields also triggers frequently.Consider the same useRef-based approach as suggested for the batch file, or explore using a library like
use-deep-compare-effectfor cleaner deep comparison.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsx
🔇 Additional comments (8)
frontend/src/page/CreateInvoicesBatch.jsx (4)
564-567: LGTM! Network-specific contract resolution.Correctly derives chainId from the connected account for network-specific contract address resolution.
586-587: LGTM! Proper cleanup on success.Correctly clears persisted form data after successful batch submission to reset the draft state.
84-84: LGTM! Proper guard pattern.The
isInitialLoadflag correctly prevents saving to storage during the initial data restoration phase.
57-62: localStorage utilities already implement comprehensive error handling.The imported utilities properly handle quota exceeded errors (with QuotaExceededError detection and old data clearing), JSON parse failures (graceful fallback to defaultValue), and SecurityErrors from private browsing (caught by general error handlers). No changes required.
frontend/src/page/CreateInvoice.jsx (4)
71-83: LGTM! Proper controlled component state.User and client form fields are now explicit state variables with controlled inputs, enabling proper persistence and state management.
484-487: LGTM! Network-specific contract resolution.Correctly derives chainId from the connected account, matching the pattern in the batch file.
505-506: LGTM! Proper cleanup on success.Correctly clears persisted form data after successful invoice creation.
667-668: LGTM! Properly implemented controlled components.All form inputs are correctly wired as controlled components with explicit
valueandonChangeprops, enabling reliable persistence and state management.Also applies to: 680-681, 696-697, 731-732, 744-745, 775-776, 788-789, 804-805, 839-840, 852-853, 1111-1270
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/page/CreateInvoice.jsx:
- Around line 215-268: The useEffect debounced auto-save is missing userEmail
and clientEmail in its dependency array causing stale saves; update the
dependency array for the effect that defines saveData (the useEffect wrapping
saveToStorage/ sanitizeDataForStorage and StorageKeys.CREATE_INVOICE) to include
userEmail and clientEmail so changes to those fields trigger the debounced save
along with the existing dependencies.
🧹 Nitpick comments (3)
frontend/src/utils/localStorage.js (1)
55-78: Consider retrying the save operation after cleanup.When
QuotaExceededErroroccurs, the function clears the specific key and expired entries but doesn't retry saving. This means the user's data won't be persisted even after freeing space. Consider adding a retry mechanism or at least logging a warning that the save failed.💾 Proposed retry logic
export const saveToStorage = (key, value, ttl = DEFAULT_TTL) => { try { const storage = getStorage(); if (!storage) return; const expiresAt = Date.now() + ttl; const dataToStore = { data: value, expiresAt, createdAt: Date.now(), }; storage.setItem(key, JSON.stringify(dataToStore)); } catch (error) { console.error(`Error saving to sessionStorage (${key}):`, error); // Handle quota exceeded error if (error.name === 'QuotaExceededError') { console.warn('sessionStorage quota exceeded. Clearing old data...'); clearStorage(key); // Try to clean up expired entries cleanupExpiredEntries(); + + // Retry once after cleanup + try { + const expiresAt = Date.now() + ttl; + const dataToStore = { + data: value, + expiresAt, + createdAt: Date.now(), + }; + storage.setItem(key, JSON.stringify(dataToStore)); + } catch (retryError) { + console.error('Failed to save after cleanup:', retryError); + } } } };frontend/src/page/CreateInvoicesBatch.jsx (1)
138-167: Fix formatting issue.Line 166 has trailing whitespace:
setIsInitialLoad(false );🧹 Fix formatting
- setIsInitialLoad(false ); + setIsInitialLoad(false);frontend/src/page/CreateInvoice.jsx (1)
476-478: Update comment to reference sessionStorage.The comment on line 477 mentions "localStorage" but the code uses sessionStorage throughout the application.
📝 Fix comment
- // Clear localStorage on successful submission + // Clear sessionStorage on successful submission clearStorage(StorageKeys.CREATE_INVOICE);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/page/CreateInvoice.jsxfrontend/src/page/CreateInvoicesBatch.jsxfrontend/src/utils/localStorage.js
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/page/CreateInvoice.jsx (1)
frontend/src/utils/localStorage.js (11)
getFromStorage(26-49)getFromStorage(26-49)StorageKeys(194-194)StorageKeys(194-194)sanitized(136-136)sanitizeDataForStorage(133-184)sanitizeDataForStorage(133-184)saveToStorage(55-78)saveToStorage(55-78)clearStorage(83-91)clearStorage(83-91)
🔇 Additional comments (16)
frontend/src/utils/localStorage.js (7)
1-20: LGTM!The use of sessionStorage with browser environment checks and TTL-based expiry is appropriate for temporary draft persistence. The 24-hour TTL provides a good balance between persistence and automatic cleanup.
26-49: LGTM!The TTL expiry check with automatic deletion of expired entries on read is well-implemented. The fallback to return
parseddirectly (line 44) provides backward compatibility for data stored without the TTL wrapper.
83-91: LGTM!The clearStorage function is straightforward with appropriate error handling.
96-124: LGTM!The cleanup function correctly collects keys to remove first (lines 101-117) before removing them (line 120), avoiding iteration issues when modifying storage during traversal.
133-184: Verify that persisting wallet addresses aligns with privacy requirements.The sanitization removes most PII (emails, names, addresses, countries, cities, postal codes) but retains
clientAddress(wallet addresses) as noted in line 178. While wallet addresses are necessary for invoice functionality, they could be considered user identifiers under privacy regulations like GDPR.Please confirm this is an intentional design decision and that wallet addresses are acceptable to persist in sessionStorage.
189-194: LGTM!The exported getters provide clean access to internal constants.
196-199: LGTM!Automatic cleanup on module load helps maintain sessionStorage hygiene. The browser environment check ensures it doesn't run in SSR contexts.
frontend/src/page/CreateInvoicesBatch.jsx (3)
56-63: LGTM!The storage utility imports are clean and include all necessary functions for the persistence feature.
187-225: LGTM!The debounced persistence with
isInitialLoadguard prevents overwriting during initial data restoration. The warning when batch size exceeds the persistence limit (lines 204-208) provides helpful feedback.
567-567: LGTM!Clearing the persisted draft after successful submission prevents stale data from being restored on future visits.
frontend/src/page/CreateInvoice.jsx (6)
49-55: LGTM!The storage utility imports include all necessary functions for single invoice persistence.
72-84: LGTM!The controlled component states for user and client fields are properly initialized, and the
isInitialLoadguard flag is correctly set up.Also applies to: 110-110
112-145: LGTM!The data restoration logic correctly:
- Restores dates, itemData, and token state
- Skips restoring clientAddress when provided via URL (lines 122-125)
- Doesn't restore PII fields (user/client names, emails) as they're sanitized before storage
147-180: LGTM!The URL parameter handling with
isInitialLoadguard ensures saved data is restored before URL params are applied. The logic to reuse saved token data when it matches the URL token (lines 164-174) is efficient and preserves additional token metadata.
494-510: LGTM!The form data collection correctly gathers all state variables for submission, matching the payload structure expected by
createInvoiceRequest.
639-640: LGTM!All form inputs have been properly converted to controlled components with correct value bindings and onChange handlers. This ensures the persistence feature works correctly for all fields.
Also applies to: 652-653, 668-669, 703-704, 716-717, 747-748, 760-761, 776-777, 811-812, 824-825, 1081-1241
ajey35
force-pushed
the
feat/added-persistant-storage-inputs
branch
from
9bba82f to
2bf85d5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @frontend/src/utils/sessionStorage.js:
- Around line 131-186: The docstring for sanitizeDataForStorage claims addresses
are removed but the function still preserves clientAddress in invoiceRows;
either update the docstring/AI-summary to state clientAddress is intentionally
retained, or actually strip/pseudonymize it: in sanitizeDataForStorage (and
where invoiceRows is handled) remove sanitizedRow.clientAddress (or replace it
with a minimal non-PII surrogate like clientAddressHash) and update any callers
that rely on clientAddress; keep MAX_PERSISTED_INVOICES and invoiceRows logic
intact and ensure userInfo and top-level address fields are treated consistently
with the chosen option.
- Around line 27-50: getFromStorage currently leaves corrupted JSON in storage
and treats stored undefined ambiguously; update getFromStorage to catch
JSON.parse errors and in that catch call storage.removeItem(key) before logging
and returning defaultValue, and when parsed === undefined treat that as missing
by returning defaultValue (or parsed.data if defined) so saving undefined
doesn’t falsely round-trip; use getStorage(), storage.getItem(key),
storage.removeItem(key) and ensure the function returns parsed.data when
present, otherwise defaultValue for undefined or corrupted entries.
🧹 Nitpick comments (1)
frontend/src/utils/sessionStorage.js (1)
6-20: MakeStorageKeysimmutable + makegetStorage()resilient to blocked storage.
Today, any consumer can mutateStorageKeys, and in some browser/privacy modes accessingwindow.sessionStoragecan throw.Proposed diff
const STORAGE_KEYS = { CREATE_INVOICE: 'chainvoice_create_invoice', CREATE_INVOICES_BATCH: 'chainvoice_create_invoices_batch', }; +Object.freeze(STORAGE_KEYS); // Default TTL: 24 hours (in milliseconds) const DEFAULT_TTL = 24 * 60 * 60 * 1000; /** * Get storage instance (sessionStorage for session-only storage) */ const getStorage = () => { if (typeof window === 'undefined') return null; - return window.sessionStorage; + try { + return window.sessionStorage; + } catch { + return null; + } };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/sessionStorage.js
🔇 Additional comments (2)
frontend/src/utils/sessionStorage.js (2)
98-126: Cleanup logic is solid (collect-then-remove avoids iteration pitfalls).
No concerns with the current approach.
191-201: Module-load cleanup guard looks good for SSR.
WithObject.freeze(STORAGE_KEYS)above, the exportedStorageKeyssurface is safer too.
ajey35
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/utils/sessionStorage.js (3)
105-155: Refactor to eliminate code duplication and reconsider retry strategy.The data preparation logic (lines 111-121) is duplicated in the retry block (lines 138-147). Additionally, line 130 calls
clearStorage(key)before retrying, which will delete the existing data for that key. If the retry fails, the user's previous data is lost.Consider:
- Extract the data preparation logic into a helper function
- Verify whether clearing the current key before retry is intentional—it may be better to only clean expired entries and retry without deleting the target key
♻️ Proposed refactor to eliminate duplication
+/** + * Prepare data for storage with TTL + */ +const prepareDataForStorage = (value, ttl) => { + const validTtl = typeof ttl === 'number' && ttl > 0 && isFinite(ttl) ? ttl : DEFAULT_TTL; + const expiresAt = Date.now() + validTtl; + return { + data: value, + expiresAt, + createdAt: Date.now(), + }; +}; + export const saveToStorage = (key, value, ttl = DEFAULT_TTL) => { try { const storage = getStorage(); if (!storage) return; - // Validate TTL: ensure it's a positive number - const validTtl = typeof ttl === 'number' && ttl > 0 && isFinite(ttl) ? ttl : DEFAULT_TTL; - - const expiresAt = Date.now() + validTtl; - const dataToStore = { - data: value, - expiresAt, - createdAt: Date.now(), - }; - + const dataToStore = prepareDataForStorage(value, ttl); const serialized = JSON.stringify(dataToStore); storage.setItem(key, serialized); } catch (error) { console.error(`Error saving to sessionStorage (${key}):`, error); // Handle quota exceeded error with retry if (isQuotaExceededError(error)) { console.warn('sessionStorage quota exceeded. Clearing old data and retrying...'); - // Clean up: remove current key and expired entries - clearStorage(key); + // Clean up expired entries only (consider not clearing the current key) cleanupExpiredEntries(); // Retry the write once after cleanup try { const storage = getStorage(); if (!storage) return; - const validTtl = typeof ttl === 'number' && ttl > 0 && isFinite(ttl) ? ttl : DEFAULT_TTL; - const expiresAt = Date.now() + validTtl; - const dataToStore = { - data: value, - expiresAt, - createdAt: Date.now(), - }; - + const dataToStore = prepareDataForStorage(value, ttl); const serialized = JSON.stringify(dataToStore); storage.setItem(key, serialized); console.info('Successfully saved to sessionStorage after quota cleanup'); } catch (retryError) { // If retry also fails, log but don't throw (graceful degradation) console.error(`Failed to save to sessionStorage after cleanup (${key}):`, retryError); } } } };
27-71: Consider simplifying undefined handling logic.The function has defensive checks for
undefinedvalues at two levels:
- Lines 47-50: Remove entry if
parsed === undefined- Lines 61-66: Check if
parsed.data !== undefinedbefore returningWhile this handles edge cases thoroughly, the logic could be streamlined. The current implementation is functionally correct but slightly verbose.
♻️ Optional simplification
export const getFromStorage = (key, defaultValue = null) => { try { const storage = getStorage(); if (!storage) return defaultValue; const item = storage.getItem(key); if (!item) return defaultValue; // Parse JSON with error handling - remove corrupted entries let parsed; try { parsed = JSON.parse(item); } catch (parseError) { // Corrupted JSON - remove it from storage storage.removeItem(key); console.error(`Corrupted JSON in sessionStorage (${key}), removed:`, parseError); return defaultValue; } - // Treat undefined as missing data - if (parsed === undefined) { - storage.removeItem(key); - return defaultValue; - } - // Check if entry has TTL and if it's expired if (parsed.expiresAt && parsed.expiresAt < Date.now()) { // Entry expired, delete it storage.removeItem(key); return defaultValue; } - // Return parsed.data if present, otherwise defaultValue - // Treat undefined data as missing to prevent false round-trip of undefined - if (parsed.data !== undefined) { - return parsed.data; - } - - // If parsed.data is undefined, treat as missing - return defaultValue; + // Return parsed.data if it exists, otherwise return defaultValue + // Note: This handles both missing data property and undefined data values + return 'data' in parsed ? parsed.data : defaultValue; } catch (error) { console.error(`Error reading from sessionStorage (${key}):`, error); return defaultValue; } };
105-121: Add serialization validation for stored values.The function doesn't validate whether
valuecan be safely serialized to JSON. Non-serializable values (like functions, symbols, or circular references) will causeJSON.stringifyto throw or produce unexpected results.♻️ Optional enhancement to catch serialization errors early
export const saveToStorage = (key, value, ttl = DEFAULT_TTL) => { try { const storage = getStorage(); if (!storage) return; + // Validate that value is serializable + if (value !== null && typeof value === 'object') { + // Quick check for circular references and non-serializable types + try { + JSON.stringify(value); + } catch (e) { + console.error(`Value for key "${key}" is not serializable:`, e); + return; + } + } + // Validate TTL: ensure it's a positive number const validTtl = typeof ttl === 'number' && ttl > 0 && isFinite(ttl) ? ttl : DEFAULT_TTL; const expiresAt = Date.now() + validTtl; const dataToStore = { data: value, expiresAt, createdAt: Date.now(), }; const serialized = JSON.stringify(dataToStore); storage.setItem(key, serialized); } catch (error) { console.error(`Error saving to sessionStorage (${key}):`, error); // ... rest of error handling } };Note: This does stringify twice, so it's a trade-off between validation and performance.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/sessionStorage.js
🔇 Additional comments (1)
frontend/src/utils/sessionStorage.js (1)
210-262: Verify PII retention policy for clientAddress field.The function intentionally retains
clientAddressin both single and batch invoice data (noted in comments at lines 208, 216, and 256). Depending on your jurisdiction and privacy requirements, client addresses may be considered PII.Additionally, the function uses shallow copying (line 213) and only handles specific nested structures (
userInfo,invoiceRows). If the data contains other nested objects with PII fields outside these known structures, they won't be sanitized.Verify:
- Whether retaining
clientAddresscomplies with your privacy policy and data protection requirements (GDPR, CCPA, etc.)- Whether there are other nested data structures in the form data that might contain PII fields not covered by this sanitization logic
Consider documenting the rationale for retaining
clientAddressif it's a deliberate decision for functional requirements.
|
@ajey35 First of try to resolve conflict. |
ajey35
force-pushed
the
feat/added-persistant-storage-inputs
branch
from
40501d2 to
4f7d3ea
Compare
|
hey @kumawatkaran523 Resolved Conflicts and Added Session storage for the GenerateLink Component As Welll |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/page/GenerateLink.jsx`:
- Around line 74-99: The restored token fields in the useEffect that reads
getFromStorage(StorageKeys.GENERATE_LINK) must be validated against the current
chain/token list before calling setSelectedToken, setUseCustomToken,
setCustomTokenAddress, setVerifiedToken, or setTokenVerificationState; update
the logic in the useEffect (and/or add a follow-up effect keyed on tokens and
chainId) to check that savedData.selectedToken exists in the current tokens list
for the active chain (and that a saved customTokenAddress matches the current
chainId format or lookup), and if it does not match clear the token-related
state instead of restoring it, then call setIsInitialLoad(false) as before.
2 participants
Fix: Add Session Storage Persistence for Invoice Forms
Issue
Form data was lost on page refresh in
CreateInvoiceandCreateInvoicesBatchpages, causing users to lose their work.Solution
Session Storageutility with safe get/set/clear functionsChanges
src/utils/session-storage.js- New utility modulesrc/page/CreateInvoice.jsx- Added persistence for all form fieldssrc/page/CreateInvoicesBatch.jsx- Added persistence for batch invoice dataResult
Users can now refresh the page without losing their form data. Data is automatically cleared after successful invoice creation.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.