[Naragod]

Proposed Changes

This fixes a 404 in the grading view. It shows up each time a grader opens the next or previous submission. David asked to split #8228 in two. This PR holds the 404 fix.

Each submission has its own files. On master, the file panel keeps the old file for a moment after a switch. It asks the new submission for that old file. The new submission lacks that file, so the server sends back a 404. The code lives in submission_file_panel.jsx.

• submission_file_panel.jsx notes which submission its file came from.
• It waits for the new file list before it asks for a file.
• It picks a file once loading turns false.
• fetchData in result.jsx drops a reply for a submission the grader has left.
• Two fast clicks now land on the right submission.

During a switch, the file URL is now null for a moment. pdf_viewer.jsx handles that step:

• It saves the scroll spot when the submission changes, before the new URL comes in.
• On scanned exams, it saves the scroll spot while the pane is in view. A hidden pane reads as scrolled to the top.
• The newest getDocument load wins.
• The viewer drops a load that ends after the URL changes.
• The viewer drops a load that ends after it closes.
• A failed load shows submissions.cannot_display.
• A 413 shows submissions.oversize_submission_file.
• A failed load clears the spinner.

Verification

We checked each fix in Chromium on a local scanned exam.

Check	Before	After
Switches that got a 404	12 of 12	0 of 12
Fast double clicks that landed on the right submission		20 of 20
Scroll spot and zoom kept after a switch		Yes

Screenshots of your changes (if applicable)

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)	X
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📦 Dependency update (change that updates a dependency)
📖 Documentation update (change that updates documentation)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

• I have performed a self-review of my changes.
  • Check that all changed files included in this pull request are intentional changes.
  • Check that all changes are relevant to the purpose of this pull request, as described above.
• I have added tests for my changes, if applicable.
  • This is required for all bug fixes and new features.
• I have updated the project documentation, if applicable.
  • This is required for new features.
• If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

• I have updated the project Changelog (this is required for all changes).
• I have verified that the pre-commit.ci checks have passed.
• I have verified that the CI tests have passed.
• I have reviewed the test coverage changes reported by Coveralls.
• I have requested a review from a project maintainer.

Questions and Comments

• A file the grader picks by hand after a switch keeps the old scroll spot. It should open at the top. Master has the same bug. We can fix it in a separate PR.

[coveralls]

Coverage Report for CI Build 40718840001

Coverage decreased (-0.09%) to 90.975%

Details

• Coverage decreased (-0.09%) from the base build.
• Patch coverage: 1 of 1 lines across 1 file are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	54236
Covered Lines:	50406
Line Coverage:	92.94%
Relevant Branches:	2627
Covered Branches:	1325
Branch Coverage:	50.44%
Branches in Coverage %:	Yes
Coverage Strength:	127.7 hits per line

---

💛 - Coveralls

[david-yz-liu]

I did leave an inline comment, but after a bit more investigation I don't think this change (mainly to pdf_viewer.jsx) is targeted at the right level, as it won't resolve the issue for other types of files. The same issue replicates across all file types.

I suspect the root issue is in SubmissionFilePanel. Because of its componentDidUpdate method, there's a delay between the result_id prop changing and the selectedFile state changing. I suspect that it's this delay that results in two separate prop updates to FileViewer (and thus resulting in two separate fetch requests, the first returning the 404).