Mattermost-server: [Help Wanted] [MM-13749] Add ability to search posts in a team to plugin API

[esethna]

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

---

Notes: Jira ticket

Add the following plugin API method:

• SearchPostsInTeam(teamId string, params *model.SearchParameter) (*model.PostList, *model.AppError)

Read about adding to the plugin API here https://developers.mattermost.com/contribute/server/plugins/.

You'll want to look at these files:

• https://github.com/mattermost/mattermost-server/blob/master/plugin/api.go
• https://github.com/mattermost/mattermost-server/blob/master/app/plugin_api.go

If you have any questions or would like feedback, feel free to ask in the Developer Toolkit channel on our Mattermost community instance.

[awbraunstein]

I'd like to take this Issue.

[hanzei]

Great @awbraunstein! Let me know, if you have any questions.

[awbraunstein]

I have a few questions.

1. Did you mean model.SearchParameter instead of model.SearchParameters?
2. Assuming the answer to 1 is yes, model.SearchParameter has "optional" fields that are pointers. Should these all be nil checked?
3. It looks like the function that I need to call also takes in a userId. Should I update the signature to require that as well?

Thanks

[awbraunstein]

I have https://github.com/mattermost/mattermost-server/pull/10106 as a possible solution pending the answers to the questions above.

Thanks

[hanzei]

1. Correct, fixed this in the description.
2. Term and IsOrSearch are required. Please take a look at https://api.mattermost.com/#tag/posts%2Fpaths%2F~1teams~1%7Bteam_id%7D~1posts~1search%2Fpost for the other fields.
3. Are you talking about SearchPostsInTeam()? This methods isn't applicable for this issue. I think you need to write your own method. If posssible, it would be great to rework SearchPostsInTeam() to keep the code DRY.

[awbraunstein]

What's the policy on "required" fields? Should they be enforced at the API layer and return an error or do you usually just return an empty response? It seems like omitting Terms or IsOrSearch from the current JSON api will yield an error. I assume we want to stick to this.

As for determining the UserId, is this necessary or can the plugin have access to any team's posts regardless of the current "user". I'm just trying to understand the difference in the permission model between the Plugin API and the REST API (api4).

[jwilander]

Yes, when something required isn't provided erroring out is the correct response 👍

For user ID, the plugin can have access to any posts on a team. You can think of it like this. The REST API allows integrations to act as a user that are subject to the permission system. Plugins act as the system and are beyond the permission system

[awbraunstein]

What should the behavior of a search like Mattermost in:@john.doe aka direct message history? Would this just not be supported?

[jwilander]

I would much rather handle that through explicit options as part of the API input rather than including it like that as part of the search term/phrase

[awbraunstein]

If that has to be an explicit option, then here's what I'm thinking.

SearchPostsInTeam(teamId string, params *model.SearchParameter, optionalUserId string) (*model.PostList, *model.AppError)

With a comment along the lines of: Searches for all the posts in a team that match the params. If you wish include direct messages for a user, pass a valid userId to the optionalUserId param. Otherwise an empty string will disable that functionality.

Personally, I don't love this API design. And would probably prefer to have two API methods. One that is SearchPostsInTeam and one that's SearchPostsInTeamForUser. This way it is clear what each method's purpose is and we don't have callers passing in an unneeded "" every time they call the function.

How does this sound?

[awbraunstein]

I'm also wondering if searching for in:@a,b should still work without a "current user" and should search the direct messages between @a and @b. Or should we just return an error for any sort of search in a group or direct channel?

[jwilander]

@awbraunstein I agree about having two separate APIs. It would be much cleaner and more obvious IMO

I don't think we should interpret in:@a,b at all in these APIs. We should just search them literally and if someone wants to search in certain channels or with other parameters, we expose those as options directly in the API via an options struct

/cc @crspeller @levb

[crspeller]

+1 for separate APIs.

Also agree that we shouldn't interpret anything just allow the plugin to directly specify the options.

[levb]

I also agree with both points.

[hanzei]

Fixed with https://github.com/mattermost/mattermost-server/pull/10106