Adding paging to elasticsearch API #9425
Conversation
crspeller
force-pushed
the
mm-8683
branch
2 times, most recently
from
0477ded
to
87d0409
Compare
| @@ -330,26 +330,42 @@ func searchPosts(c *Context, w http.ResponseWriter, r *http.Request) { | |||
| return | |||
| } | |||
|
|
|||
| includeDeletedChannels := r.URL.Query().Get("include_deleted_channels") == "true" | |||
There was a problem hiding this comment.
This won't be backwards compatibly since we're changing how this parameter is specified. We should try falling back to the query parameter if it's not specified in the body
There was a problem hiding this comment.
Yes. But this feature was never released. (it's experimental)
There was a problem hiding this comment.
@crspeller Are we removing a feature from the ExperimentalViewArchivedChannels setting?
| c.SetInvalidParam("terms") | ||
| return | ||
| } | ||
| terms := *params.Terms | ||
|
|
||
| timeZoneOffset := 0 |
There was a problem hiding this comment.
This process for parsing the search parameters is a bit complicated for me right now since it turns the body of the request into a model.SearchParameter, sets defaults, passes the fields into App.SearchPostsInTeam, and then recombines them back into an array of model.SearchParams. I don't know if it's something to change now since it'd take some extra time to refactor and work out, but it could definitely be made simpler
There was a problem hiding this comment.
Ya it's a bit strange, but I only wanted to fix what was needed.
| @@ -2168,17 +2168,6 @@ func (c *Client4) SearchPostsWithParams(teamId string, params *SearchParameter) | |||
| } | |||
| } | |||
|
|
|||
| // SearchPosts returns any posts with matching terms string including deleted channels. | |||
| func (c *Client4) SearchPostsIncludeDeletedChannels(teamId string, terms string, isOrSearch bool) (*PostList, *Response) { | |||
There was a problem hiding this comment.
I don't think we can remove this since we want backwards compatibility here
There was a problem hiding this comment.
It's for an experimental feature. So we can remove it.
|
@hmhealey Ready for re-review |
hmhealey
added
Awaiting Submitter Action
|
@crspeller The build for this one is failing, but it otherwise looks good |
jasonblais
added
4: Reviews Complete
|
@hmhealey @crspeller or @grundleborg please help merge this and the enterprise PR https://github.com/mattermost/enterprise/pull/357 We'll start PM/QA review of the webapp PR after that for Thursday's cut-off date. |
amyblais
added
Docs/Needed
amyblais
added
Docs/Done
Summary
API PR: mattermost/mattermost-api-reference#392
Enterprise PR: https://github.com/mattermost/enterprise/pull/357