From: Bryan Date: Wed, 12 Mar 2014 08:06:51 +0000 (-0400) Subject: abstracted Course-Department relationship and applied it to ProfessorTaught-Professo... X-Git-Tag: release-20150131~157 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=77f9a28cb624135281829ab2b20c8e54b2d81b6d;p=oweals%2Fkarmaworld.git abstracted Course-Department relationship and applied it to ProfessorTaught-Professor-Course-Department relationship. super button works but does not forward. template not updated. need to use ManyToMany in course instead of ProfessorAffiliation. # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # On branch feature_course_add_with_dept # Your branch is ahead of 'origin/feature_course_add_with_dept' by 1 commit. # # Changes to be committed: # (use "git reset HEAD^1 ..." to unstage) # # modified: forms.py # modified: models.py # modified: views.py # modified: ../../templates/courses/course_detail.html # modified: ../../templates/courses/course_list_entry.html # modified: ../../templates/partial/add_course.html # modified: ../../templates/user_profile.html # new file: ../../utils/forms.py # --- diff --git a/karmaworld/apps/courses/forms.py b/karmaworld/apps/courses/forms.py index 99647cc..66af081 100644 --- a/karmaworld/apps/courses/forms.py +++ b/karmaworld/apps/courses/forms.py @@ -5,54 +5,116 @@ import random from django.conf import settings -from django.forms import ModelForm from django.forms import CharField from django.forms import ValidationError -from django.forms.util import ErrorList +from ajax_select.fields import AutoCompleteField from ajax_select.fields import AutoCompleteSelectField from ajax_select_cascade.fields import AutoCompleteDependentSelectField +# generates DIV errors with appropriate classes +from karmaworld.utils.forms import NiceErrorModelForm +# supports handling autocomplete fields as a model object or a value +from karmaworld.utils.forms import ACFieldModelForm +# supports filling in Foreign Key fields with another ModelForm +from karmaworld.utils.forms import DependentModelForm + from karmaworld.apps.courses.models import Course +from karmaworld.apps.courses.models import School +from karmaworld.apps.courses.models import Professor from karmaworld.apps.courses.models import Department +from karmaworld.apps.courses.models import ProfessorTaught -# Django hard codes CSS attributes into ModelForm returned ErrorList -# https://github.com/django/django/blob/1.5.5/django/forms/util.py#L54-L60 -# https://docs.djangoproject.com/en/1.5/ref/forms/api/#customizing-the-error-list-format -# so this unfortunately doesn't do anything with ModelForms: -# https://docs.djangoproject.com/en/1.5/ref/forms/api/#django.forms.Form.error_css_class -class CSSErrorList(ErrorList): - """ Override ErrorList classes. """ - def as_ul(self, *args, **kwargs): - errorhtml = super(CSSErrorList, self).as_ul(*args, **kwargs) - # insert ') - # replace hard coded "errorlist" with something in our CSS: - errorhtml = errorhtml.replace('errorlist', 'validation_error') - return errorhtml - - -class NiceErrorModelForm(ModelForm): - """ By default use CSSErrorList for displaying errors. """ - def __init__(self, *args, **kwargs): - if 'error_class' not in kwargs: - kwargs['error_class'] = CSSErrorList - super(NiceErrorModelForm, self).__init__(*args, **kwargs) +class ProfessorForm(NiceErrorModelForm, ACFieldModelForm): + """ Find or create a Professor. """ + # AutoCompleteField would make sense for these fields because it only + # returns the value while AutoCompleteSelectField returns the object. + # This way, Javascript on the front end can autofill the other field based + # on the autocompletion of one field because the whole object is available. + # first argument is ajax channel name, defined in models as LookupChannel. + name = AutoCompleteSelectField('professor_object_by_name', help_text='', + label="Professor's name", + # allows creating a new Professor on the fly + required=False) + email = AutoCompleteSelectField('professor_object_by_email', help_text='', + label="Professor's email address", + # allows creating a new Professor on the fly + required=False) -class CourseForm(NiceErrorModelForm): + class Meta: + model = Professor + # order the fields + fields = ('name', 'email') + + def _clean_distinct_field(self, field, *args, **kwargs): + """ + Check to see if Professor model is found before grabbing the field. + Ensure that if Professor was already found, the new field corresponds. + + In theory, Javascript could and should ensure this. + In practice, better safe than incoherent. + """ + oldprof = None + if hasattr(self, 'instance') and self.instance: + # Professor was already autocompleted. Save that object. + oldprof = self.instance + # Extract the field value, possibly replacing self.instance + value = self._clean_field(field, *args, **kwargs) + if oldprof and not value: + # This field was not supplied, but another one determined the prof. + # Grab field from prof model object. + value = getattr(oldprof, field) + if oldprof and self.instance != oldprof: + # Two different Professor objects have been found across fields. + raise ValidationError('It looks like two or more different Professors have been autocompleted.') + return value + + def clean_name(self, *args, **kwargs): + return self._clean_distinct_field('name', *args, **kwargs) + + def clean_email(self, *args, **kwargs): + return self._clean_distinct_field('email', *args, **kwargs) + + +class DepartmentForm(NiceErrorModelForm, ACFieldModelForm): + """ Find and create a Department given a School. """ # first argument is ajax channel name, defined in models as LookupChannel. - school = AutoCompleteSelectField('school', help_text='') - department = AutoCompleteDependentSelectField( - 'dept_given_school', help_text='', + school = AutoCompleteSelectField('school_object_by_name', help_text='') + # first argument is ajax channel name, defined in models as LookupChannel. + name = AutoCompleteDependentSelectField( + 'dept_object_by_name_given_school', help_text='', + label='Department name', # autocomplete department based on school - dependsOn=school, + dependsOn=school, # allows creating a new department on the fly required=False ) + class Meta: + model = Department + # order the fields + fields = ('school', 'name') + + def clean_name(self, *args, **kwargs): + """ + Extract the name from the Department object if it already exists. + """ + name = self._clean_field('name', *args, **kwargs) + # this might be unnecessary + if not name: + raise ValidationError('Cannot create a Department without a name.') + return name + + +class CourseForm(NiceErrorModelForm, DependentModelForm): + """ A course form which adds a honeypot and autocompletes some fields. """ + # first argument is ajax channel name, defined in models as LookupChannel. + # note this AJAX field returns a field value, not a course object. + name = AutoCompleteField('course_name_by_name', help_text='', + label="Course name") + def __init__(self, *args, **kwargs): """ Add a dynamically named field. """ super(CourseForm, self).__init__(*args, **kwargs) @@ -65,39 +127,17 @@ class CourseForm(NiceErrorModelForm): class Meta: model = Course # order the fields - fields = ('school', 'department', 'name', 'instructor_name', - 'instructor_email', 'url') - - def clean_department(self, *args, **kwargs): - """ Create a new department if one is not provided. """ - if 'department' not in self.cleaned_data or \ - not self.cleaned_data['department']: - # Department is missing. - # Can a new one be made? - if 'school' not in self.cleaned_data or \ - not self.cleaned_data['school']: - raise ValidationError('Cannot create a new department without a school.') - if 'department_text' not in self.data or \ - not self.data['department_text']: - raise ValidationError('Cannot create a new department without a name.') - - # Build a new Department. - school = self.cleaned_data['school'] - dept_name = self.data['department_text'] - dept = Department(name=dept_name, school=school) - dept.save() - - # Fill in cleaned data as though this department were chosen. - self.cleaned_data['department'] = dept - # Return the clean Department - return self.cleaned_data['department'] + fields = ('name', 'url') + # pass department data onto DepartmentForm + model_fields = {'department': DepartmentForm} def clean(self, *args, **kwargs): - """ Additional form validation. """ + """ Honeypot validation. """ # Call ModelFormMixin or whoever normally cleans house. cleaned_data = super(CourseForm, self).clean(*args, **kwargs) + # Check the honeypot # parts of this code borrow from # https://github.com/sunlightlabs/django-honeypot hfn = settings.HONEYPOT_FIELD_NAME @@ -107,3 +147,17 @@ class CourseForm(NiceErrorModelForm): self._errors[hfn] = [settings.HONEYPOT_ERROR] del cleaned_data[hfn] return cleaned_data + + +class ProfessorTaughtForm(NiceErrorModelForm, DependentModelForm): + """ Create an association between the chosen Professor and Course. """ + + class Meta: + model = ProfessorTaught + # no actual fields + fields = tuple() + # pass professor and course data onto appropriate forms. + model_fields = { + 'professor': ProfessorForm, + 'course': CourseForm, + } diff --git a/karmaworld/apps/courses/models.py b/karmaworld/apps/courses/models.py index f04cf1e..ab21977 100644 --- a/karmaworld/apps/courses/models.py +++ b/karmaworld/apps/courses/models.py @@ -20,6 +20,23 @@ from ajax_select_cascade import DependentLookupChannel from ajax_select_cascade import register_channel_name +class AnonLookupChannel(LookupChannel): + def check_auth(self, request): + """ Allow anonymous access. """ + # By default, Lookups require request.is_staff. Don't require anything! + pass + + +class FieldLookupChannel(AnonLookupChannel): + def get_query(self, q, request): + """ + Case insensitive contain search against the given field. + Returns model objects with matching field. + """ + kwargs = { str(self.field_lookup) + '__icontains': q } + return self.model.objects.filter(**kwargs) + + class SchoolManager(models.Manager): """ Handle restoring data. """ def get_by_natural_key(self, usde_id): @@ -79,22 +96,17 @@ class School(models.Model): self.save() -@register_channel_name('school') -class SchoolLookup(LookupChannel): +@register_channel_name('school_object_by_name') +class SchoolLookup(AnonLookupChannel): """ - Handles AJAX lookups against the school model's name and value fields. + Handles AJAX lookups against the school model's name and alias fields. """ model = School def get_query(self, q, request): """ Search against both name and alias. """ query = models.Q(name__icontains=q) | models.Q(alias__icontains=q) - return School.objects.filter(query) - - def check_auth(self, request): - """ Allow anonymous access. """ - # By default, Lookups require request.is_staff. Don't require anything! - pass + return self.model.objects.filter(query) class DepartmentManager(models.Manager): @@ -110,7 +122,7 @@ class Department(models.Model): """ Department within a School. """ objects = DepartmentManager() - name = models.CharField(max_length=255) + name = models.CharField(max_length=255, verbose_name="Department name") school = models.ForeignKey(School) # Should this be optional ever? slug = models.SlugField(max_length=150, null=True) url = models.URLField(max_length=511, blank=True, null=True) @@ -140,8 +152,8 @@ class Department(models.Model): super(Department, self).save(*args, **kwargs) -@register_channel_name('dept_given_school') -class DeptGivenSchoolLookup(DependentLookupChannel): +@register_channel_name('dept_object_by_name_given_school') +class DeptGivenSchoolLookup(DependentLookupChannel, AnonLookupChannel): """ Handles AJAX lookups against the department model's name field given a school. @@ -154,13 +166,9 @@ class DeptGivenSchoolLookup(DependentLookupChannel): return Department.objects.filter(name__icontains=q, school__id=dependency) else: + # If no dependency is submit, return nothing. return [] - def check_auth(self, request): - """ Allow anonymous access. """ - # By default, Lookups require request.is_staff. Don't require anything! - pass - class ProfessorManager(models.Manager): """ Handle restoring data. """ @@ -177,8 +185,9 @@ class Professor(models.Model): """ objects = ProfessorManager() - name = models.CharField(max_length=255) - email = models.EmailField(blank=True, null=True) + name = models.CharField(max_length=255, verbose_name="Professor's name") + email = models.EmailField(blank=True, null=True, + verbose_name="Professor's Email") class Meta: """ @@ -198,6 +207,24 @@ class Professor(models.Model): return (self.name,self.email) +@register_channel_name('professor_object_by_name') +class ProfessorLookup(FieldLookupChannel): + """ + Handles AJAX lookups against the professor model's name field. + """ + model = Professor + field_lookup = 'name' + + +@register_channel_name('professor_object_by_email') +class ProfessorEmailLookup(FieldLookupChannel): + """ + Handles AJAX lookups against the professor model's email field. + """ + model = Professor + field_lookup = 'email' + + class ProfessorAffiliationManager(models.Manager): """ Handle restoring data. """ def get_by_natural_key(self, prof, dept): @@ -249,7 +276,7 @@ class Course(models.Model): objects = CourseManager() # Core metadata - name = models.CharField(max_length=255, verbose_name="Course:") + name = models.CharField(max_length=255, verbose_name="Course name") slug = models.SlugField(max_length=150, null=True) # department should remove nullable when school gets yoinked department = models.ForeignKey(Department, blank=True, null=True) @@ -260,7 +287,7 @@ class Course(models.Model): desc = models.TextField(max_length=511, blank=True, null=True) url = models.URLField(max_length=511, blank=True, null=True, - verbose_name="Course URL:") + verbose_name="Course URL") # instructor_* is vestigial, replaced by Professor+ProfessorTaught models. instructor_name = models.CharField(max_length=255, blank=True, null=True) @@ -329,6 +356,24 @@ class Course(models.Model): reversion.register(Course) + +@register_channel_name('course_name_by_name') +class CourseNameLookup(FieldLookupChannel): + """ + Handles AJAX lookups against the course model's name field. + Returns just the matching field values. + """ + model = Course + field_lookup = 'name' + + def get_query(self, q, request): + """ Return only the list of name fields. """ + # Find the matching objects. + results = super(CourseNameLookup, self).get_query(q, request) + # Only return the name field, not the object. + return results.values_list(self.field_lookup, flat=True) + + class ProfessorTaughtManager(models.Manager): """ Handle restoring data. """ def get_by_natural_key(self, prof, course): diff --git a/karmaworld/apps/courses/views.py b/karmaworld/apps/courses/views.py index da86778..6d2b814 100644 --- a/karmaworld/apps/courses/views.py +++ b/karmaworld/apps/courses/views.py @@ -16,7 +16,6 @@ from django.views.generic import TemplateView from django.views.generic.list import ListView from django.views.generic.edit import CreateView -from karmaworld.apps.courses.forms import CourseForm from karmaworld.apps.courses.models import Course from karmaworld.apps.courses.models import School from karmaworld.apps.notes.models import Note @@ -24,6 +23,10 @@ from karmaworld.apps.users.models import CourseKarmaEvent from karmaworld.apps.notes.forms import FileUploadForm from karmaworld.utils import ajax_increment, format_session_increment_field +# ProfessorTaughtForm contains CourseForm +from karmaworld.apps.courses.forms import ProfessorTaughtForm as CourseForm + + FLAG_FIELD = 'flags' USER_PROFILE_FLAGS_FIELD = 'flagged_courses' @@ -63,7 +66,7 @@ class CourseListSubView(ListView): # get the total number of notes context['note_count'] = Note.objects.count() # get the course form for the form at the bottom of the homepage - context['course_form'] = CourseAddFormView.form_class() + context['course_form'] = CourseForm() # Include "Add Course" button in header context['display_add_course'] = True diff --git a/karmaworld/templates/courses/course_detail.html b/karmaworld/templates/courses/course_detail.html index abb9787..8a731da 100644 --- a/karmaworld/templates/courses/course_detail.html +++ b/karmaworld/templates/courses/course_detail.html @@ -25,7 +25,7 @@ {% block title %} - Share Notes for {{ course.name }} | {{ course.school.name }} + Share Notes for {{ course.name }} | {% if course.school %}{{ course.school.name }}{% else %}{{ course.department.school.name }}{% endif %} {% endblock %} {% block content %} @@ -50,7 +50,7 @@
- {{ course.school.name }} + {% if course.school %}{{ course.school.name }}{% else %}{{ course.department.school.name }}{% endif %}
diff --git a/karmaworld/templates/courses/course_list_entry.html b/karmaworld/templates/courses/course_list_entry.html index 18fd2fc..48dd1da 100644 --- a/karmaworld/templates/courses/course_list_entry.html +++ b/karmaworld/templates/courses/course_list_entry.html @@ -21,6 +21,6 @@ - {{ course.school.name }} + {% if course.school %}{{ course.school.name }}{% else %}{{ course.department.school.name }}{% endif %} {{ course.instructor_name }} diff --git a/karmaworld/templates/partial/add_course.html b/karmaworld/templates/partial/add_course.html index 2c2dbee..6b21186 100644 --- a/karmaworld/templates/partial/add_course.html +++ b/karmaworld/templates/partial/add_course.html @@ -13,14 +13,16 @@
{% csrf_token %} - {% for error in course_form.non_field_errors %} + {% if course_form.non_field_errors %}
- {{ error }} +
    {% for error in course_form.non_field_errors %} +
  • + {% endfor %}
