Skip to content
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] inactivate Leave Team action for last team owner #6477

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

AlexTugarev
Copy link
Member

Fixes #4623

NONE

@@ -46,6 +47,10 @@ export default function() {
})();
}, [ team ]);

useEffect(() => {
setLeaveTeamEnabled(members.length > 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be filtered by team owners?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

absolutely! just updated

@JanKoehnlein
Copy link
Contributor

Doesn't work for me:

  1. Create a team (thus only yourself as a member and owner)
  2. Go to team page and delete yourself
    Gitpod allows that

@AlexTugarev
Copy link
Member Author

@JanKoehnlein, thanks for testing! Sorry, I just confused active state with enablement.

@AlexTugarev
Copy link
Member Author

Screen Shot 2021-11-01 at 10 55 33

This should be available with next update.

customFontStyle: 'text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300',
onClick: () => removeTeamMember(m.userId)
title: leaveTeamEnabled ? 'Leave Team' : 'Remaining owner',
customFontStyle: leaveTeamEnabled ? 'text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300' : 'text-gray-400 dark:text-gray-200',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
customFontStyle: leaveTeamEnabled ? 'text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300' : 'text-gray-400 dark:text-gray-200',
customFontStyle: leaveTeamEnabled ? 'text-red-600 dark:text-red-400 hover:text-red-800 dark:hover:text-red-300' : 'text-gray-400 dark:text-gray-200 cursor-not-allowed',

How about also adapting the cursor to show that there is no action? E.g. cursor-not-allowed or cursor-default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the label is changed to Remaining owner, I don't think it's necessary to change the cursor. Also, there is no other example in the dashboard to provide feedback using the "not-allowed" cursor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have some server-side validation for this as well?

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Nov 2, 2021

/werft run

👍 started the job as gitpod-build-at-last-owner-4623.3

@jldec jldec changed the title [teams] inactivate Leave Team action for last team member [teams] inactivate Leave Team action for last team owner Nov 2, 2021
@JanKoehnlein
Copy link
Contributor

/lgtm

@roboquat
Copy link
Contributor

roboquat commented Nov 2, 2021

LGTM label has been added.

Git tree hash: b4c1c8a0076591d28fb42b80cd49323b09f0c5a2

@roboquat
Copy link
Contributor

roboquat commented Nov 2, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JanKoehnlein

Associated issue: #4623

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 422a7fc into main Nov 2, 2021
@roboquat roboquat deleted the at/last-owner-4623 branch November 2, 2021 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent last owner from leaving the team
5 participants