Exclude organization members feature #154

enoldev · 2019-11-06T18:43:59Z

Command: we iterate through every parameter, identifying if it's a "feature" or a "flag" (for this purpose, some helper functions have been included in "utils")
Subscription: a new struct for flags has been created.
Plugin: a new method isUserOrganizationMember has been added.
Webhook: a new method excludeConfigOrgMember has been included. It's responsible for fetching configuration and creating the GitHub client.

I have created two different methods so isUserOrganizationMember could be reused without any links to the purpose of this task.

Thanks!

Fixes #125

mattermod · 2019-11-18T00:00:47Z

This issue has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @hanzei

jasonblais · 2019-11-18T00:34:00Z

@marianunez / @ali-farooq0 friendly reminder to help review of this PR some time this coming week :)

marianunez

Apologies for the delay @enolal826! Overall looks good. Some minor feedback below

server/command.go

server/plugin.go

server/webhook.go

server/command.go

Co-Authored-By: Maria A Nunez <[email protected]>

marianunez

Thanks @enolal826! 🎉

DHaussermann · 2019-11-26T16:41:37Z

@marianunez ~~would you be able to provide any high-level tests steps for this?~~ I can regression test subscriptions but I'm not sure how validate Plugin: a new method isUserOrganizationMember has been added.

Details provided by enolal826 Thank you!

DHaussermann · 2019-11-29T15:43:44Z

@enolal826 when I try to pass in the exclude-org-member flag my subscription does not work and I see no event deliveries. I have ensure that the cofig locks the organization so, the feature should be working.

There may be unrelated problem where there is not proper feedback when the subscription command is invalid. but, for now, can you help by providing a sample subscribe command where 1 or multiple members are excluded.
Can we also add this to the help?

alifarooq0

Great addition @enolal826 🎉! Just one minor concern around where we're parsing the SubscriptionFlags, looks great otherwise.

server/utils_test.go

server/webhook.go

server/plugin.go

server/subscriptions.go

DHaussermann · 2019-12-02T17:26:39Z

Hey @enolal826 thanks for updating the help to include the flag.

I'm still not seeing this work on my server. (The subscription not delivering before was because I tried to append values to the the flag which was invalid) When I use the flag I'm not seeing members being excluded.

Can you please look over these steps and let me know if you see anything else I'm doing wrong?

Build GitHub from org-members-excluded branch
Configure the plugin with an app and webhook on the GitHub side and enable it
Lock the plugin to an organization such as "DHaussermann"
Subscribe to a repo in the Org using the new flag /github subscribe DHaussermann/hello-world --exclude-org-member
As the owner of the repo (or other member) create a branch or PR
Observed The even is deliver in the subscribed channel.

alifarooq0 · 2019-12-02T19:33:24Z

@enolal826 do you mind updating the branch to resolve conflicts as well as answer @DHaussermann's comment above? Thanks!

DHaussermann · 2019-12-05T02:31:19Z

@aaronrothschild I added the PM Review label to this one. Sorry, I'm not sure what the process is for updating the read-me in cases like this. Would updates be done by the contributor and approved by you?

enoldev · 2019-12-05T10:35:43Z

I can update the README and @aaronrothschild could validate it if it's ok for him

aaronrothschild · 2019-12-09T11:54:12Z

I can update the README and @aaronrothschild could validate it if it's ok for him

Yes, that would be great and helpful for me @enolal826 ! Happy to review, then can remove the "Docs Needed" label when this is all set.

marianunez · 2019-12-09T19:13:28Z

/update-branch

marianunez · 2019-12-18T15:55:02Z

/update-branch

hanzei · 2019-12-19T14:09:20Z

@enolal826 Do you want to include the doc changes in this PR or open a separate one?

mattermod · 2019-12-30T00:00:52Z

This issue has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @hanzei

jasonblais · 2019-12-30T13:28:12Z

