[FEATURE]: View full quiz history with Show More / Show Less#645
[FEATURE]: View full quiz history with Show More / Show Less#645Ashvin-KS wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes implement functionality to display all stored quizzes with a "Show More/Show Less" toggle, replacing the previous 5-quiz limit. A new helper function safely retrieves quiz history from local storage, removing the automatic truncation logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Pull request overview
Implements issue #641 by persisting the full quiz generation history (instead of truncating to 5) while keeping the existing localStorage key and adding a “Show More / Show Less” toggle on the Quiz Dashboard.
Changes:
- Remove the “keep only last 5 quizzes” cap when writing quiz history to
localStorage. - Add safer parsing/cleanup for malformed quiz history stored under
last5Quizzes. - Add a Show More / Show Less toggle to default the dashboard view to the latest 5 quizzes while allowing access to all history.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
eduaid_web/src/pages/Text_Input.jsx |
Stops truncating stored history; adds parsing helper for last5Quizzes and appends new quiz entries. |
eduaid_web/src/pages/Previous.jsx |
Adds robust history parsing, computes a default “latest 5” view, and introduces a Show More / Show Less toggle plus state reset on clear. |
Comments suppressed due to low confidence (1)
eduaid_web/src/pages/Previous.jsx:44
handleQuizClickwritesquiz.qaPairintoqaPairswithout validating it. Ifquiz.qaPairis missing/undefined (e.g., corrupted localStorage array entries), this can result inqaPairsbeing set to the string "undefined", andOutput.jsxlater does an unguardedJSON.parse(localStorage.getItem("qaPairs"))which will throw. Add a guard/fallback (and/or remove the bad history entry) and wrap the setItem/parse path in try/catch so navigation to/outputcan't be triggered with invalid JSON.
const handleQuizClick = (quiz) => {
localStorage.setItem("qaPairs", JSON.stringify(quiz.qaPair));
navigate('/output');
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| }; | ||
|
|
||
| const [quizzes, setQuizzes] = React.useState(getQuizzesFromLocalStorage()); |
There was a problem hiding this comment.
React.useState(getQuizzesFromLocalStorage()) eagerly reads/parses localStorage on every render because the function is being invoked. Use a lazy initializer (useState(() => getQuizzesFromLocalStorage()) or pass the function directly) so parsing only happens on the initial render.
| const [quizzes, setQuizzes] = React.useState(getQuizzesFromLocalStorage()); | |
| const [quizzes, setQuizzes] = React.useState(getQuizzesFromLocalStorage); |
| const getQuizzesFromLocalStorage = () => { | ||
| const quizzes = localStorage.getItem("last5Quizzes"); | ||
| return quizzes ? JSON.parse(quizzes) : []; | ||
| if (!quizzes) { | ||
| return []; | ||
| } | ||
|
|
||
| try { | ||
| const parsedQuizzes = JSON.parse(quizzes); | ||
| if (Array.isArray(parsedQuizzes)) { | ||
| return parsedQuizzes; | ||
| } | ||
|
|
||
| localStorage.removeItem("last5Quizzes"); | ||
| return []; | ||
| } catch { | ||
| localStorage.removeItem("last5Quizzes"); | ||
| return []; | ||
| } | ||
| }; |
There was a problem hiding this comment.
The robust localStorage parsing logic here duplicates the same pattern introduced in Text_Input.jsx. To avoid the two implementations drifting over time, consider extracting a shared helper (e.g., utils/quizHistoryStorage.js) and a shared key constant for "last5Quizzes" so all reads/writes go through one codepath.
| const quizHistory = getQuizHistoryFromLocalStorage(); | ||
| quizHistory.push(quizDetails); | ||
| localStorage.setItem("last5Quizzes", JSON.stringify(quizHistory)); | ||
|
|
||
| navigate("/output"); |
There was a problem hiding this comment.
Now that history is unbounded, localStorage.setItem("last5Quizzes", ...) is more likely to hit browser storage limits and throw (QuotaExceededError), which will prevent navigation to /output and only log Error:. Consider handling quota failures explicitly (e.g., catch around setItem, optionally trimming oldest entries or skipping history write) so quiz generation still succeeds even if history can't be persisted.
| const getQuizHistoryFromLocalStorage = () => { | ||
| const storedQuizzes = localStorage.getItem("last5Quizzes"); | ||
| if (!storedQuizzes) { | ||
| return []; | ||
| } | ||
|
|
||
| try { | ||
| const parsedQuizzes = JSON.parse(storedQuizzes); | ||
| if (Array.isArray(parsedQuizzes)) { | ||
| return parsedQuizzes; | ||
| } | ||
|
|
||
| localStorage.removeItem("last5Quizzes"); | ||
| return []; | ||
| } catch { | ||
| localStorage.removeItem("last5Quizzes"); | ||
| return []; | ||
| } | ||
| }; |
There was a problem hiding this comment.
The localStorage parsing/cleanup helper introduced here is effectively duplicated in Previous.jsx. Extracting it into a shared utility and reusing it for both reading history (dashboard) and appending history (quiz generation) will reduce maintenance risk and keep parsing/cleanup behavior consistent.
![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
🧹 Nitpick comments (2)
eduaid_web/src/pages/Previous.jsx (1)
97-111: Consider adding unique IDs to quizzes for stable React keys.Using
indexas a key works here but could cause subtle rendering issues when toggling between Show More/Show Less, as the indices change for the same quiz items. A more robust solution would be to generate a unique ID (e.g.,Date.now()orcrypto.randomUUID()) when creating each quiz inText_Input.jsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/pages/Previous.jsx` around lines 97 - 111, The list uses index as a React key in quizzesToDisplay.map (key={index}), which is unstable when items reorder; update quiz objects to include a unique id when created (e.g., set id via crypto.randomUUID() or Date.now() in the quiz creation logic in Text_Input.jsx) and change the list to use that id as the key (and ensure handleQuizClick(quiz) continues to receive the quiz object with id). Locate quiz creation in Text_Input.jsx, add a stable id field to each quiz, and replace key={index} with key={quiz.id} in Previous.jsx.eduaid_web/src/pages/Text_Input.jsx (1)
102-120: Defensive parsing looks good; consider extracting to a shared utility.The safeguards for malformed localStorage data are well-implemented. However, this function is nearly identical to
getQuizzesFromLocalStorageinPrevious.jsx(lines 14-32). Consider extracting this into a shared utility (e.g.,src/utils/quizStorage.js) to avoid duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eduaid_web/src/pages/Text_Input.jsx` around lines 102 - 120, Duplicate localStorage parsing logic exists in getQuizHistoryFromLocalStorage and getQuizzesFromLocalStorage; extract this logic into a shared utility function (e.g., export function safeParseQuizzes or getQuizzesFromLocalStorage in a new module) that performs the getItem, JSON.parse, Array.isArray check, and removal on error, then import and use that function from Text_Input.jsx and Previous.jsx to remove duplication and centralize error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eduaid_web/src/pages/Text_Input.jsx`:
- Around line 142-144: The current web code appends quizzes unbounded to
quizHistory (getQuizHistoryFromLocalStorage / quizHistory /
localStorage.setItem("last5Quizzes"...)), risking localStorage quota issues and
diverging from the extension which trims to 5; change the logic to enforce a
clear cap (e.g., MAX_QUIZZES = 100, or match extension’s MAX_QUIZZES) after
pushing quizDetails by trimming older entries (shift/pop) so both web and
extension share the same behavior, rename the storage key from "last5Quizzes" to
"quizHistory", and add a migration step in getQuizHistoryFromLocalStorage that
reads "last5Quizzes" if present, converts it to the new key, and preserves/trims
entries to the cap before saving.
---
Nitpick comments:
In `@eduaid_web/src/pages/Previous.jsx`:
- Around line 97-111: The list uses index as a React key in quizzesToDisplay.map
(key={index}), which is unstable when items reorder; update quiz objects to
include a unique id when created (e.g., set id via crypto.randomUUID() or
Date.now() in the quiz creation logic in Text_Input.jsx) and change the list to
use that id as the key (and ensure handleQuizClick(quiz) continues to receive
the quiz object with id). Locate quiz creation in Text_Input.jsx, add a stable
id field to each quiz, and replace key={index} with key={quiz.id} in
Previous.jsx.
In `@eduaid_web/src/pages/Text_Input.jsx`:
- Around line 102-120: Duplicate localStorage parsing logic exists in
getQuizHistoryFromLocalStorage and getQuizzesFromLocalStorage; extract this
logic into a shared utility function (e.g., export function safeParseQuizzes or
getQuizzesFromLocalStorage in a new module) that performs the getItem,
JSON.parse, Array.isArray check, and removal on error, then import and use that
function from Text_Input.jsx and Previous.jsx to remove duplication and
centralize error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea884d8b-dbc7-4ebd-bca0-c169b42c1b55
📒 Files selected for processing (2)
eduaid_web/src/pages/Previous.jsxeduaid_web/src/pages/Text_Input.jsx
| const quizHistory = getQuizHistoryFromLocalStorage(); | ||
| quizHistory.push(quizDetails); | ||
| localStorage.setItem("last5Quizzes", JSON.stringify(quizHistory)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the extension still has the 5-quiz cap logic
rg -n "last5Quizzes" --glob "extension/**/*.jsx" -A 3 -B 1Repository: AOSSIE-Org/EduAid
Length of output: 1668
Unbounded quiz history may hit localStorage quota; web/extension now diverge.
Two concerns with removing the cap:
-
localStorage limits: With no upper bound, users who generate many quizzes over time could eventually exceed the ~5MB localStorage quota, causing silent failures. Consider adding a reasonable cap (e.g., 50-100 quizzes) or implementing a cleanup strategy.
-
Web/extension inconsistency: The extension (
extension/src/pages/text_input/TextInput.jsx:159-164) still enforces the 5-quiz limit via.shift(). If users switch between web and extension, they'll experience different behaviors and potentially lose history sync.
Additionally, the localStorage key "last5Quizzes" is now misleading since the web version stores all quizzes. Consider renaming to "quizHistory" (with a migration path for existing users).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eduaid_web/src/pages/Text_Input.jsx` around lines 142 - 144, The current web
code appends quizzes unbounded to quizHistory (getQuizHistoryFromLocalStorage /
quizHistory / localStorage.setItem("last5Quizzes"...)), risking localStorage
quota issues and diverging from the extension which trims to 5; change the logic
to enforce a clear cap (e.g., MAX_QUIZZES = 100, or match extension’s
MAX_QUIZZES) after pushing quizDetails by trimming older entries (shift/pop) so
both web and extension share the same behavior, rename the storage key from
"last5Quizzes" to "quizHistory", and add a migration step in
getQuizHistoryFromLocalStorage that reads "last5Quizzes" if present, converts it
to the new key, and preserves/trims entries to the cap before saving.
|
Added a follow-up commit to address review feedback:
Build validation:
|
Addressed Issues:
Implements issue #641 by allowing users to store all generated quizzes in history and view them with a Show More / Show Less toggle.
Fixes #641
Screenshots/Recordings:
Not attached from CLI environment.
Additional Notes:
This PR updates Quiz Dashboard history behavior while preserving existing localStorage compatibility.
Key Improvements:
AI Usage Disclosure:
Checklist
Validation:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes