feat: add batch unread endpoint #1212
Conversation
|
Size Change: +753 B (0%) Total Size: 329 kB
|
JimmyPettersson85
requested review from
vishalnarkhede,
santhoshvai,
khushal87,
szuperaz,
oliverlaz,
arnautov-anton and
MartinCupela
as code owners
| * @return {<GetUnreadCountBatchAPIResponse>} | ||
| */ | ||
| async getUnreadCountBatch(userIDs: string[]) { | ||
| return await this.post<GetUnreadCountBatchAPIResponse>(this.baseURL + '/unread_batch', { user_ids: userIDs }); |
There was a problem hiding this comment.
@JimmyPettersson85 just out of curiosity, why for a GET request is used POST method?
There was a problem hiding this comment.
For future proofing the backend we chose POST for this endpoint as we might hit querystring limits if we increase the limit of number of users. Currently the limit is 100 user IDs, and with a max length of 255 for a user ID that means a querystring of ~25k chars.
In going with POST we can handle > 100 user IDs in the future as well as support compression.
src/types.ts
Outdated
| @@ -513,6 +513,10 @@ export type GetUnreadCountAPIResponse = APIResponse & { | |||
| total_unread_count: number; | |||
| }; | |||
|
|
|||
| export type GetUnreadCountBatchAPIResponse = APIResponse & { | |||
| counts_by_user: GetUnreadCountAPIResponse[]; | |||
There was a problem hiding this comment.
Should this be a map (object), instead of an array? AFAIK, the key of the map is the user id
I think this should work (not tested)
| counts_by_user: GetUnreadCountAPIResponse[]; | |
| counts_by_user: { [string]: GetUnreadCountAPIResponse }; |
There was a problem hiding this comment.
good catch, the other parts of this code seems to be using Record so maybe
counts_by_user: Record<string, GetUnreadCountAPIResponse>;?
There was a problem hiding this comment.
Yeah probably, but my Typescript-foo isn't strong enough to say for sure 😆 @MartinCupela probably knows
There was a problem hiding this comment.
or @vishalnarkhede, help us Go devs out 😄
There was a problem hiding this comment.
Jumping in here... counts_by_user: Record<string, GetUnreadCountAPIResponse>; should be good.
But probably it will be better if we are a bit more explicit and have:
{ [userId: string]: GetUnreadCountAPIResponse }
There was a problem hiding this comment.
thanks for the input, pushed a fix for this
CLA
Description of the changes, What, Why and How?
add support for the new batch unread endpoint