feat(kms): secrets listing page

[mathieumousnier]

Description

Ticket Reference: #MANAGER-18246

Additional Information

[rjamet-ovh]

you can replace it with creation_date from @ovh-ux/manager-common-translations

[rjamet-ovh]

Suggested change

	export const getSecretList = async (okmsId: string) => {
	const response = await apiClient.v2.get<Secret[]>(
	`okms/resource/${okmsId}/secret`,
	);
	return response.data;
	};
	export const getSecretList = async (okmsId: string) => {
	const { data } = await apiClient.v2.get<Secret[]>(
	`okms/resource/${okmsId}/secret`,
	);
	return data;
	};

for the sake of consistency, what do you think?

[mathieumousnier]

For me is "bonnet blanc" / "blanc bonnet".
It does the same and is colocated in the function scope.
So you prefer it like this I can do it this way.

My main concern is to avoid returning the full axios response and manage the "data" properties in the components as it is done in kms code, this makes my eyes bleed with a lot of pain :)

[rjamet-ovh]

Suggested change

	it.skip('should display the secrets listing page', async () => {
	await renderPage();
	});
	it.skip('should display the listing table with all columns', async () => {
	await renderPage();
	const tableHeaders = [
	labels.secretManager.common.path,
	labels.secretManager.common.version,
	labels.secretManager.common.creation_date,
	];
	tableHeaders.forEach((header) => {
	expect(screen.queryAllByText(header)).toHaveLength(1);
	});
	});
	it('should display the secrets listing page', async () => {
	await renderPage();
	});
	it('should display the listing table with all columns', async () => {
	await renderPage();
	const tableHeaders = [
	labels.secretManager.common.path,
	labels.secretManager.common.version,
	labels.secretManager.common.creation_date,
	];
	tableHeaders.forEach((header) => {
	expect(screen.queryAllByText(header)).toHaveLength(1);
	});
	});

forgot to unskip?

[mathieumousnier]

I like to skip things to go faster

[rjamet-ovh]

probably better to use useParams to get the domainId instead of augmenting the secret data.

Suggested change

	export type DatagridItem = {
	okmsId: string;
	secret: Secret;
	};
	export const DatagridCellPath = (item: DatagridItem) => {
	const url = SECRET_MANAGER_ROUTES_URLS.secretDashboard(
	item.okmsId,
	item.secret.path,
	);
	return <Link href={url} label={item.secret.path} useRouter />;
	};
	export const DatagridCellPath = (item: Secret) => {
	const { okmsId } = useParams();
	const url = SECRET_MANAGER_ROUTES_URLS.secretDashboard(
	okmsId,
	item.secret.path,
	);
	return <Link href={url} label={item.secret.path} useRouter />;
	};

[mathieumousnier]

I guess your solution wins because it simplier.
But I am a bit bothered with those useParams that are implying a "context" where the component is used.
So I think I will add a .type file with the SecretListingPageParams that explicitly write the dependency.

This then opens the question about common components that would use useParams hook, and I think we just should avoid doing that.

[rjamet-ovh]

we can get rid of this with useParams in the cell components

[rjamet-ovh]

with only one child, this div with flex rules is not necessary.

Suggested change

	<div className="flex flex-col gap-6">
	<Datagrid
	columns={columns}
	items={items}
	totalItems={items.length}
	isLoading={isPending}
	contentAlignLeft
	/>
	</div>
	<Datagrid
	columns={columns}
	items={items}
	totalItems={items.length}
	isLoading={isPending}
	contentAlignLeft
	/>

[mathieumousnier]

yup, that was copy pasta I should have re-read

[rjamet-ovh]

Suggested change

	export const secretListQueryKey = (okmsId: string) => ['secrets/list', okmsId];
	export const secretListQueryKey = (okmsId: string) => ['secrets', 'list', okmsId];

[mathieumousnier]

Maybe, I took the "/" thing from other code but it already felt weird to me.
I'll think a bit about how we should manage those keys

[rjamet-ovh]

should be Listing.page.spec.tsx

[rjamet-ovh]

should be Listing.page.tsx

[rjamet-ovh]

should be listing.type.ts

[rjamet-ovh]

i'm not sure why this type is necessary?

[mathieumousnier]

We'll talk about this, we'll remove it if we consider it useless.
It feels crazy to me that there is nothing to type what to expect from the router params