-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Teams] Fix Teams menu rendering even when there are backend errors #6041
Conversation
@@ -132,6 +135,11 @@ export class UserDeletionService { | |||
} | |||
} | |||
|
|||
protected async deleteTeamMemberships(userId: string) { | |||
const teams = await this.teamDb.findTeamsByUser(userId); | |||
await Promise.all(teams.map(t => this.teamDb.removeMemberFromTeam(userId, t.id))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlexTugarev Did you mean to /lgtm
this Pull Request? 😇
/schedule |
@JanKoehnlein: Cannot schedule issue - issue does not belong to a team. Use /team to specify one. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign @jankeromnes |
/team meta |
/schedule |
@JanKoehnlein shouldn't we add only issues to groundwork, not PRs? |
/assign @jankeromnes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UX works like a charm! 🔮
Thanks @jankeromnes for always adding steps for how to test changes in the PR description. Makes everything so much better, reduces cognitive load on the reviewer side, and allows reviewers not familiar with a product feature to easily dive into a review with more confidence. 🍊
Approving to unblock merging but holding in case @AlexTugarev wants to take another look (see #6041 (review)) or @JanKoehnlein wants to make any more changes for testing groundwork automation (see #6041 (comment)).
/hold
LGTM label has been added. Git tree hash: 0ebb657213e2dc70a421d88d06208fa0b6497853
|
works as advertised! thanks! /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexTugarev, gtsiolis Associated issue: #6035 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
Description
In #6035 and #6037 we noticed that:
When a user account gets deleted, their team memberships remain active, causing errors for all other team members (now fixed in this PR ✅)
When getting the members of one of your teams fail, the Team menu rendering is broken (all members count stay as
...
, and the tabs menu is not the Team menu but the User menu) (also fixed in this PR ✅)Related Issue(s)
Fixes #6035
Fixes #6037
How to test
Release Notes
/uncc