Skip to content

Conversation

@Hubert-Szczepanski-SAP
Copy link
Contributor

What this PR does / why we need it:

Tests for Workspace creation + Deletion hook change.
Workspace deletion is using generic component, so tests are already done in - DeleteConfirmationDialog.cy.tsx

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

@andreaskienle andreaskienle requested a review from Copilot October 29, 2025 11:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors workspace creation and deletion logic by extracting it into reusable custom hooks (useCreateWorkspace and useDeleteWorkspace), improving code modularity and testability.

  • Extracts workspace creation logic from CreateWorkspaceDialogContainer into useCreateWorkspace hook
  • Creates new useDeleteWorkspace hook for workspace deletion
  • Adds comprehensive unit tests for both hooks
  • Updates components to use the new hooks with dependency injection for better testability

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/hooks/useDeleteWorkspace.tsx New hook encapsulating workspace deletion logic with toast notifications and error handling
src/hooks/useDeleteWorkspace.spec.tsx Unit tests for the delete workspace hook
src/hooks/useCreateWorkspace.tsx New hook encapsulating workspace creation logic with toast notifications and error handling
src/hooks/useCreateWorkspace.spec.tsx Unit tests for the create workspace hook
src/components/Dialogs/CreateWorkspaceDialogContainer.tsx Refactored to use the new useCreateWorkspace hook with dependency injection support
src/components/Dialogs/CreateWorkspaceDialogContainer.cy.tsx Cypress component tests for the dialog container
src/components/ControlPlanes/List/ControlPlaneListWorkspaceGridTile.tsx Updated to use the new useDeleteWorkspace hook with dependency injection support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@andreaskienle andreaskienle left a comment

Choose a reason for hiding this comment

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

Thanks for the tests! Some remarks below.

Also: Can we rename the hooks (and their tests) from .tsx to .ts? I know, I had this in the template PR but I changed it with the follow-up PR. That's on me 😅

andreaskienle
andreaskienle previously approved these changes Oct 30, 2025
Copy link
Contributor

@andreaskienle andreaskienle left a comment

Choose a reason for hiding this comment

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

👍🏻

@Hubert-Szczepanski-SAP Hubert-Szczepanski-SAP merged commit 89f48a7 into main Oct 30, 2025
5 checks passed
@Hubert-Szczepanski-SAP Hubert-Szczepanski-SAP deleted the feat/workspace-cypress-tests branch October 30, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants