[Bugfix:TAGrading] Auto-open single file#12625
Conversation
There was a problem hiding this comment.
Hey @NiharikaSaxena18 thank you for your contribution!
Change the title prefix to [Bugfix:TAGrading], since this is solving an issue.
Please use our PR description template. This is important for keeping pull requests consistent and easy to review. It can be found here:
https://raw.githubusercontent.com/Submitty/Submitty/refs/heads/main/.github/PULL_REQUEST_TEMPLATE.md
Please include a Cypress test for this change.
Please also review #12492, which is solving the same issue. If it is solving the issue, great. If not, explain why your implementation is an improvement.
Thank you!
github-project-automation
Bot
moved this from Seeking Reviewer
to Work in Progress
in Submitty Development
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12625 +/- ##
============================================
- Coverage 21.36% 21.35% -0.01%
Complexity 9990 9990
============================================
Files 274 275 +1
Lines 37300 37317 +17
Branches 499 505 +6
============================================
Hits 7969 7969
- Misses 28836 28847 +11
- Partials 495 501 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
NiharikaSaxena18
changed the title
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
github-actions
Bot
added
the
Abandoned PR - Needs New Owner
|
@NiharikaSaxena18 Please fix the JS linting |
@NiharikaSaxena18 Please add Cypress test. |
|
After trying to test this PR and reviewing the code, I have found that the current jquery selection for counting the number of student-submitted files erroneously counts auto-generated files starting with . like ".submit.timestamp". I pick up this PR and fix its bugs and then have it reviewed by someone else so this feature can be merged. And a note to reviewers: autoscroll is enabled by the "Auto Open" button. |
Changed how the number of submission files is counted to ignore files starting with . in the submission folder, because those are autogenerated on submission
|
Because folders and files are counted to detect whether any file is opened, if a grader manually opens the submissions folder and nothing else, then the single-file auto open feature will be disabled. This is expected behavior (and may be useful if e.g. the grader is only interested in checking the names of the files in the submission folder) but could be confusing. Also, I added that whenever a non-image file is opened, the submissions folder is also opened, so that the submitted file's frame can be visible without the grader manually opening the submissions folder. This behavior is technically not part of the originally mentioned new behavior of the PR, but I thought it would make the feature less confusing. Whoever reviews this, please let me know what you think of this change. |
NicholasCiuica
left a comment
NicholasCiuica
left a comment
There was a problem hiding this comment.
I am going to write a Cypress test before getting this re-reviewed.
tweaking this auto-open PR to match better with another auto-open PR I'm working on, to reduce possible merge conflicts. I also prefer the way that #12625 fetches the file clickable area in the submission browser.
Prepping to reduce merge conflicts between this PR and Submitty#12931 by matching the format of the code shared between them.
experimental change that copies over the dotfile logic from recent PR Submitty#12924. The original logic is in PHP, so the easiest way to make it accessible was to copy it over into JS, but there are probably better solutions that I'll be discussing at the meeting today.
I have copied over an array of meta file names and a meta file function to the ta-grading-panels-init so I don't have to make a connection between the php and js
automateprojectmangement
Bot
moved this from Work in Progress
to Seeking Reviewer
in Submitty Development
automateprojectmangement
Bot
moved this from Seeking Reviewer
to In Review
in Submitty Development
GarvitKhandelwal31
left a comment
GarvitKhandelwal31
left a comment
There was a problem hiding this comment.
I was curious if could we centralize the logic of creating the meta submission array and the checker function so that any future code can also use this. Furthermore it would also ensure better consistency while making edits.
I would suggest creating a file-utils.ts in site/ts/utils , the same way it was implemented in PR#12924.
Thoughts??
I believe this is a worthwhile change, so I made a commit just now implementing it, and I will bring it up at the meeting. |
GarvitKhandelwal31
left a comment
GarvitKhandelwal31
left a comment
There was a problem hiding this comment.
I did a retest again, and for me the feature does not seem to work correctly.
I tried with this submission-
Then opening the next one-
Maybe something is wrong with my environment??
We should probably wait for @mattLif to verify.
mattLif
left a comment
mattLif
left a comment
There was a problem hiding this comment.
Auto-open functionality works for me, so I think your issue might be with your environment. I agree that the usage file-utils.ts is optimal, and its implementation looks good. Only failing tests seem to be flaky, and I see no issues with the added test.
I believe your issue is due to the fact that the submissions and submissions_processed folders and one student's submission were already open. If any folder or file is already open, then those will remain auto-opened and the single-file auto open behavior will be overridden. |
GarvitKhandelwal31
left a comment
GarvitKhandelwal31
left a comment
There was a problem hiding this comment.
After retesting it again, I can verify the functionality works as expected, code looks good to me too.
### Why is this Change Important & Necessary? Fixes Submitty#12482 The existing autoscroll feature only reopens files when filenames match exactly with previously saved state. This breaks in cases where students submit files with different names, forcing graders to manually open files even for simple single-file submissions. ### What is the New Behavior? * When autoscroll is enabled: * The system first attempts to reopen previously opened files (existing behavior) * If no files are reopened **and exactly one file is submitted**, that file is automatically opened * Works for: * Regular files (`openFrame`) * Image files (`viewFileFullPanel`) * No change in behavior when: * Multiple files are submitted * Previously saved files can be successfully reopened ### What steps should a reviewer take to reproduce or test the bug or new feature? 1. Enable autoscroll in grading interface 2. Open a submission with a previously saved filename 3. Submit a new version with a **different filename** **Before this PR:** * No file is automatically opened **After this PR:** * If only one file exists, it is automatically opened Additional checks: * Test with multiple files → no auto-open should occur * Test when saved filenames match → existing behavior should remain unchanged ### Automated Testing & Documentation * No automated tests added in this PR * This change affects frontend behavior and can be validated manually through UI testing * Documentation update not required ### Other information * Not a breaking change * No database migrations required * No known security concerns --------- Co-authored-by: Justin Manion <[email protected]> Co-authored-by: Garvit Khandelwal <[email protected]> Co-authored-by: Nick Ciuica <[email protected]> Co-authored-by: roye2 <[email protected]>
* main: (78 commits) [Bugfix:TAGrading] Auto-open single file (#12625) [Feature:InstructorUI] Add sortable columns to manage students (#12957) [Dependency] Bump php-ds/php-ds from 1.7.0 to 2.0.1 in /site (#12827) [Refactor:TAGrading] Events Infrastructure & StatusBanner (#12941) [Dependency] Bump sqlalchemy from 2.0.48 to 2.0.51 in /.setup/pip (#12884) [Bugfix:Developer] trust hashicorp for vagrant install (#12968) [Feature:TAGrading] Backend for submission clustering (#12886) [Bugfix:InstructorUI] Missing Subsection in the Edit User form (#12955) [Bugfix:Submission] Warning Banners Accessibility Fix (#12956) [Bugfix:System] Preserve signup fields (#12939) [Bugfix:InstructorUI] Fix Csv Download of Subsections (#12954) [Bugfix:InstructorUI] Fix Manage Students Toggle Columns (#12953) [Feature:TAGrading] Persistent Auto-Open (#12931) [Bugfix:Submission] Dot File Upload (#12924) [Refactor:System] Declutter workers (#12815) [Bugfix:Developer] save/restore autograding_containers.json (#12952) [Bugfix:InstructorUI] Create Course Validation (#12949) [Testing:Notifications] Fix grade_inquiries.spec.js (#12950) [UI/UX:System] Add fullscreen button vue component (#12936) [Bugfix:TAGrading] Student Name Resize (#12946) ... # Conflicts: # .pylintrc
8 participants
Why is this Change Important & Necessary?
Fixes #12482
The existing autoscroll feature only reopens files when filenames match exactly with previously saved state. This breaks in cases where students submit files with different names, forcing graders to manually open files even for simple single-file submissions.
What is the New Behavior?
When autoscroll is enabled:
Works for:
openFrame)viewFileFullPanel)No change in behavior when:
What steps should a reviewer take to reproduce or test the bug or new feature?
Before this PR:
After this PR:
Additional checks:
Automated Testing & Documentation
Other information