-
Notifications
You must be signed in to change notification settings - Fork 7
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
Faster Way To Add Late Starts #285
Changes from 2 commits
50478a7
5aeb97c
2606157
57afa2f
3bc9373
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -266,7 +266,7 @@ def get_events(cls, user=None): | |
return events | ||
|
||
def clean(self): | ||
if self.start_date > self.end_date: | ||
if self.start_date != None and self.end_date != None and self.start_date > self.end_date: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reason for changing this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was mildly annoying me when testing some things. This change can 100% be dropped. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well there should never be an event without a start/end date set. that is enforced both on the model both by the super().clean() and on the database level. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it shouldn't. Honestly, I don't remember exactly why I added that. However, I do know for a fact that if you fill out the event add form and set either one of the dates to be an invalid one, when you submit it would give you a TypeError due to comparing NoneType and a datetime.datetime. That's probably why I added that when I was testing some things, but I should probably just remove this change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's up for handling in the form's logic though, not model's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea but I was just looking where the error happened and changed that. I'll just remove this change then. |
||
raise ValidationError( | ||
{ | ||
"start_date": _("Start date must be before end date"), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
{% extends "admin/base_site.html" %} | ||
{% load i18n admin_urls static admin_modify %} | ||
|
||
{% block extrahead %} | ||
{{ block.super }} | ||
<script src="{% url 'admin:jsi18n' %}"></script> | ||
<script src="{% static "admin/js/core.js" %}"></script> | ||
{{ media }} | ||
{% endblock %} | ||
|
||
{% block extrastyle %} | ||
{{ block.super }} | ||
<link rel="stylesheet" type="text/css" href="{% static "admin/css/forms.css" %}"> | ||
{% endblock %} | ||
|
||
{% if not is_popup %} | ||
{% block breadcrumbs %}<div class="breadcrumbs" style="height:0px;padding:0px"></div>{% endblock %} | ||
{% endif %} | ||
|
||
{% block coltype %}colM{% endblock %} | ||
{% block content %} | ||
<form action="{{ url }}" method="post"> | ||
{% csrf_token %} | ||
{% if form.errors %} | ||
<p class="errornote"> | ||
{% if form.errors|length == 1 %} | ||
{% translate "Please correct the error below." %} | ||
{% else %} | ||
{% translate "Please correct the errors below." %} | ||
{% endif %} | ||
</p> | ||
{% endif %} | ||
<fieldset class="module aligned "> | ||
{% for field in form %} | ||
<div class="form-row field-name"> | ||
{{ field.errors }} | ||
<div> | ||
<div class="flex-container"> | ||
{{ field.label_tag }} {{ field }} | ||
</div> | ||
</div> | ||
</div> | ||
{% endfor %} | ||
</fieldset> | ||
<div class="submit-row"> | ||
<input class="default" type="submit" value="Save"> | ||
</div> | ||
</form> | ||
{% endblock %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be querying organizations by pk instead of name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh, this way is honestly more adaptable as not all db instance will have the same SAC PK... but on this it should be the "School" org
see #285 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean the "school" organization? Is that not just SAC? Also, for creating a school if it doesn't exist, would you just make the owner and execs just the admin currently using the form?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there is a school org that is behind something like a late start or an admin related presentation.
As for the user, just pick the first created superuser (filter by date created and is_superuser).first()