[Feature:TAGrading] Persistent Auto-Open#12931
Conversation
it seems this cookie is no longer used/was never fully fleshed out?
this cookie also is never set and is no longer necessary.
changing the variable name to make its purpose easier to understand. This will result in the old cookie "files" persisting but not being used
testcase cookie is currently never set but could be used to automatically open previously opened testcases on the autograding page
still currently work in progress
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12931 +/- ##
============================================
- Coverage 21.40% 21.39% -0.02%
Complexity 9944 9944
============================================
Files 268 268
Lines 37124 37143 +19
Branches 494 499 +5
============================================
Hits 7948 7948
- Misses 28686 28700 +14
- Partials 490 495 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The goal is that closing a file should remove it from the auto-opened list. I will work more on this later
clicking the auto-open checkbox now flushes the list of auto-opened files
There was a problem hiding this comment.
Currently (Prior to noticing it is labeled as Work in Progress) I was able to reproduce the behavior on main where previously opened files do not automatically reopen when returning to a student submission. However, I was unable to verify the expected behavior on this branch. Also may you fix the Lint issues, and then perhaps the behavior can be properly verified.
This commit includes some slight changes I made to my findAllClosedFiles to correctly get all closed files; it also includes an attempt I made at combining the open and closed file finder functions into one, but I decided it was not worth the complications
now when a file is closed, it is removed from the auto-open list
fixes an issue where intermediary folders being incorrect does not stop files from being opening (e.g. submissions/A/B/c.txt being saved as auto-opened, would also open submissions/B/c.txt)
jndlansh
left a comment
There was a problem hiding this comment.
I think the core approach of accumulating open files and removing closed ones is good. Just a few comments I have added here and there, I understand it is WIP, address them before merging. Also see CI failing, Fix JS lint after you are done working. Great work!
if the open_files cookies is read as no value, it is interpreted as [] so the rest of the open_files cookies set logic works. This was causing an issue with Cypress tests and I tested that it was also causing errors in the console
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.
There was a problem hiding this comment.
I tested this PR and generally approve. I was able to verify the original behavior on main. On the branch, I found that auto open is now persistent and recognizes file names between users.
One thing I noted is that if one user submits a directory with 'small_file.txt' in it, and another user only submits a 'small_file.txt', these files will not be considered for auto opening since they have different paths.
I am wondering whether we would prefer the auto open to go off of file name or file path, since it seems like it might be useful to auto open the file even if it's in a differently named directory.
Edit: We decided this additional functionality would not be necessary.
### Why is this Change Important & Necessary? Currently while auto-open is enabled, opening one or more new files in the gradeable interface will flush the list of auto-opened files that were opened in previous viewed submissions. This reduces the usefulness of auto-open for gradeables where submission files can be named differently. ### What is the New Behavior? Previous files designated as auto-opened will persist when additional files are opened while auto-open is enabled. Any files that you closed will be removed from the list of auto-opened files. The list of auto-opened files will be flushed when you disable and re-enable auto-open. ### What steps should a reviewer take to reproduce or test the bug or new feature? 1. on main, create a new gradeable where students can submit files 2. have one student submit a non-image file, and another student submit another non-image file with a different name 3. open the grading interface to the first student, enable auto open, and open the student's submitted file 4. navigate to the other student, and open their submitted file 5. go back to the first student the first student's submitted file WILL NOT be auto-opened 7. now go to the PR branch and repeat steps 3-5 the first student's submitted file WILL be auto-opened Additional testing: - Test that auto-open works with nested folder structures (by submitting a .zip of a folder with files and following a similar switching procedure to the one described above) - Test that image file behavior is unchanged (image files are not auto-openable) - Test that disabling auto-open stops any files from being auto-opened, and re-enabling auto-open does not result in any files being auto-opened by default (because clicking the auto-open checkbox flushes the auto-open list) ### Automated Testing & Documentation The current auto-open Cypress test has been updated to test this new behavior ### Other Information This PR changes the name of the "files" cookie to "open_files" for more clarity on its purpose; please let me know if you have wording suggestions. This PR also removes mentions of the "opened_mark" and "scroll_pixel" cookies, which are never generated and may be leftovers from features that are no longer supported or desired. I have left behind the unfinished "testcases" cookie logic, as it seems to be mostly completed and just needs to be set properly. Please let me know what you think of keeping/removing these cookies and if another PR to implement them might be worth it. --------- Co-authored-by: Barb Cutler <[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
Why is this Change Important & Necessary?
Currently while auto-open is enabled, opening one or more new files in the gradeable interface will flush the list of auto-opened files that were opened in previous viewed submissions. This reduces the usefulness of auto-open for gradeables where submission files can be named differently.
What is the New Behavior?
Previous files designated as auto-opened will persist when additional files are opened while auto-open is enabled. Any files that you closed will be removed from the list of auto-opened files. The list of auto-opened files will be flushed when you disable and re-enable auto-open.
What steps should a reviewer take to reproduce or test the bug or new feature?
the first student's submitted file WILL NOT be auto-opened
the first student's submitted file WILL be auto-opened
Additional testing:
Automated Testing & Documentation
The current auto-open Cypress test has been updated to test this new behavior
Other Information
This PR changes the name of the "files" cookie to "open_files" for more clarity on its purpose; please let me know if you have wording suggestions.
This PR also removes mentions of the "opened_mark" and "scroll_pixel" cookies, which are never generated and may be leftovers from features that are no longer supported or desired. I have left behind the unfinished "testcases" cookie logic, as it seems to be mostly completed and just needs to be set properly. Please let me know what you think of keeping/removing these cookies and if another PR to implement them might be worth it.