Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

a button to email the registrar requesting Student SL Course participation #940

Merged
merged 87 commits into from
Jul 25, 2023

Conversation

doucoureb
Copy link
Contributor

@doucoureb doucoureb commented Jun 16, 2023

Issue:
We need a new button to upload excel sheets to the admin page to allow the automation of courses and course participant creation in the database.

Fixes:
Added two main logic functions parseUpload and pushCourseParticipantToDatabase which control the file upload from the manageServiceLearning page. These functions parse the data in an excel sheet prepare it for preview and upload the data to the database when the user confirms. Added modals to preview and submit data from the excel sheet.

Tests:
To test our changes, create excel sheet in the format of:
Term
Course Abbreviation
Participants(their B numbers and full names)

If a future term doesn't exist or a participant's username is not in the database, there should be an error message and it should disable the submit button. Otherwise, it should display the Term, Course and added students FirstName LastName.

Resolves #646

@AndersonStettner AndersonStettner linked an issue Jun 20, 2023 that may be closed by this pull request
@AndersonStettner AndersonStettner changed the title a button to email the registrar requesting Student SL Course particip… a button to email the registrar requesting Student SL Course participation Jun 21, 2023
@BrianRamsay BrianRamsay marked this pull request as draft June 23, 2023 19:42
Copy link
Contributor

@BrianRamsay BrianRamsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move the test files into a files/ directory inside tests

app/controllers/main/routes.py Outdated Show resolved Hide resolved
app/models/course.py Outdated Show resolved Hide resolved
app/logic/serviceLearningCoursesData.py Show resolved Hide resolved
app/logic/serviceLearningCoursesData.py Outdated Show resolved Hide resolved
app/logic/serviceLearningCoursesData.py Outdated Show resolved Hide resolved
@andersoncedu andersoncedu removed their request for review July 24, 2023 15:40
Copy link
Contributor

@BrianRamsay BrianRamsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I got an error when I made a course abbreviation 4 digits instead of 3:
    previousTerm = list(Term.select().order_by(Term.termOrder))[-1].termOrder > Term.convertDescriptionToTermOrder(cellVal) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TypeError: '>' not supported between instances of 'str' and 'NoneType'

  2. I got an error when I made the term "Fall 21" instead of "Fall 2021"

  3. The whole course and other participants disappears from the preview when there is an error with one of the bnumbers. Also, I would probably put the error in-line with the course and other participants, rather than put it at the end.

  4. The whole course disappears if there is a blank line after the course abbreviation and before the participant list starts.

  5. Same problem if there is a blank line after the term

Copy link
Contributor

@BrianRamsay BrianRamsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make sure the buttons look ok when the page width shrinks.

app/logic/serviceLearningCoursesData.py Outdated Show resolved Hide resolved
app/logic/serviceLearningCoursesData.py Show resolved Hide resolved
app/templates/main/manageServiceLearningFaculty.html Outdated Show resolved Hide resolved
app/static/js/event_list.js Outdated Show resolved Hide resolved
BrianRamsay and others added 3 commits July 25, 2023 09:52
…nstead of any whitespace character, described their function, added check for A and B for spring and fall terms. Addressed additional changes as requested.
@BrianRamsay BrianRamsay marked this pull request as draft July 25, 2023 15:09
@BrianRamsay BrianRamsay marked this pull request as ready for review July 25, 2023 15:10
@BrianRamsay BrianRamsay reopened this Jul 25, 2023
@github-actions
Copy link

View Code Coverage

@BrianRamsay BrianRamsay merged commit 7cd1a73 into development Jul 25, 2023
10 checks passed
@BrianRamsay BrianRamsay deleted the buttonRegistrar branch July 25, 2023 15:24
@AndersonStettner AndersonStettner linked an issue Sep 19, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request student participation log from registrar Receiving the SLC-designated course list.
6 participants