Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions .changeset/component-delete-from-project-listing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@openchoreo/backstage-plugin': minor
---

Add a per-row "Delete" action to the Project Contents table so a component
can be deleted directly from the project listing without opening the
component page first. The action surfaces in a row "more actions" kebab for
component rows (hidden for resources and for rows already marked for
deletion), reuses the shared delete-confirmation dialog, and refreshes the
listing once the component is marked for deletion.
Comment on lines +5 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Changeset says "hidden for resources" but code supports resource deletion.

RowActionsCell renders RowDeleteButton for both component and resource items, and RowActionsCell.test.tsx confirms the delete button appears for resource rows. The companion changeset listing-row-delete-everywhere.md also states it "extends the Project Contents row action to resource rows." Since both changesets ship together, the "hidden for resources" clause is inaccurate for the final state and will confuse changelog readers.

📝 Suggested changeset revision
 Add a per-row "Delete" action to the Project Contents table so a component
 can be deleted directly from the project listing without opening the
-component page first. The action surfaces in a row "more actions" kebab for
-component rows (hidden for resources and for rows already marked for
-deletion), reuses the shared delete-confirmation dialog, and refreshes
-the listing once the component is marked for deletion.
+component page first. The action surfaces as a per-row delete button for
+both component and resource rows (hidden for rows already marked for
+deletion), reuses the shared delete-confirmation dialog, and optimistically
+marks the row for deletion until the next catalog sync drops it.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Add a per-row "Delete" action to the Project Contents table so a component
can be deleted directly from the project listing without opening the
component page first. The action surfaces in a row "more actions" kebab for
component rows (hidden for resources and for rows already marked for
deletion), reuses the shared delete-confirmation dialog, and refreshes the
listing once the component is marked for deletion.
Add a per-row "Delete" action to the Project Contents table so a component
can be deleted directly from the project listing without opening the
component page first. The action surfaces as a per-row delete button for
both component and resource rows (hidden for rows already marked for
deletion), reuses the shared delete-confirmation dialog, and optimistically
marks the row for deletion until the next catalog sync drops it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.changeset/component-delete-from-project-listing.md around lines 5 - 10, The
changeset summary is inconsistent with the implemented behavior: RowActionsCell
and RowDeleteButton support deletion for both component and resource rows, and
the related listing-row-delete-everywhere.md changeset confirms that scope.
Update the wording in the changeset to remove the “hidden for resources” claim
and describe the delete action as applying to both components and resources, so
the changelog matches the final behavior.

13 changes: 13 additions & 0 deletions .changeset/listing-row-delete-everywhere.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@openchoreo/backstage-plugin': minor
---

Generalize the listing row delete beyond components. The new
`useDeleteEntityDialog` hook (replacing `useDeleteComponentDialog`) dispatches
deletes for every kind the OpenChoreo API supports — components, projects,
namespaces, resources and the platform resource kinds — sharing one dispatch
with the entity-page context menu. A reusable `RowDeleteButton` +
`usePendingDeletionOverlay` pair wires per-row delete (with an optimistic
"marked for deletion" badge) into the catalog "All ..." pages, the namespace
projects/resources cards, and extends the Project Contents row action to
resource rows.
10 changes: 10 additions & 0 deletions .changeset/refresh-system-on-component-delete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@openchoreo/backstage-plugin-catalog-backend-module': patch
---

