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

contrib: names: update field boosts and mapping #370

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

sakshamarora1
Copy link
Contributor

@sakshamarora1 sakshamarora1 commented Jul 24, 2024

Fixes: inveniosoftware/invenio-app-rdm#2756

The names in the database and index are inverted, therefore the suggest API first tries to search on the last name and prioritizes that due to how edge ngram works for the suggest API.
This solution add suggest API for given name for better results.

ATTENTION

tests fail due to #372, to be rebased and merged after #372

Copy link
Contributor

@jrcastro2 jrcastro2 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a couple of comments.

@sakshamarora1 sakshamarora1 force-pushed the fix/names_vocab branch 2 times, most recently from ac52df8 to 557337c Compare July 24, 2024 15:32
@carlinmack
Copy link
Contributor

carlinmack commented Jul 24, 2024

can you add a test of the suggest endpoint like https://github.com/inveniosoftware/invenio-vocabularies/blob/master/tests/contrib/funders/test_funders_resource.py#L116 :)

@jrcastro2
Copy link
Contributor

ES tests are failing

@sakshamarora1
Copy link
Contributor Author

@jrcastro2 ES tests are removed in #372 😅
Didn't wanna duplicate it here to prevent merge conflicts

Copy link
Contributor

@kpsherva kpsherva left a comment

Choose a reason for hiding this comment

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

looks, good, needs the tests to pass

Copy link
Member

@zzacharo zzacharo left a comment

Choose a reason for hiding this comment

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

@sakshamarora1 there is only one comment on this from @slint. Otherwise, it seems good to go!

@ntarocco ntarocco merged commit 109fdb4 into inveniosoftware:master Aug 20, 2024
4 checks passed
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.

Searching for names when adding creators return inconsistent results
7 participants