[Refactor:Submission] Submission file hiding in UI#7804
Conversation
Codecov Report
@@ Coverage Diff @@
## main #7804 +/- ##
============================================
- Coverage 22.21% 20.36% -1.85%
- Complexity 7673 8185 +512
============================================
Files 205 205
Lines 24170 26360 +2190
Branches 66 89 +23
============================================
Hits 5369 5369
- Misses 18737 20904 +2167
- Partials 64 87 +23
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
I left some comments but biggest issue is with the change of removing scanned_exam as a database stored type. I think the new method to base it off of if students can download submissions after grades are released isn't the best method
If an instructor prevents students from downloading submission until after grading is complete it doesn't necessarily imply a gradeable is a bulk uploaded gradeable.
I'm not sure if any of this has been discussed but was there an issue for having bulk upload use better defaults/config to let students only download after grades have been released? A scanned exam / bulk upload is technically only an electronic gradeable that has that flag set to true on creation - it should still have all the other settings and configs of a normal electronic gradeable
I guess the overall question is why does the scanned gradeable flag need to be removed and silently calculated based on configs instead of just explicitly set and have the gradeable use good defaults introduced with the rest of this PR to get the expected behavior of an exam
|
Should be noted this PR required rebuilding gradeables |
williamjallen
left a comment
There was a problem hiding this comment.
Code looks good overall, and would be fine to merge as-is. I noted a few stylistic changes below, but they do not necessarily have to be made for this to be merged.
site/app/templates/admin/admin_gradeable/AdminGradeableAuto.twig
Outdated
Show resolved
Hide resolved
site/app/templates/admin/admin_gradeable/AdminGradeableAuto.twig
Outdated
Show resolved
Hide resolved
site/app/templates/admin/admin_gradeable/AdminGradeableAuto.twig
Outdated
Show resolved
Hide resolved
site/app/templates/admin/admin_gradeable/AdminGradeableAuto.twig
Outdated
Show resolved
Hide resolved
site/app/templates/admin/admin_gradeable/AdminGradeableAuto.twig
Outdated
Show resolved
Hide resolved
| <span class="required_type" >(Required)</span> | ||
| {% else %} | ||
| <fieldset><legend><b style='font-size:18px'>Gradeable type</b>: <span id="gradeable-type-string">{{ type_string }}</span></legend> | ||
| <fieldset><legend><b style='font-size:18px'>Gradeable type</b>: <span id="gradeable-type-string" data-hw="{{ gradeable_type_strings['electronic_hw'] | e('html_attr') }}" data-bulk="{{ gradeable_type_strings['electronic_bulk'] | e ('html_attr') }}">{{ type_string }}</span></legend> |
There was a problem hiding this comment.
Is there supposed to be a space in e ('html_attr')? It's used both with and without the space in this line.
site/app/templates/admin/admin_gradeable/AdminGradeableCreate.twig
Outdated
Show resolved
Hide resolved
tkoz0
left a comment
There was a problem hiding this comment.
Works as expected with a test bulk upload gradeable.
...rator/migrations/course/20220330154141_update_electronic_gradeable_student_access_options.py
Outdated
Show resolved
Hide resolved
Eceptonsu
left a comment
There was a problem hiding this comment.
Code looks fine and the changes work as expected.
When "Make new submissions and access all prior versions?" is selected as "No", the "Will this assignment have a due date?" option in the date section disappears. Is this intentional?
### Why is this Change Important & Necessary? Lingering change from #7804 See warning/error in https://github.com/Submitty/Submitty/actions/runs/21500146068/job/61944467809#step:5:12258 ### What is the New Behavior? hide_submitted_files, an invalid config value, is now removed from the notebook_expected_string config example.
What is the current behavior?
hide_submitted_filesis a gradeable config option.A scanned exam/bulk upload gradeable is separated from a
electronic_hwgradeable based on the gradeable type set initially."Make new submissions and access all prior versions?" under "What are students allowed to do?" for electronic gradeables does not work as expected for student access to prior versions; there is also a gradeable autograding config option which hides the gradeable version as part of it (
hide_version_and_test_details.What is the new behavior?
Student download (can students view/download submitted files?) is added as an option for a gradeable that is configurable in the UI. By default, this is set to the opposite of whether the gradeable was previously a bulk upload/scanned exam option.
hide_submitted_filesis removed as an option and will no longer affect whether gradeables will display submitted files. This also affects previous gradeables.Scanned exam/bulk upload is removed as a gradeable parameter; in the frontend, the gradeable type instead dynamically changes from a student gradeable (
electronic_hw- "Students will submit one or more files by direct upload to the Submitty website") to a scanned exam/bulk upload (electronic_bulk- "TA/Instructor will (bulk) upload scanned .pdf for online manual grading"); if "View the gradeable on the course home page?" is set to "When grades are released", it will be a bulk upload gradeable; otherwise, it will be a student gradeable.Fixed already-existing UI version hiding option ("Make new submissions and access all prior versions?") to properly hide other versions from the student, and only show the active version to them if applicable.
hide_version_and_test_detailsis renamed tohide_test_detailsand no longer affects whether the version is shown or hidden. This also affects previous gradeables.Other information?
Functionality that hid individual images and bulk upload data of PDFs from student view has been removed; future images/bulk upload data will have a prefix of
.to hide them, but current files will become visible - this was discussed and decided on because the current implementation of hiding/restricting access to certain files for bulk upload gradeables is unclear.hide_version_and_test_detailswill still hide test details but will no longer hide the version.eg_scanned_exam under the electronic_gradeable table is being removed - the field wasn't necessarily accurate since instructors would sometimes select one option and then change the uploaded gradeable config to match the other option.
When reverting this PR, eg_scanned_exam in the database is set based on if "View the gradeable on the course home page?" is set to "When grades are released".