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.

Sign up for GitHub

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

Jump to bottom

Feature/mx 1604 wikidata search endpoint #91

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Feature/mx 1604 wikidata search endpoint #91

wants to merge 13 commits into from
+488 −37

Conversation

mr-kamran-ali
Copy link
Contributor

Added

  • /wikidata endpoint to fetch all matching organizations from wikidata

@mr-kamran-ali
Copy link
Contributor Author

Depends on robert-koch-institut/mex-common#210

@mr-kamran-ali mr-kamran-ali added the blocked Waiting for preconditions to resolve label Jun 26, 2024
cutoffthetop
cutoffthetop requested changes Jul 2, 2024
View reviewed changes
Copy link
Contributor

@cutoffthetop cutoffthetop left a comment

Choose a reason for hiding this comment

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

looks nice and clean!

just 2 small things, mex-common version bump, and a main merge please

mex/backend/auxiliary/wikidata.py Outdated Show resolved Hide resolved
mex/backend/auxiliary/wikidata.py Show resolved Hide resolved
@cutoffthetop cutoffthetop removed the blocked Waiting for preconditions to resolve label Jul 8, 2024
cutoffthetop
cutoffthetop requested changes Jul 8, 2024
View reviewed changes
Copy link
Contributor

@cutoffthetop cutoffthetop left a comment

Choose a reason for hiding this comment

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

looks almost good, but i did find a glitch with the pagination though

mex/backend/auxiliary/wikidata.py
Comment on lines +50 to +51
offset=offset,
limit=limit,
Copy link
Contributor

Choose a reason for hiding this comment

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

total is supposed to be the total number of search hits found on wikidata, not the number of returned items, any request-sender could figure that out by itself. (same is true for offset and limit, we don't need to mirror that back.)

e.g. if you'd search for "institute" you might get a couple of hundred hits on wikidata, but we only return limit / up to 100 of them. and a UI would need to know how many there are overall, to display a proper pagination bar.

i know this might complicate this PR a lot and i'm sorry i didn't spot that earlier in the mex-common pr. so at this point, i'd suggest you add a stop-gap comment in this line and create a new ticket stub to return the correct pagination total later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cutoffthetop cutoffthetop cutoffthetop requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@mr-kamran-ali mr-kamran-ali

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@mr-kamran-ali @cutoffthetop