Skip to content

Commit

Permalink
Merge pull request #737 from rottencandy/fix/release-pr-link-473
Browse files Browse the repository at this point in the history
Use correct workspace for release pipelinerun link
  • Loading branch information
openshift-merge-robot authored Jul 28, 2023
2 parents d2847aa + 551e451 commit cae73cf
Show file tree
Hide file tree
Showing 11 changed files with 165 additions and 21 deletions.
3 changes: 1 addition & 2 deletions src/components/ApplicationDetails/WorkspaceSwitcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ContextMenuItem, ContextSwitcher } from '../ContextSwitcher';

export const WorkspaceSwitcher: React.FC<{ selectedWorkspace?: string }> = () => {
const navigate = useNavigate();
const { workspace, setWorkspace, workspaces } = React.useContext(WorkspaceContext);
const { workspace, workspaces } = React.useContext(WorkspaceContext);

const menuItems = React.useMemo(
() => workspaces?.map((app) => ({ key: app.metadata.name, name: app.metadata.name })) || [],
Expand All @@ -15,7 +15,6 @@ export const WorkspaceSwitcher: React.FC<{ selectedWorkspace?: string }> = () =>

const onSelect = (item: ContextMenuItem) => {
navigate(`/application-pipeline/workspaces/${item.name}/applications`);
setWorkspace(item.name);
};

return workspaces.length > 0 ? (
Expand Down
2 changes: 1 addition & 1 deletion src/components/Releases/ReleaseDetailsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const ReleaseDetailsView: React.FC<ReleaseDetailsViewProps> = ({
key: 'overview',
label: 'Overview',
isFilled: true,
component: <ReleaseOverviewTab applicationName={applicationName} release={release} />,
component: <ReleaseOverviewTab release={release} />,
},
]}
/>
Expand Down
20 changes: 14 additions & 6 deletions src/components/Releases/ReleaseOverviewTab.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';
import { Link } from 'react-router-dom';
import { useK8sWatchResource } from '@openshift/dynamic-plugin-sdk-utils';
import {
DescriptionList,
DescriptionListDescription,
Expand All @@ -10,21 +11,28 @@ import {
Title,
} from '@patternfly/react-core';
import { useReleaseStatus } from '../../hooks/useReleaseStatus';
import { useWorkspaceResource } from '../../hooks/useWorkspaceResource';
import { ReleasePlanGroupVersionKind } from '../../models';
import { Timestamp } from '../../shared/components/timestamp/Timestamp';
import { ReleaseKind } from '../../types';
import { ReleasePlanKind } from '../../types/coreBuildService';
import { calculateDuration } from '../../utils/pipeline-utils';
import { useWorkspaceInfo } from '../../utils/workspace-context-utils';
import MetadataList from '../PipelineRunDetailsView/MetadataList';
import { StatusIconWithText } from '../topology/StatusIcon';

type ReleaseOverviewTabProps = {
applicationName: string;
release: ReleaseKind;
};

const ReleaseOverviewTab: React.FC<ReleaseOverviewTabProps> = ({ applicationName, release }) => {
const { workspace } = useWorkspaceInfo();
const pipelineRun = release.status?.processing?.pipelineRun?.split('/')[1];
const ReleaseOverviewTab: React.FC<ReleaseOverviewTabProps> = ({ release }) => {
const { namespace } = useWorkspaceInfo();
const [pipelineRun, prWorkspace] = useWorkspaceResource(release.status?.processing?.pipelineRun);
const [releasePlan, releasePlanLoaded] = useK8sWatchResource<ReleasePlanKind>({
name: release.spec.releasePlan,
groupVersionKind: ReleasePlanGroupVersionKind,
namespace,
});
const duration = calculateDuration(
typeof release.status?.startTime === 'string' ? release.status?.startTime : '',
typeof release.status?.completionTime === 'string' ? release.status?.completionTime : '',
Expand Down Expand Up @@ -114,9 +122,9 @@ const ReleaseOverviewTab: React.FC<ReleaseOverviewTabProps> = ({ applicationName
<DescriptionListGroup>
<DescriptionListTerm>Pipeline Run</DescriptionListTerm>
<DescriptionListDescription>
{pipelineRun ? (
{pipelineRun && prWorkspace && releasePlanLoaded ? (
<Link
to={`/application-pipeline/workspaces/${workspace}/applications/${applicationName}/pipelineruns/${pipelineRun}`}
to={`/application-pipeline/workspaces/${prWorkspace}/applications/${releasePlan.spec.application}/pipelineruns/${pipelineRun}`}
>
{pipelineRun}
</Link>
Expand Down
12 changes: 10 additions & 2 deletions src/components/Releases/__tests__/ReleaseOverviewTab.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,17 @@ jest.mock('../../../utils/workspace-context-utils', () => ({
useWorkspaceInfo: jest.fn(() => ({ namespace: 'test-ns', workspace: 'test-ws' })),
}));

jest.mock('@openshift/dynamic-plugin-sdk-utils', () => ({
useK8sWatchResource: jest.fn(() => [{ spec: { application: 'test-app' } }, true]),
}));

jest.mock('../../../hooks/useWorkspaceResource', () => ({
useWorkspaceResource: jest.fn(() => ['test-pipelinerun', 'target-ws']),
}));

describe('ReleaseOverviewTab', () => {
it('should render correct details', () => {
render(<ReleaseOverviewTab applicationName="test-app" release={mockRelease} />);
render(<ReleaseOverviewTab release={mockRelease} />);
expect(screen.getByText('Duration')).toBeVisible();
expect(screen.getByText('10 seconds')).toBeVisible();

Expand All @@ -39,7 +47,7 @@ describe('ReleaseOverviewTab', () => {
expect(screen.getByText('Pipeline Run')).toBeVisible();
expect(screen.getByText('test-strategy')).toBeVisible();
expect(screen.getByRole('link', { name: 'test-pipelinerun' }).getAttribute('href')).toBe(
'/application-pipeline/workspaces/test-ws/applications/test-app/pipelineruns/test-pipelinerun',
'/application-pipeline/workspaces/target-ws/applications/test-app/pipelineruns/test-pipelinerun',
);
});
});
39 changes: 39 additions & 0 deletions src/hooks/__tests__/useWorkspaceForNamespace.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import * as React from 'react';
import { renderHook } from '@testing-library/react-hooks';
import { WorkspaceProvider } from '../../utils/workspace-context-utils';
import { useWorkspaceForNamespace } from '../useWorkspaceForNamespace';

describe('useWorkspaceForNamespace', () => {
it('should find correct workspace', () => {
const wrapper = ({ children }) => (
<WorkspaceProvider
value={{
namespace: 'test-ns',
lastUsedWorkspace: 'test-ws',
workspace: '',
workspaces: [
{ metadata: { name: 'ws1' }, status: { namespaces: [{ name: 'my-ns' }] } },
{ metadata: { name: 'ws2' }, status: { namespaces: [{ name: 'my-ns-2' }] } },
{ metadata: { name: 'ws3' }, status: { namespaces: [{ name: 'test-ns' }] } },
] as any,
setWorkspace: () => {},
workspacesLoaded: false,
}}
>
{children}
</WorkspaceProvider>
);
const { result } = renderHook(() => useWorkspaceForNamespace('my-ns'), { wrapper });
expect(result.current.metadata.name).toBe('ws1');
});

it('should return undefined if workspace is not found', () => {
const { result } = renderHook(() => useWorkspaceForNamespace('my-ns-1'));
expect(result.current).toBe(undefined);
});

it('should return undefined if namespace is not provided', () => {
const { result } = renderHook(() => useWorkspaceForNamespace(''));
expect(result.current).toBe(undefined);
});
});
24 changes: 24 additions & 0 deletions src/hooks/__tests__/useWorkspaceResource.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { renderHook } from '@testing-library/react-hooks';
import { useWorkspaceForNamespace } from '../useWorkspaceForNamespace';
import { useWorkspaceResource } from '../useWorkspaceResource';

jest.mock('../useWorkspaceForNamespace', () => ({
useWorkspaceForNamespace: jest.fn(),
}));

const useWorkspaceMock = useWorkspaceForNamespace as jest.Mock;

describe('useWorkspaceResource', () => {
it('should return correct resource name and workspace', () => {
useWorkspaceMock.mockReturnValue({ metadata: { name: 'ws1' } });
const { result } = renderHook(() => useWorkspaceResource('my-ns/my-resource'));
expect(useWorkspaceMock).toHaveBeenCalledWith('my-ns');
expect(result.current).toEqual(['my-resource', 'ws1']);
});

it('should return undefined if invalid string is provided', () => {
useWorkspaceMock.mockReturnValue(undefined);
const { result } = renderHook(() => useWorkspaceResource(''));
expect(result.current).toEqual([undefined, undefined]);
});
});
14 changes: 14 additions & 0 deletions src/hooks/useWorkspaceForNamespace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as React from 'react';
import { WorkspaceContext } from '../utils/workspace-context-utils';

export const useWorkspaceForNamespace = (namespace?: string) => {
const { workspaces } = React.useContext(WorkspaceContext);

return React.useMemo(() => {
if (!namespace) {
return undefined;
}

return workspaces.find((w) => w.status?.namespaces?.some((ns) => ns.name === namespace));
}, [namespace, workspaces]);
};
21 changes: 21 additions & 0 deletions src/hooks/useWorkspaceResource.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import * as React from 'react';
import { useWorkspaceForNamespace } from './useWorkspaceForNamespace';

/**
* @param namespacedObj string with the format `<namespace>/<resource>`
* @returns resource name and workspace if accessible
*/
export const useWorkspaceResource = (
namespacedObj?: string,
): [resource?: string, workspace?: string] => {
const [namespace, resource] = React.useMemo(() => {
if (!namespacedObj) {
return [undefined, undefined];
}
return namespacedObj.split('/');
}, [namespacedObj]);

const workspace = useWorkspaceForNamespace(namespace);

return [resource, workspace?.metadata.name];
};
2 changes: 1 addition & 1 deletion src/types/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { K8sResourceCommon } from '@openshift/dynamic-plugin-sdk-utils';

export interface Workspace extends K8sResourceCommon {
status: {
type: string;
type?: string;
namespaces: Namespace[];
owner: string;
role: string;
Expand Down
14 changes: 14 additions & 0 deletions src/utils/__tests__/workspace-context-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,18 @@ describe('useActiveWorkspace', () => {
);
});
});

it('should update workspace if url segment changes', async () => {
k8sListResourceItemsMock.mockReturnValue(mockWorkspaces);
window.location.pathname = '/application-pipeline/workspaces/workspace-one/applications';
const { result, waitForNextUpdate, rerender } = renderHook(() => useActiveWorkspace());
await waitForNextUpdate();

expect(result.current.workspace).toBe('workspace-one');

window.location.pathname = '/application-pipeline/workspaces/workspace-two/applications';
rerender();

expect(result.current.workspace).toBe('workspace-two');
});
});
35 changes: 26 additions & 9 deletions src/utils/workspace-context-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,25 @@ export const useWorkspaceInfo = () => {
const { namespace, workspace } = React.useContext(WorkspaceContext);
return { namespace, workspace };
};
export const getHomeWorkspace = (workspaces) => workspaces?.find((w) => w?.status?.type === 'home');
export const getHomeWorkspace = (workspaces: Workspace[]) =>
workspaces?.find((w) => w?.status?.type === 'home');

export const useActiveWorkspace = (): WorkspaceContextData => {
const lastUsedWorkspace = useLastUsedWorkspace();
const navigate = useNavigate();
const [, workspaceFromUrl = ''] = window.location.pathname.match(workspacePathMatcher) || [];
const [workspace, setWorkspace] = React.useState<string>(getActiveWorkspace);
const [namespace, setNamespace] = React.useState<string>('');
const [workspaces, setWorkspaces] = React.useState<any>([]);
const [workspaces, setWorkspaces] = React.useState<Workspace[]>([]);
const [workspacesLoaded, setWorkspacesLoaded] = React.useState<boolean>(false);

const getDefaultNsForWorkspace = React.useCallback((allWorkspaces, currentWorkspace) => {
const obj = allWorkspaces?.find((w) => w.metadata.name === currentWorkspace);
return obj?.status?.namespaces.find((n) => n.type === 'default');
}, []);
const getDefaultNsForWorkspace = React.useCallback(
(allWorkspaces: Workspace[], currentWorkspace: string) => {
const obj = allWorkspaces?.find((w) => w.metadata.name === currentWorkspace);
return obj?.status?.namespaces.find((n) => n.type === 'default');
},
[],
);

React.useEffect(() => {
if (workspace && workspaces?.length > 0) {
Expand All @@ -61,13 +65,24 @@ export const useActiveWorkspace = (): WorkspaceContextData => {
}
}, [getDefaultNsForWorkspace, setNamespace, workspace, workspaces]);

// switch workspace if URL segment has changed
React.useEffect(() => {
if (
workspace &&
workspaceFromUrl &&
workspaceFromUrl !== workspace &&
workspaces.some((w) => w.metadata.name === workspaceFromUrl)
) {
setWorkspace(workspaceFromUrl);
}
}, [workspace, workspaceFromUrl, workspaces]);

React.useEffect(() => {
let unmounted = false;
const fetchWorkspaces = async () => {
let allWorkspaces = [];
let allWorkspaces: Workspace[] = [];
try {
setActiveWorkspace(''); // to fetch root level workspaces
allWorkspaces = await k8sListResourceItems({
allWorkspaces = await k8sListResourceItems<Workspace>({
model: WorkspaceModel,
});
} catch (e) {
Expand All @@ -79,6 +94,8 @@ export const useActiveWorkspace = (): WorkspaceContextData => {
return;
}

setActiveWorkspace(''); // to fetch root level workspaces

let ws: string;
if (Array.isArray(allWorkspaces)) {
const workspaceNames = allWorkspaces.map((dataResource) => dataResource.metadata.name);
Expand Down

0 comments on commit cae73cf

Please sign in to comment.