Fix Add Course button to use Department and Professor models

[btbonval]

School is a nullable foreign key transitioning out of the Course model.

Unfortunately we haven't updated the Add Course button to make use of the Department model in Course which is replacing School's functionality. Javascript should prevent any problems, but it looks like a possible spammer added a course without a school (likely bypassing the javascript).

Course's "get_absolute_url" relies on self.school.slug, so if there is no school, that line crashes. I installed a hotfix on production which tries that, and upon error, makes the absolute url /course/<course-slug> since the school slug in /<school-slug>/<course-slug>/ is not meaningful (we ignore it).

The correct "fix" is to support adding the school by way of department. If no department is specified, each school may have an empty department for the time being, but the empty department will still tie together a course to a school.

There is also a professor model which exists but is not used by Add Course. This shouldn't break things, but we are transitioning away from professor_email and professor_name inside Course.

[btbonval]

Once school is chosen, a dropdown / custom field for Department would be good, showing all departments registered for the school. It should accept custom written text, unlike School does. Upon a novel department, associate it backwards with the school.

This will take some more JSON hook things.

[btbonval]

This basically sounds like what we want for School / Department / Course: https://github.com/dragonx/django-hier-ajax-form

[btbonval]

The above link looked a bit ... fresh. Found this for using AJAX to subselect on a form based on other selections: https://pypi.python.org/pypi/django-clever-selects/0.6.2 with code here: https://github.com/PragmaticMates/django-clever-selects which seems to be a rip of https://github.com/s-block/django-chained-selectbox

[btbonval]

Trying to see if the above is supported in Django 1.5 (as it claims to be tested in 1.4.5), found this more active thing that does ForeignKey UI stuff through AJAX (which could also accomplish what this ticket looks to do): https://github.com/crucialfelix/django-ajax-selects

[btbonval]

AJAX Selects looks a little more generic. It won't do hierarchical stuff out of the box, but each field can independently do partial-search lookups and so forth. Once something is selected, other fields are populated.

This should work well enough to select school, then do a similar department lookup with a hook to select by the selected school. Worth a try.

[btbonval]

Probably need to change this as well: https://github.com/FinalsClub/karmaworld/blob/06f8f7a5f500458afac830b7d082e68542e8af59/karmaworld/templates/partial/filepicker.html#L18

[charlesconnell]

Can't we just write a couple of custom endpoints like we've been doing? This seems like overkill.

[btbonval]

We'll talk about it tomorrow. There are many ways we could do this, but by saying "custom endpoints", what I hear you say is "shouldn't we just write this whole thing by hand?" The hierarchical stuff would make use of AJAX endpoints. The difference is we don't have to write it by hand quite to the same degree. Instead of overkill, it should be easier to leverage someone else's work.

On Mon, Jan 27, 2014 at 3:24 PM, Charles Connell notifications@github.comwrote:

Can't we just write a couple of custom endpoints like we've been doing? This seems like overkill.

— Reply to this email directly or view it on GitHubhttps://github.com/FinalsClub/karmaworld/issues/294#issuecomment-33417684 .

[btbonval]

