Skip to content

Block scene/performer submits with pending URLs #1123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
smith113-p wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Block scene/performer submits with pending URLs #1123

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
480c72b
Block scene submit with pending URL
smith113-p May 29, 2026
a552f9a
Block performer submit with pending URL
smith113-p May 29, 2026
fe875e9
Address review comments
smith113-p Jun 14, 2026
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
20 changes: 17 additions & 3 deletions frontend/src/components/urlInput/urlInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,17 @@ interface URLInputProps {
lens: Lens<URLItem[]>;
type: ValidSiteTypeEnum;
errors?: ErrorsType;
pendingURLError?: string;
onPendingURLChange?: (hasPendingURL: boolean) => void;
}

const URLInput: FC<URLInputProps> = ({ lens, type, errors }) => {
const URLInput: FC<URLInputProps> = ({
lens,
type,
errors,
pendingURLError,
onPendingURLChange,
}) => {
const interop = lens.interop();
const {
fields: urls,
Expand Down Expand Up @@ -70,6 +78,7 @@ const URLInput: FC<URLInputProps> = ({ lens, type, errors }) => {
if (inputRef.current) inputRef.current.value = "";
setSelectedSite(undefined);
setNewURL("");
onPendingURLChange?.(false);
};

const handleInput = (url: string) => {
Expand Down Expand Up @@ -159,14 +168,19 @@ const URLInput: FC<URLInputProps> = ({ lens, type, errors }) => {
ref={inputRef}
onBlur={(e) => handleInput(e.currentTarget.value)}
placeholder="URL"
onChange={(e) => setNewURL(e.currentTarget.value)}
onChange={(e) => {
const value = e.currentTarget.value;
setNewURL(value);
onPendingURLChange?.(value.trim().length > 0);
}}

@Gykes Gykes May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If im reading this correctly the onPendingURLChange notification is wired into onChange. It looks like the existing onPaste handler also calls setNewURL directly if it matches a site. I think if a user pastes in a URL, doesnt add, then saves it will get missed.

@smith113-p smith113-p Jun 14, 2026

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.

Refactored this, thanks for the suggestion, the new code is a lot cleaner I think.

onPaste={handlePaste}
className="w-50"
className={`w-50 ${pendingURLError ? "is-invalid" : ""}`}
/>
<Button onClick={handleAdd} disabled={!newURL || !selectedSite}>
Add
</Button>
</InputGroup>
{pendingURLError && <div className="text-danger">{pendingURLError}</div>}
</div>
);
};
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/pages/performers/performerForm/PerformerForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ const PerformerForm: FC<PerformerProps> = ({
piercings: initial?.piercings ?? performer?.piercings ?? [],
images: initial?.images ?? performer?.images ?? [],
urls: initial?.urls ?? performer?.urls ?? [],
pendingUrl: "",
},
});

Expand Down Expand Up @@ -300,6 +301,7 @@ const PerformerForm: FC<PerformerProps> = ({
error: errors.urls?.find?.((u) => u?.url?.message)?.url?.message,
tab: "links",
},
{ error: errors.pendingUrl?.message, tab: "links" },
].filter((e) => e.error) as { error: string; tab: string }[];

return (
Expand Down Expand Up @@ -668,6 +670,12 @@ const PerformerForm: FC<PerformerProps> = ({
lens={lens.focus("urls").defined()}
type={ValidSiteTypeEnum.PERFORMER}
errors={errors.urls}
pendingURLError={errors.pendingUrl?.message}
onPendingURLChange={(hasPendingURL) =>
setValue("pendingUrl", hasPendingURL ? "pending" : "", {

@Gykes Gykes May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Soring pending vs "" to drive a boolean check seems weird to me and potentially a smell down the line. I would make pendingURL a yup.boolean() or store the pending url.

@Gykes Gykes May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like this is also the same on SceneForm.tsx and schema.ts

@smith113-p smith113-p Jun 14, 2026

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.

Made it store the pending URL.

shouldValidate: true,
})
}
/>

<NavButtons onNext={() => setActiveTab("images")} />
Expand Down
18 changes: 18 additions & 0 deletions frontend/src/pages/performers/performerForm/__tests__/PerformerForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,24 @@ describe("PerformerForm", () => {
expect(callback).not.toHaveBeenCalled();
});

it("blocks submit when a URL is entered but not added", async () => {
const callback = vi.fn();
const { user } = renderEdit(callback);
await user.click(screen.getByRole("tab", { name: "Links" }));
const urlInput = (await waitFor(() => {
const el = document.querySelector('.URLInput input[placeholder="URL"]');
if (!el) throw new Error("URLInput not ready");
return el;
})) as HTMLInputElement;
await user.type(urlInput, "https://example.org/pending");
await submit(user);
const matches = await screen.findAllByText(
"Click Add to include the entered URL before submitting",
);
expect(matches.length).toBeGreaterThan(0);
expect(callback).not.toHaveBeenCalled();
});

it("hides breast type when gender is MALE", async () => {
const { user } = renderEdit();
expect(screen.getByLabelText("Breast type")).toBeInTheDocument();
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/pages/performers/performerForm/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ export const PerformerSchema = yup.object({
}),
)
.ensure(),
pendingUrl: yup
.string()
.test(
"no-pending-url",
"Click Add to include the entered URL before submitting",
(value) => !value,
),
note: yup.string().required("Edit note is required"),
});

Expand Down
9 changes: 9 additions & 0 deletions frontend/src/pages/scenes/sceneForm/SceneForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const SceneForm: FC<SceneProps> = ({
control,
handleSubmit,
watch,
setValue,
formState: { errors },
} = useForm({
resolver: yupResolver(SceneSchema),
Expand All @@ -83,6 +84,7 @@ const SceneForm: FC<SceneProps> = ({
images: initial?.images ?? scene?.images ?? [],
studio: initial?.studio ?? scene?.studio ?? undefined,
tags: initial?.tags ?? scene?.tags ?? [],
pendingUrl: "",
performers: (initial?.performers ?? scene?.performers ?? []).map((p) => ({
performerId: p.performer.id,
name: p.performer.name,
Expand Down Expand Up @@ -318,6 +320,7 @@ const SceneForm: FC<SceneProps> = ({
error: errors.urls?.find?.((u) => u?.url?.message)?.url?.message,
tab: "links",
},
{ error: errors.pendingUrl?.message, tab: "links" },
].filter((e) => e.error) as { error: string; tab: string }[];

return (
Expand Down Expand Up @@ -500,6 +503,12 @@ const SceneForm: FC<SceneProps> = ({
lens={lens.focus("urls").defined()}
type={ValidSiteTypeEnum.SCENE}
errors={errors.urls}
pendingURLError={errors.pendingUrl?.message}
onPendingURLChange={(hasPendingURL) =>
setValue("pendingUrl", hasPendingURL ? "pending" : "", {
shouldValidate: true,
})
}

@Gykes Gykes May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like this is duplicated. My genera rule of thumb is 2+ times to break it out into a hook or a shared component to reduce LOC

@smith113-p smith113-p Jun 14, 2026

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.

Done.

/>

<NavButtons onNext={() => setActiveTab("images")} />
Expand Down
18 changes: 18 additions & 0 deletions frontend/src/pages/scenes/sceneForm/__tests__/SceneForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,24 @@ describe("SceneForm", () => {
expect(callback).not.toHaveBeenCalled();
});

it("blocks submit when a URL is entered but not added", async () => {
const callback = vi.fn();
const { user } = renderEdit(callback);
await user.click(screen.getByRole("tab", { name: "Links" }));
const urlInput = (await waitFor(() => {
const el = document.querySelector('.URLInput input[placeholder="URL"]');
if (!el) throw new Error("URLInput not ready");
return el;
})) as HTMLInputElement;
await user.type(urlInput, "https://pending.example");
await submit(user);
const matches = await screen.findAllByText(
"Click Add to include the entered URL before submitting",
);
expect(matches.length).toBeGreaterThan(0);
expect(callback).not.toHaveBeenCalled();
});

it("disables submit when saving=true", async () => {
const { user } = renderForm(
<SceneForm scene={baseScene} callback={vi.fn()} saving={true} />,
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/pages/scenes/sceneForm/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ export const SceneSchema = yup.object({
}),
)
.ensure(),
pendingUrl: yup
.string()
.test(
"no-pending-url",
"Click Add to include the entered URL before submitting",
(value) => !value,
),
note: yup.string().required("Edit note is required"),
});

Expand Down
Loading