Builds cleanup/fixes (#2957)

app/components/BuildCard.tsx

@@ -72,7 +72,7 @@ export function BuildCard({ build, owner, canEdit = false }: BuildProps) {
 	} = build;
 
 	const isNoGear = [headGearSplId, clothesGearSplId, shoesGearSplId].some(
-		(id) => id === -1,
+		(id) => typeof id !== "number",
 	);
 
 	return (
@@ -274,7 +274,7 @@ function AbilitiesRowWithGear({
 }: {
 	gearType: GearType;
 	abilities: AbilityType[];
-	gearId: number;
+	gearId: number | null;
 }) {
 	const { t } = useTranslation(["gear"]);
 	const translatedGearName = t(
@@ -283,7 +283,7 @@ function AbilitiesRowWithGear({
 
 	return (
 		<>
-			{gearId !== -1 ? (
+			{typeof gearId === "number" ? (
 				<Image
 					height={64}
 					width={64}

app/db/tables.ts

@@ -150,14 +150,14 @@ export type BadgeOwner = {
 };
 
 export interface Build {
-	clothesGearSplId: number;
+	clothesGearSplId: number | null;
 	description: string | null;
-	headGearSplId: number;
+	headGearSplId: number | null;
 	id: GeneratedAlways<number>;
 	modes: JSONColumnTypeNullable<ModeShort[]>;
 	ownerId: number;
 	private: DBBoolean | null;
-	shoesGearSplId: number;
+	shoesGearSplId: number | null;
 	title: string;
 	updatedAt: Generated<number>;
 }

app/features/build-stats/build-stats-utils.ts

@@ -1,3 +1,4 @@
+import * as R from "remeda";
 import { abilities } from "~/modules/in-game-lists/abilities";
 import type { Ability } from "~/modules/in-game-lists/types";
 import invariant from "~/utils/invariant";
@@ -101,18 +102,14 @@ type AbilityCountsMap = Map<Ability, number>;
 const POPULAR_BUILDS_TO_SHOW = 25;
 
 export function popularBuilds(builds: Array<AbilitiesByWeapon>) {
-	const counts = new Map<string, number>();
-	for (const build of builds) {
-		const summedUpAbilities = sumUpAbilities(build);
-		const serializedAbilities = serializeAbilityCountsMap(summedUpAbilities);
-
-		counts.set(serializedAbilities, (counts.get(serializedAbilities) ?? 0) + 1);
-	}
-
-	const serializedToShow = Array.from(counts.entries())
-		.sort((a, b) => b[1] - a[1])
-		.filter(([, count]) => count > 1)
-		.slice(0, POPULAR_BUILDS_TO_SHOW);
+	const serializedToShow = R.pipe(
+		builds,
+		R.countBy((build) => serializeAbilityCountsMap(sumUpAbilities(build))),
+		R.entries(),
+		R.sortBy([([, count]) => count, "desc"]),
+		R.filter(([, count]) => count > 1),
+		R.take(POPULAR_BUILDS_TO_SHOW),
+	);
 
 	return serializedToShowToResultType(serializedToShow);
 }

app/features/builds/BuildRepository.server.ts

@@ -1,5 +1,6 @@
 import type { ExpressionBuilder, Transaction } from "kysely";
 import { jsonArrayFrom } from "kysely/helpers/sqlite";
+import * as R from "remeda";
 import { db } from "~/db/sql";
 import type { BuildWeapon, DB, Tables, TablesInsertable } from "~/db/tables";
 import { modesShort } from "~/modules/in-game-lists/modes";
@@ -10,6 +11,7 @@ import type {
 	ModeShort,
 } from "~/modules/in-game-lists/types";
 import { weaponIdToArrayWithAlts } from "~/modules/in-game-lists/weapon-ids";
+import { dateToDatabaseTimestamp } from "~/utils/dates";
 import { LimitReachedError } from "~/utils/errors";
 import invariant from "~/utils/invariant";
 import { commonUserJsonObject } from "~/utils/kysely.server";
@@ -76,22 +78,15 @@ function dbAbilitiesToArrayOfArrays(
 		Pick<Tables["BuildAbility"], "ability" | "gearType" | "slotIndex">
 	>,
 ): BuildAbilitiesTuple {
-	const sorted = abilities
-		.slice()
-		.sort((a, b) => {
-			if (a.gearType === b.gearType) return a.slotIndex - b.slotIndex;
-
-			return gearOrder.indexOf(a.gearType) - gearOrder.indexOf(b.gearType);
-		})
-		.map((a) => a.ability);
+	const sorted = R.sortBy(
+		abilities,
+		(a) => gearOrder.indexOf(a.gearType),
+		(a) => a.slotIndex,
+	).map((a) => a.ability);
 
 	invariant(sorted.length === 12, "expected 12 abilities");
 
-	return [
-		[sorted[0], sorted[1], sorted[2], sorted[3]],
-		[sorted[4], sorted[5], sorted[6], sorted[7]],
-		[sorted[8], sorted[9], sorted[10], sorted[11]],
-	];
+	return R.chunk(sorted, 4) as BuildAbilitiesTuple;
 }
 
 interface CreateArgs {
@@ -107,6 +102,14 @@ interface CreateArgs {
 	private: TablesInsertable["Build"]["private"];
 }
 
+function serializeModes(modes: Array<ModeShort> | null) {
+	if (!modes || modes.length === 0) return null;
+
+	return JSON.stringify(
+		modes.slice().sort((a, b) => modesShort.indexOf(a) - modesShort.indexOf(b)),
+	);
+}
+
 async function createInTrx({
 	args,
 	trx,
@@ -120,22 +123,29 @@ async function createInTrx({
 			ownerId: args.ownerId,
 			title: args.title,
 			description: args.description,
-			modes:
-				args.modes && args.modes.length > 0
-					? JSON.stringify(
-							args.modes
-								.slice()
-								.sort((a, b) => modesShort.indexOf(a) - modesShort.indexOf(b)),
-						)
-					: null,
-			headGearSplId: args.headGearSplId ?? -1,
-			clothesGearSplId: args.clothesGearSplId ?? -1,
-			shoesGearSplId: args.shoesGearSplId ?? -1,
+			modes: serializeModes(args.modes),
+			headGearSplId: args.headGearSplId,
+			clothesGearSplId: args.clothesGearSplId,
+			shoesGearSplId: args.shoesGearSplId,
 			private: args.private,
 		})
 		.returningAll()
 		.executeTakeFirstOrThrow();
 
+	await populateBuildChildrenInTrx({ trx, buildId, updatedAt, args });
+}
+
+async function populateBuildChildrenInTrx({
+	trx,
+	buildId,
+	updatedAt,
+	args,
+}: {
+	trx: Transaction<DB>;
+	buildId: number;
+	updatedAt: number;
+	args: CreateArgs;
+}) {
 	await trx
 		.insertInto("BuildWeapon")
 		.values(
@@ -204,8 +214,37 @@ export async function create(args: CreateArgs) {
 
 export async function update(args: CreateArgs & { id: number }) {
 	return db.transaction().execute(async (trx) => {
-		await trx.deleteFrom("Build").where("id", "=", args.id).execute();
-		await createInTrx({ args, trx });
+		const { updatedAt } = await trx
+			.updateTable("Build")
+			.set({
+				title: args.title,
+				description: args.description,
+				modes: serializeModes(args.modes),
+				headGearSplId: args.headGearSplId,
+				clothesGearSplId: args.clothesGearSplId,
+				shoesGearSplId: args.shoesGearSplId,
+				private: args.private,
+				updatedAt: dateToDatabaseTimestamp(new Date()),
+			})
+			.where("id", "=", args.id)
+			.returning("updatedAt")
+			.executeTakeFirstOrThrow();
+
+		await trx
+			.deleteFrom("BuildWeapon")
+			.where("buildId", "=", args.id)
+			.execute();
+		await trx
+			.deleteFrom("BuildAbility")
+			.where("buildId", "=", args.id)
+			.execute();
+
+		await populateBuildChildrenInTrx({
+			trx,
+			buildId: args.id,
+			updatedAt,
+			args,
+		});
 	});
 }
 
@@ -408,8 +447,8 @@ function hasXRankPlacement(eb: ExpressionBuilder<DB, "BuildWeapon">) {
 		eb
 			.selectFrom("Build")
 			.select("BuildWeapon.buildId")
-			.leftJoin("SplatoonPlayer", "SplatoonPlayer.userId", "Build.ownerId")
-			.leftJoin(
+			.innerJoin("SplatoonPlayer", "SplatoonPlayer.userId", "Build.ownerId")
+			.innerJoin(
 				"XRankPlacement",
 				"XRankPlacement.playerId",
 				"SplatoonPlayer.id",

app/features/builds/builds-constants.ts

@@ -4,9 +4,11 @@ export const MAX_BUILD_FILTERS = 6;
 
 export const FILTER_SEARCH_PARAM_KEY = "f";
 
-export const PATCHES = [
+type Patch = { patch: string; date: string };
+
+export const PATCHES: Array<Patch> = [
 	{
-		path: "11.1.0",
+		patch: "11.1.0",
 		date: "2026-03-18",
 	},
 	{

app/features/builds/builds-schemas.server.ts

@@ -1,11 +1,12 @@
 import { z } from "zod";
+import { MAX_AP } from "~/features/build-analyzer/analyzer-constants";
 import { ability, modeShort, safeJSONParse } from "~/utils/zod";
 import { MAX_BUILD_FILTERS } from "./builds-constants";
 
 const abilityFilterSchema = z.object({
 	type: z.literal("ability"),
 	ability: z.string().toUpperCase().pipe(ability),
-	value: z.union([z.number(), z.boolean()]),
+	value: z.union([z.int().min(0).max(MAX_AP), z.boolean()]),
 	comparison: z
 		.string()
 		.toUpperCase()
@@ -20,7 +21,7 @@ const modeFilterSchema = z.object({
 
 const dateFilterSchema = z.object({
 	type: z.literal("date"),
-	date: z.string(),
+	date: z.iso.date(),
 });
 
 export const buildFiltersSearchParams = z.preprocess(

app/features/builds/builds-types.ts

@@ -9,26 +9,24 @@ export interface BuildWeaponWithTop500Info {
 	isTop500: number;
 }
 
-type WithId<T> = T & { id: string };
-
-export type AbilityBuildFilter = WithId<{
+export type AbilityBuildFilter = {
 	type: "ability";
 	ability: Ability;
 	/** Ability points value or "has"/"doesn't have" */
-	value?: number | boolean;
+	value: number | boolean;
 	comparison?: "AT_LEAST" | "AT_MOST";
-}>;
+};
 
-export type ModeBuildFilter = WithId<{
+export type ModeBuildFilter = {
 	type: "mode";
 	mode: ModeShort;
-}>;
+};
 
-export type DateBuildFilter = WithId<{
+export type DateBuildFilter = {
 	type: "date";
 	/** YYYY-MM-DD */
 	date: string;
-}>;
+};
 
 export type BuildFilter =
 	| AbilityBuildFilter

app/features/builds/components/FilterSection.tsx

@@ -12,7 +12,7 @@ import type {
 	Ability as AbilityType,
 	ModeShort,
 } from "~/modules/in-game-lists/types";
-import { dateToYYYYMMDD } from "~/utils/dates";
+import { dateToYYYYMMDD, isValidDate } from "~/utils/dates";
 import { PATCHES } from "../builds-constants";
 import type {
 	AbilityBuildFilter,
@@ -198,27 +198,20 @@ function DateFilter({
 	const { t } = useTranslation(["builds"]);
 	const { formatDate } = useTimeFormat();
 
-	const selectValue = () => {
-		const dateString = dateToYYYYMMDD(new Date(filter.date));
-
-		if (
-			PATCHES.find(({ date }) => {
-				return new Date(date).toISOString().split("T")[0] === dateString;
-			})
-		) {
-			return dateString;
-		}
-
-		return "CUSTOM";
-	};
+	const selectValue = () =>
+		PATCHES.some(({ date }) => date === filter.date) ? filter.date : "CUSTOM";
 
 	// on Saturday so it doesn't overlap with actual path dates (no patches on Saturdays)
 	const oneMonthAgoOnSaturday = new Date();
-	oneMonthAgoOnSaturday.setDate(oneMonthAgoOnSaturday.getDate() - 30);
-	oneMonthAgoOnSaturday.setDate(
-		oneMonthAgoOnSaturday.getDate() - oneMonthAgoOnSaturday.getDay() + 6,
+	oneMonthAgoOnSaturday.setUTCDate(oneMonthAgoOnSaturday.getUTCDate() - 30);
+	oneMonthAgoOnSaturday.setUTCDate(
+		oneMonthAgoOnSaturday.getUTCDate() - oneMonthAgoOnSaturday.getUTCDay() + 6,
 	);
 
+	const customDate = isValidDate(new Date(filter.date))
+		? new Date(filter.date)
+		: oneMonthAgoOnSaturday;
+
 	return (
 		<div className={clsx(styles.filter, styles.filterDate)}>
 			<label className="mb-0">{t("builds:filters.date.since")}</label>
@@ -255,7 +248,7 @@ function DateFilter({
 			{selectValue() === "CUSTOM" ? (
 				<input
 					type="date"
-					value={dateToYYYYMMDD(new Date(filter.date))}
+					value={dateToYYYYMMDD(customDate)}
 					onChange={(e) => onChange({ date: e.target.value })}
 					max={dateToYYYYMMDD(new Date())}
 					data-testid="date-input"

app/features/builds/core/ability-sorting.server.ts

@@ -54,24 +54,13 @@ const sortAbilityCount = (a: [Ability, number], b: [Ability, number]) => {
 	return b[1] - a[1];
 };
 function subAbilitiesSorted(abilities: BuildAbilitiesTuple): Ability[] {
-	const subAbilitiesUnsorted = [
-		abilities[0].slice(1),
-		abilities[1].slice(1),
-		abilities[2].slice(1),
-	].flat();
+	const subAbilitiesUnsorted = abilities.flatMap((row) => row.slice(1));
 
-	const counts = Array.from(
-		subAbilitiesUnsorted
-			.reduce((acc, cur) => {
-				if (!acc.has(cur)) {
-					acc.set(cur, 1);
-				} else {
-					acc.set(cur, acc.get(cur)! + 1);
-				}
-				return acc;
-			}, new Map<Ability, number>())
-			.entries(),
-	).sort(sortAbilityCount);
+	const countsMap = new Map<Ability, number>();
+	for (const ability of subAbilitiesUnsorted) {
+		countsMap.set(ability, (countsMap.get(ability) ?? 0) + 1);
+	}
+	const counts = Array.from(countsMap).sort(sortAbilityCount);
 
 	const subAbilities: Ability[][] = [[], [], []];
 	while (counts.length > 0) {

app/features/builds/core/filter.server.ts

@@ -73,7 +73,7 @@ function matchesAbilityFilter({
 	filter,
 }: {
 	build: PartialBuild;
-	filter: Omit<AbilityBuildFilter, "id">;
+	filter: AbilityBuildFilter;
 }) {
 	if (typeof filter.value === "boolean") {
 		const hasAbility = build.abilities.flat().includes(filter.ability);
@@ -94,7 +94,7 @@ function matchesModeFilter({
 	filter,
 }: {
 	build: PartialBuild;
-	filter: Omit<ModeBuildFilter, "id">;
+	filter: ModeBuildFilter;
 }) {
 	if (!build.modes) return false;
 
@@ -106,7 +106,7 @@ function matchesDateFilter({
 	filter,
 }: {
 	build: PartialBuild;
-	filter: Omit<DateBuildFilter, "id">;
+	filter: DateBuildFilter;
 }) {
 	const date = new Date(filter.date);
 

app/features/builds/routes/builds.$slug.test.ts

@@ -0,0 +1,143 @@
+import { describe, expect, test } from "vitest";
+import { buildFiltersMeaningfullyChanged } from "./builds.$slug";
+
+const sp = (filters?: unknown, extra: Record<string, string> = {}) => {
+	const params = new URLSearchParams(extra);
+	if (filters !== undefined) params.set("f", JSON.stringify(filters));
+	return params;
+};
+
+const ability = (
+	value: number,
+	comparison: "AT_LEAST" | "AT_MOST" = "AT_LEAST",
+	abilityName = "ISM",
+) => ({ type: "ability", ability: abilityName, comparison, value });
+
+const mode = (modeShort: "SZ" | "TC" | "RM" | "CB") => ({
+	type: "mode",
+	mode: modeShort,
+});
+
+const date = (dateString: string) => ({ type: "date", date: dateString });
+
+describe("buildFiltersMeaningfullyChanged", () => {
+	test("no filters either side -> not changed", () => {
+		expect(buildFiltersMeaningfullyChanged(sp(), sp())).toBe(false);
+	});
+
+	test("identical filters in same order -> not changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([ability(10), mode("SZ")]),
+				sp([ability(10), mode("SZ")]),
+			),
+		).toBe(false);
+	});
+
+	test("identical filters in different order -> not changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([ability(10), mode("SZ")]),
+				sp([mode("SZ"), ability(10)]),
+			),
+		).toBe(false);
+	});
+
+	test("ability filter value differs -> changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(sp([ability(10)]), sp([ability(20)])),
+		).toBe(true);
+	});
+
+	test("different filter count -> changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([ability(10)]),
+				sp([ability(10), mode("SZ")]),
+			),
+		).toBe(true);
+	});
+
+	test("duplicate ability filters with different values are not equal", () => {
+		// Regression test for the subset-check bug.
+		// old = [ability(5), ability(10)], new = [ability(10), ability(10)]
+		// the previous implementation would incorrectly return "not changed".
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([ability(5), ability(10)]),
+				sp([ability(10), ability(10)]),
+			),
+		).toBe(true);
+	});
+
+	test("AT_LEAST 0 ability filter added on the new side -> not changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([ability(10)]),
+				sp([ability(10), ability(0)]),
+			),
+		).toBe(false);
+	});
+
+	test("both sides only have meaningless filters -> not changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(sp([ability(0)]), sp([ability(0)])),
+		).toBe(false);
+	});
+
+	test("mode filter changed -> changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(sp([mode("SZ")]), sp([mode("TC")])),
+		).toBe(true);
+	});
+
+	test("mode filter same -> not changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(sp([mode("SZ")]), sp([mode("SZ")])),
+		).toBe(false);
+	});
+
+	test("date filter changed -> changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([date("2026-01-01")]),
+				sp([date("2026-02-01")]),
+			),
+		).toBe(true);
+	});
+
+	test("ability comparison flipped (AT_LEAST -> AT_MOST) -> changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([ability(10, "AT_LEAST")]),
+				sp([ability(10, "AT_MOST")]),
+			),
+		).toBe(true);
+	});
+
+	test("old has filters, new has none -> changed", () => {
+		expect(buildFiltersMeaningfullyChanged(sp([ability(10)]), sp())).toBe(true);
+	});
+
+	test("old has only meaningless filters, new has none -> not changed", () => {
+		expect(buildFiltersMeaningfullyChanged(sp([ability(0)]), sp())).toBe(false);
+	});
+
+	test("malformed JSON in `f` param does not throw and is treated as empty", () => {
+		const malformed = new URLSearchParams();
+		malformed.set("f", "not-json");
+		expect(buildFiltersMeaningfullyChanged(malformed, sp())).toBe(false);
+		expect(buildFiltersMeaningfullyChanged(sp([ability(10)]), malformed)).toBe(
+			true,
+		);
+	});
+
+	test("mixed filter types in different orders -> not changed", () => {
+		expect(
+			buildFiltersMeaningfullyChanged(
+				sp([ability(10), mode("SZ"), date("2026-01-01")]),
+				sp([date("2026-01-01"), ability(10), mode("SZ")]),
+			),
+		).toBe(false);
+	});
+});

app/features/builds/routes/builds.$slug.tsx

@@ -6,8 +6,6 @@ import {
 	Funnel,
 	Map as MapIcon,
 } from "lucide-react";
-import { nanoid } from "nanoid";
-import * as React from "react";
 import { useTranslation } from "react-i18next";
 import type { MetaFunction } from "react-router";
 import {
@@ -49,9 +47,63 @@ export { loader };
 
 import styles from "./builds.$slug.module.css";
 
-const filterOutMeaninglessFilters = (
-	filter: Unpacked<BuildFiltersFromSearchParams>,
-) => {
+type ParsedFilter = Unpacked<BuildFiltersFromSearchParams>;
+
+/**
+ * Returns true if the meaningful build filters in `next` differ from those in `current`.
+ * Order-insensitive and duplicate-safe; AT_LEAST 0 ability filters are treated as no-ops.
+ */
+export function buildFiltersMeaningfullyChanged(
+	current: URLSearchParams,
+	next: URLSearchParams,
+): boolean {
+	const oldFilters = extractMeaningfulFilters(current);
+	const newFilters = extractMeaningfulFilters(next);
+
+	return !R.isDeepEqual(
+		R.sortBy(oldFilters, filterKey),
+		R.sortBy(newFilters, filterKey),
+	);
+}
+
+export const shouldRevalidate: ShouldRevalidateFunction = (args) => {
+	if (isRevalidation(args)) return true;
+
+	if (
+		args.currentUrl.searchParams.get("limit") !==
+		args.nextUrl.searchParams.get("limit")
+	) {
+		return true;
+	}
+
+	if (
+		buildFiltersMeaningfullyChanged(
+			args.currentUrl.searchParams,
+			args.nextUrl.searchParams,
+		)
+	) {
+		return args.defaultShouldRevalidate;
+	}
+
+	return false;
+};
+
+function parseFiltersFromSearchParams(
+	searchParams: URLSearchParams,
+): BuildFilter[] {
+	const raw = searchParams.get(FILTER_SEARCH_PARAM_KEY);
+	if (!raw) return [];
+
+	return safeJSONParse<BuildFilter[]>(raw, []);
+}
+
+function extractMeaningfulFilters(
+	searchParams: URLSearchParams,
+): BuildFiltersFromSearchParams {
+	return parseFiltersFromSearchParams(searchParams).filter(isMeaningfulFilter);
+}
+
+function isMeaningfulFilter(filter: ParsedFilter): boolean {
 	if (filter.type !== "ability") return true;
 
 	return (
@@ -59,74 +111,13 @@ const filterOutMeaninglessFilters = (
 		typeof filter.value !== "number" ||
 		filter.value > 0
 	);
-};
-export const shouldRevalidate: ShouldRevalidateFunction = (args) => {
-	if (isRevalidation(args)) return true;
+}
 
-	const oldLimit = args.currentUrl.searchParams.get("limit");
-	const newLimit = args.nextUrl.searchParams.get("limit");
-
-	// limit was changed -> revalidate
-	if (oldLimit !== newLimit) {
-		return true;
-	}
-
-	const rawOldFilters = args.currentUrl.searchParams.get(
-		FILTER_SEARCH_PARAM_KEY,
-	);
-	const oldFilters = rawOldFilters
-		? safeJSONParse<BuildFiltersFromSearchParams>(rawOldFilters, []).filter(
-				filterOutMeaninglessFilters,
-			)
-		: null;
-	const rawNewFilters = args.nextUrl.searchParams.get(FILTER_SEARCH_PARAM_KEY);
-	const newFilters = rawNewFilters
-		? // no safeJSONParse as the value should be coming from app code and should be trustworthy
-			(JSON.parse(rawNewFilters) as BuildFiltersFromSearchParams).filter(
-				filterOutMeaninglessFilters,
-			)
-		: null;
-
-	// meaningful filter was added/removed -> revalidate
-	if (oldFilters && newFilters && oldFilters.length !== newFilters.length) {
-		return true;
-	}
-	// no meaningful filters were or going to be in use -> skip revalidation
-	if (
-		oldFilters &&
-		newFilters &&
-		oldFilters.length === 0 &&
-		newFilters.length === 0
-	) {
-		return false;
-	}
-	// all meaningful filters identical -> skip revalidation
-	if (
-		newFilters?.every((f1) =>
-			oldFilters?.some((f2) => {
-				if (f1.type !== f2.type) return false;
-
-				if (f1.type === "mode" && f2.type === "mode") {
-					return f1.mode === f2.mode;
-				}
-				if (f1.type === "date" && f2.type === "date") {
-					return f1.date === f2.date;
-				}
-				if (f1.type !== "ability" || f2.type !== "ability") return false;
-
-				return (
-					f1.ability === f2.ability &&
-					f1.comparison === f2.comparison &&
-					f1.value === f2.value
-				);
-			}),
-		)
-	) {
-		return false;
-	}
-
-	return args.defaultShouldRevalidate;
-};
+function filterKey(filter: ParsedFilter): string {
+	if (filter.type === "mode") return `mode:${filter.mode}`;
+	if (filter.type === "date") return `date:${filter.date}`;
+	return `ability:${filter.ability}:${filter.comparison}:${filter.value}`;
+}
 
 export const meta: MetaFunction<typeof loader> = (args) => {
 	if (!args.data) return [];
@@ -185,22 +176,14 @@ export function BuildCards({ data }: { data: SerializeFrom<typeof loader> }) {
 export default function WeaponsBuildsPage() {
 	const data = useLoaderData<typeof loader>();
 	const { t } = useTranslation(["common", "builds"]);
-	const [, setSearchParams] = useSearchParams();
-	const [filters, setFilters] = React.useState<BuildFilter[]>(
-		data.filters ? data.filters.map((f) => ({ ...f, id: nanoid() })) : [],
-	);
+	const [searchParams, setSearchParams] = useSearchParams();
+	const filters = parseFiltersFromSearchParams(searchParams);
 
-	const filtersForSearchParams = (filters: BuildFilter[]) =>
-		JSON.stringify(
-			filters.map((f) => {
-				return R.omit(f, ["id"]);
-			}),
-		);
 	const syncSearchParams = (newFilters: BuildFilter[]) => {
 		setSearchParams(
-			filtersForSearchParams.length > 0
+			newFilters.length > 0
 				? {
-						[FILTER_SEARCH_PARAM_KEY]: filtersForSearchParams(newFilters),
+						[FILTER_SEARCH_PARAM_KEY]: JSON.stringify(newFilters),
 					}
 				: {},
 		);
@@ -210,7 +193,6 @@ export default function WeaponsBuildsPage() {
 		const newFilter: BuildFilter =
 			type === "ability"
 				? {
-						id: nanoid(),
 						type: "ability",
 						ability: "ISM",
 						comparison: "AT_LEAST",
@@ -218,43 +200,32 @@ export default function WeaponsBuildsPage() {
 					}
 				: type === "date"
 					? {
-							id: nanoid(),
 							type: "date",
 							date: PATCHES[0].date,
 						}
 					: {
-							id: nanoid(),
 							type: "mode",
 							mode: "SZ",
 						};
 
-		const newFilters = [...filters, newFilter];
-		setFilters(newFilters);
-
-		// no need to sync new ability filter as this doesn't have effect till they make other choices
-		if (type !== "ability") {
-			syncSearchParams(newFilters);
-		}
+		syncSearchParams([...filters, newFilter]);
 	};
 
 	const handleFilterChange = (i: number, newFilter: Partial<BuildFilter>) => {
-		const newFilters = structuredClone(filters);
-
-		newFilters[i] = {
-			...(filters[i] as AbilityBuildFilter),
-			...(newFilter as AbilityBuildFilter),
-		};
-
-		setFilters(newFilters);
+		const newFilters = filters.map((f, index) =>
+			index === i
+				? ({
+						...(f as AbilityBuildFilter),
+						...(newFilter as AbilityBuildFilter),
+					} as BuildFilter)
+				: f,
+		);
 
 		syncSearchParams(newFilters);
 	};
 
 	const handleFilterDelete = (i: number) => {
-		const newFilters = filters.filter((_, index) => index !== i);
-		setFilters(newFilters);
-
-		syncSearchParams(newFilters);
+		syncSearchParams(filters.filter((_, index) => index !== i));
 	};
 
 	const loadMoreLink = () => {
@@ -263,7 +234,7 @@ export default function WeaponsBuildsPage() {
 		params.set("limit", String(data.limit + BUILDS_PAGE_BATCH_SIZE));
 
 		if (filters.length > 0) {
-			params.set(FILTER_SEARCH_PARAM_KEY, filtersForSearchParams(filters));
+			params.set(FILTER_SEARCH_PARAM_KEY, JSON.stringify(filters));
 		}
 
 		return `?${params.toString()}`;
@@ -338,7 +309,7 @@ export default function WeaponsBuildsPage() {
 				<div className="stack md">
 					{filters.map((filter, i) => (
 						<FilterSection
-							key={filter.id}
+							key={i}
 							number={i + 1}
 							filter={filter}
 							onChange={(newFilter) => handleFilterChange(i, newFilter)}

app/features/sendouq/core/SendouQ.server.test.ts

@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
 import { db } from "~/db/sql";
 import { refreshUserSkills } from "~/features/mmr/tiered.server";
 import * as PrivateUserNoteRepository from "~/features/sendouq/PrivateUserNoteRepository.server";
+import { databaseTimestampNow } from "~/utils/dates";
 import { dbInsertUsers, dbReset } from "~/utils/Test";
 import * as SQGroupRepository from "../SQGroupRepository.server";
 import { refreshSendouQInstance, SendouQ } from "./SendouQ.server";
@@ -393,9 +394,18 @@ describe("SendouQ", () => {
 				await insertSkill(3, 2000);
 				await insertSkill(4, 1050);
 
-				await createGroup([4]);
-				await createGroup([2]);
-				await createGroup([3]);
+				const g4Id = await createGroup([4]);
+				const g2Id = await createGroup([2]);
+				const g3Id = await createGroup([3]);
+				// Force identical latestActionAt so the sort comparator's
+				// recency tie-breaker stays neutral and the assertion does
+				// not depend on whether the group inserts straddle a
+				// millisecond boundary (which they can on slow CI).
+				await db
+					.updateTable("Group")
+					.set({ latestActionAt: databaseTimestampNow() })
+					.where("id", "in", [g4Id, g2Id, g3Id])
+					.execute();
 				await refreshSendouQInstance();
 
 				const notes = await PrivateUserNoteRepository.byAuthorUserId(1);

app/features/user-page/actions/u.$identifier.builds.server.ts

@@ -26,15 +26,9 @@ export const action: ActionFunction = async ({ request }) => {
 
 	switch (data._action) {
 		case "DELETE_BUILD": {
-			const usersBuilds = await BuildRepository.allByUserId(user.id, {
-				showPrivate: true,
-			});
+			const ownerId = await BuildRepository.ownerIdById(data.buildToDeleteId);
 
-			const buildToDelete = usersBuilds.find(
-				(build) => build.id === data.buildToDeleteId,
-			);
-
-			errorToastIfFalsy(buildToDelete, "Build to delete not found");
+			errorToastIfFalsy(ownerId === user.id, "Build to delete not found");
 
 			await BuildRepository.deleteById(data.buildToDeleteId);
 

app/features/user-page/core/build-sorting.server.test.ts

@@ -329,6 +329,41 @@ describe("sortBuilds()", () => {
 
 			expect(sortedBuilds[2].id).toBe(1);
 		});
+
+		it(`sorts ${identifier} with null gear last`, () => {
+			const key = (
+				{
+					HEADGEAR_ID: "headGearSplId",
+					CLOTHES_ID: "clothesGearSplId",
+					SHOES_ID: "shoesGearSplId",
+				} as const
+			)[identifier]!;
+
+			const builds = [
+				mockBuild({
+					id: 1,
+					[key]: null,
+				}),
+				mockBuild({
+					id: 2,
+					[key]: 5,
+				}),
+				mockBuild({
+					id: 3,
+					[key]: 1,
+				}),
+			];
+
+			const sortedBuilds = sortBuilds({
+				builds,
+				buildSorting: [identifier as any],
+				weaponPool: [],
+			});
+
+			expect(sortedBuilds[0].id).toBe(3);
+			expect(sortedBuilds[1].id).toBe(2);
+			expect(sortedBuilds[2].id).toBe(1);
+		});
 	}
 
 	it("sorts when buildSort not given", () => {

app/features/user-page/core/build-sorting.server.ts

@@ -28,9 +28,12 @@ export function sortBuilds({
 			Math.min(...a.weapons.map((wpn) => wpn.weaponSplId)) -
 			Math.min(...b.weapons.map((wpn) => wpn.weaponSplId)),
 		UPDATED_AT: (a, b) => b.updatedAt - a.updatedAt,
-		HEADGEAR_ID: (a, b) => a.headGearSplId - b.headGearSplId,
-		CLOTHES_ID: (a, b) => a.clothesGearSplId - b.clothesGearSplId,
-		SHOES_ID: (a, b) => a.shoesGearSplId - b.shoesGearSplId,
+		HEADGEAR_ID: (a, b) =>
+			compareNullableNumbers(a.headGearSplId, b.headGearSplId),
+		CLOTHES_ID: (a, b) =>
+			compareNullableNumbers(a.clothesGearSplId, b.clothesGearSplId),
+		SHOES_ID: (a, b) =>
+			compareNullableNumbers(a.shoesGearSplId, b.shoesGearSplId),
 		MODE: (a, b) => {
 			const aLowestModeIdx = modesShort.findIndex((mode) =>
 				a.modes?.includes(mode),
@@ -104,3 +107,10 @@ export function sortBuilds({
 		return 0;
 	});
 }
+
+function compareNullableNumbers(a: number | null, b: number | null) {
+	if (a === null && b === null) return 0;
+	if (a === null) return 1;
+	if (b === null) return -1;
+	return a - b;
+}

app/features/user-page/loaders/u.$identifier.builds.new.server.ts

@@ -68,13 +68,9 @@ function resolveDefaultValues(
 	return {
 		buildToEditId: buildToEdit?.id,
 		weapons,
-		head: buildToEdit?.headGearSplId === -1 ? null : buildToEdit?.headGearSplId,
-		clothes:
-			buildToEdit?.clothesGearSplId === -1
-				? null
-				: buildToEdit?.clothesGearSplId,
-		shoes:
-			buildToEdit?.shoesGearSplId === -1 ? null : buildToEdit?.shoesGearSplId,
+		head: buildToEdit?.headGearSplId,
+		clothes: buildToEdit?.clothesGearSplId,
+		shoes: buildToEdit?.shoesGearSplId,
 		abilities,
 		title: buildToEdit?.title,
 		description: buildToEdit?.description ?? null,

app/features/user-page/loaders/u.$identifier.builds.server.ts

@@ -1,8 +1,8 @@
 import type { LoaderFunctionArgs } from "react-router";
+import * as R from "remeda";
 import { getUser } from "~/features/auth/core/user.server";
 import * as BuildRepository from "~/features/builds/BuildRepository.server";
 import * as UserRepository from "~/features/user-page/UserRepository.server";
-import type { MainWeaponId } from "~/modules/in-game-lists/types";
 import type { SerializeFrom } from "~/utils/remix";
 import { notFoundIfFalsy, privatelyCachedJson } from "~/utils/remix.server";
 import { sortBuilds } from "../core/build-sorting.server";
@@ -37,19 +37,9 @@ export const loader = async ({ params }: LoaderFunctionArgs) => {
 	return privatelyCachedJson({
 		buildSorting: user.buildSorting,
 		builds: sortedBuilds,
-		weaponCounts: calculateWeaponCounts(),
+		weaponCounts: R.countBy(
+			builds.flatMap((build) => build.weapons),
+			(weapon) => weapon.weaponSplId,
+		),
 	});
-
-	function calculateWeaponCounts() {
-		return builds.reduce(
-			(acc, build) => {
-				for (const weapon of build.weapons) {
-					acc[weapon.weaponSplId] = (acc[weapon.weaponSplId] ?? 0) + 1;
-				}
-
-				return acc;
-			},
-			{} as Record<MainWeaponId, number>,
-		);
-	}
 };

e2e/builds.spec.ts

@@ -1,8 +1,9 @@
-import type { Page } from "@playwright/test";
+import type { Locator, Page } from "@playwright/test";
 import { NZAP_TEST_DISCORD_ID, NZAP_TEST_ID } from "~/db/seed/constants";
 import type { GearType } from "~/db/tables";
 import { ADMIN_DISCORD_ID } from "~/features/admin/admin-constants";
 import { newBuildBaseSchema } from "~/features/user-page/user-page-schemas";
+import invariant from "~/utils/invariant";
 import { expect, impersonate, navigate, seed, test } from "~/utils/playwright";
 import { createFormHelpers } from "~/utils/playwright-form";
 import { BUILDS_PAGE, userBuildsPage, userNewBuildPage } from "~/utils/urls";
@@ -69,6 +70,10 @@ test.describe("Builds", () => {
 			url: userBuildsPage({ discordId: ADMIN_DISCORD_ID }),
 		});
 
+		const buildIdBefore = await buildIdFromEditLink(
+			page.getByTestId("edit-build").first(),
+		);
+
 		await page.getByTestId("edit-build").first().click();
 
 		const form = createFormHelpers(page, newBuildBaseSchema);
@@ -83,6 +88,11 @@ test.describe("Builds", () => {
 			"Private",
 		);
 
+		const buildIdAfter = await buildIdFromEditLink(
+			page.getByTestId("edit-build").first(),
+		);
+		expect(buildIdAfter).toBe(buildIdBefore);
+
 		await impersonate(page, NZAP_TEST_ID);
 		await navigate({
 			page,
@@ -157,3 +167,11 @@ async function selectGear({
 		.getByTestId(`gear-select-option-${name}`)
 		.click();
 }
+
+async function buildIdFromEditLink(locator: Locator) {
+	const href = await locator.getAttribute("href");
+	invariant(href, "edit-build link missing href");
+	const match = href.match(/buildId=(\d+)/);
+	invariant(match, `buildId not found in href: ${href}`);
+	return Number(match[1]);
+}

migrations/132-build-gear-nullable.js

@@ -0,0 +1,53 @@
+export function up(db) {
+	db.pragma("foreign_keys = OFF");
+
+	db.transaction(() => {
+		db.prepare(
+			/*sql*/ `
+      create table "Build_new" (
+        "id" integer primary key,
+        "ownerId" integer not null,
+        "title" text not null,
+        "description" text,
+        "modes" text,
+        "headGearSplId" integer,
+        "clothesGearSplId" integer,
+        "shoesGearSplId" integer,
+        "updatedAt" integer default (strftime('%s', 'now')) not null,
+        "private" integer default 0,
+        foreign key ("ownerId") references "User"("id") on delete restrict
+      ) strict
+    `,
+		).run();
+
+		db.prepare(
+			/*sql*/ `
+      insert into "Build_new" ("id", "ownerId", "title", "description", "modes", "headGearSplId", "clothesGearSplId", "shoesGearSplId", "updatedAt", "private")
+      select
+        "id",
+        "ownerId",
+        "title",
+        "description",
+        "modes",
+        case when "headGearSplId" = -1 then null else "headGearSplId" end,
+        case when "clothesGearSplId" = -1 then null else "clothesGearSplId" end,
+        case when "shoesGearSplId" = -1 then null else "shoesGearSplId" end,
+        "updatedAt",
+        "private"
+      from "Build"
+    `,
+		).run();
+
+		db.prepare(/*sql*/ `drop table "Build"`).run();
+
+		db.prepare(/*sql*/ `alter table "Build_new" rename to "Build"`).run();
+
+		db.prepare(
+			/*sql*/ `create index build_owner_id on "Build"("ownerId")`,
+		).run();
+
+		db.pragma("foreign_key_check");
+	})();
+
+	db.pragma("foreign_keys = ON");
+}