From 0932e8a5b5068892d5567dfb78269322b38e761e Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Wed, 9 Oct 2024 20:41:08 +0300 Subject: [PATCH] Fix main team switch handling when leaving/deleting team + add secondary team tests --- app/db/seed/constants.ts | 1 + app/features/lfg/LFGRepository.server.ts | 11 +- app/features/sendouq/routes/q.looking.test.ts | 11 +- app/features/team/TeamRepository.server.ts | 52 ++++- .../team/actions/t.$customUrl.server.ts | 2 + .../team/queries/deleteTeam.server.ts | 11 -- .../team/routes/t.$customUrl.edit.tsx | 5 +- app/features/team/routes/t.$customUrl.test.ts | 182 ++++++++++++++++++ app/utils/Test.ts | 24 ++- db-test.sqlite3 | Bin 860160 -> 864256 bytes 10 files changed, 265 insertions(+), 34 deletions(-) delete mode 100644 app/features/team/queries/deleteTeam.server.ts create mode 100644 app/features/team/routes/t.$customUrl.test.ts diff --git a/app/db/seed/constants.ts b/app/db/seed/constants.ts index 695d2a0dc..f6f08158d 100644 --- a/app/db/seed/constants.ts +++ b/app/db/seed/constants.ts @@ -3,5 +3,6 @@ export const ADMIN_TEST_AVATAR = "6fc41a44b069a0d2152ac06d1e496c6c"; export const NZAP_TEST_DISCORD_ID = "455039198672453645"; export const NZAP_TEST_AVATAR = "f809176af93132c3db5f0a5019e96339"; // https://cdn.discordapp.com/avatars/455039198672453645/f809176af93132c3db5f0a5019e96339.webp?size=160 export const NZAP_TEST_ID = 2; +export const REGULAR_USER_TEST_ID = 2; export const AMOUNT_OF_CALENDAR_EVENTS = 200; diff --git a/app/features/lfg/LFGRepository.server.ts b/app/features/lfg/LFGRepository.server.ts index 91c917e30..246bdbf99 100644 --- a/app/features/lfg/LFGRepository.server.ts +++ b/app/features/lfg/LFGRepository.server.ts @@ -1,8 +1,8 @@ import { sub } from "date-fns"; -import { type NotNull, sql } from "kysely"; +import { type NotNull, type Transaction, sql } from "kysely"; import { jsonArrayFrom, jsonObjectFrom } from "kysely/helpers/sqlite"; import { db } from "~/db/sql"; -import type { TablesInsertable } from "~/db/tables"; +import type { DB, TablesInsertable } from "~/db/tables"; import { databaseTimestampNow, dateToDatabaseTimestamp } from "~/utils/dates"; import { COMMON_USER_FIELDS } from "~/utils/kysely.server"; import { LFG } from "./lfg-constants"; @@ -143,6 +143,9 @@ export function deletePost(id: number) { return db.deleteFrom("LFGPost").where("id", "=", id).execute(); } -export function deletePostsByTeamId(teamId: number) { - return db.deleteFrom("LFGPost").where("teamId", "=", teamId).execute(); +export function deletePostsByTeamId(teamId: number, trx?: Transaction) { + return (trx ?? db) + .deleteFrom("LFGPost") + .where("teamId", "=", teamId) + .execute(); } diff --git a/app/features/sendouq/routes/q.looking.test.ts b/app/features/sendouq/routes/q.looking.test.ts index bb94ea0f7..864a26d72 100644 --- a/app/features/sendouq/routes/q.looking.test.ts +++ b/app/features/sendouq/routes/q.looking.test.ts @@ -204,9 +204,10 @@ describe("Private user note sorting", () => { }); const matchAction = wrappedAction({ action: rawMatchAction, - params: { id: "1" }, }); + const matchActionParams = { id: "1" }; + test("users with positive note sorted first", async () => { await matchAction( { @@ -215,7 +216,7 @@ describe("Private user note sorting", () => { sentiment: "POSITIVE", comment: "test", }, - { user: "admin" }, + { user: "admin", params: matchActionParams }, ); const data = await lookingLoader({ user: "admin" }); @@ -231,7 +232,7 @@ describe("Private user note sorting", () => { sentiment: "NEGATIVE", comment: "test", }, - { user: "admin" }, + { user: "admin", params: matchActionParams }, ); const data = await lookingLoader({ user: "admin" }); @@ -249,7 +250,7 @@ describe("Private user note sorting", () => { sentiment: "POSITIVE", comment: "test", }, - { user: "admin" }, + { user: "admin", params: matchActionParams }, ); await matchAction( { @@ -258,7 +259,7 @@ describe("Private user note sorting", () => { sentiment: "NEGATIVE", comment: "test", }, - { user: "admin" }, + { user: "admin", params: matchActionParams }, ); const data = await lookingLoader({ user: "admin" }); diff --git a/app/features/team/TeamRepository.server.ts b/app/features/team/TeamRepository.server.ts index f7839653d..c289fb944 100644 --- a/app/features/team/TeamRepository.server.ts +++ b/app/features/team/TeamRepository.server.ts @@ -4,6 +4,7 @@ import { nanoid } from "nanoid"; import { INVITE_CODE_LENGTH } from "~/constants"; import { db } from "~/db/sql"; import type { DB, Tables } from "~/db/tables"; +import * as LFGRepository from "~/features/lfg/LFGRepository.server"; import { databaseTimestampNow } from "~/utils/dates"; import invariant from "~/utils/invariant"; import { COMMON_USER_FIELDS } from "~/utils/kysely.server"; @@ -107,6 +108,7 @@ export async function teamsByMemberUserId( .select([ "TeamMemberWithSecondary.teamId as id", "TeamMemberWithSecondary.isOwner", + "TeamMemberWithSecondary.isMainTeam", ]) .where("userId", "=", userId) .execute(); @@ -200,6 +202,52 @@ export function switchMainTeam({ }); } +export function del(teamId: number) { + return db.transaction().execute(async (trx) => { + const members = await trx + .selectFrom("TeamMember") + .select(["TeamMember.userId"]) + .where("teamId", "=", teamId) + .execute(); + + // switch main team to another if they at least one secondary team + for (const member of members) { + const currentTeams = await teamsByMemberUserId(member.userId, trx); + + const teamToSwitchTo = currentTeams.find((team) => team.id !== teamId); + + if (!teamToSwitchTo) continue; + + await trx + .updateTable("AllTeamMember") + .set({ + isMainTeam: 1, + }) + .where("userId", "=", member.userId) + .where("teamId", "=", teamToSwitchTo.id) + .execute(); + } + + await trx + .updateTable("AllTeamMember") + .set({ + isMainTeam: 0, + }) + .where("AllTeamMember.teamId", "=", teamId) + .execute(); + + await LFGRepository.deletePostsByTeamId(teamId, trx); + + await trx + .updateTable("AllTeam") + .set({ + deletedAt: databaseTimestampNow(), + }) + .where("id", "=", teamId) + .execute(); + }); +} + export function addNewTeamMember({ userId, teamId, @@ -245,8 +293,9 @@ export function removeTeamMember({ invariant(teamToLeave, "User is not a member of this team"); invariant(!teamToLeave.isOwner, "Owner cannot leave the team"); + const wasMainTeam = teamToLeave.isMainTeam; const newMainTeam = currentTeams.find((team) => team.id !== teamId); - if (newMainTeam) { + if (wasMainTeam && newMainTeam) { await trx .updateTable("AllTeamMember") .set({ @@ -261,6 +310,7 @@ export function removeTeamMember({ .updateTable("AllTeamMember") .set({ leftAt: databaseTimestampNow(), + isMainTeam: 0, }) .where("userId", "=", userId) .where("teamId", "=", teamId) diff --git a/app/features/team/actions/t.$customUrl.server.ts b/app/features/team/actions/t.$customUrl.server.ts index 56ce97896..5b764990e 100644 --- a/app/features/team/actions/t.$customUrl.server.ts +++ b/app/features/team/actions/t.$customUrl.server.ts @@ -30,6 +30,7 @@ export const action: ActionFunction = async ({ request, params }) => { teamId: team.id, userId: user.id, }); + break; } case "MAKE_MAIN_TEAM": { @@ -37,6 +38,7 @@ export const action: ActionFunction = async ({ request, params }) => { userId: user.id, teamId: team.id, }); + break; } default: { diff --git a/app/features/team/queries/deleteTeam.server.ts b/app/features/team/queries/deleteTeam.server.ts deleted file mode 100644 index 20cb38734..000000000 --- a/app/features/team/queries/deleteTeam.server.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { sql } from "~/db/sql"; - -const stm = sql.prepare(/* sql */ ` - update "AllTeam" - set "deletedAt" = strftime('%s', 'now') - where "id" = @id -`); - -export function deleteTeam(id: number) { - stm.run({ id }); -} diff --git a/app/features/team/routes/t.$customUrl.edit.tsx b/app/features/team/routes/t.$customUrl.edit.tsx index b832948d7..a1d719593 100644 --- a/app/features/team/routes/t.$customUrl.edit.tsx +++ b/app/features/team/routes/t.$customUrl.edit.tsx @@ -18,7 +18,6 @@ import { Label } from "~/components/Label"; import { Main } from "~/components/Main"; import { SubmitButton } from "~/components/SubmitButton"; import { requireUserId } from "~/features/auth/core/user.server"; -import * as LFGRepository from "~/features/lfg/LFGRepository.server"; import { isAdmin } from "~/permissions"; import { type SendouRouteHandle, @@ -36,7 +35,6 @@ import { uploadImagePage, } from "~/utils/urls"; import * as TeamRepository from "../TeamRepository.server"; -import { deleteTeam } from "../queries/deleteTeam.server"; import { TEAM } from "../team-constants"; import { editTeamSchema, teamParamsSchema } from "../team-schemas.server"; import { canAddCustomizedColors, isTeamOwner } from "../team-utils"; @@ -89,8 +87,7 @@ export const action: ActionFunction = async ({ request, params }) => { switch (data._action) { case "DELETE": { - await LFGRepository.deletePostsByTeamId(team.id); - deleteTeam(team.id); + await TeamRepository.del(team.id); throw redirect(TEAM_SEARCH_PAGE); } diff --git a/app/features/team/routes/t.$customUrl.test.ts b/app/features/team/routes/t.$customUrl.test.ts new file mode 100644 index 000000000..b772988da --- /dev/null +++ b/app/features/team/routes/t.$customUrl.test.ts @@ -0,0 +1,182 @@ +import type { SerializeFrom } from "@remix-run/node"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { REGULAR_USER_TEST_ID } from "~/db/seed/constants"; +import { db } from "~/db/sql"; +import { + dbInsertUsers, + dbReset, + wrappedAction, + wrappedLoader, +} from "~/utils/Test"; +import { loader as userProfileLoader } from "../../user-page/loaders/u.$identifier.index.server"; +import * as TeamRepository from "../TeamRepository.server"; +import { action as _teamPageAction } from "../actions/t.$customUrl.server"; +import { action as teamIndexPageAction } from "../actions/t.server"; +import { action as _editTeamAction } from "../routes/t.$customUrl.edit"; +import type { + createTeamSchema, + editTeamSchema, + teamProfilePageActionSchema, +} from "../team-schemas.server"; + +const loadUserTeamLoader = wrappedLoader< + SerializeFrom +>({ + loader: userProfileLoader, +}); + +const createTeamAction = wrappedAction({ + action: teamIndexPageAction, +}); +const teamPageAction = wrappedAction({ + action: _teamPageAction, +}); +const editTeamAction = wrappedAction({ + action: _editTeamAction, +}); + +async function loadTeams() { + const data = await loadUserTeamLoader({ + user: "regular", + params: { + identifier: String(REGULAR_USER_TEST_ID), + }, + }); + + return { team: data.user.team, secondaryTeams: data.user.secondaryTeams }; +} + +describe("Secondary teams", () => { + beforeEach(async () => { + await dbInsertUsers(2); + }); + afterEach(() => { + dbReset(); + }); + + it("first team created becomes main team", async () => { + const { team, secondaryTeams } = await loadTeams(); + + expect(team!.name).toBe("Team 1"); + expect(secondaryTeams).toHaveLength(0); + }); + + it("second team created becomes secondary", async () => { + await createTeamAction({ name: "Team 1" }, { user: "regular" }); + await createTeamAction({ name: "Team 2" }, { user: "regular" }); + + const { team, secondaryTeams } = await loadTeams(); + + expect(team!.name).toBe("Team 1"); + expect(secondaryTeams[0].name).toBe("Team 2"); + }); + + it("makes secondary team main team", async () => { + await createTeamAction({ name: "Team 1" }, { user: "regular" }); + await createTeamAction({ name: "Team 2" }, { user: "regular" }); + + const { team, secondaryTeams } = await loadTeams(); + + expect(team!.name).toBe("Team 1"); + expect(secondaryTeams[0].name).toBe("Team 2"); + }); + + it("sets main team (2 team)", async () => { + await createTeamAction({ name: "Team 1" }, { user: "regular" }); + await createTeamAction({ name: "Team 2" }, { user: "regular" }); + + await teamPageAction( + { _action: "MAKE_MAIN_TEAM" }, + { user: "regular", params: { customUrl: "team-2" } }, + ); + + const { team } = await loadTeams(); + + expect(team!.name).toBe("Team 2"); + }); + + it("when deleting the main team, the secondary team becomes main", async () => { + await createTeamAction({ name: "Team 1" }, { user: "regular" }); + await createTeamAction({ name: "Team 2" }, { user: "regular" }); + + await editTeamAction( + { + _action: "DELETE", + }, + { + user: "regular", + params: { customUrl: "team-1" }, + }, + ); + + const { team, secondaryTeams } = await loadTeams(); + + expect(team!.name).toBe("Team 2"); + expect(secondaryTeams).toHaveLength(0); + }); + + it("when leaving the main team, the secondary team becomes main", async () => { + // has to be made by "admin" because can't leave team you own + await createTeamAction({ name: "Team 1" }, { user: "admin" }); + await createTeamAction({ name: "Team 2" }, { user: "admin" }); + + await TeamRepository.addNewTeamMember({ + userId: REGULAR_USER_TEST_ID, + teamId: 1, + maxTeamsAllowed: 2, + }); + await TeamRepository.addNewTeamMember({ + userId: REGULAR_USER_TEST_ID, + teamId: 2, + maxTeamsAllowed: 2, + }); + + const { team, secondaryTeams } = await loadTeams(); + + expect(team!.name).toBe("Team 1"); + expect(secondaryTeams[0].name).toBe("Team 2"); + + await teamPageAction( + { + _action: "LEAVE_TEAM", + }, + { + user: "regular", + params: { customUrl: "team-1" }, + }, + ); + + const { team: newTeam, secondaryTeams: newSecondaryTeams } = + await loadTeams(); + + expect(newTeam!.name).toBe("Team 2"); + expect(newSecondaryTeams).toHaveLength(0); + }); + + it("creates max 2 teams as non-patron", async () => { + await createTeamAction({ name: "Team 1" }, { user: "regular" }); + await createTeamAction({ name: "Team 2" }, { user: "regular" }); + + expect( + createTeamAction({ name: "Team 3" }, { user: "regular" }), + ).rejects.toThrow("status code: 400"); + }); + + const makeUserPatron = () => + db + .updateTable("User") + .set({ patronTier: 2 }) + .where("id", "=", REGULAR_USER_TEST_ID) + .execute(); + + it("creates more than 2 teams as patron", async () => { + await makeUserPatron(); + + await createTeamAction({ name: "Team 1" }, { user: "regular" }); + await createTeamAction({ name: "Team 2" }, { user: "regular" }); + await createTeamAction({ name: "Team 3" }, { user: "regular" }); + + const { secondaryTeams } = await loadTeams(); + expect(secondaryTeams).toHaveLength(2); + }); +}); diff --git a/app/utils/Test.ts b/app/utils/Test.ts index 967c11774..59b12b090 100644 --- a/app/utils/Test.ts +++ b/app/utils/Test.ts @@ -1,7 +1,8 @@ import type { ActionFunctionArgs, LoaderFunctionArgs } from "@remix-run/node"; +import type { Params } from "@remix-run/react"; import type { z } from "zod"; import { ADMIN_ID } from "~/constants"; -import { NZAP_TEST_ID } from "~/db/seed/constants"; +import { REGULAR_USER_TEST_ID } from "~/db/seed/constants"; import { db, sql } from "~/db/sql"; import { SESSION_KEY } from "~/features/auth/core/authenticator.server"; import { authSessionStorage } from "~/features/auth/core/session.server"; @@ -14,15 +15,16 @@ export function arrayContainsSameItems(arr1: T[], arr2: T[]) { export function wrappedAction({ action, - params = {}, }: { // TODO: strongly type this action: (args: ActionFunctionArgs) => any; - params?: ActionFunctionArgs["params"]; }) { return async ( args: z.infer, - { user }: { user?: "admin" | "regular" } = {}, + { + user, + params = {}, + }: { user?: "admin" | "regular"; params?: Params } = {}, ) => { const body = new URLSearchParams(args); const request = new Request("http://app.com/path", { @@ -58,7 +60,10 @@ export function wrappedLoader({ // TODO: strongly type this loader: (args: LoaderFunctionArgs) => any; }) { - return async ({ user }: { user?: "admin" | "regular" } = {}) => { + return async ({ + user, + params = {}, + }: { user?: "admin" | "regular"; params?: Params } = {}) => { const request = new Request("http://app.com/path", { method: "GET", headers: await authHeader(user), @@ -67,7 +72,7 @@ export function wrappedLoader({ try { const data = await loader({ request, - params: {}, + params, context: {}, }); @@ -87,7 +92,7 @@ async function authHeader(user?: "admin" | "regular"): Promise { const session = await authSessionStorage.getSession(); - session.set(SESSION_KEY, user === "admin" ? ADMIN_ID : NZAP_TEST_ID); + session.set(SESSION_KEY, user === "admin" ? ADMIN_ID : REGULAR_USER_TEST_ID); return [["Cookie", await authSessionStorage.commitSession(session)]]; } @@ -106,11 +111,12 @@ export const dbReset = () => { sql.prepare("PRAGMA foreign_keys = ON").run(); }; -export const dbInsertUsers = (count: number) => +export const dbInsertUsers = (count?: number) => db .insertInto("User") .values( - Array.from({ length: count }).map((_, i) => ({ + // defaults to 2 = admin & regular "NZAP" + Array.from({ length: count ?? 2 }).map((_, i) => ({ id: i + 1, discordName: `user${i + 1}`, discordId: String(i), diff --git a/db-test.sqlite3 b/db-test.sqlite3 index 8ce9fdd9f9ed067a734cdd716c1a0c8dfc115d86..1aaafb3b39ca68077ad0d6b222de22479cdd8ce0 100644 GIT binary patch delta 2646 zcmai$e`p(J7{}ksU7IG&^0r&kOM6N0n*K^`-tLmSbn7N{vlhEQR_QvNtd((1uIsHo z(k0imPG&cqs}=S`5A8sd;Xf*D;4ChP2vZc(o#&aP&ssV21@DQR6hrlMRn zud6wSlzeqdhgD6ig8>OiOtqu@s5UmXp`h97fMkts*v5b+94({#w3c_6M&sLRK4pz- zN--dY5Y zV4Eb$5;}?|)M+%XOrl9GJ?c>PFN1aVocDqudr4wuNo4VVO43qK}sMcrz2fC?68LFc?Q(uL?UUoeyVsKo)qJkTFzbzKUY-#9bC~y*SJP|;5(dP zcirHMO8t;)wC@Vm+!e_rn#m~>POG9X#3ilOiUiE_E-9j8*^EOWjQ2uC(!UE8iTXR# zWS5=@H`{eqm^u?Fq^rufQ-`mIi#q%)9JU{7o6LDkB$1xQUv|tXo3Ac<&^qeo^0iPA z4Qn%GD>;Nlyk>R8jt$xv=jw{Yeo+^+i*=hlitQ+^jHweD#p$>#=WDiG#YRgQuvHR8 z8EN|Q4KFWakT1HJ&hY$zHD9BP0WCORV#8cblzBFYe})J%eQr$}UnT7KJ~ar@W^+{ce_Jsh0d$zg@`hx zk7RRuG7&+W(AC3gE+*vloR-$Tg78}0cUQ=&Ht8h&p)|%@O~M+35Su(IU?xl zjJ`PZ*F2UAz?D~4R_9h$slv*tYjc#t9xuHNU?08gU-h5&Wqq=D$vfq_Te=%R3<|XO z^yo8gmgA^-RCpMtXQL~i@e*AD9xb?+d;;~zsN8ky!2VOyrEDwf z%9|Il`4~C0R)#}b=1KB40n1(cx_9sI7WVW$)%~1cYa(PPgoxQL68*1!u|oe3zAL?N zip;YyCL z6Vwre2|@%MK`lWIL6CqYs3r&yR1s_;*i2B#(0c(@LHU2@5+EC92Et;OcIAaVj?(ngR$H<73#&khC^LAbTq_1 zWtN{xQD9KwGZLJI)5-!oF3-RjwQ$Vl{9lJU@tA*x!gxvgjFM#d|0&6?c1u@O+q?1j zSK1?Zqr{Cyk#bNj7GYsZ)?lGrRBR*@9TDDq=4dwsIEj}saK2ze+TRg6WhQYk3Pcb| zgtJ?75>aCqBmv5E8r0+?Hk}aTC=f$|_)Wj9L!~;=rPS_puy`%}tflx)4d=I$I2#LKbR@eB3VK+Yw8`&m$3hsXbxbqaZ@9jJ|ToE*jn0`?C4e3Ev|s z4=2^?<|x@vySzqq5ZT_R_D-io>=&IOT&&B;$md`|E-BMR*=}9>xb_`pvAs?T3`jhm zfvR>A?f&8s+ue1Lv;3IeFS%$@ND)dUt}TMe{#;-{KrDCKx0x#A<2(yY*Xt zRmu9{`)3Ji@kzhy_4SRV^$nu2zTw!`+wZ;tykHqLkI{GOWjYO(J@;IDntpG3(Q(yr zmH3tT0KFRc-v7Y=pw^o2@-8MuUCaw3#Nuv!<)O39GlPa!EvPfHe(`5I;M*r_g{kKZ ztWi#pQAcAWLEdwEy=mf^U@Yc7GojyJr3dxVJ5-C_zecy~qib|TSJvneecvk0JLTzO z)T{rpM(6Y|S82vEG_HU0TT@tnK1D{n05|}XOcF~&jT0%7CG?lZJVE`nL!PvrzfN4z z&!@?6yk`zQ>ta}zIJ>=ZtDCG6`uzjQX(3KtUm6=Akx|H;$eNJ3khzh0ka>~$8e;== Z{W951jO(ryI$GbBPZ5prpXiVG{|lvrA0Ge!