We forgot to talk about this today :(

We should talk about this at some point: I don't understand the implication of why doing this would be overkill. It'll be part of a pretty major overhaul with respect to Filepicker API stuff, so we should make sure everyone's on board before I begin to cut in.

[charlesconnell]

All this ticket asks for is that we make the form on the homepage work correctly. Said form uses some AJAX endpoints in courses/views.py, and adding a department field would probably require writing one new endpoint. Let's just get the homepage working correctly before doing anything fancy.

[btbonval]

Ahh I see the problem now.

So beforehand, all we needed was "School" which had to exist in the database.

But now we need to have both "School" (which still needs to be in the database) as well as "Department" (which must be associated with the school). There are two fields which are not independent from each other.

If we don't do it right the first time, we could easily have the same department added a bunch of times to a school, once for each course. So take "EECS" department at MIT. If users created 4 courses within the EECS department at MIT, we want to make sure that EECS is a single department in the database, not four different departments each associated with MIT and a single course.

[btbonval]

So yeah, we need endpoints for School and Department. Easy.

But we need the front end to be intelligent about handling the dependence between School and Department. That's new. That's more than just adding endpoints. It's a bit of extra validation code which needs to go somewhere.

What I've done is found a few different Django bits that can simplify that dependence checking for us, so we don't have to write crazy endpoint logic to handle the dependence.

[btbonval]

It looks like django-ajax-selects should be good for replacing hand-written AJAX autocomplete. Something a bit more custom (like three lines of code) might be needed for adding hierarchy.

Unfortunately it looks like in order to use any of these solutions we'll need to switch over to using Django Forms rather than the hand-crafted HTML fields presently in use in the Add Course partial template. That's going to take more time.

[btbonval]

django-ajax-selects actually looks like a perfect replacement for the current School autocomplete, and with a custom LookupChannel, it should be easy to find all Departments associated with a school AND autocomplete those (or just make it a dropdown).

Instructions for django-ajax-selects do not indicate how to include its javascript. It's easy enough to do by hand, but it would be nice to know that one is supposed to do it by hand as opposed to some template doohickery.

[btbonval]

No javascript? https://github.com/crucialfelix/django-ajax-selects/blob/develop/example/templates/search_form.html

But... pretty sure this needs to be included: https://github.com/crucialfelix/django-ajax-selects/blob/develop/ajax_select/static/ajax_select/js/ajax_select.js

[btbonval]

It's added to the form Media, which explains the example. https://github.com/crucialfelix/django-ajax-selects/blob/afcb7bc24596fae7e3d0a15874fbba6c58b069d7/ajax_select/fields.py#L25

I'm guessing since I'm using just a single field rather than a full form, I don't get magical js import injection. They should mention that in the README: https://github.com/crucialfelix/django-ajax-selects/tree/master#formspy

[btbonval]

Just toss this in any ol' place: {{ course_form.media }}. It'll inherit the media of all fields and stuff, so that works. I guess it isn't documented because its one of those Django things that normal Django people just know to do. Who has two thumbs and isn't a Django guy? This guy. You can't see it, but I indeed have two thumbs pointing at myself.

[btbonval]

AJAX 403 Forbidden. Nothing in the error logs. I wonder if this is a CSRF problem.

[btbonval]

Used CSRF trap as before: https://github.com/FinalsClub/karmaworld/issues/324#issuecomment-34404100

403 does not appear to be CSRF.

[btbonval]

Logged in with an admin account. 403 is now 200. Okay, so need to flip on anonymous access to ajax-selects methods.

Also, completion isn't happening. Keeps returning empty set. Check the field.

[btbonval]

ajax-selects version:

# make_ajax_field(Model, 'field', 'channel')
schoolajax = make_ajax_field(Course, 'school', 'school')
...
+AJAX_LOOKUP_CHANNELS = {
+    #  simple: search School.objects.filter(alias__icontains=q)
+    'school': { 'model': 'courses.school', 'search_field': 'alias' },

hand written version:

matching_school_aliases = list(School.objects.filter(alias__icontains=_query))
matching_school_names = list(School.objects.filter(name__icontains=_query)[:20])

I see, there are two fields being searched against, not just one. The school I was looking for doesn't have an alias.

karmaworld=> SELECT count(*) FROM courses_school WHERE alias IS NOT null;
 count 
-------
     0
(1 row)

Yeah, no schools have an alias. So that's pointless to check. Switching to name... Boom! Looks almost the same, but it returns "School <ID>: <name>" whereas the hand written one returns "<name>" from the dropdown selection list.

[btbonval]

Here's where the Anonymous bit comes in (good docs!): https://github.com/crucialfelix/django-ajax-selects/tree/master#check_authselfrequest

[btbonval]

That fixed easily enough with a custom lookup. Alright, now for a department lookup which also needs to know the school... that might be harder. How does one pass other options in?

[btbonval]

One does not simply pass options into Mordor. https://github.com/crucialfelix/django-ajax-selects/pull/13

[btbonval]

Add Course has become one form to input multiple models:

• Course (create a new)
• Department (old or new)
• School (old only)
• Professor (old or new)

The form will get deconvolved into one to three new objects, and then link all the chosen objects together in the database.

A ModelForm is no longer appropriate. FormSet appears to repeat the same Form over and over. It looks like in order to encapsulate all these models into a single form, we need the most basic Django Form.

[btbonval]

wow. Did some random searching around on the internets instead of just Github and found this for hierarchical selects: https://github.com/digi604/django-smart-selects

It's about as popular as django-ajax-selects, but a little more stale. More info about it in English: http://www.nickbewley.com/development/chained-select-boxes-in-django/

django-ajax-selects is pretty spiffy for autocomplete while django-smart-selects will generate new fields with limited dropdowns based on previous values.

In this case we'd want to keep on with School being django-ajax-selected, but from that field, Department and Professor are generated by django-smart-selects based on the school.

Course is completely new, so there's no need to chain it or anything.

[btbonval]

Things to do.

1. convert Course ModelForm into a generic Form
2. make sure School still autocompletes via make_ajax_field of django-ajax-selects
3. rewrite ForeignKeys to ChainedForeignKeys using django-smart-selects
4. write Department and Professor into the Form using ChainedModelChoiceField of django-smart-selects

[btbonval]

These two packages might not get along so great, but there is a workaround: https://github.com/digi604/django-smart-selects/issues/32

[btbonval]

The general wisdom, dating back to 2008, is not to even try smooshing multiple models into one form. Instead, make a bunch of ModelForms and display them in the same template, then process the results in a single view. http://collingrady.wordpress.com/2008/02/18/editing-multiple-objects-in-django-with-newforms/

This will work consistently with django-ajax-selects.

It is unclear if django-smart-selects will work across different Forms. The docs are not so good on this project. Will have to read source code, find examples, or just try it. Although django-smart-selects has a lot of street cred, it might be worth looking back at the newer things mentioned earlier (https://github.com/FinalsClub/karmaworld/issues/294#issuecomment-33002344)

[btbonval]

django-smart-selects uses ChainedModelChoiceField which calls the widget ChainedSelect. There's no reason to think this can't bunny hop through models, but it looks like ChainedSelect creates chained forms on the fly. The behavior was described as adding fields as older fields were selected. This isn't really the desired behavior: it lacks control over newly created fields.

[AndrewMagliozzi]

Hey Bryan,

On prod, several schools have aliases. UMass, MIT, etc.

We should make sure to auto-complete on those too.

On Feb 7, 2014, at 11:59 PM, Bryan Bonvallet notifications@github.com wrote:

ajax-selects version:

make_ajax_field(Model, 'field', 'channel')

schoolajax = make_ajax_field(Course, 'school', 'school') ... +AJAX_LOOKUP_CHANNELS = {

• simple: search School.objects.filter(alias__icontains=q)

• 'school': { 'model': 'courses.school', 'search_field': 'alias' }, hand written version:

matching_school_aliases = list(School.objects.filter(alias__icontains=_query)) matching_school_names = list(School.objects.filter(name__icontains=_query)[:20]) I see, there are two fields being searched against, not just one. The school I was looking for doesn't have an alias.

karmaworld=> SELECT count(*) FROM courses_school WHERE alias IS NOT null;

count

 0

(1 row) Yeah, no schools have an alias. So that's pointless to check. Switching to name... Boom! Looks almost the same, but it returns "School : " whereas the hand written one returns "" from the dropdown selection list.

— Reply to this email directly or view it on GitHub.

[btbonval]

@AndrewMagliozzi Yeah, I can add alias back. We don't have any way to programmatically generate those aliases. It seems that they are curated by hand?

[AndrewMagliozzi]

Yes, I did them by hand. In the future, we could allow users to add them, but it's not a priority.

On Feb 8, 2014, at 2:47 PM, Bryan Bonvallet notifications@github.com wrote:

@AndrewMagliozzi Yeah, I can add alias back. We don't have any way to programmatically generate those aliases. It seems that they are curated by hand?

— Reply to this email directly or view it on GitHub.

[btbonval]

Wrote code so that alias is checked along with name.

Also made a neat little thing to ease use of django-ajax-selects. You're supposed to write a list of AJAX handlers into the settings file in order to identify which AJAX channels are handled by which handler. I wrote a class decorator instead. Specify the channel in the decorator and decorate the AJAX handler directly. It's so much more clear. Not sure why they don't have that little ditty upstream but it works.

[btbonval]

django-smart-selects does not work across Models. In this case, Course would need to have both School and Department with FKs and also Department would need an FK to School. This is extraneous from a relational schema point of view, it wouldn't be done following normal forms. It is seen in the README example and another example:

• https://github.com/digi604/django-smart-selects/blob/master/README.md#chained-selects
• http://www.nickbewley.com/development/chained-select-boxes-in-django/

In both examples, the FKs are redundant.

• Location FKs to Area, Area FKs to Country, Country FKs to Content. By the transitive property of logic (and relational algebra), location only needs a FK to Area and both Country and Continent are uniquely defined; yet Location has FKs to Country and Continent (this is redundant).
• Phone FKs to Make, Make FKs to Model, Model FKs to Series. Once again, Model and Series are uniquely identified through Phone's Make FK. Yet, Phone FKs to Model and Series as well.

Either I'm missing something vital, or django-smart-selects only functions on really poor database designs. I think I'm going to have to give this module a pass and look into the other choices.

[btbonval]

django-smart-selects has been abandoned. I'm going to give django-clever-selects a quick try, but otherwise it'll be down to writing custom AJAX endpoints and junk. For a common problem, hierarchical/chained selection is surprisingly not well tackled by the Django community.

[btbonval]

django-clever-selects isn't reducing as much redundant code as I'd like, and also it doesn't seem to make the AJAX callbacks anyway. So there's that.

Welp, can't say I didn't try. So now I have to write this by hand. I really wish I could pass more information from the form back to django-ajax-selects, because that'd make this an absolute snap.

[btbonval]

Check out how django-ajax-selects triggers the field made by make_ajax_field. Basically all the code would be the same for channels and LookupChannels. The query comes from the triggered field, call it field A, and then the result of the LookupChannel populates field A.

If the triggered field could be switched to field B instead, then field B's information becomes the query for the LookupChannel, which can perform a FK search, and the returned data would populate field A.

[btbonval]

Super dug into this.

The text input field is triggered using the jQuery UI Autocomplete Widget. http://api.jqueryui.com/autocomplete/

As the user types in stuff, the Autocomplete Widget checks against source, which is the AJAX endpoint that performs the query and returns a response. The response is then mapped into a dropdown which the user selects from. The user selection prompts select, which maps back to django-ajax-selects code, which then populates a hidden "deck".

There won't be any triggering field A and populating field B, because the Autocomplete Widget handles all that on one input.

However, the deck supports triggers which can be written into the template. When an item is added to the deck, it's primary key and some kind of serialized object are returned with the triggered event.

The Autocomplete Widget's source can be written to take use a JS function that passes in the query. A small function could be written to submit an AJAX call with the query and the aforementioned primary key, returning a reduced set based on the foreign key and query. This function could be assigned upon the triggering of the dependent field's deck and use the event contents.

I might be able to extend the django-ajax-selects to make this a fairly simple process and add the functionality for everybody upstream. Honestly, that would probably be easier than writing this by hand just one time for this one case.

[btbonval]

Fun fact: to change an autocomplete source (as per the solution above), one does not change the source attribute on the DOM object. http://stackoverflow.com/questions/3399139/jquery-possible-to-dynamically-change-source-of-autocomplete-widget#3399169

Gotta change the autocomplete attribute. So that should help things along.

I guess it is sort of clear in the documentation if one read's the examples. Somehow I missed that. http://api.jqueryui.com/autocomplete/#method-option

[btbonval]

First successful dependent query. The code is fairly simple right now, but figuring it out was not.

Need to see where I can trap the form response. I've been in AJAX land for so long I forgot what Django does about this stuff upon submit.

[btbonval]

I might have accidentally made this so it will cross forms in the browser... Yeah, it's by HTML element id, so I can put separate School, Department, Professor, and Course forms all together and have them all feed back and forth on the client.

[btbonval]

It looks like course and instructor name make calls back to the server when creating a Course, but the server never actually returns anything. Now that I've mucked with the form, the server returns a bunch of 400 bad requests. Since it doesn't seem to be functional anyway, I'm very tempted to strip that off so I can get this done.

[btbonval]

Nevermind, those random AJAX calls don't hurt anything (besides bandwidth).

Okay so now I'm at a weird spot. I'd like to be able to enter arbitrary strings into the Department field, but I got bounced with an error message saying Department is required. This is probably due to how django-ajax-selects' AutoComplete stuff works. Much like the hand written stuff, there are some hidden fields that keep track of the autocompleted data. It'd be great if there was a fallback to push the string instead of the hidden field if there is no hidden field data. It's probably an option I need to find.

Unfortunately, I have a flight in about 12 hours. I put in over 30 hours this week trying to get this thing done before I flew out, but I think it's going on hiatus until I get back into town.

[btbonval]

oh I deleted too much AJAX handling. Meant to nuke the old school_list stuff and nailed instructor and course name AJAX. Still not sure what it does.

[btbonval]

From an email I sent on Feb 10:

Problems and Yet to Dos:

1. Add Course is no longer just changing the Course model and reading from the School model. It will be reading from the school model, changing the Course, Department, and Professor models. Still a lot of work left to do here, although the django-ajax-selects-cascade will use DOM IDs, so it can autocomplete based on fields from other forms. this will be insanely useful for having a ModelForm for each Model, but still having the forms interconnected.
2. write a form_valid to read in all these crazy models and connect them or create them or do all that stuff based on a lot of conditional circumstances (does the department already exist or is it new? does the prof exist or is (s)he new?). I'm hoping most of this can be simplified by step 1 using separate ModelForms.
3. I have to figure out how to handle the case when a thing doesn't autocomplete, which indicates a new thing should be created. I noticed some of the school_list javascript had TODOs written about how to prompt for a new school if one isn't found. Again, i'm hoping most of this can be simplified by step 1 using separate ModelForms.

[btbonval]

Revising the above to dos now that I understand what's going on here:

1. Autocomplete across forms using django-ajax-selects-cascade.
2. form_valid will work if no new objects need to be created besides the one course.
3. Use form.clean() on each ModelForm can check for certain errors (like "missing department"). I could assume a missing department really means make a new one. A problem might arise that ModelForm.clean() can only access the data for its particular model, thus will not know which School was chosen which is needed to associate with the Department.

There might need to be some back and forth between form_valid and clean, which to me indicates this is the wrong approach. Check out Django docs to see if there's something that covers this select-or-create use case a little more concisely than writing some ugly hack.

[btbonval]

The heart of this problem can be described simply: there are multiple ModelForms which all submit to a single view with a single form_valid/form_invalid to do any special processing.

The Forms assume preexisting data will be selected when using ModelFields, so any individual form which breaks that assumption will lead the view to form_invalid.

A possible answer is to override the ModelField with a TextField (all the AJAX autocomplete should work just the same), and then perform select-or-create based off the text field within form_valid of the view. There must be a nice way to do this sort of select-or-create against a ModelField, and hopefully Django already has something to offer.

An alternate answer is to create a Django super Form which inherits from each ModelForm, thus exposing all attributes of all models to the singular clean() method for the super Form. This seems like a hack and I don't think anyone would want to inherit such a hack.

[btbonval]

Oh, we could have a hidden "Create" field for each model that may be created (prof and department). Upon model failure, we could send back the form saying "these don't exist, if you submit again it will be created" with the sent back form having the create checkbox ticked.

That doesn't solve any of the above problems, but it does add a layer of user supervision.

[btbonval]

Here's one example of multiple forms against one view. The example pretty much side steps Django views entirely, instead operating on the forms and manipulating the raw request. Basically they're running a state machine with 3 states (initial: no form, first form submit, final: second form submit) to handle the serial process of filling out some complicated form. http://stackoverflow.com/questions/17050302/django-multiple-forms-1-view-form-errors-issue

Example snippet to take in two forms with one view, using legit Django classes. However, this case submits each form independently and separately. It does show a way to write logic like the previous URL into well founded Django classes. http://chriskief.com/2012/12/30/django-class-based-views-with-multiple-forms/

Here's a disembodied example of using Django to store one ModelForm's newly created object into another form. This really contains some of the logic that should make its way into the code. http://blog.byjakt.com/multiple-django-forms-in-one-form.html

Alright, with those examples of common practice and good Django use, the problems I had above might be a little more straight forward.