[Feature:InstructorUI] Parse CSV Headers in Classlist Upload#12601
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12601 +/- ##
============================================
- Coverage 21.64% 21.55% -0.09%
- Complexity 9654 9733 +79
============================================
Files 268 268
Lines 36245 36413 +168
Branches 487 487
============================================
+ Hits 7846 7850 +4
- Misses 27916 28080 +164
Partials 483 483
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
automateprojectmangement
Bot
moved this from Seeking Reviewer
to In Review
in Submitty Development
hyeenaa
left a comment
hyeenaa
left a comment
There was a problem hiding this comment.
There's a few issues I found with the PR related to edge cases.
- when inserting with an invalid first name, it gets the row # wrong, (potentially starting index at 0 without correction).
- When cascading issues arise, the error message only shows the first one.
- When the section number is negative, there is no catch, and it enters into the database
- integers are allowed for user id's
Take a look at these, and let me know if you have any questions about how I encountered them.
github-project-automation
Bot
moved this from In Review
to Work in Progress
in Submitty Development
|
Hi @hyeenaa, |
|
Cool, all good. Forgot that numbers are usually expected in user IDs. I am curious about negative section numbers, though, I don't necessarily see an instance where that would be used, but I might be missing something. I'll give it all another look-over tomorrow. |
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
hyeenaa
left a comment
hyeenaa
left a comment
There was a problem hiding this comment.
I've pretty thoroughly tested the input filtering, and it seems to be responsive to all inputs in all fields. Everything else also looks good.
github-project-automation
Bot
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Done
in Submitty Development
…y#12601) <!-- ** Please remove all comment blocks in the description before submitting this PR. ** --> <!-- NOTE: Please ensure your title and description align with Submitty conventions (see https://submitty.org/developer/getting_started/make_a_pull_request for more details). Each title has a prefix, and a limit of 40 chars. A description template has been provided and must be completed in full. Please also ensure that all CI tests are passing. Pull requests that do not meet these requirements are ineligible for review and may be closed. --> ### Why is this Change Important & Necessary? Closes Submitty#12599 and Submitty#12424 ### What is the New Behavior? Right now the classlist upload feature reads a .csv file without using column headers, and instead hard-codes the location of fields like First Name, User ID, Registration Section, ect. The new behavior is that the first row of the .csv file is interpreted as column headers, which are used to determine how different fields are parsed. Example of a correctly formatted file: <img width="1229" height="569" alt="Screenshot_20260317_173617" src="https://github.com/user-attachments/assets/31264a1b-bdf6-440b-aecc-cc561733eeee" /> The file format is compatible with the .csv files generated by the "Download Classlist" button on the same page, so the downloaded file can be edited and re-uploaded without any hassle. ### What steps should a reviewer take to reproduce or test the bug or new feature? 1. Go to the "Manage Students" tab. 2. Use the "Download Users" button to download the classlist file. 3. Modify the registration section for some of the users; add additional users with a similar format to those above; test multiple fields at the same time; ect. 4. Check for any errors. ### Automated Testing & Documentation This feature need substantial testing, and the documentation on submitty.org needs to be updated to match the new behavior. Neither of these have been done yet. ### Other information Should probably add a warning about the change, in case instructors are used to the old format. Maybe check if all the column titles are invalid, then they are probably missing the headers altogether. --------- Co-authored-by: Justin Manion <[email protected]>
4 participants
Why is this Change Important & Necessary?
Closes #12599 and #12424
What is the New Behavior?
Right now the classlist upload feature reads a .csv file without using column headers, and instead hard-codes the location of fields like First Name, User ID, Registration Section, ect.
The new behavior is that the first row of the .csv file is interpreted as column headers, which are used to determine how different fields are parsed.
Example of a correctly formatted file:
The file format is compatible with the .csv files generated by the "Download Classlist" button on the same page, so the downloaded file can be edited and re-uploaded without any hassle.
What steps should a reviewer take to reproduce or test the bug or new feature?
Automated Testing & Documentation
This feature need substantial testing, and the documentation on submitty.org needs to be updated to match the new behavior. Neither of these have been done yet.
Other information
Should probably add a warning about the change, in case instructors are used to the old format. Maybe check if all the column titles are invalid, then they are probably missing the headers altogether.