Fix problem switcher not update#928
Closed
def-WA2025 wants to merge 2 commits intoXMOJ-Script-dev:masterfrom
Closed
Fix problem switcher not update#928def-WA2025 wants to merge 2 commits intoXMOJ-Script-dev:masterfrom
def-WA2025 wants to merge 2 commits intoXMOJ-Script-dev:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAlways refresh and persist the contest problem list from the server so the problem switcher reflects the latest problems on page reload, instead of reusing potentially stale data from localStorage. Sequence diagram for refreshed contest problem list on page reloadsequenceDiagram
actor User
participant BrowserPage
participant XMOJScript
participant LocalStorage
participant XMOJServer
User->>BrowserPage: Reload contest problem page
BrowserPage->>XMOJScript: Execute main
XMOJScript->>LocalStorage: getItem(UserScript-Contest-cid-ProblemList)
LocalStorage-->>XMOJScript: ContestProblemList (ignored, set to null)
XMOJScript->>XMOJServer: GET /contest.php?cid
XMOJServer-->>XMOJScript: HTML contest page
XMOJScript->>XMOJScript: Parse HTML and extract problemset rows
XMOJScript->>XMOJScript: Build problemList array
XMOJScript->>LocalStorage: setItem(UserScript-Contest-cid-ProblemList, JSON(problemList))
LocalStorage-->>XMOJScript: Persisted problemList
XMOJScript->>BrowserPage: Create and render problemSwitcher using fresh ContestProblemList
BrowserPage-->>User: Display updated contest problems in switcher
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- You’re now forcing a network request on every page load by setting
ContestProblemList = nullunconditionally; consider keeping the cache check and adding an explicit refresh strategy (e.g., timestamp-based invalidation or a manual refresh) instead of always re-fetching. - The initial
localStorage.getItemassignment toContestProblemListis immediately overwritten withnull, so that read can be removed or the logic restructured to avoid dead code and make the intent of the cache/update behavior clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You’re now forcing a network request on every page load by setting `ContestProblemList = null` unconditionally; consider keeping the cache check and adding an explicit refresh strategy (e.g., timestamp-based invalidation or a manual refresh) instead of always re-fetching.
- The initial `localStorage.getItem` assignment to `ContestProblemList` is immediately overwritten with `null`, so that read can be removed or the logic restructured to avoid dead code and make the intent of the cache/update behavior clearer.
## Individual Comments
### Comment 1
<location path="XMOJ.user.js" line_range="2271" />
<code_context>
document.getElementsByTagName("h2")[0].innerHTML += " (" + pid + ")";
}
let ContestProblemList = localStorage.getItem("UserScript-Contest-" + SearchParams.get("cid") + "-ProblemList");
- if (ContestProblemList == null) {
- const contestReq = await fetch("https://www.xmoj.tech/contest.php?cid=" + SearchParams.get("cid"));
</code_context>
<issue_to_address>
**suggestion:** The value read from localStorage is immediately discarded and never used.
After reading `ContestProblemList` from localStorage, it’s immediately set to `null` and the data is always fetched from the network, so the `localStorage.getItem(...)` call has no effect. Either remove the `getItem` if you always want a fresh fetch, or keep the conditional logic to retain caching behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -2269,23 +2269,22 @@ async function main() { | |||
| document.getElementsByTagName("h2")[0].innerHTML += " (" + pid + ")"; | |||
| } | |||
| let ContestProblemList = localStorage.getItem("UserScript-Contest-" + SearchParams.get("cid") + "-ProblemList"); | |||
There was a problem hiding this comment.
suggestion: The value read from localStorage is immediately discarded and never used.
After reading ContestProblemList from localStorage, it’s immediately set to null and the data is always fetched from the network, so the localStorage.getItem(...) call has no effect. Either remove the getItem if you always want a fresh fetch, or keep the conditional logic to retain caching behavior.
2 tasks
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="XMOJ.user.js">
<violation number="1" location="XMOJ.user.js:2272">
P2: Don't clear the cached problem list before the refresh succeeds; otherwise a failed contest fetch breaks the switcher path instead of using the last cached list.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+2272
to
2288
| ContestProblemList = null; | ||
| const contestReq = await fetch("https://www.xmoj.tech/contest.php?cid=" + SearchParams.get("cid")); | ||
| const res = await contestReq.text(); | ||
| if (contestReq.status === 200 && res.indexOf("比赛尚未开始或私有，不能查看题目。") === -1) { | ||
| const parser = new DOMParser(); | ||
| const dom = parser.parseFromString(res, "text/html"); | ||
| const rows = (dom.querySelector("#problemset > tbody")).rows; | ||
| let problemList = []; | ||
| for (let i = 0; i < rows.length; i++) { | ||
| problemList.push({ | ||
| "title": rows[i].children[2].innerText, | ||
| "url": rows[i].children[2].children[0].href | ||
| }); | ||
| } | ||
| localStorage.setItem("UserScript-Contest-" + SearchParams.get("cid") + "-ProblemList", JSON.stringify(problemList)); | ||
| ContestProblemList = JSON.stringify(problemList); | ||
| } |
There was a problem hiding this comment.
P2: Don't clear the cached problem list before the refresh succeeds; otherwise a failed contest fetch breaks the switcher path instead of using the last cached list.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At XMOJ.user.js, line 2272:
<comment>Don't clear the cached problem list before the refresh succeeds; otherwise a failed contest fetch breaks the switcher path instead of using the last cached list.</comment>
<file context>
@@ -2269,23 +2269,22 @@ async function main() {
- }
- localStorage.setItem("UserScript-Contest-" + SearchParams.get("cid") + "-ProblemList", JSON.stringify(problemList));
- ContestProblemList = JSON.stringify(problemList);
+ ContestProblemList = null;
+ const contestReq = await fetch("https://www.xmoj.tech/contest.php?cid=" + SearchParams.get("cid"));
+ const res = await contestReq.text();
</file context>
Suggested change
| ContestProblemList = null; | |
| const contestReq = await fetch("https://www.xmoj.tech/contest.php?cid=" + SearchParams.get("cid")); | |
| const res = await contestReq.text(); | |
| if (contestReq.status === 200 && res.indexOf("比赛尚未开始或私有，不能查看题目。") === -1) { | |
| const parser = new DOMParser(); | |
| const dom = parser.parseFromString(res, "text/html"); | |
| const rows = (dom.querySelector("#problemset > tbody")).rows; | |
| let problemList = []; | |
| for (let i = 0; i < rows.length; i++) { | |
| problemList.push({ | |
| "title": rows[i].children[2].innerText, | |
| "url": rows[i].children[2].children[0].href | |
| }); | |
| } | |
| localStorage.setItem("UserScript-Contest-" + SearchParams.get("cid") + "-ProblemList", JSON.stringify(problemList)); | |
| ContestProblemList = JSON.stringify(problemList); | |
| } | |
| const contestReq = await fetch("https://www.xmoj.tech/contest.php?cid=" + SearchParams.get("cid")); | |
| const res = await contestReq.text(); | |
| if (contestReq.status === 200 && res.indexOf("比赛尚未开始或私有，不能查看题目。") === -1) { | |
| const parser = new DOMParser(); | |
| const dom = parser.parseFromString(res, "text/html"); | |
| const rows = (dom.querySelector("#problemset > tbody")).rows; | |
| let problemList = []; | |
| for (let i = 0; i < rows.length; i++) { | |
| problemList.push({ | |
| "title": rows[i].children[2].innerText, | |
| "url": rows[i].children[2].children[0].href | |
| }); | |
| } | |
| localStorage.setItem("UserScript-Contest-" + SearchParams.get("cid") + "-ProblemList", JSON.stringify(problemList)); | |
| ContestProblemList = JSON.stringify(problemList); | |
| } else if (ContestProblemList == null) { | |
| ContestProblemList = "[]"; | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR aim to accomplish?
Fix *EX problems not display.
How does this PR accomplish the above?
Update contestProblemList when the page reload.
By submitting this pull request, I confirm the following:
git rebase)Summary by Sourcery
Bug Fixes:
Summary by cubic
Fixes the Problem Switcher not updating after reload by always refreshing the contest problem list from the server so all problems show up.
localStoragekeyUserScript-Contest-<cid>-ProblemListto avoid stale cache.Written for commit 76fb133. Summary will update on new commits.