Refactor url form field

app/features/calendar/actions/calendar.new.server.test.ts

@@ -5,6 +5,7 @@ import * as TournamentFactory from "~/db/seed/factories/TournamentFactory";
 import * as TournamentOrganizationFactory from "~/db/seed/factories/TournamentOrganizationFactory";
 import * as UserFactory from "~/db/seed/factories/UserFactory";
 import * as CalendarRepository from "~/features/calendar/CalendarRepository.server";
+import invariant from "~/utils/invariant";
 import { wrappedAction } from "~/utils/Test";
 import type { calendarNewSchemaServer } from "../calendar-new-schemas.server";
 import { defaultBracketsFormValues } from "../calendar-progression-form";
@@ -127,3 +128,84 @@ describe("calendar new action: editing an event with badge prizes", () => {
 		expect(await badgePrizeIds(tournament.eventId)).toEqual([badge.id]);
 	});
 });
+
+describe("calendar new action: bracket URL", () => {
+	beforeEach(async () => {
+		await UserFactory.createRegular(null, { roles: ["TOURNAMENT_ORGANIZER"] });
+	});
+
+	const newEventFields = (
+		overrides: Partial<Parameters<typeof editAction>[0]>,
+	) => ({
+		toToolsEnabled: false,
+		name: "In The Zone",
+		description: "",
+		organizationId: "",
+		rules: "",
+		date: [addDays(new Date(), 7).toISOString() as never],
+		startTime: null,
+		bracketUrl: "",
+		discordInviteCode: "",
+		tags: [],
+		badges: [],
+		trophyId: null,
+		avatarImgId: null,
+		regClosesAt: "0" as const,
+		minMembersPerTeam: "4" as const,
+		maxMembersPerTeam: undefined,
+		toToolsMode: "TO" as const,
+		pool: "",
+		...defaultBracketsFormValues(),
+		isRanked: true,
+		enableNoScreenToggle: true,
+		enableSubs: true,
+		autonomousSubs: true,
+		requireInGameNames: false,
+		isInvitational: false,
+		isTest: false,
+		isDraft: false,
+		requireSendouQParticipation: false,
+		...overrides,
+	});
+
+	test.each([
+		{
+			why: "missing",
+			bracketUrl: "",
+			expectedError: "forms:errors.bracketUrlRequired",
+		},
+		{
+			why: "javascript: protocol",
+			bracketUrl: "javascript:alert(1)",
+			expectedError: "forms:errors.invalidUrl",
+		},
+	])("rejects bracket URL ($why)", async ({ bracketUrl, expectedError }) => {
+		const res = await editAction(newEventFields({ bracketUrl }), {
+			user: "regular",
+		});
+
+		expect(res.fieldErrors.bracketUrl).toBe(expectedError);
+	});
+
+	test("tournament with no bracket URL gets the default one", async () => {
+		const res = await editAction(
+			newEventFields({
+				toToolsEnabled: true,
+				date: [],
+				startTime: addDays(new Date(), 7).toISOString() as never,
+			}),
+			{ user: "regular" },
+		);
+
+		expect(res.fieldErrors).toBeUndefined();
+
+		const location =
+			res instanceof Response ? res.headers.get("Location") : null;
+		invariant(location, "expected a redirect to the created event");
+
+		const created = await CalendarRepository.findById(
+			Number(location.split("/").at(-1)),
+		);
+		expect(created?.bracketUrl).toBe("https://sendou.ink");
+	});
+});

app/features/calendar/routes/calendar.new.tsx

