From c58aa14cac9d0a33c2abdb2716632d714309fcbd Mon Sep 17 00:00:00 2001 From: zkldi <20380519+zkldi@users.noreply.github.com> Date: Thu, 14 Apr 2022 03:10:06 +0100 Subject: [PATCH] Remove parentMilestones as a feature. Handle it through database queries instead. --- server/package.json | 2 +- server/pnpm-lock.yaml | 8 +-- .../framework/goals/goals.test.ts | 3 - server/src/lib/targets/goals.ts | 59 +++++++++++++++++-- server/src/lib/targets/milestones.ts | 36 +---------- .../_game/_playtype/targets/goals/router.ts | 10 ++-- .../_playtype/targets/goals/router.test.ts | 11 +--- .../_game/_playtype/targets/goals/router.ts | 10 +++- server/src/test-utils/test-data.ts | 1 - 9 files changed, 78 insertions(+), 62 deletions(-) diff --git a/server/package.json b/server/package.json index 1ef5e28af..dc567c1c5 100644 --- a/server/package.json +++ b/server/package.json @@ -85,7 +85,7 @@ "rimraf": "3.0.2", "safe-json-stringify": "1.2.0", "seq-logging": "1.1.1", - "tachi-common": "0.7.14", + "tachi-common": "0.7.15", "ts-node": "10.4.0", "tsconfig-paths": "3.12.0", "typescript": "4.5.5", diff --git a/server/pnpm-lock.yaml b/server/pnpm-lock.yaml index b451adbaa..d279f9574 100644 --- a/server/pnpm-lock.yaml +++ b/server/pnpm-lock.yaml @@ -65,7 +65,7 @@ specifiers: safe-json-stringify: 1.2.0 seq-logging: 1.1.1 supertest: 6.2.2 - tachi-common: 0.7.14 + tachi-common: 0.7.15 tap: 15.1.6 ts-node: 10.4.0 tsconfig-paths: 3.12.0 @@ -112,7 +112,7 @@ dependencies: rimraf: 3.0.2 safe-json-stringify: 1.2.0 seq-logging: 1.1.1 - tachi-common: 0.7.14 + tachi-common: 0.7.15 ts-node: 10.4.0_2615db9039ce432b4abf2fc39ef336ec tsconfig-paths: 3.12.0 typescript: 4.5.5 @@ -5363,8 +5363,8 @@ packages: engines: {node: '>= 0.4'} dev: true - /tachi-common/0.7.14: - resolution: {integrity: sha512-RZfmSvCrITGm3jClZZc4JJAJwo6lUU+Wkq2+PAQh7gHdlR5OjVlTNDQr67F3F2KTagz6Sw4y1BDKw78j2aPl8Q==} + /tachi-common/0.7.15: + resolution: {integrity: sha512-rZNTYZFmDVUDcqaVJZAwLhH3Sskoks6Votc62JAOIrxVKEGu9JwGClEpU7YXtD4Uq2Qwt9ZesMtRatQ9rgcMcg==} dependencies: '@types/mongodb': 3.6.18 dev: false diff --git a/server/src/lib/score-import/framework/goals/goals.test.ts b/server/src/lib/score-import/framework/goals/goals.test.ts index 7ae5074d7..9d37b44a2 100644 --- a/server/src/lib/score-import/framework/goals/goals.test.ts +++ b/server/src/lib/score-import/framework/goals/goals.test.ts @@ -131,7 +131,6 @@ t.test("#GetRelevantGoals", (t) => { progressHuman: "NO DATA", timeSet: Date.now(), userID: 1, - parentMilestones: [], })) ); }); @@ -194,7 +193,6 @@ t.test("#UpdateGoalsForUser", (t) => { timeAchieved: null, timeSet: 0, userID: 1, - parentMilestones: [], }; t.test("Should correctly update goals when user achieves goal.", async (t) => { @@ -396,7 +394,6 @@ t.test("#ProcessGoal", (t) => { lastInteraction: null, outOf: 5, outOfHuman: "HARD CLEAR", - parentMilestones: [], playtype: "SP", progress: 6, progressHuman: "EX HARD CLEAR", diff --git a/server/src/lib/targets/goals.ts b/server/src/lib/targets/goals.ts index a6e1386e4..ca9ac8a40 100644 --- a/server/src/lib/targets/goals.ts +++ b/server/src/lib/targets/goals.ts @@ -11,6 +11,8 @@ import { PBScoreDocument, Playtypes, GoalSubscriptionDocument, + MilestoneSubscriptionDocument, + MilestoneDocument, } from "tachi-common"; import { GetFolderChartIDs } from "utils/folder"; import { CreateGoalTitle as CreateGoalName, ValidateGoalChartsAndCriteria } from "./goal-utils"; @@ -283,7 +285,6 @@ export async function ConstructGoal( export async function SubscribeToGoal( userID: integer, goalDocument: GoalDocument, - parentMilestone?: string, cancelIfAchieved = true ) { const goalExists = await db.goals.findOne({ goalID: goalDocument.goalID }); @@ -323,9 +324,6 @@ export async function SubscribeToGoal( lastInteraction: null, timeAchieved: result.achieved ? Date.now() : null, timeSet: Date.now(), - // if this goal subscription came from a milestone, add the milestone to - // the list of parents. - parentMilestones: parentMilestone ? [parentMilestone] : [], game: goalDocument.game, playtype: goalDocument.playtype, goalID: goalDocument.goalID, @@ -337,3 +335,56 @@ export async function SubscribeToGoal( return goalSub; } + +export function GetMilestonesThatContainGoal(goalID: string) { + return db.milestones.find({ + "milestoneData.goalID": goalID, + }); +} + +/** + * Unsubscribing from a goal may not be legal, because the goal might be part of + * a milestone the user is subscribed to. This function returns all milestones + * and milestoneSubs that a goal is attached to. + * + * If this query matches none, an empty array is returned. + */ +export async function GetBlockingParentMilestoneSubs( + goalSub: GoalSubscriptionDocument +): Promise<(MilestoneSubscriptionDocument & { milestone: MilestoneDocument })[]> { + const blockers = await db["milestone-subs"].aggregate([ + { + // find all milestones that this user is subscribed to + $match: { + userID: goalSub.userID, + game: goalSub.game, + playtype: goalSub.playtype, + }, + }, + { + // look up the parent milestones + $lookup: { + from: "milestones", + localField: "milestoneID", + foreignField: "milestoneID", + as: "parentMilestoneSubs", + }, + }, + { + // then project it onto the $milestone field. This will be null + // if the milestone has no parent, which we hopefully won't have + // to consider (illegal) + $set: { + milestone: { $arrayElemAt: ["$parentMilestoneSubs", 0] }, + }, + }, + { + // then finally, filter to only milestones that pertain to this goal. + $match: { + "$milestone.milestoneData.goalID": goalSub.goalID, + }, + }, + ]); + + return blockers; +} diff --git a/server/src/lib/targets/milestones.ts b/server/src/lib/targets/milestones.ts index 629baa426..0f16af1ed 100644 --- a/server/src/lib/targets/milestones.ts +++ b/server/src/lib/targets/milestones.ts @@ -230,25 +230,8 @@ export async function SubscribeToMilestone( // from result.goalResults ourselves. // evaluating goals is fairly cheap though. await Promise.all( - result.goals.map(async (goal) => { - const res = await SubscribeToGoal(userID, goal, milestone.milestoneID, false); - - // If the user is already subscribed to this goal -- i.e. manually or as part - // of another milestone - // add this milestoneID to the list of parents instead. - if (res === SubscribeFailReasons.ALREADY_SUBSCRIBED) { - await db["goal-subs"].update( - { - userID, - milestoneID: milestone.milestoneID, - }, - { - $push: { - parentMilestones: milestone.milestoneID, - }, - } - ); - } + result.goals.map((goal) => { + SubscribeToGoal(userID, goal, milestone.milestoneID, false); }) ); @@ -262,20 +245,7 @@ export async function SubscribeToMilestone( export async function UnsubscribeFromMilestone(userID: integer, milestone: MilestoneDocument) { const goalIDs = GetGoalIDsFromMilestone(milestone); - // Pull this milestone ID from all of the goalSubscriptions that have it. - // since it's no longer going to be their parent. - await db["goal-subs"].update( - { - goalID: { $in: goalIDs }, - userID, - parentMilestones: milestone.milestoneID, - }, - { - $pull: { - parentMilestones: milestone.milestoneID, - }, - } - ); + // TODO COME BACK HERE // then, remove all of the ones that now have no parent blocking their demise. // that's pretty morbid, jesus christ. diff --git a/server/src/server/router/api/v1/games/_game/_playtype/targets/goals/router.ts b/server/src/server/router/api/v1/games/_game/_playtype/targets/goals/router.ts index ca6cf1753..e828ac353 100644 --- a/server/src/server/router/api/v1/games/_game/_playtype/targets/goals/router.ts +++ b/server/src/server/router/api/v1/games/_game/_playtype/targets/goals/router.ts @@ -2,7 +2,11 @@ import { RequestHandler, Router } from "express"; import db from "external/mongo/db"; import { SYMBOL_TachiData } from "lib/constants/tachi"; import CreateLogCtx from "lib/logger/logger"; -import { EvaluatedGoalReturn, EvaluateGoalForUser } from "lib/targets/goals"; +import { + EvaluatedGoalReturn, + EvaluateGoalForUser, + GetMilestonesThatContainGoal, +} from "lib/targets/goals"; import prValidate from "server/middleware/prudence-validate"; import { FormatGame } from "tachi-common"; import { GetMostSubscribedGoals } from "utils/db"; @@ -66,9 +70,7 @@ router.get("/:goalID", ResolveGoalID, async (req, res) => { const users = await GetUsersWithIDs(goalSubs.map((e) => e.userID)); - const parentMilestones = await db.milestones.find({ - "milestoneData.goalID": goal.goalID, - }); + const parentMilestones = await GetMilestonesThatContainGoal(goal.goalID); return res.status(200).json({ success: true, diff --git a/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.test.ts b/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.test.ts index ddc2f436d..2a8d35948 100644 --- a/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.test.ts +++ b/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.test.ts @@ -656,9 +656,7 @@ t.test("GET /api/v1/users/:userID/games/:game/:playtype/targets/goals/:goalID", await db.goals.insert(dupedGoal); await db["goal-subs"].insert( // @ts-expect-error Not sure why the types break here, but they do. - dm(HC511UserGoal, { - parentMilestones: [TestingIIDXSPMilestone.milestoneID], - }) + dm(HC511UserGoal) ); await db.milestones.insert(dm(TestingIIDXSPMilestone, {})); @@ -739,12 +737,7 @@ t.test("DELETE /api/v1/users/:userID/games/:game/:playtype/targets/goals/:goalID t.test("Should reject a goal deletion if goal has parent milestones.", async (t) => { await db.goals.insert(dupedGoal); - await db["goal-subs"].insert( - // @ts-expect-error deepmerge type error - dm(dupedGoalSub, { - parentMilestones: [TestingIIDXSPMilestone.milestoneID], - }) - ); + await db["goal-subs"].insert(dm(dupedGoalSub, {})); const res = await mockApi .delete(`/api/v1/users/1/games/iidx/SP/targets/goals/${dupedGoalSub.goalID}`) diff --git a/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.ts b/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.ts index 6a3c5f5b5..6bd5d1c87 100644 --- a/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.ts +++ b/server/src/server/router/api/v1/users/_userID/games/_game/_playtype/targets/goals/router.ts @@ -4,7 +4,7 @@ import { SubscribeFailReasons } from "lib/constants/err-codes"; import { SYMBOL_TachiData } from "lib/constants/tachi"; import CreateLogCtx from "lib/logger/logger"; import { ServerConfig } from "lib/setup/config"; -import { ConstructGoal, SubscribeToGoal } from "lib/targets/goals"; +import { ConstructGoal, GetBlockingParentMilestoneSubs, SubscribeToGoal } from "lib/targets/goals"; import p from "prudence"; import { RequirePermissions } from "server/middleware/auth"; import prValidate from "server/middleware/prudence-validate"; @@ -256,10 +256,14 @@ router.delete( const goalSub = req[SYMBOL_TachiData]!.goalSubDoc!; - if (goalSub.parentMilestones.length) { + const parentMilestones = await GetBlockingParentMilestoneSubs(goalSub); + + if (parentMilestones.length) { return res.status(400).json({ success: false, - description: `This goal is part of a milestone you are subscribed to. It can only be removed by unsubscribing from the relevant milestones.`, + description: `This goal is part of a milestone you are subscribed to. It can only be removed by unsubscribing from the relevant milestones: ${parentMilestones + .map((e) => `'${e.milestone.name}'`) + .join(", ")}.`, }); } diff --git a/server/src/test-utils/test-data.ts b/server/src/test-utils/test-data.ts index 0fa2d545a..c06cf2036 100644 --- a/server/src/test-utils/test-data.ts +++ b/server/src/test-utils/test-data.ts @@ -438,7 +438,6 @@ export const HC511UserGoal: GoalSubscriptionDocument = { progressHuman: "NO DATA", timeSet: 0, userID: 1, - parentMilestones: [], }; export const TestingIIDXFolderSP10: FolderDocument = {