diff --git a/app/components/BuildCard.tsx b/app/components/BuildCard.tsx index b148249c4..ae37a5669 100644 --- a/app/components/BuildCard.tsx +++ b/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" ? ( ; modes: JSONColumnTypeNullable; ownerId: number; private: DBBoolean | null; - shoesGearSplId: number; + shoesGearSplId: number | null; title: string; updatedAt: Generated; } diff --git a/app/features/build-stats/build-stats-utils.ts b/app/features/build-stats/build-stats-utils.ts index 89c8e9c37..ec19d81de 100644 --- a/app/features/build-stats/build-stats-utils.ts +++ b/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; const POPULAR_BUILDS_TO_SHOW = 25; export function popularBuilds(builds: Array) { - const counts = new Map(); - 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); } diff --git a/app/features/builds/BuildRepository.server.ts b/app/features/builds/BuildRepository.server.ts index ff3839911..332b829ff 100644 --- a/app/features/builds/BuildRepository.server.ts +++ b/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 >, ): 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 | 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; + 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) { 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", diff --git a/app/features/builds/builds-constants.ts b/app/features/builds/builds-constants.ts index 92dcb9cbf..9e5413eba 100644 --- a/app/features/builds/builds-constants.ts +++ b/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 = [ { - path: "11.1.0", + patch: "11.1.0", date: "2026-03-18", }, { diff --git a/app/features/builds/builds-schemas.server.ts b/app/features/builds/builds-schemas.server.ts index 784a32ca7..40335090b 100644 --- a/app/features/builds/builds-schemas.server.ts +++ b/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( diff --git a/app/features/builds/builds-types.ts b/app/features/builds/builds-types.ts index 3e32599da..f1e3c1f0f 100644 --- a/app/features/builds/builds-types.ts +++ b/app/features/builds/builds-types.ts @@ -9,26 +9,24 @@ export interface BuildWeaponWithTop500Info { isTop500: number; } -type WithId = 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 diff --git a/app/features/builds/components/FilterSection.tsx b/app/features/builds/components/FilterSection.tsx index 40d08aaae..fa4d7bfda 100644 --- a/app/features/builds/components/FilterSection.tsx +++ b/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 (
@@ -255,7 +248,7 @@ function DateFilter({ {selectValue() === "CUSTOM" ? ( onChange({ date: e.target.value })} max={dateToYYYYMMDD(new Date())} data-testid="date-input" diff --git a/app/features/builds/core/ability-sorting.server.ts b/app/features/builds/core/ability-sorting.server.ts index 915323185..0b0093c35 100644 --- a/app/features/builds/core/ability-sorting.server.ts +++ b/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()) - .entries(), - ).sort(sortAbilityCount); + const countsMap = new Map(); + 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) { diff --git a/app/features/builds/core/filter.server.ts b/app/features/builds/core/filter.server.ts index d1c0e5aa2..f28fd223f 100644 --- a/app/features/builds/core/filter.server.ts +++ b/app/features/builds/core/filter.server.ts @@ -73,7 +73,7 @@ function matchesAbilityFilter({ filter, }: { build: PartialBuild; - filter: Omit; + 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; + filter: ModeBuildFilter; }) { if (!build.modes) return false; @@ -106,7 +106,7 @@ function matchesDateFilter({ filter, }: { build: PartialBuild; - filter: Omit; + filter: DateBuildFilter; }) { const date = new Date(filter.date); diff --git a/app/features/builds/routes/builds.$slug.test.ts b/app/features/builds/routes/builds.$slug.test.ts new file mode 100644 index 000000000..d7445fe11 --- /dev/null +++ b/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 = {}) => { + 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); + }); +}); diff --git a/app/features/builds/routes/builds.$slug.tsx b/app/features/builds/routes/builds.$slug.tsx index eaf503866..c43b42231 100644 --- a/app/features/builds/routes/builds.$slug.tsx +++ b/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, -) => { +type ParsedFilter = Unpacked; + +/** + * 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(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(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 = (args) => { if (!args.data) return []; @@ -185,22 +176,14 @@ export function BuildCards({ data }: { data: SerializeFrom }) { export default function WeaponsBuildsPage() { const data = useLoaderData(); const { t } = useTranslation(["common", "builds"]); - const [, setSearchParams] = useSearchParams(); - const [filters, setFilters] = React.useState( - 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) => { - 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() {
{filters.map((filter, i) => ( handleFilterChange(i, newFilter)} diff --git a/app/features/sendouq/core/SendouQ.server.test.ts b/app/features/sendouq/core/SendouQ.server.test.ts index 1ceb5a359..76e27b2fc 100644 --- a/app/features/sendouq/core/SendouQ.server.test.ts +++ b/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); diff --git a/app/features/user-page/actions/u.$identifier.builds.server.ts b/app/features/user-page/actions/u.$identifier.builds.server.ts index 37729b879..54da58a64 100644 --- a/app/features/user-page/actions/u.$identifier.builds.server.ts +++ b/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); diff --git a/app/features/user-page/core/build-sorting.server.test.ts b/app/features/user-page/core/build-sorting.server.test.ts index 12156fae6..83e8ad44e 100644 --- a/app/features/user-page/core/build-sorting.server.test.ts +++ b/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", () => { diff --git a/app/features/user-page/core/build-sorting.server.ts b/app/features/user-page/core/build-sorting.server.ts index b2ee5b8d6..b47880c4a 100644 --- a/app/features/user-page/core/build-sorting.server.ts +++ b/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; +} diff --git a/app/features/user-page/loaders/u.$identifier.builds.new.server.ts b/app/features/user-page/loaders/u.$identifier.builds.new.server.ts index fc7170648..30edf1315 100644 --- a/app/features/user-page/loaders/u.$identifier.builds.new.server.ts +++ 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, diff --git a/app/features/user-page/loaders/u.$identifier.builds.server.ts b/app/features/user-page/loaders/u.$identifier.builds.server.ts index 77c51464a..4c4ea0c97 100644 --- a/app/features/user-page/loaders/u.$identifier.builds.server.ts +++ b/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, - ); - } }; diff --git a/db-test.sqlite3 b/db-test.sqlite3 index 910c5f0b2..206fb8b82 100644 Binary files a/db-test.sqlite3 and b/db-test.sqlite3 differ diff --git a/e2e/builds.spec.ts b/e2e/builds.spec.ts index 8d8f21cbc..4b8662188 100644 --- a/e2e/builds.spec.ts +++ b/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]); +} diff --git a/e2e/seeds/db-seed-DEFAULT.sqlite3 b/e2e/seeds/db-seed-DEFAULT.sqlite3 index a7c61ba5d..9cfd870d4 100644 Binary files a/e2e/seeds/db-seed-DEFAULT.sqlite3 and b/e2e/seeds/db-seed-DEFAULT.sqlite3 differ diff --git a/e2e/seeds/db-seed-FINALIZED_BRACKET.sqlite3 b/e2e/seeds/db-seed-FINALIZED_BRACKET.sqlite3 index d4f0b96bc..fd9b1cbae 100644 Binary files a/e2e/seeds/db-seed-FINALIZED_BRACKET.sqlite3 and b/e2e/seeds/db-seed-FINALIZED_BRACKET.sqlite3 differ diff --git a/e2e/seeds/db-seed-NO_SCRIMS.sqlite3 b/e2e/seeds/db-seed-NO_SCRIMS.sqlite3 index acbea5962..6d47ec50f 100644 Binary files a/e2e/seeds/db-seed-NO_SCRIMS.sqlite3 and b/e2e/seeds/db-seed-NO_SCRIMS.sqlite3 differ diff --git a/e2e/seeds/db-seed-NO_SQ_GROUPS.sqlite3 b/e2e/seeds/db-seed-NO_SQ_GROUPS.sqlite3 index 6aa8c7ea8..091f384a3 100644 Binary files a/e2e/seeds/db-seed-NO_SQ_GROUPS.sqlite3 and b/e2e/seeds/db-seed-NO_SQ_GROUPS.sqlite3 differ diff --git a/e2e/seeds/db-seed-NO_TOURNAMENT_TEAMS.sqlite3 b/e2e/seeds/db-seed-NO_TOURNAMENT_TEAMS.sqlite3 index b5110d850..555a64f6f 100644 Binary files a/e2e/seeds/db-seed-NO_TOURNAMENT_TEAMS.sqlite3 and b/e2e/seeds/db-seed-NO_TOURNAMENT_TEAMS.sqlite3 differ diff --git a/e2e/seeds/db-seed-NZAP_IN_TEAM.sqlite3 b/e2e/seeds/db-seed-NZAP_IN_TEAM.sqlite3 index ebc2defa1..782dd37e5 100644 Binary files a/e2e/seeds/db-seed-NZAP_IN_TEAM.sqlite3 and b/e2e/seeds/db-seed-NZAP_IN_TEAM.sqlite3 differ diff --git a/e2e/seeds/db-seed-REG_OPEN.sqlite3 b/e2e/seeds/db-seed-REG_OPEN.sqlite3 index 956204816..a35250680 100644 Binary files a/e2e/seeds/db-seed-REG_OPEN.sqlite3 and b/e2e/seeds/db-seed-REG_OPEN.sqlite3 differ diff --git a/e2e/seeds/db-seed-SMALL_SOS.sqlite3 b/e2e/seeds/db-seed-SMALL_SOS.sqlite3 index a40a1ca4d..8797d21d7 100644 Binary files a/e2e/seeds/db-seed-SMALL_SOS.sqlite3 and b/e2e/seeds/db-seed-SMALL_SOS.sqlite3 differ diff --git a/e2e/seeds/db-seed-TEAM_MAP_PREFS.sqlite3 b/e2e/seeds/db-seed-TEAM_MAP_PREFS.sqlite3 index 7f108342b..cb3e70730 100644 Binary files a/e2e/seeds/db-seed-TEAM_MAP_PREFS.sqlite3 and b/e2e/seeds/db-seed-TEAM_MAP_PREFS.sqlite3 differ diff --git a/migrations/132-build-gear-nullable.js b/migrations/132-build-gear-nullable.js new file mode 100644 index 000000000..035db381b --- /dev/null +++ b/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"); +}