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

Fix crash in autocomplete API for Institutions #2795

Conversation

iulianav
Copy link

@iulianav iulianav commented Sep 26, 2017

Description

I fixed and improved the autocomplete feature for institutions by using
a completion type suggester.

Signed-off-by: Iuliana Voinea [email protected]

Related Issue

Motivation and Context

Checklist:

  • I have all the information that I need (if not, move to RFC and look for it).
  • I linked the related issue(s) in the corresponding commit logs.
  • I wrote good commit log messages.
  • My code follows the code style of this project.
  • I've added any new docs if API/utils methods were added.
  • I have updated the existing documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I fixed and improved the autocomplete feature for institutions by using
a completion type suggester.

Signed-off-by: Iuliana Voinea <[email protected]>
@iulianav
Copy link
Author

#2755

* Adds real INSPIRE use cases to user related fixtures.

* Amends permission tests to use the new fixtures.
@@ -31,7 +31,7 @@ define([
this.dataEngine = new Bloodhound({
name: 'affiliations',
remote: {
url: '/api/institutions?q=affautocomplete:%QUERY*',
url: '/api/institutions?q=affiliation_suggest:%QUERY*',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe, url shoud be, '/api/institutions/_suggest?affiliation=%QUERY'

@@ -31,7 +31,7 @@ define([
this.dataEngine = new Bloodhound({
name: 'affiliations',
remote: {
url: '/api/institutions?q=affautocomplete:%QUERY*',
url: '/api/institutions?q=affiliation_suggest:%QUERY*',
filter: function(response) {
return $.map(response.hits.hits, function(el) { return el });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the hits are probably under, response.affiliation[0].options instead .hits.hits thing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return el.payload maybe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And be sure everything inside https://github.com/inspirehep/inspire-next/pull/2795/files#diff-d5957147cc784ce12f5bc9c0855e7628R47 template is being populated https://github.com/inspirehep/inspire-next/blob/master/inspirehep/modules/records/receivers.py#L335 .Otherwise they should be removed from the template or if necessary should be added to payload.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you know, why map is used here since mapper returns the element itself in the first place. @jmartinm ?

Iuliana Voinea added 3 commits September 26, 2017 13:30
I fixed and improved the autocomplete feature for institutions by using
a completion type suggester.

Signed-off-by: Iuliana Voinea <[email protected]>
Signed-off-by: Iuliana Voinea <[email protected]>
Signed-off-by: Iuliana Voinea <[email protected]>
@jacquerie
Copy link
Contributor

Superseded by #3006.

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

Successfully merging this pull request may close these issues.

5 participants