[Feature:InstructorUI] Submission Open Date Constraints#12663
Conversation
Added a warning description for when the ta beta testing date is after the submission open date for students.
Spent about 2 hours trying to learn how migrations work on Submitty and ended up removing what appears to be the TA view (beta testing) constraint from the Gradeable table. This should hopefully prevent constraint issue when choosing dates.
Removed these PHP constraints for all gradeable types.
Fixed an issue where the ta view date could not be after the team lock date.
Fixed ordering for both: Warning messages when setting gradeable dates. Gradeable categorization when ta view date was after the present and open submission date was before the present.
This reverts unnecessary DB Dump changes.
Fixed ta view date and submission open date warning popping up when submission open date is not visible.
This button would previously set all dates to now. Made it so that it only changed the TA view date to now.
Fixed a test that should show all changes saved when TA view date is after submission open date. Also added an additional test that makes sure the gradable is visible to all when submission open date is in the past and ta view date is in the future.
Using testid instead of id for submission open date.
- removed this because it was already set to past date
- Need to log back in as instructor after testing gradeable visibility with students, graders, and Tas.
Fixed students not being able to view the gradeable if the ta view date was in the future and the submission open date in the past.
Fixed an issue where TAs could view FUTURE gradeables where the open submission date did not exist.
If open submission > now() and ta beta testing > now(), then the gradeable should only be visible to instructors
When ta beta testing date is after the manual grading date, a warning will now pop up.
Removed redundant operation in cypress testing
Finished lab gradeables cypress tests
Added check to make sure date_ta_view is accessible.
I removed this update dates because on github, it is already set to future date. I will remove this comment once it appears to be working properly.
dates on github cypress != dates on local cypress
made 'All Changes Saved' acceptable and '' acceptable
Manual grading in the past and TA viewing in the future means it is visible for graders, tas, and instructors.
If submission open date is after due date and due date exists, warning should display.
Removed DB constraint for open submission date
Removed PHP constraint for open submission date.
Fixed the open submission now button not working properly and moving all the date forward.
Added basic cypress tests which test to make sure submission open constraints have been removed
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12663 +/- ##
============================================
- Coverage 21.64% 21.64% -0.01%
Complexity 9654 9654
============================================
Files 268 268
Lines 36245 36244 -1
Branches 487 487
============================================
- Hits 7846 7845 -1
Misses 27916 27916
Partials 483 483
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Farhanxikram
left a comment
Farhanxikram
left a comment
There was a problem hiding this comment.
Functionality review:
- Every assignment i tested with seemed to switch to the correct section when prompted to do so via changing dates (ex. if the due date is in the past it would be in the Past due section and so on)
- All of the buttons that should've been avaialble for the cassignments were still available, and worked as intended
- as a student i wasn't able to open the assignments that were labeled futre or beta
OS: Windows
Browsers: Chrome, Firefox
github-project-automation
Bot
moved this from Seeking Reviewer
to Awaiting Maintainer Review
in Submitty Development
Fixed php stan file.
automateprojectmangement
Bot
moved this from Awaiting Maintainer Review
to In Review
in Submitty Development
Fixed gradeables not being listed correctly and viewed by correct users when due date was in the past and open submission date was in the future.
Fixed failing cypress test (date due can now go before submission open)
JManion32
left a comment
JManion32
left a comment
There was a problem hiding this comment.
Functionality is as expected and code changes are reasonable. I was unable to verify the Cypress test locally (same issue as before), but the new additions look fine to me, and it passes CI. It would be nice to fix that local issue so we could verify future tests, but that's beyond the scope of this PR. Nice work with this, hopefully we can merge it soon and continue to chip away at these constraints!
github-project-automation
Bot
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
aconfo
left a comment
aconfo
left a comment
There was a problem hiding this comment.
I tested functionality and could not find anything wrong with it. I tried all the various recommended tests and couldn't find any bugs or crashes while interacting with gradeables. Overall, it looks really good and I don't have any suggestions for improvement
QuackHonk
left a comment
QuackHonk
left a comment
There was a problem hiding this comment.
After looking through it and running the code, the pr fixes the issues of having dates in the far future that dont match. However while messing around with the dates I did run into an internal server error when the grade inquiries were in the far future and I changed the TA Beta testing date to be past the submissions, open date, and grading dates.
To be fair, this may be a problem on my system as my computer was running into issues recently with my index file corrupting and me having to reset my virtual box, but I'm not totally sure.
The error I got:
This is likely because you ran it with submitty_install_site rather than submitty_install. This usually means that there is no PHP constraint, but there is a database constraint. Try testing the PR again and run submitty_install |
automateprojectmangement
Bot
moved this from Awaiting Maintainer Review
to In Review
in Submitty Development
github-project-automation
Bot
moved this from In Review
to Done
in Submitty Development
6 participants
CAUTION: Since this PR removes a date constraint, if it is installed on a production system and then needs to be reverted after a moderate period of time where gradeables might have been created/edited and taking advantage of the more flexible date settings, rolling back / reverting the change might fail.
Why is this Change Important & Necessary?
Partially addresses #12349
This PR specifically removes all constraints for the Open Submission date for gradeables. This is important and necessary because instructors felt that the current UI for setting dates on gradeables was confusing, and they want to have more freedom over what dates they set for gradeables, even if they are invalid, since a lot of those dates may not matter for the specific class or assignment.
What is the New Behavior?
Now, the Open Submission date has no constraints, it can be set to whatever date regardless of other dates. When one of the lower dates is set to before the present day and the Open Submission date is set to the future, the lower date wins when assigning a gradeable status on the gradeables page.
A date with actual constraints:
Previously, the Open Submission date also had a constraint (where its box would turn red and changes would fail to save) if it was after any of the dates below it (For example: due date).
Now, when the Open Submission date is ahead of one of the dates below it, such as the due date, or if any other dates are conflicting with each other, it will display messages/warnings like the ones shown here:
Additionally, when the Open Submission date is in the future, and the due date is in the past, the due date wins and the assignment shows as past due. This is also true for all dates below the Open Submission date.
What steps should a reviewer take to reproduce or test the bug or new feature?
Log in as instructor. Go to the sample course, try editing all gradeables and try changing their open submission date in all different combinations. Make sure that not only can the date be changed to any valid date, but also that the listing of the gradeable on the gradeables page is valid. For example, if due date is in the past, and open submission in the future, then the assignment should show under CLOSED, not under FUTURE, not under BETA, and not under OPEN.
Secondly, On the gradeable page, make sure all the buttons are visible and function properly. For example, if the assignment shows as OPEN or CLOSED (past due), all appropriate buttons should be visible and should function like normal.
Finally, go to a gradeable labeled FUTURE and a gradeable labeled BETA and try to copy the link. Then log out and log in as a student and try to paste the link. You should NOT be able to access the gradeable. If you can, please report it immediately.
Note: While testing, the only dates that should have full free access to be changed without causing any constraint issues currently are the grade inquiry dates, the ta view date, and the open submission date. The rest of the dates still have constraints on them.
Automated Testing & Documentation
This feature is sufficiently tested by unit tests and cypress tests. Cypress tests have been added to ensure that the gradables are visible to the proper users when the Open Submission date is changed along with the other dates, especially in ways that would previously violate a constraint. I have not updated lab gradeable dates cypress tests, since those gradeables do not have an open submission date option.
Other information
This is not a breakage change. However, this PR DID add a create a new migration for the course database. It removed any DB constraints from the gradeable table relating to eg_submission_date.
Link hijacking might be a security concern with this PR and should be tested. Ensure that ONLY instructors have access to FUTURE gradeables and instructors and TAs/Mentors have access to BETA gradeables.