-
-
Notifications
You must be signed in to change notification settings - Fork 218
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
feat(#9193): add functionality of getting places as pages or async iterables in cht-datasource #9368
feat(#9193): add functionality of getting places as pages or async iterables in cht-datasource #9368
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! I made it though all the implementation code and unit tests (just need to finish looking at the integration tests). But here are some minor comments I have so far!
export const fetchAndFilter = ( | ||
getFunction: (limit: number, skip: number) => Promise<Nullable<Doc>[]>, | ||
filterFunction: (settings: SettingsService, doc: Nullable<Doc>, uuid: string | undefined) => unknown, | ||
settings: SettingsService, | ||
limit: number, | ||
): typeof recursionInner => { | ||
const recursionInner = async ( | ||
currentLimit: number, | ||
currentSkip: number, | ||
currentDocs: Nullable<Doc>[] = [], | ||
): Promise<Page<unknown>> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to the wonders of generic types, we can do something like this:
export const fetchAndFilter = ( | |
getFunction: (limit: number, skip: number) => Promise<Nullable<Doc>[]>, | |
filterFunction: (settings: SettingsService, doc: Nullable<Doc>, uuid: string | undefined) => unknown, | |
settings: SettingsService, | |
limit: number, | |
): typeof recursionInner => { | |
const recursionInner = async ( | |
currentLimit: number, | |
currentSkip: number, | |
currentDocs: Nullable<Doc>[] = [], | |
): Promise<Page<unknown>> => { | |
export const fetchAndFilter = <T extends Doc> ( | |
getFunction: (limit: number, skip: number) => Promise<Nullable<Doc>[]>, | |
filterFunction: (settings: SettingsService, doc: Nullable<Doc>, uuid: string | undefined) => doc is T, | |
settings: SettingsService, | |
limit: number, | |
): typeof recursionInner => { | |
const recursionInner = async ( | |
currentLimit: number, | |
currentSkip: number, | |
currentDocs: T[] = [], | |
): Promise<Page<T>> => { |
And then we do not have to have the as Page<Place.v1.Place>
assertions in the person/place code! The only other required change is that we update the filter
call below to be explicit about the assertion: const newDocs = docs.filter((doc): doc is T => filterFunction(settings, doc, doc?._id));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this would eliminate the use of unknown in the function, I couldn't get this to work without the as Page<Place.v1.Place>
assertions because <T extends Doc>
would mean they have properties from Doc
but Place
and Person
have additional properties for which errors are thrown in other parts of the code.
Co-authored-by: Joshua Kuestersteffen <[email protected]>
Co-authored-by: Joshua Kuestersteffen <[email protected]>
Co-authored-by: Joshua Kuestersteffen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! This should be good to go! 🚀
Description
Added functionality of getting places using pages or as
AsyncGenerator
usingcht-datasource
along with the addition of REST endpoint/api/v1/person
which provides people as pages.Issues: #9239 , #9240 , #9242
Usage:
Query Params:
contact_type
of the place to fetchCode review checklist
Compose URLs
If Build CI hasn't passed, these may 404:
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.