- {% endfor %} + {% endif %} {{ course_form.as_p }}
diff --git a/karmaworld/templates/user_profile.html b/karmaworld/templates/user_profile.html index 78c9921..5c5be2b 100644 --- a/karmaworld/templates/user_profile.html +++ b/karmaworld/templates/user_profile.html @@ -93,7 +93,7 @@ {{ event.note.name }}
{% endif %} {% if item.0 == 'CourseKarmaEvent' %} @@ -101,7 +101,7 @@ {{ event.course.name }}
- {{ event.course.school.name }} + {% if event.course.school %}{{ event.course.school.name }}{% else %}{{ event.course.department.school.name }}{% endif %}
{% endif %} diff --git a/karmaworld/utils/forms.py b/karmaworld/utils/forms.py new file mode 100644 index 0000000..296ed2a --- /dev/null +++ b/karmaworld/utils/forms.py @@ -0,0 +1,184 @@ +#!/usr/bin/env python +# -*- coding:utf8 -*- +# Copyright (C) 2012 FinalsClub Foundation + + +import inspect + +from django.forms import ModelForm +from django.forms.util import ErrorList +from django.utils.safestring import mark_safe + + +# Django hard codes CSS attributes into ModelForm returned ErrorList +# https://github.com/django/django/blob/1.5.5/django/forms/util.py#L54-L60 +# https://docs.djangoproject.com/en/1.5/ref/forms/api/#customizing-the-error-list-format +# so this unfortunately doesn't do anything with ModelForms: +# https://docs.djangoproject.com/en/1.5/ref/forms/api/#django.forms.Form.error_css_class +class CSSErrorList(ErrorList): + """ Override ErrorList classes. """ + def as_ul(self, *args, **kwargs): + # It might be more efficient to rewrite this properly rather than string + # hack it. For now, this is more flexible to changes in Django. + errorhtml = super(CSSErrorList, self).as_ul(*args, **kwargs) + # insert ') + # replace hard coded "errorlist" with something in our CSS: + errorhtml = errorhtml.replace('errorlist', 'validation_error') + return errorhtml + + +class NiceErrorModelForm(ModelForm): + """ By default use CSSErrorList for displaying errors and prefix fields. """ + def __init__(self, *args, **kwargs): + if 'prefix' not in kwargs: + # extract class name. this is ugly because Django magic classes. + # use class name as prefix. + kwargs['prefix'] = str(type(self)).split('.')[-1].split("'")[0] + if 'error_class' not in kwargs: + kwargs['error_class'] = CSSErrorList + super(NiceErrorModelForm, self).__init__(*args, **kwargs) + + +class ACFieldModelForm(ModelForm): + """ + Treats an AutoCompleteSelectField as either a field or a model instance. + + Such a field will either find a result or create a new instance to hold the + result, so the concept of a required Field is a bit different. Ensure that + required=False on any AutoComplete*Fields used in this way. + + This should be used in the correct clean function in the subclass like so: + def clean_fieldname(self, *args, **kwargs): + return self._clean_field('fieldname', *args, **kwargs) + """ + def _clean_field(self, field, *args, **kwargs): + """ + Given a form field name, decide if the form field contains a model. + """ + # If the object already exists, its cleaned field will contain an object + modelobject = self.cleaned_data.get(field, None) + if modelobject: + # tell this Form the object already exists in the database + self.instance = modelobject + # return the appropriate value for this field. + return getattr(modelobject, field) + # Otherwise we need to extract the field's value from the form by hand. + return self.data.get(self.add_prefix(field + '_text'), None) + + +class DependentModelForm(ModelForm): + """ + Support sending POST data to other ModelForms and returning their objects. + + Include a model_fields dictionary in Meta, which maps ForeignKey attributes + to a corresponding ModelForm. Assuming a Car model has fields color, year, + engine, and make, with the latter two being FKs that have corresponding + ModelForms, it's Meta class might look like this: + class Meta: + model = Car + fields = ('color', 'year') + model_fields = { 'engine': EngineModelForm, 'make': MakeModelForm } + """ + def __init__(self, *args, **kwargs): + """ Clears cached modelforms. """ + # Prefix is practically required when using multiple forms against the + # same POST data. + if 'prefix' not in kwargs: + # extract class name. this is ugly because Django magic classes. + # use class name as prefix. + kwargs['prefix'] = str(type(self)).split('.')[-1].split("'")[0] + # indicate no dependent form objects have been made + self.dependent_modelforms = {} + self.dependent_modelforms_data = {} + super(DependentModelForm, self).__init__(*args, **kwargs) + + def _get_forms(self, with_data=False): + """ Memoize dependent form objects. """ + data = (self.data,) if with_data else tuple() + memo = self.dependent_modelforms_data if with_data else \ + self.dependent_modelforms + if not memo: + # Any attributes which need a model object (defined in + # Meta.model_fields) will have ModelForms created at this time. + for attribute, modelform in self.Meta.model_fields.iteritems(): + # Mebbe pass POST data into the foreign model form. + memo[attribute] = modelform(*data) + return memo + + def _media(self): + """ Render all dependent form media as well as this form's. """ + # too bad Django doesn't have a nice way to process this? + # prepare base Media object. + superself = super(DependentModelForm, self) + if inspect.ismethod(superself.media): + selfmedia = superself.media() + else: + selfmedia = superself.media + + # search through each dependent form for media + for modelform in self._get_forms().itervalues(): + if inspect.ismethod(modelform.media): + media = modelform.media() + else: + media = modelform.media + # update CSS dict + selfmedia.add_css(media._css) + # uniquely concatenate JS sources + selfmedia.add_js(media._js) + + # send back the results + return selfmedia + + # https://docs.djangoproject.com/en/1.5/topics/forms/media/#media-as-a-dynamic-property + media = property(_media) + + def is_valid(self, *args, **kwargs): + """ Check all subforms for validity and then this form. """ + all_valid = True + # Perform validation and error checking for each ModelForm. + for attribute, modelform in self._get_forms(with_data=True).iteritems(): + if not modelform.is_valid(): + all_valid = False + + # Process this form's validity to generate errors. + # This form is invalid if it is invalid or its subforms are invalid. + return super(DependentModelForm, self).is_valid(*args, **kwargs) and all_valid + + def _post_clean(self, *args, **kwargs): + """ Inject objects created from required ModelForms. """ + super(ModelForm, self)._post_clean(*args, **kwargs) + + # If self.instance has not been created by _post_clean, create it now. + # This happens when only model_fields are present and no fields. + try: + str(self.instance) + except: + self.instance = self.Meta.model() + + # Don't create objects if this form is not valid. + if not self.is_valid(): + return + + for attribute, modelform in self._get_forms(with_data=True).iteritems(): + # create foreign model object and associate it internally here + setattr(self.instance, attribute, modelform.save()) + + def _render_dependencies_first(self, method, *args, **kwargs): + """ Render dependent forms prior to rendering this form. """ + html = '' + # render each form + for modelform in self._get_forms().itervalues(): + html += getattr(modelform, method)(*args, **kwargs) + # render this form + supermethod = getattr(super(DependentModelForm, self), method) + html += supermethod(*args, **kwargs) + return mark_safe(html) + + def as_p(self, *args, **kwargs): + return self._render_dependencies_first('as_p', *args, **kwargs) + def as_ul(self, *args, **kwargs): + return self._render_dependencies_first('as_ul', *args, **kwargs) + def as_table(self, *args, **kwargs): + return self._render_dependencies_first('as_table', *args, **kwargs)