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.

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

create a course form hangs on school entry #376

Closed
AndrewMagliozzi opened this issue Sep 30, 2014 · 7 comments
Closed

create a course form hangs on school entry #376

AndrewMagliozzi opened this issue Sep 30, 2014 · 7 comments

Comments

@AndrewMagliozzi
Copy link
Member

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
Copy link
Member

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
#376.

@btbonval
Copy link
Member

btbonval commented Oct 1, 2014

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
Copy link
Member Author

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
Copy link
Member

btbonval commented Oct 1, 2014

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
Copy link
Member

tested on beta successfully.

@btbonval
Copy link
Member

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

@AndrewMagliozzi
Copy link
Member Author

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
#376 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants