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

[dashboard]: only display delete button if not preparing #5186

Closed
wants to merge 1 commit into from

Conversation

mrsimonemms
Copy link
Contributor

Proposed fix for #4173

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign svenefftinge after the PR has been reviewed.
You can assign the PR to them by writing /assign @svenefftinge in a comment when ready.

Associated issue: #4173

The full list of commands accepted by this bot can be found 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 requested a review from jankeromnes August 13, 2021 10:42
@mrsimonemms mrsimonemms requested a review from gtsiolis August 13, 2021 10:47
@mrsimonemms mrsimonemms linked an issue Aug 13, 2021 that may be closed by this pull request
@gtsiolis
Copy link
Contributor

gtsiolis commented Aug 13, 2021

Looking at this now! 👀

@jankeromnes jankeromnes removed their request for review August 13, 2021 13:18
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Lunch happened but I'm back! 🍝

Changes look good, @mrsimonemms!

Ideally, we could allow users to click the delete action and handle all the necessary checks on the backstage even if that's moving the to-be-deleted entry to outside the active workspaces list, as described in option 🅱️ in #4173 (comment). 🌤️

However, I understand there's a technical limitation to do this (#4173 (comment)) so removing the delete option sounds like a good MVC (minimum viable change) especially since this removal is visible at rare times as the user usually switches context to starting a workspace (loading screen) away from the dashboard workspaces list. ✔️

I've added one small nitpick issue about borders but could be fine to skip this, too. Let me know what you think.

components/dashboard/src/workspaces/WorkspaceEntry.tsx Outdated Show resolved Hide resolved
);

// Workspaces can only be deleted if not preparing
if (state !== 'preparing') {
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: Sounds safe to use a similar if statement here as above options.

@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Aug 18, 2021

/werft run

👍 started the job as gitpod-build-sje-cannot-delete-ws.3

@mrsimonemms mrsimonemms force-pushed the sje/cannot-delete-ws branch 2 times, most recently from 189f983 to 7770b3c Compare August 18, 2021 14:30
@mrsimonemms mrsimonemms force-pushed the sje/cannot-delete-ws branch from 7770b3c to 5c79b20 Compare August 18, 2021 14:40
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Aug 18, 2021

/werft run

👍 started the job as gitpod-build-sje-cannot-delete-ws.7

@mrsimonemms mrsimonemms requested a review from gtsiolis August 18, 2021 15:04
@gtsiolis
Copy link
Contributor

gtsiolis commented Aug 18, 2021

issue: @mrsimonemms For some reason I cannot creaste any new workspace, getting the following two errors:

cannot pull image: rpc error: code = Unknown desc = failed to pull and unpack image "reg.sje-cannot-delete-ws.staging.gitpod-dev.com:30474/remote/d0b360df-d75f-496a-a3ef-1b8a19cee9bb:latest": failed to resolve reference "reg.sje-cannot-delete-ws.staging.gitpod-dev.com:30474/remote/d0b360df-d75f-496a-a3ef-1b8a19cee9bb:latest": failed to do request: Head https://reg.sje-cannot-delete-ws.staging.gitpod-dev.com:30474/v2/remote/d0b360df-d75f-496a-a3ef-1b8a19cee9bb/manifests/latest: dial tcp 127.0.0.1:30474: connect: connection refused
cannot pull image: Back-off pulling image "reg.sje-cannot-delete-ws.staging.gitpod-dev.com:30474/remote/bcce5a7d-da3a-431d-8ebc-0b83386bace9"

Maybe there's an issue with preview environments at the moment? 🤷

@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Aug 25, 2021

/werft run

👍 started the job as gitpod-build-sje-cannot-delete-ws.8

@gtsiolis
Copy link
Contributor

gtsiolis commented Sep 3, 2021

/werft run

👍 started the job as gitpod-build-sje-cannot-delete-ws.9

@gtsiolis
Copy link
Contributor

gtsiolis commented Sep 3, 2021

/werft with-clean-slate-deployment

👎 unknown command: with-clean-slate-deployment
Use /werft help to list the available commands

@corneliusludmann
Copy link
Contributor

corneliusludmann commented Sep 10, 2021

Due to #4173 (comment):

Deleting workspaces in any phase but RUNNING or STOPPED is very difficult. I reckon we should rather disable the "Delete" item in the menu if the workspace is neither running nor stopped.

What about checking if the workspace is in RUNNING or STOPPED instead and only provide the delete button during these 2 phases?

→ This would probably fix this as well: https://community.gitpod.io/t/unable-to-delete-workspace/4956

@stale
Copy link

stale bot commented Sep 28, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Sep 28, 2021
@stale stale bot closed this Oct 3, 2021
@mrsimonemms mrsimonemms deleted the sje/cannot-delete-ws branch January 5, 2022 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dashboard meta: stale This issue/PR is stale and will be closed soon size/S user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot delete a workspace in PREPARING phase
4 participants