@@ -198,9 +198,9 @@ function useDefaultValues() {
 				? databaseTimestampToDate(data.eventToEdit.startTimes[0])
 				: getDateAtNextFullHour(new Date())
 			: null,
-		// tournaments hide this field, so seed a valid URL for the url-format validation (the action coalesces to the same default)
+		// tournaments hide this field, the action coalesces the empty value to the default
 		bracketUrl: data.isAddingTournament
-			? "https://sendou.ink"
+			? ""
 			: (data.eventToEdit?.bracketUrl ?? ""),
 		discordInviteCode: baseEvent?.discordInviteCode ?? "",
 		tags: baseEvent?.tags ?? [],

app/form/fields.test.ts

@@ -3,7 +3,7 @@ import { describe, expect, test } from "vitest";
 import { textField, textFieldOptional } from "./fields";
 
 describe("textField", () => {
-	const schema = textField({ validate: "url", maxLength: 150 });
+	const urlSchema = textField({ validate: "url", maxLength: 30 });
 
 	test.each([
 		["https://sendou.ink", true, "https URL"],
@@ -12,18 +12,27 @@ describe("textField", () => {
 		["JavaScript:alert(1)", false, "javascript URL with mixed case protocol"],
 		["data:text/html,<script>alert(1)</script>", false, "data URL"],
 		["not a url", false, "not a URL at all"],
+		["https://sendou.ink/aaaaaaaaaaaaaaaaaaaaaa", false, "URL over maxLength"],
 	])("%s -> %s (%s)", (input, expected) => {
-		expect(v.safeParse(schema, input).success).toBe(expected);
+		expect(v.safeParse(urlSchema, input).success).toBe(expected);
 	});
 });
 
 describe("textFieldOptional", () => {
-	const schema = textFieldOptional({ validate: "url", maxLength: 150 });
+	const urlSchema = textFieldOptional({ validate: "url", maxLength: 30 });
 
 	test.each([
 		["https://sendou.ink", true, "https URL"],
 		["javascript:alert(1)", false, "javascript URL"],
+		["https://sendou.ink/aaaaaaaaaaaaaaaaaaaaaa", false, "URL over maxLength"],
 	])("%s -> %s (%s)", (input, expected) => {
-		expect(v.safeParse(schema, input).success).toBe(expected);
+		expect(v.safeParse(urlSchema, input).success).toBe(expected);
+	});
+
+	test.each([
+		{ input: "", why: "empty string" },
+		{ input: undefined, why: "missing value" },
+	])("parses to null ($why)", ({ input }) => {
+		expect(v.parse(urlSchema, input)).toBeNull();
 	});
 });

app/form/fields.ts

@@ -40,18 +40,6 @@ import type {
 	TrophyOption,
 } from "./types";
 
-const httpUrlSchema = v.pipe(
-	v.string(),
-	v.url(),
-	v.check((value) => {
-		try {
-			return ["http:", "https:"].includes(new URL(value).protocol);
-		} catch {
-			return false;
-		}
-	}, "Only http(s) URLs are allowed."),
-);
-
 export const formRegistry = new WeakMap<object, FormField>();
 
 /** Clones the schema first so shared instances (e.g. `id`, `stageId`) each get their own registry entry. */
@@ -165,11 +153,6 @@ type TextFieldArgs = WithTypedTranslationKeys<
 export function textFieldOptional(
 	args: TextFieldArgs,
 ): v.GenericSchema<string | null, string | null> {
-	// validated as a plain string, so unlike other optional text fields it has no null fallback and its key stays required
-	if (args.validate === "url") {
-		return registerTextField(httpUrlSchema, args, false, false) as never;
-	}
-
 	return registerTextField(
 		safeNullableStringSchema({ min: args.minLength, max: args.maxLength }),
 		args,
@@ -179,12 +162,12 @@ export function textFieldOptional(
 }
 
 export function textField(args: TextFieldArgs): v.GenericSchema<string> {
-	const schema =
-		args.validate === "url"
-			? httpUrlSchema
-			: safeStringSchema({ min: args.minLength, max: args.maxLength });
-
-	return registerTextField(schema, args, true, false) as never;
+	return registerTextField(
+		safeStringSchema({ min: args.minLength, max: args.maxLength }),
+		args,
+		true,
+		false,
+	) as never;
 }
 
 function registerTextField<T extends v.GenericSchema<any, string | null>>(
@@ -214,6 +197,16 @@ function textFieldRefined<T extends v.GenericSchema<any, string | null>>(
 ): v.GenericSchema<any, string | null> {
 	let result: v.GenericSchema<any, string | null> = schema;
 
+	if (args.validate === "url") {
+		result = v.pipe(
+			result,
+			v.check(
+				(val) => val === null || isHttpUrl(val),
+				"forms:errors.invalidUrl",
+			),
+		);
+	}
+
 	if (args.regExp) {
 		result = v.pipe(
 			result,
@@ -246,6 +239,15 @@ function textFieldRefined<T extends v.GenericSchema<any, string | null>>(
 	return result;
 }
 
+/** Only http(s) is allowed so that e.g. a `javascript:` URL can never end up in a rendered link. */
+function isHttpUrl(value: string) {
+	try {
+		return ["http:", "https:"].includes(new URL(value).protocol);
+	} catch {
+		return false;
+	}
+}
+
 export function inGameName(
 	args: WithTypedTranslationKeys<{
 		label?: FormsTranslationKey;