Refresh the parent System when a component is removed so its now-dangling
`hasPart` relation is dropped. Without this, deleting a component left the
project page showing "Some related entities could not be found in the catalog"
until the catalog was rebuilt, because the System's relations are only
recomputed when it is re-processed and the periodic full sync re-emits it
unchanged.
23 changes: 21 additions & 2 deletions packages/app/src/components/catalog/CatalogCardList.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type ReactNode } from 'react';
import { useMemo, type ReactNode } from 'react';
import {
Box,
Chip,
Expand All @@ -21,6 +21,9 @@ import { useNavigate } from 'react-router-dom';
import {
DeletionBadge,
isMarkedForDeletion,
RowDeleteButton,
useDeleteEntityDialog,
usePendingDeletionOverlay,
} from '@openchoreo/backstage-plugin';
import { Entity } from '@backstage/catalog-model';
import { useCardListStyles } from './styles';
Expand Down Expand Up @@ -165,6 +168,19 @@ export const CatalogCardList = ({ actionButton }: CatalogCardListProps) => {
navigate(url);
};

// Row-level delete for every kind the OC API can delete (projects,
// components, resources, namespaces, environments, platform types, ...).
// The catalog lags a delete by a sync/event, so deleted rows are overlaid
// with the deletion mark until the next refetch drops them.
const { markDeleted, overlay } = usePendingDeletionOverlay();
const { requestDelete, DeleteDialog } = useDeleteEntityDialog({
onDeleted: markDeleted,
});
const displayedEntities = useMemo(
() => overlay(entities),
[entities, overlay],
);

const kindLabel = filters.kind?.label || filters.kind?.value || 'Entity';
const pluralLabel = kindPluralNames[kindLabel] || `${kindLabel}s`;
const titleText = `All ${totalItems === 1 ? kindLabel : pluralLabel}${
Expand Down Expand Up @@ -211,7 +227,7 @@ export const CatalogCardList = ({ actionButton }: CatalogCardListProps) => {
))}
</Box>

{entities.map(entity => {
{displayedEntities.map(entity => {
const name =
entity.metadata.title || entity.metadata.name || 'Unnamed';
const description = entity.metadata.description || '';
Expand Down Expand Up @@ -442,6 +458,7 @@ export const CatalogCardList = ({ actionButton }: CatalogCardListProps) => {
</IconButton>
</Tooltip>
)}
<RowDeleteButton entity={entity} onDelete={requestDelete} />
</Box>
</Box>
);
Expand All @@ -468,6 +485,8 @@ export const CatalogCardList = ({ actionButton }: CatalogCardListProps) => {
/>
</Box>
)}

<DeleteDialog />
</Box>
);
};
17 changes: 10 additions & 7 deletions packages/app/src/components/catalog/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export const usePersonalFilterStyles = makeStyles(theme => ({

export const useCardListStyles = makeStyles(theme => {
const mobileGrid = {
gridTemplateColumns: '32px 1fr 60px',
gridTemplateColumns: '32px 1fr 100px',
};

return {
Expand All @@ -150,32 +150,32 @@ export const useCardListStyles = makeStyles(theme => {
// Grid templates per kind — icon + actions fixed, rest spread evenly
// Component: Icon | Name | Description | Namespace | Project | Type | Actions
gridTemplateComponent: {
gridTemplateColumns: '40px 1fr 1.5fr 1fr 1fr 1fr 80px',
gridTemplateColumns: '40px 1fr 1.5fr 1fr 1fr 1fr 116px',
[theme.breakpoints.down('xs')]: mobileGrid,
},
// API: Icon | Name | Description | Namespace | Project | Component | Type | Actions
gridTemplateApi: {
gridTemplateColumns: '40px 1.2fr 2fr 0.8fr 0.8fr 0.8fr 0.7fr 80px',
gridTemplateColumns: '40px 1.2fr 2fr 0.8fr 0.8fr 0.8fr 0.7fr 116px',
[theme.breakpoints.down('xs')]: mobileGrid,
},
// Environment: Icon | Name | Description | Namespace | Type | Actions
gridTemplateEnvironment: {
gridTemplateColumns: '40px 1fr 1.5fr 1fr 1fr 80px',
gridTemplateColumns: '40px 1fr 1.5fr 1fr 1fr 116px',
[theme.breakpoints.down('xs')]: mobileGrid,
},
// Planes: Icon | Name | Description | Namespace | Agent | Actions
gridTemplatePlane: {
gridTemplateColumns: '40px 1fr 1.5fr 1fr 1fr 80px',
gridTemplateColumns: '40px 1fr 1.5fr 1fr 1fr 116px',
[theme.breakpoints.down('xs')]: mobileGrid,
},
// Project/simple: Icon | Name | Description | Namespace | Actions
gridTemplateSimple: {
gridTemplateColumns: '40px 1fr 1.5fr 1fr 80px',
gridTemplateColumns: '40px 1fr 1.5fr 1fr 116px',
[theme.breakpoints.down('xs')]: mobileGrid,
},
// Namespace/domain: Icon | Name | Description | Actions
gridTemplateMinimal: {
gridTemplateColumns: '40px 1fr 1.5fr 80px',
gridTemplateColumns: '40px 1fr 1.5fr 116px',
[theme.breakpoints.down('xs')]: mobileGrid,
},

Expand Down Expand Up @@ -239,6 +239,9 @@ export const useCardListStyles = makeStyles(theme => {
'& > :first-child': {
marginLeft: -8,
},
'& .MuiIconButton-root': {
padding: theme.spacing(0.75),
},
},
columnCell: {
fontSize: '0.8rem',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -841,3 +841,103 @@ describe('EventDeltaApplier.handleEvent', () => {
);
});
});

// ---------------------------------------------------------------------------
// Removing a Component leaves the parent System's reverse `hasPart` relation
// dangling until the System is re-processed. With a catalog read-side wired,
// the applier resolves the owning System and forces a refresh so the stale
// relation (and the "related entities could not be found" warning) clears.
// ---------------------------------------------------------------------------
describe('EventDeltaApplier component removal refreshes the parent System', () => {
let applyMutation: jest.Mock;
let connection: EntityProviderConnection;
let getEntityByRef: jest.Mock;
let refreshEntity: jest.Mock;

function newApplierWithCatalog(component: any) {
getEntityByRef = jest.fn().mockResolvedValue(component);
refreshEntity = jest.fn().mockResolvedValue(undefined);
return new EventDeltaApplier({
logger: mkLogger(),
baseUrl: 'http://test:8080',
defaultOwner: 'group:default/test-owner',
translatorContext: {
providerName: 'OpenChoreoEntityProvider',
defaultOwner: 'group:default/test-owner',
componentTypeUtils: ComponentTypeUtils.fromConfig(
new ConfigReader({
openchoreo: { componentTypes: { mappings: [] } },
}),
),
},
getConnection: () => connection,
ctdConverter: new CtdToTemplateConverter(mkLogger()),
rtdConverter: new RtdToTemplateConverter(mkLogger()),
ptdConverter: new PtdToTemplateConverter(mkLogger()),
catalogService: { getEntityByRef, refreshEntity } as any,
auth: {
getOwnServiceCredentials: jest.fn().mockResolvedValue({}),
} as any,
});
}

beforeEach(() => {
jest.clearAllMocks();
mockGET.mockResolvedValue(notFound());
applyMutation = jest.fn();
connection = { applyMutation, refresh: jest.fn() } as any;
});

it('refreshes the System from the component partOf relation', async () => {
const applier = newApplierWithCatalog({
kind: 'Component',
metadata: { name: 'order', namespace: 'test-ns' },
spec: { system: 'shop' },
relations: [
{ type: 'partOf', targetRef: 'system:test-ns/shop' },
{ type: 'ownedBy', targetRef: 'group:default/team' },
],
});

await applier.handleEvent('Component', 'order', 'test-ns', 'deleted');

// Parent resolved before removal, component removed, parent refreshed.
expect(getEntityByRef).toHaveBeenCalledWith(
'component:test-ns/order',
expect.anything(),
);
expect(
applyMutation.mock.calls[0][0].removed.map((r: any) => r.entityRef),
).toEqual(['component:test-ns/order']);
expect(refreshEntity).toHaveBeenCalledWith(
'system:test-ns/shop',
expect.anything(),
);
});

it('falls back to spec.system when there is no partOf relation', async () => {
const applier = newApplierWithCatalog({
kind: 'Component',
metadata: { name: 'order', namespace: 'test-ns' },
spec: { system: 'shop' },
});

await applier.handleEvent('Component', 'order', 'test-ns', 'deleted');

expect(refreshEntity).toHaveBeenCalledWith(
'system:test-ns/shop',
expect.anything(),
);
});

it('removes the component without refreshing when the owner cannot be resolved', async () => {
const applier = newApplierWithCatalog(undefined);

await applier.handleEvent('Component', 'order', 'test-ns', 'deleted');

expect(
applyMutation.mock.calls[0][0].removed.map((r: any) => r.entityRef),
).toEqual(['component:test-ns/order']);
expect(refreshEntity).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { Entity, stringifyEntityRef } from '@backstage/catalog-model';
import {
Entity,
stringifyEntityRef,
RELATION_PART_OF,
} from '@backstage/catalog-model';
import { AuthService, LoggerService } from '@backstage/backend-plugin-api';
import {
CatalogService,
Expand Down Expand Up @@ -644,6 +648,69 @@ export class EventDeltaApplier {
]);
}

/**
* Resolve the Backstage System (OC Project) ref a component belongs to, by
* reading the still-present catalog Component entity. Prefers the `partOf`
* relation (the exact ref Backstage assigned); falls back to `spec.system`.
* Returns undefined when the catalog read-side isn't wired or the owner
* can't be resolved — the caller then just skips the parent refresh.
*/
private async findParentSystemRef(
ns: string,
name: string,
): Promise<string | undefined> {
if (!this.catalogService || !this.auth) {
return undefined;
}
try {
const credentials = await this.auth.getOwnServiceCredentials();
const entity = await this.catalogService.getEntityByRef(
this.buildEntityRef('component', ns, name),
{ credentials },
);
if (!entity) {
return undefined;
}
const partOf = entity.relations?.find(
rel =>
rel.type === RELATION_PART_OF && rel.targetRef.startsWith('system:'),
);
if (partOf) {
return partOf.targetRef;
}
const system = (entity.spec as { system?: string } | undefined)?.system;
return system
? this.buildEntityRef(
'system',
entity.metadata.namespace ?? 'default',
system,
)
: undefined;
} catch (err) {
this.logger.warn(
`Could not resolve parent System for component ${ns}/${name}: ${err}`,
);
return undefined;
}
}

/**
* Force a re-process (and thus re-stitch) of a single entity. Best-effort:
* logs and continues on failure so it never blocks the mutation that
* triggered it.
*/
private async refreshEntityRef(entityRef: string): Promise<void> {
if (!this.catalogService || !this.auth) {
return;
}
try {
const credentials = await this.auth.getOwnServiceCredentials();
await this.catalogService.refreshEntity(entityRef, { credentials });
} catch (err) {
this.logger.warn(`Failed to refresh ${entityRef}: ${err}`);
}
}

private async refreshComponent(
ns: string,
name: string,
Expand All @@ -657,7 +724,19 @@ export class EventDeltaApplier {
const client = await this.createApiClient();
const component = await this.fetchComponent(client, ns, name);
if (!component) {
// Resolve the owning System *before* removing the component — once the
// component entity is gone we can no longer read its `partOf` relation.
const parentSystemRef = await this.findParentSystemRef(ns, name);
await this.removeEntityRefs([this.buildEntityRef('component', ns, name)]);
// Removing the component leaves the System's reverse `hasPart` relation
// dangling: the System's denormalized relations are only recomputed when
// it is itself re-processed, and the periodic full sync re-emits it
// unchanged (so it isn't). Until then the project page shows "Some
// related entities could not be found in the catalog". Force a refresh
// of the parent System so it re-stitches and drops the dangling edge.
if (parentSystemRef) {
await this.refreshEntityRef(parentSystemRef);
}
return;
}

Expand Down
Loading
Loading