Skip to content

Commit

Permalink
fix(storage) simplify volume detail page and fix small issues
Browse files Browse the repository at this point in the history
  • Loading branch information
edlerd committed Sep 27, 2023
1 parent 836f0f5 commit d0ca988
Show file tree
Hide file tree
Showing 19 changed files with 97 additions and 55 deletions.
2 changes: 1 addition & 1 deletion src/pages/projects/forms/DiskSizeSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const DiskSizeSelector: FC<Props> = ({ value, setMemoryLimit, disabled }) => {
step="Any"
placeholder="Enter value"
onChange={(e) => setMemoryLimit(e.target.value + limit.unit)}
value={limit.value}
value={value?.match(/^\d/) ? limit.value : ""}
disabled={disabled}
/>
<Select
Expand Down
5 changes: 5 additions & 0 deletions src/pages/storage/CustomIsoList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
List,
MainTable,
SearchBox,
useNotify,
} from "@canonical/react-components";
import { humanFileSize, isoTimeToString } from "util/helpers";
import { useQuery } from "@tanstack/react-query";
Expand All @@ -20,6 +21,7 @@ interface Props {
}

const CustomIsoList: FC<Props> = ({ project }) => {
const notify = useNotify();
const [query, setQuery] = useState<string>("");

const { data: images = [], isLoading } = useQuery({
Expand Down Expand Up @@ -57,6 +59,9 @@ const CustomIsoList: FC<Props> = ({ project }) => {
pool={image.pool ?? ""}
volume={image.volume}
project={project}
onFinish={() =>
notify.success(`Custom iso ${image.aliases} deleted.`)
}
/>,
]}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/storage/StorageDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const StorageDetail: FC = () => {

return (
<main className="l-main">
<div className="p-panel instance-detail-page">
<div className="p-panel">
<StorageDetailHeader
name={name}
storagePool={storagePool}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/storage/StorageDetailHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as Yup from "yup";
import { LxdStoragePool } from "types/storage";
import { renameStoragePool } from "api/storage-pools";
import DeleteStorageBtn from "pages/storage/actions/DeleteStorageBtn";
import { testDuplicateName } from "util/storagePool";
import { testDuplicateStoragePoolName } from "util/storagePool";
import { useNotify } from "@canonical/react-components";

interface Props {
Expand All @@ -22,7 +22,7 @@ const StorageDetailHeader: FC<Props> = ({ name, storagePool, project }) => {

const RenameSchema = Yup.object().shape({
name: Yup.string()
.test(...testDuplicateName(project, controllerState))
.test(...testDuplicateStoragePoolName(project, controllerState))
.required("This field is required"),
});

Expand Down
6 changes: 4 additions & 2 deletions src/pages/storage/StoragePoolForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
zfsDriver,
} from "util/storageOptions";
import ItemName from "components/ItemName";
import { testDuplicateName } from "util/storagePool";
import { testDuplicateStoragePoolName } from "util/storagePool";
import NotificationRow from "components/NotificationRow";

const StoragePoolForm: FC = () => {
Expand All @@ -37,7 +37,9 @@ const StoragePoolForm: FC = () => {

const StorageSchema = Yup.object().shape({
name: Yup.string()
.test(...testDuplicateName(panelParams.project, controllerState))
.test(
...testDuplicateStoragePoolName(panelParams.project, controllerState)
)
.required("This field is required"),
});

Expand Down
2 changes: 1 addition & 1 deletion src/pages/storage/StorageUsedBy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const StorageUsedBy: FC<Props> = ({ storage, project }) => {
<td>
<ExpandableList
items={data[SNAPSHOTS].map((item) => (
<div key={`${item.name}-${item.project}`}>
<div key={`${item.instance}-${item.name}-${item.project}`}>
<Link
to={`/ui/project/${item.project}/instances/detail/${item.instance}/snapshots`}
>
Expand Down
4 changes: 2 additions & 2 deletions src/pages/storage/StorageVolumeDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const StorageVolumeDetail: FC = () => {

return (
<main className="l-main">
<div className="p-panel instance-detail-page">
<div className="p-panel">
<StorageVolumeHeader
storagePool={pool}
volume={volume}
Expand All @@ -104,7 +104,7 @@ const StorageVolumeDetail: FC = () => {
)}

{activeTab === "configuration" && (
<div role="tabpanel" aria-labelledby="volumes">
<div role="tabpanel" aria-labelledby="configuration">
<StorageVolumeEdit volume={volume} pool={pool} />
</div>
)}
Expand Down
28 changes: 16 additions & 12 deletions src/pages/storage/StorageVolumeHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import { useFormik } from "formik";
import * as Yup from "yup";
import { LxdStorageVolume } from "types/storage";
import { renameStorageVolume } from "api/storage-pools";
import { testDuplicateName } from "util/storageVolume";
import { testDuplicateStorageVolumeName } from "util/storageVolume";
import { useNotify } from "@canonical/react-components";
import { useQueryClient } from "@tanstack/react-query";
import { queryKeys } from "util/queryKeys";
import DeleteStorageVolumeBtn from "pages/storage/actions/DeleteStorageVolumeBtn";

interface Props {
Expand All @@ -21,12 +19,17 @@ const StorageVolumeHeader: FC<Props> = ({ volume, project, storagePool }) => {
const navigate = useNavigate();
const notify = useNotify();
const controllerState = useState<AbortController | null>(null);
const queryClient = useQueryClient();

const RenameSchema = Yup.object().shape({
name: Yup.string()
.test(
...testDuplicateName(project, volume.type, storagePool, controllerState)
...testDuplicateStorageVolumeName(
project,
volume.type,
storagePool,
controllerState,
volume.name
)
)
.required("This field is required"),
});
Expand Down Expand Up @@ -56,13 +59,6 @@ const StorageVolumeHeader: FC<Props> = ({ volume, project, storagePool }) => {
})
.finally(() => {
formik.setSubmitting(false);
void queryClient.invalidateQueries([
queryKeys.storage,
storagePool,
project,
volume.type,
volume.name,
]);
});
},
});
Expand All @@ -89,6 +85,14 @@ const StorageVolumeHeader: FC<Props> = ({ volume, project, storagePool }) => {
project={project}
appearance=""
hasIcon={false}
onFinish={() => {
navigate(
`/ui/project/${project}/storage/detail/${storagePool}/volumes`,
notify.queue(
notify.success(`Storage volume ${volume.name} deleted.`)
)
);
}}
/>
}
isLoaded={true}
Expand Down
28 changes: 19 additions & 9 deletions src/pages/storage/StorageVolumeOverview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,26 @@ const StorageVolumeOverview: FC<Props> = ({ project, volume }) => {
<td>{isoTimeToString(volume.created_at)}</td>
</tr>
<tr>
<th className="p-muted-heading">Config</th>
<th className="p-muted-heading">Custom config</th>
<td>
{Object.entries(volume.config).map(([key, value], id) => {
return (
<div key={id}>
<div>{key}</div>
<div>{value}</div>
</div>
);
})}
{Object.entries(volume.config).length === 0 ? (
"-"
) : (
<table>
<tbody>
{Object.entries(volume.config).map(
([key, value], id) => {
return (
<tr key={id}>
<th className="u-text--muted">{key}</th>
<td>{value}</td>
</tr>
);
}
)}
</tbody>
</table>
)}
</td>
</tr>
</tbody>
Expand Down
4 changes: 4 additions & 0 deletions src/pages/storage/StorageVolumes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const StorageVolumes: FC = () => {
return {
columns: [
{
// If the volume name contains a slash, it's a snapshot, and we don't want to link to it
content: volume.name.includes("/") ? (
volume.name
) : (
Expand Down Expand Up @@ -124,6 +125,9 @@ const StorageVolumes: FC = () => {
pool={pool}
volume={volume}
project={project}
onFinish={() => {
notify.success(`Storage volume ${volume.name} deleted.`);
}}
/>
</>
),
Expand Down
11 changes: 3 additions & 8 deletions src/pages/storage/actions/DeleteStorageVolumeBtn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import {
Icon,
useNotify,
} from "@canonical/react-components";
import { useNavigate } from "react-router-dom";

interface Props {
pool: string;
volume: LxdStorageVolume;
project: string;
onFinish: () => void;
appearance?: string;
hasIcon?: boolean;
label?: string;
Expand All @@ -23,11 +23,11 @@ const DeleteStorageVolumeBtn: FC<Props> = ({
pool,
project,
volume,
onFinish,
appearance = "base",
hasIcon = true,
label,
}) => {
const navigate = useNavigate();
const notify = useNotify();
const [isLoading, setLoading] = useState(false);
const queryClient = useQueryClient();
Expand All @@ -40,12 +40,7 @@ const DeleteStorageVolumeBtn: FC<Props> = ({
setLoading(true);

deleteStorageVolume(volume.name, pool, project)
.then(() =>
navigate(
`/ui/project/${project}/storage/detail/${pool}/volumes`,
notify.queue(notify.success(`Storage volume ${volume.name} deleted.`))
)
)
.then(onFinish)
.catch((e) => {
notify.failure("Storage volume deletion failed", e);
})
Expand Down
12 changes: 10 additions & 2 deletions src/pages/storage/forms/StorageVolumeCreate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import SubmitButton from "components/SubmitButton";
import { createStorageVolume } from "api/storage-pools";
import NotificationRow from "components/NotificationRow";
import { useNavigate, useParams } from "react-router-dom";
import { testDuplicateName } from "util/storageVolume";
import { testDuplicateStorageVolumeName } from "util/storageVolume";
import BaseLayout from "components/BaseLayout";
import {
StorageVolumeFormValues,
Expand All @@ -33,13 +33,21 @@ const StorageVolumeCreate: FC = () => {

const StorageSchema = Yup.object().shape({
name: Yup.string()
.test(...testDuplicateName(project, "custom", pool, controllerState))
.test(
...testDuplicateStorageVolumeName(
project,
"custom",
pool,
controllerState
)
)
.required("This field is required"),
});

const formik = useFormik<StorageVolumeFormValues>({
initialValues: {
content_type: "filesystem",
type: "custom",
name: "",
project: project,
pool: pool,
Expand Down
3 changes: 2 additions & 1 deletion src/pages/storage/forms/StorageVolumeForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface StorageVolumeFormValues {
pool: string;
size?: string;
content_type: "filesystem" | "block";
type: string;
security_shifted?: string;
security_unmapped?: string;
snapshots_expiry?: string;
Expand Down Expand Up @@ -67,7 +68,7 @@ export const volumeFormToPayload = (
[getVolumeKey("zfs_reserve_space")]: values.zfs_reserve_space,
},
project,
type: "custom",
type: values.type,
content_type: values.content_type,
description: "",
location: "",
Expand Down
9 changes: 7 additions & 2 deletions src/pages/storage/forms/StorageVolumeFormMain.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@ const StorageVolumeFormMain: FC<Props> = ({ formik }) => {
{...getFormProps(formik, "name")}
type="text"
label="Name"
disabled={formik.values.isReadOnly}
required
disabled={formik.values.isReadOnly || !formik.values.isCreating}
required={formik.values.isCreating}
help={
formik.values.isCreating
? undefined
: "Click the name in the header to rename the instance"
}
/>
<label className="p-form__label" htmlFor="limits_disk">
Size
Expand Down
6 changes: 4 additions & 2 deletions src/util/snapshots.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const isInstanceStateful = (instance: LxdInstance) => {
export const getExpiresAt = (expirationDate: string, expirationTime: string) =>
`${expirationDate}T${expirationTime}`;

export const testDuplicateName = (
export const testDuplicateSnapshotName = (
instance: LxdInstance,
controllerState: AbortControllerState,
excludeName?: string
Expand Down Expand Up @@ -124,7 +124,9 @@ export const getSnapshotSchema = (
return Yup.object().shape({
name: Yup.string()
.required("This field is required")
.test(...testDuplicateName(instance, controllerState, snapshotName))
.test(
...testDuplicateSnapshotName(instance, controllerState, snapshotName)
)
.test(...testForbiddenChars()),
expirationDate: Yup.string()
.nullable()
Expand Down
2 changes: 1 addition & 1 deletion src/util/storagePool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { AbortControllerState, checkDuplicateName } from "util/helpers";
import { TestFunction } from "yup";
import { AnyObject } from "yup/lib/types";

export const testDuplicateName = (
export const testDuplicateStoragePoolName = (
project: string,
controllerState: AbortControllerState
): [string, string, TestFunction<string | undefined, AnyObject>] => {
Expand Down
18 changes: 11 additions & 7 deletions src/util/storageVolume.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,25 @@ import { TestFunction } from "yup";
import { AnyObject } from "yup/lib/types";
import { LxdStoragePool } from "types/storage";

export const testDuplicateName = (
export const testDuplicateStorageVolumeName = (
project: string,
volumeType: string,
storagePool: string,
controllerState: AbortControllerState
controllerState: AbortControllerState,
previousName?: string
): [string, string, TestFunction<string | undefined, AnyObject>] => {
return [
"deduplicate",
"A storage volume with this name already exists",
(value?: string) => {
return checkDuplicateName(
value,
project,
controllerState,
`storage-pools/${storagePool}/volumes/${volumeType}`
return (
value === previousName ||
checkDuplicateName(
value,
project,
controllerState,
`storage-pools/${storagePool}/volumes/${volumeType}`
)
);
},
];
Expand Down
Loading

0 comments on commit d0ca988

Please sign in to comment.