From 71013b36ec68126195530d7b6f3d09dfc7fb1d25 Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Wed, 18 Dec 2024 15:31:20 -0500 Subject: [PATCH 1/4] Return merged class ID in all data rather than actual class ID. --- src/stories/hubbles_law/database.ts | 30 +++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/stories/hubbles_law/database.ts b/src/stories/hubbles_law/database.ts index aef50f5..b8f510d 100644 --- a/src/stories/hubbles_law/database.ts +++ b/src/stories/hubbles_law/database.ts @@ -8,6 +8,7 @@ import { HubbleClassData } from "./models/hubble_class_data"; import { IgnoreStudent } from "../../models/ignore_student"; import { logger } from "../../logger"; import { HubbleClassMergeGroup } from "./models/hubble_class_merge_group"; +import { QueryOptions } from "mysql2"; const galaxyAttributes = ["id", "ra", "decl", "z", "type", "name", "element"]; @@ -320,7 +321,7 @@ async function getClassIDsForSyncClass(classID: number): Promise { return classIDs; } -export async function getMergedIDsForClass(classID: number, ignoreMergeOrder=false): Promise { +export async function getMergedIDsForClass(classID: number, ignoreMergeOrder=false, mergeOrderSort: "ASC" | "DESC" | null = null): Promise { // TODO: Currently this uses two queries: // The first to get the merge group (if there is one) // Then a second to get all of the classes in the merge group @@ -342,7 +343,11 @@ export async function getMergedIDsForClass(classID: number, ignoreMergeOrder=fal [Op.lte]: mergeGroup.merge_order, }; } - const mergeEntries = await HubbleClassMergeGroup.findAll({ where }); + const query: FindOptions = { where }; + if (mergeOrderSort) { + query.order = [["merge_order", mergeOrderSort]]; + } + const mergeEntries = await HubbleClassMergeGroup.findAll(query); return mergeEntries.map(entry => entry.class_id); } @@ -453,7 +458,7 @@ export async function getAllHubbleMeasurements(before: Date | null = null, } const exclude = minimal ? Object.keys(HubbleMeasurement.getAttributes()).filter(key => !MINIMAL_MEASUREMENT_FIELDS.includes(key)) : []; - return HubbleMeasurement.findAll({ + const measurements = await HubbleMeasurement.findAll({ raw: true, attributes: { // The "student" here comes from the alias below @@ -492,7 +497,24 @@ export async function getAllHubbleMeasurements(before: Date | null = null, } }] }] - }); + }) as (HubbleMeasurement & { class_id: number })[]; + + const mergeIDsToUse: Record = {}; + for (const measurement of measurements) { + const cid = measurement.class_id; + let mergedID = mergeIDsToUse[cid]; + if (mergedID === undefined) { + const mergedIDs = await getMergedIDsForClass(cid, true, "DESC"); + mergedID = mergedIDs[0]; + mergedIDs.forEach(id => { + mergeIDsToUse[id] = mergedID; + }); + } + measurement.class_id = mergedID; + } + + return measurements; + } const MINIMAL_STUDENT_DATA_FIELDS = ["student_id", "age_value"]; From aadae3d523cc0b429e1d800ce6a3769edc22c3e1 Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Wed, 18 Dec 2024 16:19:00 -0500 Subject: [PATCH 2/4] Make it so that the data returned from the all data endpoint gives the class ID as whichever ID is being used for the relevant merge group. --- src/stories/hubbles_law/database.ts | 41 ++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/src/stories/hubbles_law/database.ts b/src/stories/hubbles_law/database.ts index b8f510d..0ddc50a 100644 --- a/src/stories/hubbles_law/database.ts +++ b/src/stories/hubbles_law/database.ts @@ -499,19 +499,36 @@ export async function getAllHubbleMeasurements(before: Date | null = null, }] }) as (HubbleMeasurement & { class_id: number })[]; - const mergeIDsToUse: Record = {}; - for (const measurement of measurements) { - const cid = measurement.class_id; - let mergedID = mergeIDsToUse[cid]; - if (mergedID === undefined) { - const mergedIDs = await getMergedIDsForClass(cid, true, "DESC"); - mergedID = mergedIDs[0]; - mergedIDs.forEach(id => { - mergeIDsToUse[id] = mergedID; - }); + const classIDs = new Set(); + measurements.forEach(measurement => classIDs.add(measurement.class_id)); + const mergeGroupData = await HubbleClassMergeGroup.findAll({ + attributes: [ + "class_id", + "group_id", + // We use a RANK rather than the merge order because we don't know what the highest merge order value will be + // But the best rank will always be 1 + [Sequelize.literal("RANK() OVER(PARTITION BY group_id ORDER BY merge_order DESC)"), "rk"], + ], + order: [["rk", "ASC"]], + }) as (HubbleClassMergeGroup & { rk: number })[]; + + const mergeIDsToUse: Record = {}; + const groupIDs: Record = {}; + mergeGroupData.forEach(data => { + // Because this is a column constructed inside the query, + // it doesn't work to do `data.rk`, or `data.getDataValue("rk")` + const rank = data.get("rk"); + if (rank === 1) { + mergeIDsToUse[data.class_id] = data.class_id; + groupIDs[data.group_id] = data.class_id; + } else { + mergeIDsToUse[data.class_id] = groupIDs[data.group_id] ?? data.class_id; } - measurement.class_id = mergedID; - } + }); + + measurements.forEach(measurement => { + measurement.class_id = mergeIDsToUse[measurement.class_id] ?? measurement.class_id; + }); return measurements; From cffd023db6187cefb145f4fd7a17ed4b851fe615 Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Wed, 18 Dec 2024 16:26:50 -0500 Subject: [PATCH 3/4] Remove unused import. --- src/stories/hubbles_law/database.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/stories/hubbles_law/database.ts b/src/stories/hubbles_law/database.ts index 0ddc50a..d47573b 100644 --- a/src/stories/hubbles_law/database.ts +++ b/src/stories/hubbles_law/database.ts @@ -8,7 +8,6 @@ import { HubbleClassData } from "./models/hubble_class_data"; import { IgnoreStudent } from "../../models/ignore_student"; import { logger } from "../../logger"; import { HubbleClassMergeGroup } from "./models/hubble_class_merge_group"; -import { QueryOptions } from "mysql2"; const galaxyAttributes = ["id", "ra", "decl", "z", "type", "name", "element"]; From dffe2034376197fa6782515276283dbf219fea74 Mon Sep 17 00:00:00 2001 From: Carifio24 Date: Wed, 18 Dec 2024 16:28:22 -0500 Subject: [PATCH 4/4] Remove the merge order sorting in merged IDs query; we don't actually need it. --- src/stories/hubbles_law/database.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/stories/hubbles_law/database.ts b/src/stories/hubbles_law/database.ts index d47573b..ff8be30 100644 --- a/src/stories/hubbles_law/database.ts +++ b/src/stories/hubbles_law/database.ts @@ -320,7 +320,7 @@ async function getClassIDsForSyncClass(classID: number): Promise { return classIDs; } -export async function getMergedIDsForClass(classID: number, ignoreMergeOrder=false, mergeOrderSort: "ASC" | "DESC" | null = null): Promise { +export async function getMergedIDsForClass(classID: number, ignoreMergeOrder=false): Promise { // TODO: Currently this uses two queries: // The first to get the merge group (if there is one) // Then a second to get all of the classes in the merge group @@ -342,11 +342,7 @@ export async function getMergedIDsForClass(classID: number, ignoreMergeOrder=fal [Op.lte]: mergeGroup.merge_order, }; } - const query: FindOptions = { where }; - if (mergeOrderSort) { - query.order = [["merge_order", mergeOrderSort]]; - } - const mergeEntries = await HubbleClassMergeGroup.findAll(query); + const mergeEntries = await HubbleClassMergeGroup.findAll({ where }); return mergeEntries.map(entry => entry.class_id); }