FinalsClub / karmaworld

KarmaNotes.org v3.0
GNU Affero General Public License v3.0
7 stars 6 forks source link

create a course form hangs on school entry #376

Closed AndrewMagliozzi closed 9 years ago

AndrewMagliozzi commented 9 years ago

if you only type one letter in the school field, then wait three or more seconds, the entire page crashes. It seems like too many entries are being returned from the sql query to the 9000+ colleges in our database.

sethwoodworth commented 9 years ago

This used to be configurable so it only autocompletes after the first 3-5 characters.

On Tue, Sep 30, 2014 at 4:14 PM, Andrew Magliozzi notifications@github.com wrote:

if you only type one letter in the school field, then wait three or more seconds, the entire page crashes. It seems like too many entries are being returned from the sql query to the 9000+ colleges in our database.

— Reply to this email directly or view it on GitHub https://github.com/FinalsClub/karmaworld/issues/376.

btbonval commented 9 years ago

The autocomplete code is no longer hand written, but uses existing code that was written as a ruby gem or something. More eyes on it, more better. Although this is a Django project, so it isn't a Ruby gem, so I don't know what I'm thinking of.

I'll take a look.

AndrewMagliozzi commented 9 years ago

Thanks Brian. It's not a horrible bug but it's not good.

On Oct 1, 2014, at 4:12 PM, Bryan Bonvallet notifications@github.com wrote:

The autocomplete code is no longer hand written, but uses existing code that was written as a ruby gem or something. More eyes on it, more better.

I'll take a look.

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

btbonval commented 9 years ago

Right, this should be handled by django-ajax-selects. Not a gem, but it was an externally written thing which has more eyes on it than our code ever will. https://github.com/FinalsClub/karmaworld/blob/master/requirements.txt#L25

It looks like autocomplete is being written by hand instead of using the django-ajax-selects piece? There is a min-length specified in the AJAX. Here's where the JS is handled for add course: [edit: this actually appears to do nothing at all anymore] https://github.com/FinalsClub/karmaworld/blob/master/karmaworld/assets/js/add-course.js#L35-L104

For the Add Course form, however, django-ajax-selects does appear to be in use: https://github.com/FinalsClub/karmaworld/blob/master/karmaworld/apps/courses/forms.py#L86-L88

I'm unsure what add-course.js does or if it interferes with the school autocomplete of django-ajax-selects. I'm going to move forward assuming django-ajax-selects is in use (which also assumes there's a lot of unused code in the add-course.js file). Here's how to specify a minimum length, via the LookupChannel: https://github.com/crucialfelix/django-ajax-selects/blob/master/README.md#methods-you-can-override-in-your-lookupchannel

School's AJAX LookupChannel is here. There is no min_length option specified in it or its parent class. https://github.com/FinalsClub/karmaworld/blob/master/karmaworld/apps/courses/models.py#L99-L109 https://github.com/FinalsClub/karmaworld/blob/master/karmaworld/apps/courses/models.py#L23-L27

It looks like we need to pass an minLength dictionary option, which gets passed onto jQuery. The documentation says this should be supplied not to the LookupChannel, but to the field in the model's form. https://github.com/crucialfelix/django-ajax-selects/blob/master/README.md#setting-plugin-options

The School is supplied in terms of the Department in the form context. So the autocomplete field is defined here: https://github.com/FinalsClub/karmaworld/blob/master/karmaworld/apps/courses/forms.py#L86-L88

That'd be the place to pass the options.

So something like this should fix the problem:

    school = AutoCompleteSelectField('school_object_by_name',
                                     help_text='',
                                     plugin_options={'minLength': 3}
                                     label=mark_safe('School <span class="required-field">(required)</span>'))
btbonval commented 9 years ago

tested on beta successfully.

btbonval commented 9 years ago

pushed to production heroku server. looks like it's in action.

AndrewMagliozzi commented 9 years ago

awesome. Thanks Bryan!

On Sun, Oct 12, 2014 at 1:41 PM, Bryan Bonvallet notifications@github.com wrote:

pushed to production heroku server. looks like it's in action.

— Reply to this email directly or view it on GitHub https://github.com/FinalsClub/karmaworld/issues/376#issuecomment-58812090 .