mattermost / mattermost-plugin-apps

Powers the Mattermost App Framework
https://developers.mattermost.com/integrate/apps/
Apache License 2.0
33 stars 29 forks source link

MM-48793, MM-48137: improved bot_joined|left_channel|team #443

Closed levb closed 1 year ago

levb commented 1 year ago

Replaces #405

Upon a conversation with @hanzei we decided to change the semantics of UserJoined|Left... rather than introducing new subjects.

Summary

  1. Makes bot_(joined|left)_(channel|team) global, and ensures reliable attribute expansion.
  2. Makes user_(joined|left)_(channel|team) either accept the entity ID in which case they fire for all users joining/leaving the specified entity; or without the entity ID fire only for the subscriber, if joined/left any relevant entity.
  3. Changed Expand to bypass permission checks for these events since they are limited to the member of the team|channel.
  4. Replaced server/mmclient with server/proxy/ExpandGetter
  5. A small logging improvement in goapp

Ticket Link

https://mattermost.atlassian.net/browse/MM-48137 https://mattermost.atlassian.net/browse/MM-48793

TODO

codecov-commenter commented 1 year ago

Codecov Report

Base: 20.31% // Head: 20.78% // Increases project coverage by +0.47% :tada:

Coverage data is based on head (5eac984) compared to base (8bfe37b). Patch coverage: 21.84% of modified lines in pull request are covered.

:exclamation: Current head 5eac984 differs from pull request most recent head f55b189. Consider uploading reports for the commit f55b189 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #443 +/- ## ========================================== + Coverage 20.31% 20.78% +0.47% ========================================== Files 78 79 +1 Lines 5266 5445 +179 ========================================== + Hits 1070 1132 +62 - Misses 4066 4181 +115 - Partials 130 132 +2 ``` | [Impacted Files](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) | Coverage Δ | | |---|---|---| | [apps/goapp/http.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-YXBwcy9nb2FwcC9odHRwLmdv) | `0.00% <0.00%> (ø)` | | | [server/proxy/bindings.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L2JpbmRpbmdzLmdv) | `29.50% <0.00%> (-4.46%)` | :arrow_down: | | [server/proxy/enable.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L2VuYWJsZS5nbw==) | `4.05% <0.00%> (-0.36%)` | :arrow_down: | | [server/proxy/expand\_getter.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L2V4cGFuZF9nZXR0ZXIuZ28=) | `0.00% <0.00%> (ø)` | | | [server/proxy/install.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L2luc3RhbGwuZ28=) | `0.00% <0.00%> (ø)` | | | [server/proxy/invoke\_webhook.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L2ludm9rZV93ZWJob29rLmdv) | `0.00% <0.00%> (ø)` | | | [server/proxy/notify.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L25vdGlmeS5nbw==) | `0.00% <0.00%> (ø)` | | | [server/proxy/service.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L3NlcnZpY2UuZ28=) | `6.97% <ø> (ø)` | | | [utils/mattermost.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-dXRpbHMvbWF0dGVybW9zdC5nbw==) | `0.00% <0.00%> (ø)` | | | [server/proxy/expand.go](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost#diff-c2VydmVyL3Byb3h5L2V4cGFuZC5nbw==) | `49.21% <56.25%> (+0.30%)` | :arrow_up: | | ... and [5 more](https://codecov.io/gh/mattermost/mattermost-plugin-apps/pull/443?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mattermost)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

hanzei commented 1 year ago

@mickmister Would you mind also taking a look at the PR?

hanzei commented 1 year ago

@levb How can @DHaussermann test this PR?

levb commented 1 year ago

@hanzei @DHaussermann the PR is covered with REST API tests; I think I fixed the test app, too.

hanzei commented 1 year ago

@levb I will take a look at the test app later and check if I can get it to work