Hi @enolal826! Wasn't sure if you and @aaronrothschild were planning to make the doc changes in this PR or separately. Looks like this is otherwise good to merge. Thanks for your contribution! :)

hanzei · 2020-01-07T12:55:47Z

@aaronrothschild Would you be fine merging the PR as it is and doing the doc changes in a different PR?

aaronrothschild · 2020-01-07T20:47:57Z

@aaronrothschild Would you be fine merging the PR as it is and doing the doc changes in a different PR?

@hanzei Yes, no problem.

hanzei · 2020-01-08T09:02:09Z

/update-branch

hanzei · 2020-01-08T09:06:05Z

Thanks for adding this valuable feature @enolal826 🕺

#192) * Revert "Update main.go (#154)" This reverts commit be4a281d0cc791d10e6e5ae917b325b2f054e475. * Revert "[MM-33506] Use embed package to include plugin manifest (#145)" This reverts commit ca9ee3c17c6920a636a33f378e17395afd6f329f. * Revert "Don't generate manifest.ts (#127)" This reverts commit 18d30b50bc7ba800c9f05bfd82970781db0aea3e. * install-go-tools target, adopt gotestsum * bring back make apply + automatic versioning * Update build/manifest/main.go Co-authored-by: Michael Kochell <[email protected]> * suppress git describe error when no tags match * make version/release notes opt-in * fix whitespace in Makefile * document version management options --------- Co-authored-by: Michael Kochell <[email protected]>

