search

mattermost / mattermost-plugin-github

GitHub plugin for Mattermost
Apache License 2.0
157 stars 148 forks source link

cleanup / removal of inactive users #749

Open nilesingalls opened 6 months ago

nilesingalls commented 6 months ago

As a Mattermost administrator, what is the best approach to remove subscriptions for inactive users?

We have a few former users showing up in mattermost logs that I'd like to address.

"timestamp":"2024-02-23 13:30:54.563 Z","level":"warn","msg":"Failed to search for review","caller":"app/plugin_api.go:987","plugin_id":"github","query":"is:pr is:open review-requested:REDACTED archived:false ","userid":"REDACTED","github username":"REDACTED","error":"GET https://api.github.com/search/issues?q=is%3Apr+is%3Aopen+review-requested%3AREDACTED+archived%3Afalse+: 403 API rate limit exceeded for user ID REDACTED. If you reach out to GitHub Support for help, please include the request ID REDACTED [rate reset in 10s]"}

ayusht2810 commented 5 months ago

@mickmister Regarding the approach for the above issue, we can implement the UserHasBeenDeactivated Hook, and whenever a user is deactivated, we can check for the subscriptions linked to the user, if any and delete the subscription. What are your thoughts on this approach, or do you have any other solutions for the approach?

mickmister commented 4 months ago

@ayusht2810 Yeah that sounds great :+1:

@nilesingalls Are you wanting to also affect existing deactivated users? That would likely require SQL that we may be able to provide for this

ayusht2810 commented 4 months ago

@mickmister Regarding the above issue, I misunderstood it and posted the wrong approach to it. Sorry for that. I think the issue states that they don't want to receive the subscription for a user who is inactive on GitHub, as shown in the logs "query":"is:pr is:open review-requested:REDACTED archived:false ","userid":"REDACTED","github username":"REDACTED". But I think the issue is arising from the function HasUnreads (which posts the user's GitHub todos in certain time interval) https://github.com/mattermost/mattermost-plugin-github/blob/fba634d51d347d2c0eddfd9b9110faef289662fc/server/plugin/plugin.go#L916 This function is invoked from the /connected API, which is called at various places from the client, due to which we are getting the above issue. Isn't deactivating the user from Mattermost server (as the user is also inactive from GitHub) a way to resolve or should we check if the user exist on GitHub and perform further actions? What are your thoughts on this? Let me know if I am missing something here.

mickmister commented 4 months ago

@ayusht2810 Yeah it doesn't make much sense that these queries would be getting executed if the user is deactivated, and thus not logged into Mattermost. @nilesingalls Are you certain these particular errors are occurring for users that are deactivated from Mattermost?

nilesingalls commented 4 months ago

These are users who are active in our Mattermost instance but no longer have a github account associated to our github organization. Perhaps our use case is unique in that regard, but we split off a group of users into a separate company.

mickmister commented 3 months ago

@nilesingalls Thanks that makes sense. Are you able to have them each run /github disconnect?