Skip to content

SRVOCF-1071: Change the delete action to an undeploy button #181

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.

Sign up for GitHub

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

Open
Cragsmann wants to merge 1 commit into
base: master
Choose a base branch
from
+44 −26
Open

SRVOCF-1071: Change the delete action to an undeploy button #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions e2e/use-cases/delete/function-delete.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ test.describe('Delete function', () => {
await deleteFunction(page, PRESEEDED_FUNC_NAME, PRESEEDED_FUNC_NAMESPACE);
});

test('delete button is disabled for not deployed functions', async ({ page }) => {
test('undeploy button is disabled for not deployed functions', async ({ page }) => {
await navigateToFunctionsList(page);

const grid = page.getByRole('grid', { name: 'Functions' });
await expect(grid).toBeVisible({ timeout: 30_000 });

const row = grid.locator('tbody tr').filter({ hasText: PRESEEDED_FUNC_NAME });
await expect(row.getByText('NotDeployed')).toBeVisible();
await expect(row.getByRole('button', { name: 'Delete' })).toBeDisabled();
await expect(row.getByRole('button', { name: 'Undeploy' })).toBeDisabled();
});

test('delete button removes function from cluster', async ({ page }) => {
test('undeploy button removes function from cluster', async ({ page }) => {
test.setTimeout(600_000);

await test.step('make sure deletion target function is deployed in cluster', async () => {
Expand All @@ -52,9 +52,9 @@ test.describe('Delete function', () => {
await test.step('undeploy function', async () => {
const grid = page.getByRole('grid', { name: 'Functions' });
const row = grid.locator(`tbody tr:has(td:text-is("${PRESEEDED_FUNC_NAME}"))`);
const deleteBtn = row.getByRole('button', { name: 'Delete' });
await expect(deleteBtn).toBeEnabled({ timeout: 30_000 });
await deleteBtn.click();
const undeployBtn = row.getByRole('button', { name: 'Undeploy' });
await expect(undeployBtn).toBeEnabled({ timeout: 30_000 });
await undeployBtn.click();

const modal = page.getByRole('dialog');
await expect(modal).toBeVisible({ timeout: 5_000 });
Expand Down
2 changes: 1 addition & 1 deletion e2e/use-cases/list/functions-list.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test.describe('Functions list', () => {
await expect(row).toBeVisible();
await expect(row.getByText('NotDeployed')).toBeVisible();
await expect(row.getByRole('button', { name: 'Edit' })).toBeEnabled();
await expect(row.getByRole('button', { name: 'Delete' })).toBeDisabled();
await expect(row.getByRole('button', { name: 'Undeploy' })).toBeDisabled();
});

await test.step('refresh re-fetches the list', async () => {
Expand Down
6 changes: 4 additions & 2 deletions locales/en/plugin__console-functions-plugin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"Branch": "Branch",
"Cancel": "Cancel",
"Click \"Create new function\", choose a runtime, and add any environment variables (plain values or from a secret). Submitting creates a GitHub repository, pushes the function scaffold, and starts a GitHub Actions workflow that deploys the function to your cluster. It appears here as \"NotDeployed\" until the workflow finishes, then the status changes to \"Running\".": "Click \"Create new function\", choose a runtime, and add any environment variables (plain values or from a secret). Submitting creates a GitHub repository, pushes the function scaffold, and starts a GitHub Actions workflow that deploys the function to your cluster. It appears here as \"NotDeployed\" until the workflow finishes, then the status changes to \"Running\".",
"Click the undeploy button in the Actions column to remove the running function and its Knative Service from the cluster. The GitHub repository and its code remain, so you can redeploy it later.": "Click the undeploy (power off) button in the Actions column to remove the running function and its Knative Service from the cluster. The GitHub repository and its code remain, so you can redeploy it later.",
"Close": "Close",
"Coming soon": "Coming soon",
"ConfigMap": "ConfigMap",
Expand All @@ -23,7 +24,6 @@
"Create function": "Create function",
"Create new function": "Create new function",
"Create or select a project for your function. If it needs credentials such as an API key, create a secret in that namespace so you can reference it as an environment variable.": "Create or select a project for your function. If it needs credentials such as an API key, create a secret in that namespace so you can reference it as an environment variable.",
"Delete": "Delete",
"Edit": "Edit",
"Edit and redeploy": "Edit and redeploy",
"Edit function": "Edit function",
Expand Down Expand Up @@ -62,12 +62,14 @@
"Select a namespace first": "Select a namespace first",
"Select...": "Select...",
"Serverless functions in your repository and deployed to your cluster. Manage lifecycle, monitor status, and scale on demand.": "Serverless functions in your repository and deployed to your cluster. Manage lifecycle, monitor status, and scale on demand.",
"Set up guide": "Set up guide",
"Setup guide": "Setup guide",
"Sign in with GitHub": "Sign in with GitHub",
"Start editing": "Start editing",
"Status": "Status",
"Stay": "Stay",
"Undeploy": "Undeploy",
"Undeploy a function": "Undeploy a function",
"Undeploying removes the running function and its Knative Service from the cluster. The GitHub repository and its code remain, so you can redeploy it later.": "Undeploying removes the running function and its Knative Service from the cluster. The GitHub repository and its code remain, so you can redeploy it later.",
"Unsaved changes": "Unsaved changes",
"URL": "URL",
"Value": "Value",
Expand Down
12 changes: 6 additions & 6 deletions src/pages/function-list/components/FunctionTable.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ vi.mock('@openshift-console/dynamic-plugin-sdk', () => ({
vi.mock('@patternfly/react-icons', () => ({
ExclamationTriangleIcon: () => 'WarningIcon',
PencilAltIcon: () => 'EditIcon',
TrashIcon: () => 'DeleteIcon',
PowerOffIcon: () => 'UndeployIcon',
}));

const mockKnativeService = {
Expand Down Expand Up @@ -203,7 +203,7 @@ describe('FunctionTable', () => {
expect(onEdit).toHaveBeenCalledWith('my-repo');
});

it('launches delete modal when delete button is clicked', async () => {
it('launches undeploy modal when undeploy button is clicked', async () => {
const mockLauncher = vi.fn();
mockUseDeleteModal.mockReturnValue(mockLauncher);
const user = userEvent.setup();
Expand All @@ -214,24 +214,24 @@ describe('FunctionTable', () => {
</MemoryRouter>,
);

await user.click(screen.getByRole('button', { name: 'Delete' }));
await user.click(screen.getByRole('button', { name: 'Undeploy' }));
expect(mockLauncher).toHaveBeenCalled();
expect(mockUseDeleteModal).toHaveBeenCalledWith(
mockKnativeService,
undefined,
undefined,
expect.anything(),
'Undeploy',
);
});

it('disables delete button for NotDeployed functions', () => {
it('disables undeploy button for NotDeployed functions', () => {
render(
<MemoryRouter>
<FunctionTable functions={[mockFunctions[1]]} onEdit={vi.fn()} showNamespace />
</MemoryRouter>,
);

expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled();
expect(screen.getByRole('button', { name: 'Undeploy' })).toBeDisabled();
});

it('disables edit button for cluster-only functions', () => {
Expand Down
28 changes: 18 additions & 10 deletions src/pages/function-list/components/FunctionTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import {
SuccessStatus,
useDeleteModal,
} from '@openshift-console/dynamic-plugin-sdk';
import { ActionList, ActionListItem, Button, Tooltip } from '@patternfly/react-core';
import { ExclamationTriangleIcon, PencilAltIcon, TrashIcon } from '@patternfly/react-icons';
import { ActionList, ActionListItem, Button, Content, Tooltip } from '@patternfly/react-core';
import { ExclamationTriangleIcon, PencilAltIcon, PowerOffIcon } from '@patternfly/react-icons';
import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table';
import { useTranslation } from 'react-i18next';
import { FunctionSource, FunctionStatus } from '../../../common/types';
Expand Down Expand Up @@ -80,7 +80,7 @@ export function FunctionTable({
<EditActionButton source={fn.source} repoName={fn.repoName} onEdit={onEdit} />
</ActionListItem>
<ActionListItem>
<DeleteActionButton mainResource={fn.mainResource} />
<UndeployActionButton mainResource={fn.mainResource} />
</ActionListItem>
</ActionList>
</Td>
Expand Down Expand Up @@ -154,22 +154,30 @@ function EditActionButton({
return <Tooltip content={t('No source repository to edit')}>{button}</Tooltip>;
}

function DeleteActionButton({ mainResource }: { mainResource?: K8sResourceCommon }) {
function UndeployActionButton({ mainResource }: { mainResource?: K8sResourceCommon }) {
const { t } = useTranslation('plugin__console-functions-plugin');
const launchDelete = useDeleteModal(
const launchUndeploy = useDeleteModal(
mainResource as K8sResourceCommon,
undefined,
undefined,
<Content component="p">
{t(
'Undeploying removes the running function and its Knative Service from the cluster. The GitHub repository and its code remain, so you can redeploy it later.',
)}
</Content>,
t('Undeploy'),
);

return (
const button = (
<Button
variant="plain"
aria-label={t('Delete')}
icon={<TrashIcon />}
aria-label={t('Undeploy')}
icon={<PowerOffIcon />}

@pmeida pmeida Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I personally dont feel like the icon choice is intuitive.
The problem is:

  • A power icon reads as "power off on click," not "undeploy on click."
  • A greyed-out power icon reads as "click to power on," not "undeploy is unavailable."
    Even if "power on" were mapped to deploy, the visual state is inverted - the button appears actionable when it should be disabled, and vice versa.

This is also not adaptable for when we have the deploy functionality, a single-state icon button cannot represent both undeploy and deploy as the feature evolves

My options would be:

  1. kebab menu with the actual text - no risk of misinterpretation
  2. UndoIcon that switches to a disabled PlayIcon after undeploy - visually communicates the inverse action is coming. PlayIcon can be enabled once deploy functionality is available.
  3. UndoIcon that switches to a disabled state after undeploy - simpler variant with no forward hint. Undeploy is clear, but the disabled state does not suggest a future deploy action.

We could also just go with option 2 and directly implement https://redhat.atlassian.net/browse/SRVOCF-1072 as it is the root of the confusion.

isDisabled={!mainResource}
onClick={() => launchDelete()}
onClick={() => launchUndeploy()}
/>
);

if (!mainResource) return button;

return <Tooltip content={t('Undeploy')}>{button}</Tooltip>;
}
2 changes: 2 additions & 0 deletions src/pages/function-list/components/SetupGuide.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ describe('SetupGuide', () => {
await user.click(screen.getByRole('button', { name: 'View setup guide.' }));

expect(screen.getByRole('dialog')).toBeInTheDocument();
expect(screen.getByText('Setup guide')).toBeInTheDocument();
expect(screen.getByText('Connect GitHub')).toBeInTheDocument();
expect(screen.getByText('Create a namespace and secret')).toBeInTheDocument();
expect(screen.getByText('Create a function')).toBeInTheDocument();
expect(screen.getByText('Edit and redeploy')).toBeInTheDocument();
expect(screen.getByText('Undeploy a function')).toBeInTheDocument();
});

it('closes the modal when Close is clicked', async () => {
Expand Down
8 changes: 7 additions & 1 deletion src/pages/function-list/components/SetupGuide.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function SetupGuideModal({ isOpen, onClose }: SetupGuideModalProps) {

return (
<Modal isOpen={isOpen} onClose={onClose} variant="medium" data-test="setup-guide-modal">
<ModalHeader title={t('Set up guide')} />
<ModalHeader title={t('Setup guide')} />
<ModalBody>
<Content component="p">
{t('Follow these steps to create and deploy your serverless function.')}
Expand Down Expand Up @@ -96,5 +96,11 @@ function steps(t: (key: string) => string): Step[] {
'Open the function from the list to edit its files, then click "Save & Deploy". This pushes your changes to GitHub, which runs the same workflow again to redeploy the function.',
),
},
{
title: t('Undeploy a function'),
body: t(
'Click the undeploy button in the Actions column to remove the running function and its Knative Service from the cluster. The GitHub repository and its code remain, so you can redeploy it later.',
),
},
];
}