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
5 changes: 5 additions & 0 deletions .changeset/sweet-jars-stand.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"landscape-ui": patch
---

Make the behavior of adding a publication and publishing a mirror or local to a new publication consistent
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,13 @@ const PublishRepositoryExistingForm: FC<PublishRepositoryExistingFormProps> = ({
<ReadOnlyField
label="Publication target"
value={publication.publicationTarget}
tooltipMessage={
"The publication target is defined by the publication."
}
tooltipMessage="The publication target is defined by the publication."
/>

<ReadOnlyField
label="Signing GPG key"
value={publication.gpgKey?.armor}
tooltipMessage={"The GPG key is defined by the publication."}
tooltipMessage="The GPG key is defined by the publication."
/>
</Blocks.Item>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe("MirrorPublicationsLink", () => {
</Suspense>,
);

expect(await screen.findByText("1 publication")).toBeInTheDocument();
expect(await screen.findByText("2 publications")).toBeInTheDocument();
Comment thread
gesquivelgaghi marked this conversation as resolved.
});

it("renders a link with a limited count", async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,34 @@
import Blocks from "@/components/layout/Blocks";
import type { FC } from "react";
import ReadOnlyField from "@/components/form/ReadOnlyField";
import type { Mirror } from "@canonical/landscape-openapi";
import type { Mirror, Publication } from "@canonical/landscape-openapi";

interface PublishMirrorContentsBlockProps {
readonly mirror: Mirror;
readonly publication: Publication;
}

const PublishMirrorContentsBlock: FC<PublishMirrorContentsBlockProps> = ({
mirror,
publication,
}) => {
const architectures = publication.architectures ?? mirror.architectures ?? [];
return (
<Blocks.Item title="Contents">
<ReadOnlyField
label="Distribution"
value={mirror.distribution ?? ""}
tooltipMessage={"The distribution is defined by the mirror."}
value={publication.distribution ?? mirror.distribution}
tooltipMessage="You can’t change the contents of an existing publication."
/>
<ReadOnlyField
label="Components"
value={mirror.components.join(", ")}
tooltipMessage={"The components are defined by the mirror."}
tooltipMessage="The components are defined by the mirror."
/>
<ReadOnlyField
label="Architectures"
value={mirror.architectures?.join(", ")}
tooltipMessage={"The architectures are defined by the mirror."}
value={architectures.join(", ")}
tooltipMessage="You can’t change the contents of an existing publication."
/>
</Blocks.Item>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const PublishMirrorExistingForm: FC<PublishMirrorExistingFormProps> = ({

onSubmit: async (values) => {
try {
await publishPublication({ name: values.name ?? "" });
await publishPublication({ name: values.name });

closeSidePanel();

Expand Down Expand Up @@ -79,7 +79,7 @@ const PublishMirrorExistingForm: FC<PublishMirrorExistingFormProps> = ({
<Blocks>
<Blocks.Item title="Details">
<Select
label="Publication"
label="Publication name"
required
options={publicationOptions}
error={getFormikError(formik, "name")}
Expand All @@ -99,7 +99,7 @@ const PublishMirrorExistingForm: FC<PublishMirrorExistingFormProps> = ({
/>
</Blocks.Item>

<PublishMirrorContentsBlock mirror={mirror} />
<PublishMirrorContentsBlock mirror={mirror} publication={publication} />

<PublicationSettingsBlock publication={publication} />
</Blocks>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ import Blocks from "@/components/layout/Blocks";
import useDebug from "@/hooks/useDebug";
import usePageParams from "@/hooks/usePageParams";
import { getFormikError } from "@/utils/formikErrors";
import { Form, Input, Select, Textarea } from "@canonical/react-components";
import {
Form,
Input,
type MultiSelectItem,
Select,
Textarea,
} from "@canonical/react-components";
import { useFormik } from "formik";
import type { FC } from "react";
import useNotify from "@/hooks/useNotify";
Expand All @@ -13,13 +19,13 @@ import {
PublicationSettingsBlock,
useCreatePublication,
usePublishPublication,
VALIDATION_SCHEMA_NEW,
VALIDATION_SCHEMA_NEW_MIRROR,
} from "@/features/publications";
import PublishMirrorContentsBlock from "../PublishMirrorContentsBlock";
import type { Mirror, PublicationTarget } from "@canonical/landscape-openapi";
import type { SelectOption } from "@/types/SelectOption";
import ReadOnlyField from "@/components/form/ReadOnlyField";
import type { PublishNewFormValues } from "@/features/publications";
import MultiSelectField from "@/components/form/MultiSelectField";

interface PublishMirrorNewFormProps {
readonly mirror: Mirror;
Expand All @@ -39,7 +45,11 @@ const PublishMirrorNewForm: FC<PublishMirrorNewFormProps> = ({
usePublishPublication();

const formik = useFormik({
initialValues: getInitialValues(publicationTargets[0]?.name),
initialValues: {
...getInitialValues(publicationTargets[0]?.name),
distribution: mirror.distribution,
architectures: mirror.architectures,
},
Comment thread
gesquivelgaghi marked this conversation as resolved.

onSubmit: async (values: PublishNewFormValues) => {
const { notAutomatic, butAutomaticUpgrades } =
Expand All @@ -51,7 +61,8 @@ const PublishMirrorNewForm: FC<PublishMirrorNewFormProps> = ({
displayName: values.name,
publicationTarget: values.publicationTarget,
source: mirror.name ?? "",
distribution: mirror.distribution,
distribution: values.distribution,
architectures: values.architectures,
acquireByHash: values.hashIndexing,
notAutomatic,
butAutomaticUpgrades,
Expand Down Expand Up @@ -79,7 +90,7 @@ const PublishMirrorNewForm: FC<PublishMirrorNewFormProps> = ({
}
},

validationSchema: VALIDATION_SCHEMA_NEW,
validationSchema: VALIDATION_SCHEMA_NEW_MIRROR,
validateOnMount: true,
});

Expand All @@ -91,6 +102,22 @@ const PublishMirrorNewForm: FC<PublishMirrorNewFormProps> = ({
}),
);

const architectureOptions =
mirror.architectures?.map((architecture) => ({
label: architecture,
value: architecture,
})) ?? [];

const handleArchitectureChange = async (
items: MultiSelectItem[],
): Promise<void> => {
await formik.setFieldTouched("architectures", true);
await formik.setFieldValue(
"architectures",
items.map(({ value }) => String(value)),
);
};

return (
<Form onSubmit={formik.handleSubmit} noValidate>
<Blocks>
Expand Down Expand Up @@ -128,7 +155,40 @@ const PublishMirrorNewForm: FC<PublishMirrorNewFormProps> = ({
)}
</Blocks.Item>

<PublishMirrorContentsBlock mirror={mirror} />
<Blocks.Item title="Contents">
{mirror.preserveSignatures ? (
<ReadOnlyField
label="Distribution"
value={formik.values.distribution ?? ""}
tooltipMessage="You can't change the distribution of a signature-preserving mirror."
/>
) : (
<Input
type="text"
label="Distribution"
required
error={getFormikError(formik, "distribution")}
{...formik.getFieldProps("distribution")}
/>
Comment thread
gesquivelgaghi marked this conversation as resolved.
)}
<ReadOnlyField
label="Components"
value={mirror.components.join(", ")}
tooltipMessage="The components are defined by the mirror."
/>
<MultiSelectField
variant="condensed"
hasSelectedItemsFirst={false}
label="Architectures"
required
items={architectureOptions}
selectedItems={architectureOptions.filter(({ value }) =>
formik.values.architectures?.includes(value),
)}
onItemsUpdate={handleArchitectureChange}
error={getFormikError(formik, "architectures")}
/>
</Blocks.Item>

<PublicationSettingsBlock formik={formik} />
</Blocks>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { expectLoadingState } from "@/tests/helpers";
import Suspense from "@/components/layout/SidePanel/Suspense";
import LoadingState from "@/components/layout/SidePanel/LoadingState";
import { mirrors } from "@/tests/mocks/mirrors";
import type { Mirror } from "@canonical/landscape-openapi";
import type { Mirror, Publication } from "@canonical/landscape-openapi";
import { publications } from "@/tests/mocks/publications";

const COPIED_FEEDBACK_TIMEOUT = 2010;
Expand Down Expand Up @@ -160,7 +160,7 @@ describe("ViewLogsSidePanel", () => {
});

it("shows empty state when operation has no error details", async () => {
const noLogsPublication = publications.find(
const noLogsPublication = (publications as Publication[]).find(
(pub) => pub.lastOperation === "operations/ssss-cccc-dddd",
);
Comment thread
gesquivelgaghi marked this conversation as resolved.
assert(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ describe("PublicationTargetList", () => {
renderWithProviders(<PublicationTargetList targets={publicationTargets} />);

await waitFor(() => {
expect(screen.getAllByText("1 publication")).toHaveLength(3);
expect(screen.getByText("1 publication")).toBeInTheDocument();
expect(screen.getAllByText("2 publications")).toHaveLength(2);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as above, if it's not too hard could be worth filtering the mock data to get these numbers instead of hardcoding them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there's a lot of cases where this is a problem and I have another PR which also changes the mocks and affects some of these, so i'll address these separately

expect(screen.getByText(NO_DATA_TEXT)).toBeInTheDocument();
});
expect(screen.getByText(NO_DATA_TEXT)).toBeInTheDocument();
});

it("renders table correctly with empty targets array", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,16 @@ describe("AddPublicationForm", () => {
expect(screen.getByText("distribution 1")).toBeInTheDocument();
});

it("hides signing key field when local repository is selected", async () => {
it("shows signing key field when local repository is selected", async () => {
const user = userEvent.setup();

renderForm();

await selectLocalSource(user);

expect(
screen.queryByRole("textbox", { name: "Signing GPG key" }),
).not.toBeInTheDocument();
screen.getByRole("textbox", { name: "Signing GPG key" }),
).toBeInTheDocument();
});

it("shows validation error when all architectures are deselected", async () => {
Expand Down
Loading
Loading