* Make use of github.com/mattermost/mattermost/server/public * [MM-36] Update plugin with respect to phase 1 upgrades * [MM-36] Update makefile command for testcases * Sync with playbooks: install-go-tools, gotestsum, and dynamic versions (#192) * Revert "Update main.go (#154)" This reverts commit be4a281d0cc791d10e6e5ae917b325b2f054e475. * Revert "[MM-33506] Use embed package to include plugin manifest (#145)" This reverts commit ca9ee3c17c6920a636a33f378e17395afd6f329f. * Revert "Don't generate manifest.ts (#127)" This reverts commit 18d30b50bc7ba800c9f05bfd82970781db0aea3e. * install-go-tools target, adopt gotestsum * bring back make apply + automatic versioning * Update build/manifest/main.go Co-authored-by: Michael Kochell <[email protected]> * suppress git describe error when no tags match * make version/release notes opt-in * fix whitespace in Makefile * document version management options --------- Co-authored-by: Michael Kochell <[email protected]> * Fetch plugin logs from server (#193) Co-authored-by: Jesse Hallam <[email protected]> * [MM-36] Update plugin.json file --------- Co-authored-by: Ben Schumacher <[email protected]> Co-authored-by: Mattermost Build <[email protected]> Co-authored-by: Jesse Hallam <[email protected]> Co-authored-by: Michael Kochell <[email protected]>

Exclude org member feature added

766f3c7

hanzei requested review from hanzei and marianunez November 6, 2019 19:00

hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Nov 6, 2019

hanzei requested review from alifarooq0 and removed request for hanzei November 6, 2019 19:02

hanzei added this to the v0.13.0 milestone Nov 6, 2019

tests for utils added

b89d7b6

mattermod added the Lifecycle/1:stale label Nov 18, 2019

marianunez requested changes Nov 19, 2019

View reviewed changes

server/command.go Outdated Show resolved Hide resolved

server/plugin.go Show resolved Hide resolved

server/webhook.go Show resolved Hide resolved

marianunez removed the Lifecycle/1:stale label Nov 19, 2019

parameterList name changed and error logging added

0dd1e4f

marianunez reviewed Nov 19, 2019

View reviewed changes

server/command.go Outdated Show resolved Hide resolved

marianunez requested a review from DHaussermann November 19, 2019 19:31

Update feedback message

49119c0

Co-Authored-By: Maria A Nunez <[email protected]>

marianunez approved these changes Nov 19, 2019

View reviewed changes

alifarooq0 suggested changes Nov 29, 2019

View reviewed changes

server/utils_test.go Show resolved Hide resolved

server/utils_test.go Outdated Show resolved Hide resolved

server/webhook.go Outdated Show resolved Hide resolved

server/plugin.go Outdated Show resolved Hide resolved

server/subscriptions.go Outdated Show resolved Hide resolved

Format fixes and help command text updated

2188223

hanzei requested a review from alifarooq0 December 2, 2019 09:31

enoldev added 2 commits December 2, 2019 22:31

support for push, create and delete events

f47bd30

subscriptionFlags added when parsing command

498c438

alifarooq0 approved these changes Dec 3, 2019

View reviewed changes

Merge branch 'master' into org-members-excluded

39a840a

DHaussermann requested a review from aaronrothschild December 5, 2019 02:32

DHaussermann added the Docs/Needed Requires documentation label Dec 6, 2019

Updated README

92fff18

Merge branch 'master' into org-members-excluded

7de8deb

levb assigned aaronrothschild Dec 10, 2019

Merge branch 'master' into org-members-excluded

f573b53

mattermod added the Lifecycle/1:stale label Dec 30, 2019

hanzei unassigned aaronrothschild Jan 8, 2020

hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 1: PM Review Requires review by a product manager Lifecycle/1:stale labels Jan 8, 2020

Merge branch 'master' into org-members-excluded

ded0481

hanzei merged commit 23f77f0 into mattermost:master Jan 8, 2020

hanzei added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Mar 4, 2020

hanzei mentioned this pull request Mar 4, 2020

Require read:org scope for OAuth connect #200

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exclude organization members feature #154

Exclude organization members feature #154

enoldev commented Nov 6, 2019 •

edited

Loading

mattermod commented Nov 18, 2019

jasonblais commented Nov 18, 2019

marianunez left a comment

marianunez left a comment

DHaussermann commented Nov 26, 2019 •

edited

Loading

DHaussermann commented Nov 29, 2019

alifarooq0 left a comment

DHaussermann commented Dec 2, 2019

alifarooq0 commented Dec 2, 2019

DHaussermann commented Dec 5, 2019

enoldev commented Dec 5, 2019

aaronrothschild commented Dec 9, 2019

marianunez commented Dec 9, 2019

marianunez commented Dec 18, 2019

hanzei commented Dec 19, 2019

mattermod commented Dec 30, 2019

jasonblais commented Dec 30, 2019

hanzei commented Jan 7, 2020

aaronrothschild commented Jan 7, 2020

hanzei commented Jan 8, 2020

hanzei commented Jan 8, 2020

Exclude organization members feature #154

Exclude organization members feature #154

Conversation

enoldev commented Nov 6, 2019 • edited Loading

mattermod commented Nov 18, 2019

jasonblais commented Nov 18, 2019

marianunez left a comment

Choose a reason for hiding this comment

marianunez left a comment

Choose a reason for hiding this comment

DHaussermann commented Nov 26, 2019 • edited Loading

DHaussermann commented Nov 29, 2019

alifarooq0 left a comment

Choose a reason for hiding this comment

DHaussermann commented Dec 2, 2019

alifarooq0 commented Dec 2, 2019

DHaussermann commented Dec 5, 2019

enoldev commented Dec 5, 2019

aaronrothschild commented Dec 9, 2019

marianunez commented Dec 9, 2019

marianunez commented Dec 18, 2019

hanzei commented Dec 19, 2019

mattermod commented Dec 30, 2019

jasonblais commented Dec 30, 2019

hanzei commented Jan 7, 2020

aaronrothschild commented Jan 7, 2020

hanzei commented Jan 8, 2020

hanzei commented Jan 8, 2020

enoldev commented Nov 6, 2019 •

edited

Loading

DHaussermann commented Nov 26, 2019 •

edited

Loading