Fixing the List of Summer Terms on the Request Summer Proposal page - #1767
Fixing the List of Summer Terms on the Request Summer Proposal page#1767ArtemKurasov wants to merge 17 commits into
Conversation
|
Fixes issue ##1747 SummaryUpdated the CCE Minor Summer Experience form so students see summer years based on their estimated enrollment period. What Changed
For example, a Senior in Summer 2021 sees Summer 2018–2021, while a Sophomore sees Summer 2020–2022. TestingAdded tests covering:
|
| return redirect(url_for('minor.viewCceMinor', username=username, tab="manageProposals")) | ||
|
|
||
| student = User.get_by_id(username) | ||
| year_name = User.rawClassLevel |
There was a problem hiding this comment.
This should be classLevel instead of year_name
| flash("Proposal successfully created.", "success") | ||
| return redirect(url_for('minor.viewCceMinor', username=username, tab="manageProposals")) | ||
|
|
||
| student = User.get_by_id(username) |
There was a problem hiding this comment.
incase username is not student I know this is not the case above it should be as cceMinor is reserve for students only so we need to ensure isStudent is user here
| """ | ||
| Select the summer terms during which a CCE Minor student could be enrolled. | ||
|
|
||
| The user record does not store an admission date, so the admission academic |
There was a problem hiding this comment.
docstrings should give a reason for input and what returns and a single sentence of what it should do.
| "Sophomore": 2, | ||
| "Junior": 3, | ||
| "Senior": 4, | ||
| "Graduating": 5, |
There was a problem hiding this comment.
graduating is troublesome classLevel because when you run this command: SELECT * FROM user AS u WHERE u.rawClassLevel = 'Graduating' AND u.isGraduated = TRUE; you will see those two attributes can occur at the same time.
| {{ super() }} | ||
| <script type="module" src="/static/js/minorProfilePage.js"></script> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery.inputmask/3.3.4/jquery.inputmask.bundle.min.js"></script> | ||
| <script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/bootstrap-select/1.5.4/bootstrap-select.js"></script> |
| ], | ||
| ) | ||
| def test_selectAllSummerTerms_uses_estimated_enrollment_window( | ||
| class_level, current_description, current_year, expected_years): |
There was a problem hiding this comment.
make sure to test graduating too as you have special logic for graduating when populating for the student.
|
|
||
| firstSummer = inferredAdmissionYear + 1 | ||
| lastSummer = inferredAdmissionYear + max(4, classYear) - 1 | ||
| if currentTerm.description.startswith("Summer"): |
There was a problem hiding this comment.
currentTerm.isSummer should and can be use here as we have a field for this.
| # A Spring/Summer term belongs to the academic year that began the prior fall. | ||
| academicYearStart = (currentTerm.year if currentTerm.description.startswith("Fall") | ||
| else currentTerm.year - 1) | ||
| inferredAdmissionYear = academicYearStart - (classYear - 1) |
There was a problem hiding this comment.
what we can do here is a way to shrink these lines 63,65,66 to two lines in two of these forms:
eg 1:
firstSummer = (academicYearStart - (classYear - 1)) + 1
lastSummer = (firstSummer - 1) + max(4, classYear) - 1
eg2:
firstSummer, lastSummer = inferredAdmissionYear + 1, inferredAdmissionYear + max(4, classYear) - 1```
| {% endif %} | ||
| {% for term in selectableTerms %} | ||
| <option value="{{ term }}" | ||
| <option value="{{ term.id }}" |
There was a problem hiding this comment.
changing it from term to term.id doesn't really matter as i tried doing {{ term.description }}, {{term}}, {{term.id}} and term and term.id are the same.
| @@ -1,6 +1,9 @@ | |||
| import pytest | |||
| from types import SimpleNamespace | |||
There was a problem hiding this comment.
we do not use SimpleNamespace unless we are trying to let's say fake a request in the test so that we can avoid create a whole class for it. For instance: mockRequestProposalObject = SimpleNamespace(
form=defaultProposal,
files=SimpleNamespace(
getlist=lambda key: [],
get=lambda key: None
), in test_nimor.py file. Here your simplenamespace is just use to convert traditional dictionary access into attribute access. but, if my understanding is wrong and there is a rationale do tell me that can change my comment.
| isCurrentTerm=False, | ||
| termOrder=f"{year}-2") | ||
|
|
||
| student = SimpleNamespace(rawClassLevel=class_level, |
There was a problem hiding this comment.
From the above comment regarding simplenamespace convert those so that it reflects the dictionary usage.
4 participants
Issue Description
Fixes issue #1747
Changes
Testing