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.

Sign up for GitHub

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

Jump to bottom

Empty date fields when adding a course instance results in 500 internal server error #1174

Closed
lainets opened this issue Apr 11, 2023 · 1 comment · Fixed by #1186
Closed

Empty date fields when adding a course instance results in 500 internal server error #1174

lainets opened this issue Apr 11, 2023 · 1 comment · Fixed by #1186
Assignees
@lainets
Labels
area: admin Issues related to server administration and service upkeep effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: good first issue Good for newcomers requester: team The issue is raised by a person inside the A+ developer team type: bug This is a bug
Milestone
v1.19

Comments

@lainets
Copy link
Contributor

lainets commented Apr 11, 2023

Saving a new course instance with the required date fields left empty causes an internal server error. Seems like the field validation doesn't check for empty fields.
@markkuriekkinen
Copy link
Collaborator

The crash occurred in the Django admin page for creating new CourseInstances. The code in CourseInstance.clean() clearly tries to use the mandatory starting and ending times before checking if the values are None. The less than comparison with the None value causes a crash.

a-plus/course/models.py

Line 704 in 7e4bd40

if self.ending_time <= self.starting_time:

@markkuriekkinen markkuriekkinen added type: bug This is a bug experience: good first issue Good for newcomers area: admin Issues related to server administration and service upkeep effort: hours Estimated to take less than one day, from the creation of a new branch to the merging requester: team The issue is raised by a person inside the A+ developer team labels Apr 15, 2023
@markkuriekkinen markkuriekkinen added this to the v1.19 milestone Apr 15, 2023
@lainets lainets self-assigned this May 30, 2023
lainets added a commit to lainets/a-plus that referenced this issue May 30, 2023
@lainets
Check that starting and ending times are not None
7e0d543 
Closes apluslms#1174.
lainets added a commit to lainets/a-plus that referenced this issue May 30, 2023
@lainets
Check that starting and ending times are not None
1c5bdc3 
Closes apluslms#1174.
@lainets lainets mentioned this issue May 30, 2023
Merged
3 tasks
markkuriekkinen pushed a commit that referenced this issue May 30, 2023
@lainets @markkuriekkinen
Check that starting and ending times are not None
4db0acb 
Closes #1174.
markkuriekkinen pushed a commit to markkuriekkinen/a-plus that referenced this issue Jun 2, 2023
@lainets @markkuriekkinen
Check that starting and ending times are not None
e4b63b1 
Closes apluslms#1174.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lainets lainets

Labels
area: admin Issues related to server administration and service upkeep effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: good first issue Good for newcomers requester: team The issue is raised by a person inside the A+ developer team type: bug This is a bug
Projects
A+ sprints
Status: Done
Milestone
v1.19
Development

Successfully merging a pull request may close this issue.

Check that starting and ending times are not None lainets/a-plus
2 participants
@markkuriekkinen @lainets