[Bugfix:TAGrading] CSV Upload Numeric #12972
Merged
Merged
Conversation
this css class was not used for anything in the code except identification, and the id is better for that since its unique
doesn't work. I don't think I am accessing the component data correctly to check whether the numeric score is 0
automateprojectmangement
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 #12972 +/- ##
============================================
- Coverage 21.30% 21.29% -0.01%
Complexity 10055 10055
============================================
Files 277 277
Lines 37621 37626 +5
Branches 513 513
============================================
Hits 8014 8014
- Misses 29105 29110 +5
Partials 502 502
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
NicholasCiuica
changed the title
NicholasCiuica
changed the title
NicholasCiuica
changed the title
this color change makes uploaded cells of value 0 have a grey text color, which is not needed because we want the user to see clearly that this cell was changed by the upload, and it already has the darker background color just like other altered uploaded cells
Previous behavior: if uploaded cell value was above clamp, that cell would still be saved to the db as a component of value 0, which should not be happening
NicholasCiuica
marked this pull request as ready for review
July 1, 2026 16:02
automateprojectmangement
Bot
moved this from Work in Progress
to Seeking Reviewer
in Submitty Development
dandrecollins07-ctrl
approved these changes
Jul 3, 2026
dandrecollins07-ctrl
left a comment
dandrecollins07-ctrl
left a comment
Contributor
There was a problem hiding this comment.
Code looks good, and functionality works properly. Approving.
github-project-automation
Bot
moved this from Seeking Reviewer
to Awaiting Maintainer Review
in Submitty Development
Member
|
@NicholasCiuica |
...so I don't wanna accidentally add it back when this PR gets merged? Not sure if it would work out like that but I wanna be safe
dandrecollins07-ctrl
approved these changes
Jul 6, 2026
dandrecollins07-ctrl
left a comment
dandrecollins07-ctrl
left a comment
Contributor
There was a problem hiding this comment.
Re-tested after the merge conflict resolution. Verified that uploading a CSV with valid numeric values still works as expected. Also confirmed that uploading a value of 0 does not create a gcd_score = 0 entry in the database, and uploading a value above the component max no longer inserts a 0 into the database either. The functionality described in the PR is working correctly.
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Done
in Submitty Development
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this Change Important & Necessary?
Closes #12903
What is the New Behavior?
If a CSV is uploaded to a numeric gradeable, any cells with value 0 will not be added as gradeable component data to the database. In the previous behavior, these value-0 cells made deleting "empty" columns after csv upload difficult. This issue may be related to recent issues where adding additional numeric columns after doing a CSV upload could make some columns unalterable.
What steps should a reviewer take to reproduce or test the bug or new feature?
EDIT: Also try uploading values above the max score set on a numeric column. On main, a component will be saved to the database with a value of 0 at the location of the high value. On this PR's branch, any invalid score that is uploaded from a CSV should have no effect on the database (no new component data added).
Automated Testing & Documentation
Cypress tests for CSV uploading to numeric gradeables will be updated/introduced in another PR.
Other information