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

[CHAT-1814] Updates to message search #677

Merged
merged 19 commits into from
Jul 13, 2021
Merged

[CHAT-1814] Updates to message search #677

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8207d3e
add channel.searchv2 and client.searchv2 endpoints
miagilepner May 10, 2021
4e984ff
Merge remote-tracking branch 'origin/master' into feat/messages-es
miagilepner May 19, 2021
ec4e53f
Merge remote-tracking branch 'origin/master' into feat/messages-es
miagilepner May 25, 2021
118f916
combine into one search endpoint
miagilepner May 28, 2021
fc84437
Merge remote-tracking branch 'origin/master' into feat/messages-es
miagilepner May 28, 2021
e8626a3
correctly handle sort parameters
miagilepner May 28, 2021
b70378f
Merge branch 'master' into feat/messages-es
miagilepner May 28, 2021
0418a37
Merge remote-tracking branch 'origin/master' into feat/messages-es
miagilepner Jun 7, 2021
f101145
Merge remote-tracking branch 'origin/master' into feat/messages-es
miagilepner Jun 14, 2021
134fe15
Merge remote-tracking branch 'origin/master' into feat/messages-es
miagilepner Jun 23, 2021
a2fa704
Merge branch 'master' into feat/messages-es
miagilepner Jul 1, 2021
ee05ba7
Merge branch 'master' into feat/messages-es
miagilepner Jul 5, 2021
60e91e9
Update src/channel.ts
miagilepner Jul 6, 2021
986edfd
fix
miagilepner Jul 6, 2021
d09614e
fixes
miagilepner Jul 6, 2021
2b221cb
fixes
miagilepner Jul 8, 2021
12277bc
Apply suggestions from code review
miagilepner Jul 9, 2021
a14d3a1
fix types of normalize sort function
miagilepner Jul 9, 2021
8960261
Merge branch 'master' into feat/messages-es
miagilepner Jul 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/channel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
BanUserOptions,
ChannelAPIResponse,
ChannelData,
ChannelFilters,
ChannelMemberAPIResponse,
ChannelMemberResponse,
ChannelQueryOptions,
Expand All @@ -33,6 +34,8 @@ import {
QueryMembersOptions,
Reaction,
ReactionAPIResponse,
SearchOptions,
SearchPayload,
SearchAPIResponse,
SendMessageAPIResponse,
TruncateChannelAPIResponse,
Expand Down Expand Up @@ -299,7 +302,7 @@ export class Channel<
* search - Query messages
*
* @param {MessageFilters<AttachmentType, ChannelType, CommandType, MessageType, ReactionType, UserType> | string} query search query or object MongoDB style filters
* @param {{client_id?: string; connection_id?: string; limit?: number; offset?: number; query?: string; message_filter_conditions?: MessageFilters<AttachmentType, ChannelType, CommandType, MessageType, ReactionType, UserType>}} options Option object, {user_id: 'tommaso'}
* @param {{client_id?: string; connection_id?: string; query?: string; message_filter_conditions?: MessageFilters<AttachmentType, ChannelType, CommandType, MessageType, ReactionType, UserType>}} options Option object, {user_id: 'tommaso'}
*
* @return {Promise<SearchAPIResponse<AttachmentType, ChannelType, CommandType, MessageType, ReactionType, UserType>>} search messages response
*/
Expand All @@ -314,10 +317,9 @@ export class Channel<
UserType
>
| string,
options: {
options: SearchOptions & {
client_id?: string;
connection_id?: string;
limit?: number;
message_filter_conditions?: MessageFilters<
AttachmentType,
ChannelType,
Expand All @@ -326,14 +328,25 @@ export class Channel<
ReactionType,
UserType
>;
offset?: number;
query?: string;
} = {},
) {
// Return a list of channels
const payload = {
filter_conditions: { cid: this.cid },
...options,
const { sort, ...optionsWithoutSort } = { ...options };
Copy link
Contributor

Choose a reason for hiding this comment

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

So sort here is throwing a TS error, this is because only one of the 2 search options types has sort on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think you need to spread options here.

const payload: SearchPayload<
AttachmentType,
ChannelType,
CommandType,
MessageType,
ReactionType,
UserType
> = {
filter_conditions: { cid: this.cid } as ChannelFilters<
ChannelType,
CommandType,
UserType
>,
...optionsWithoutSort,
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
};
if (typeof query === 'string') {
payload.query = query;
Expand All @@ -343,6 +356,9 @@ export class Channel<
throw Error(`Invalid type ${typeof query} for query parameter`);
}

if (sort) {
payload.sort = normalizeQuerySort(sort);
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
}
// Make sure we wait for the connect promise if there is a pending one
await this.getClient().wsPromise;

Expand Down
9 changes: 6 additions & 3 deletions src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ import {
PermissionAPIResponse,
PermissionsAPIResponse,
ReactionResponse,
SearchAPIResponse,
SearchOptions,
SearchPayload,
SearchAPIResponse,
SendFileAPIResponse,
StreamChatOptions,
TestPushDataInput,
Expand Down Expand Up @@ -1779,7 +1779,7 @@ export class StreamChat<
>,
options: SearchOptions = {},
) {
// Return a list of channels
const { sort: sort_value, ...options_without_sort } = { ...options };
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in channel but also we can probably avoid the rest spread as well, and pop everything to one line via

      sort: sort &&= normalizeQuerySort(sort_value)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure what you want to do here. Which lines will be replaced with your suggestion?

const payload: SearchPayload<
AttachmentType,
ChannelType,
Expand All @@ -1789,7 +1789,7 @@ export class StreamChat<
UserType
> = {
filter_conditions: filterConditions,
...options,
...options_without_sort,
};
if (typeof query === 'string') {
payload.query = query;
Expand All @@ -1798,6 +1798,9 @@ export class StreamChat<
} else {
throw Error(`Invalid type ${typeof query} for query parameter`);
}
if (sort_value) {
payload.sort = normalizeQuerySort(sort_value);
}

// Make sure we wait for the connect promise if there is a pending one
await this.setUserPromise;
Expand Down
37 changes: 34 additions & 3 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,8 +649,17 @@ export type SearchAPIResponse<
UserType
>;
}[];
next?: string;
previous?: string;
results_warning?: SearchWarning;
};

export type SearchWarning = {
channel_search_cids: string[];
channel_search_count: number;
warning_code: number;
warning_description: string;
};
export type SendFileAPIResponse = APIResponse & { file: string };

export type SendMessageAPIResponse<
Expand Down Expand Up @@ -935,11 +944,19 @@ export type QueryMembersOptions = {
user_id_lte?: string;
};

export type SearchOptions = {
export type SearchOptions = LimitOffsetSearchOptions | NextSearchOptions;

export type LimitOffsetSearchOptions = {
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
limit?: number;
offset?: number;
};

export type NextSearchOptions = {
limit?: number;
next?: string;
sort?: SearchMessageSort;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export type SearchOptions = LimitOffsetSearchOptions | NextSearchOptions;
export type LimitOffsetSearchOptions = {
limit?: number;
offset?: number;
};
export type NextSearchOptions = {
limit?: number;
next?: string;
sort?: SearchMessageSort;
};
export type LimitOffsetSearchOptions = {
limit?: number;
offset?: number;
};
export type NextSearchOptions = {
limit?: number;
next?: string;
sort?: SearchMessageSort;
};
export type SearchOptions = LimitOffsetSearchOptions | NextSearchOptions;

Copy link
Contributor

Choose a reason for hiding this comment

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

Defining before using ... makes it a little easier to read!!

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, do we really need separate LimitOffsetSearchOptions and NextSearchOptions? Because anyways user can't switch between v1 or v2 search, right?

export type SearchOptions = {
  limit?: number;
  next?: string;
  sort?: SearchMessageSort;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it was better to keep them separate - A user should not be allowed to use offset with next or with sort.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah my bad. I thought sort was the only difference there :) Thanks for clarification


export type StreamChatOptions = AxiosRequestConfig & {
/**
* Used to disable warnings that are triggered by using connectUser or connectAnonymousUser server-side.
Expand Down Expand Up @@ -1407,10 +1424,20 @@ export type UserSort<UserType = UnknownType> =
| Sort<UserResponse<UserType>>
| Array<Sort<UserResponse<UserType>>>;

export type SearchRelevanceSort = { relevance?: AscDesc };

export type SearchMessageSortBase =
| SearchRelevanceSort
| Sort<Message>
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
| { [field: string]: AscDesc };
miagilepner marked this conversation as resolved.
Show resolved Hide resolved

export type SearchMessageSort = SearchMessageSortBase | Array<SearchMessageSortBase>;

export type QuerySort<ChannelType = UnknownType, UserType = UnknownType> =
| BannedUsersSort
| ChannelSort<ChannelType>
| UserSort<UserType>;
| UserSort<UserType>
| SearchMessageSort;
miagilepner marked this conversation as resolved.
Show resolved Hide resolved

/**
* Base Types
Expand Down Expand Up @@ -1898,7 +1925,7 @@ export type SearchPayload<
MessageType = UnknownType,
ReactionType = UnknownType,
UserType = UnknownType
> = SearchOptions & {
> = Omit<SearchOptions, 'sort'> & {
client_id?: string;
connection_id?: string;
filter_conditions?: ChannelFilters<ChannelType, CommandType, UserType>;
Expand All @@ -1911,6 +1938,10 @@ export type SearchPayload<
UserType
>;
query?: string;
sort?: Array<{
direction: AscDesc;
field: string | number;
miagilepner marked this conversation as resolved.
Show resolved Hide resolved
}>;
};

export type TestPushDataInput = {
Expand Down