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

Check for missing parent object in confidence score computation #1006

Merged
merged 1 commit into from
Sep 25, 2017

Conversation

dianashk
Copy link
Contributor

@dianashk dianashk commented Sep 25, 2017

We have been experiences a significant number of these errors recently.

500 Cannot read property 'region_a' of undefined

screen shot 2017-09-25 at 12 49 17 pm

For example: /v1/search?debug=true&layers=neighbourhood&text=cuba, new me

screen shot 2017-09-25 at 12 47 22 pm

@dianashk dianashk self-assigned this Sep 25, 2017
@dianashk dianashk added the bug label Sep 25, 2017
Copy link
Member

@orangejulius orangejulius left a comment

Choose a reason for hiding this comment

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

ahh, yeah. this sort of error probably still lives all over our codebase

res += propMatch(text.state, (hit.parent.region_a ? hit.parent.region_a[0] : null), true);
res += propMatch(text.country, (hit.parent.country_a ? hit.parent.country_a[0] :null), true);
res += propMatch(text.state, ((hit.parent && hit.parent.region_a) ? hit.parent.region_a[0] : null), true);
res += propMatch(text.country, ((hit.parent && hit.parent.country_a) ? hit.parent.country_a[0] :null), true);
Copy link
Member

@orangejulius orangejulius Sep 25, 2017

Choose a reason for hiding this comment

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

subtle hint to @trescube that refactoring all this code to use lodash _.get would be amazing :P

Copy link
Contributor Author

@dianashk dianashk Sep 25, 2017

Choose a reason for hiding this comment

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

indeed, not only a hint to @trescube but rather to the whole team

Copy link
Member

Choose a reason for hiding this comment

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

I singled out @trescube because i know he has no greater joy in life than refactoring code to use lodash :P

@dianashk dianashk merged commit 2ea514f into master Sep 25, 2017
@orangejulius orangejulius deleted the no-parent-confidence branch November 14, 2017 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants