From 94600dc01359821e342bd4a4a925b23f4574f657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikko=20Pyykk=C3=B6?= Date: Wed, 28 Sep 2022 17:44:05 +0300 Subject: [PATCH 1/2] add course instance to completion from where it's possible --- backend/api/routes/completions.ts | 38 ++++++------ backend/api/routes/progress.ts | 21 ++++--- backend/api/routes/storedData.ts | 6 +- .../common/getUserWithRaceCondition.ts | 6 +- .../bin/kafkaConsumer/common/userFunctions.ts | 10 ++-- backend/bin/sendAiStatistics.ts | 9 +-- backend/bin/updateBAICompletionTiers.ts | 4 +- backend/graphql/Completion/model.ts | 19 +++++- backend/graphql/Completion/queries.ts | 25 +++++--- backend/graphql/Course/model.ts | 11 +++- backend/graphql/User/model.ts | 9 ++- backend/graphql/UserCourseSummary.ts | 30 ++++++---- backend/prisma/schema.prisma | 11 ++-- backend/util/db-functions.ts | 59 +++++++++++++++++++ backend/util/notEmpty.ts | 2 +- backend/util/server-functions.ts | 17 +++--- frontend/components/CertificateButton.tsx | 2 +- frontend/lib/certificates.ts | 2 +- 18 files changed, 196 insertions(+), 85 deletions(-) diff --git a/backend/api/routes/completions.ts b/backend/api/routes/completions.ts index 112f403f3..534921fb8 100644 --- a/backend/api/routes/completions.ts +++ b/backend/api/routes/completions.ts @@ -11,6 +11,7 @@ import { } from "@prisma/client" import { generateUserCourseProgress } from "../../bin/kafkaConsumer/common/userCourseProgress/generateUserCourseProgress" +import { mapCompletionsWithCourseInstanceId } from "../../util/db-functions" import { err } from "../../util/result" import { ApiContext, Controller } from "../types" @@ -53,10 +54,12 @@ export class CompletionController extends Controller { return res.status(404).json({ message: "Course not found" }) } - const sql = knex - .select("completion.*") + const sql = knex("completion") + .select([ + "completion.*", + knex.raw(`'${course.id}' as course_instance_id`) + ]) .distinctOn("completion.user_id", "completion.course_id") - .from("completion") .fullOuterJoin( "completion_registered", "completion.id", @@ -97,9 +100,8 @@ export class CompletionController extends Controller { } const instructions = ( - await knex - .select("instructions") - .from("course_translation") + await knex("course_translation") + .select("instructions") .where("course_id", course.id) .where("language", languageMap[language] ?? "fi_FI") )[0]?.instructions @@ -132,9 +134,8 @@ export class CompletionController extends Controller { } const completion = ( - await knex - .select("tier") - .from("completion") + await knex("completion") + .select("tier") .where("course_id", course.completions_handled_by_id ?? course.id) .andWhere("user_id", user.id) .orderBy("created_at", "asc") @@ -147,9 +148,8 @@ export class CompletionController extends Controller { // TODO/FIXME: note - this now happily ignores completion_language and just gets the first one // - as it's now only used in BAI, shouldn't be a problem? const tiers = ( - await knex - .select("tiers") - .from("open_university_registration_link") + await knex("open_university_registration_link") + .select("tiers") .where("course_id", course.id) )?.[0].tiers @@ -247,14 +247,13 @@ export class CompletionController extends Controller { }, }) - return res.status(200).json(updatedCompletion) + return res + .status(200) + .json({ ...updatedCompletion, course_instance_id: course.id }) } - private getCompletion = async ( - course: Course, - user: User, - ): Promise => { - return ( + private getCompletion = async (course: Course, user: User) => { + return mapCompletionsWithCourseInstanceId( await this.ctx.prisma.user .findUnique({ where: { @@ -267,7 +266,8 @@ export class CompletionController extends Controller { }, orderBy: { created_at: "asc" }, take: 1, - }) + }), + course.id, )?.[0] } diff --git a/backend/api/routes/progress.ts b/backend/api/routes/progress.ts index b9c60975d..dad77e091 100644 --- a/backend/api/routes/progress.ts +++ b/backend/api/routes/progress.ts @@ -1,12 +1,13 @@ import { Request, Response } from "express" import { chunk, groupBy, omit } from "lodash" -import { Completion, User, UserCourseProgress } from "@prisma/client" +import { Completion, ExerciseCompletion, User, UserCourseProgress } from "@prisma/client" import { generateUserCourseProgress } from "../../bin/kafkaConsumer/common/userCourseProgress/generateUserCourseProgress" import { BAIParentCourse, BAItiers } from "../../config/courseConfig" import { notEmpty } from "../../util/notEmpty" import { ApiContext, Controller } from "../types" +import { mapCompletionsWithCourseInstanceId } from "../../util/db-functions" interface ExerciseCompletionResult { user_id: string @@ -65,7 +66,7 @@ export class ProgressController extends Controller { "section", "max_points", "completed", - "custom_id as quizzes_id", + "custom_id as quizzes_id" ) .distinctOn("ec.exercise_id") .join("exercise as e", { "ec.exercise_id": "e.id" }) @@ -152,11 +153,16 @@ export class ProgressController extends Controller { const exercise_completions = await exerciseCompletionQuery - const completions = await knex("completion as c") - .select("*") + const completions = mapCompletionsWithCourseInstanceId( + await knex(knex.ref("completion").as("c")) + .select( + "*", + ) .where("course_id", course.completions_handled_by_id ?? course.id) .andWhere("user_id", user.id) - .orderBy("created_at", "asc") + .orderBy("created_at", "asc"), + course.id + ) const exercise_completions_map = (exercise_completions ?? []).reduce( (acc, curr) => ({ @@ -227,12 +233,11 @@ export class ProgressController extends Controller { id = course.id } - const data = await knex - .select[]>( + const data = await knex("user_course_progress") + .select( "course_id", "extra", ) - .from("user_course_progress") .where("user_course_progress.course_id", id) .andWhere("user_course_progress.user_id", user.id) .orderBy("created_at", "asc") diff --git a/backend/api/routes/storedData.ts b/backend/api/routes/storedData.ts index 692866e64..9483a7956 100644 --- a/backend/api/routes/storedData.ts +++ b/backend/api/routes/storedData.ts @@ -1,6 +1,7 @@ import { Request, Response } from "express-serve-static-core" import { omit } from "lodash" +import { mapCompletionsWithCourseInstanceId } from "../../util/db-functions" import { requireCourseOwnership } from "../../util/server-functions" import { ApiContext, Controller } from "../types" @@ -120,7 +121,10 @@ export class StoredDataController extends Controller { const mappedStoredData = storedData.map((data) => ({ user: omit(data.user, "completions"), - completions: data.user?.completions, + completions: mapCompletionsWithCourseInstanceId( + data.user?.completions, + course.id, + ), storedData: omit(data, "user"), })) diff --git a/backend/bin/kafkaConsumer/common/getUserWithRaceCondition.ts b/backend/bin/kafkaConsumer/common/getUserWithRaceCondition.ts index 8fe9353dc..09c308f34 100644 --- a/backend/bin/kafkaConsumer/common/getUserWithRaceCondition.ts +++ b/backend/bin/kafkaConsumer/common/getUserWithRaceCondition.ts @@ -11,14 +11,16 @@ export async function getUserWithRaceCondition( let user: User | null const { knex, logger, prisma } = context - user = (await knex("user").where("upstream_id", user_id).limit(1))[0] + user = (await knex("user").where("upstream_id", user_id).limit(1))[0] if (!user) { try { user = await getUserFromTMCAndCreate(prisma, user_id) } catch (e) { try { - user = (await knex("user").where("upstream_id", user_id).limit(1))[0] + user = ( + await knex("user").where("upstream_id", user_id).limit(1) + )[0] } catch {} if (!user) { diff --git a/backend/bin/kafkaConsumer/common/userFunctions.ts b/backend/bin/kafkaConsumer/common/userFunctions.ts index 016846df0..b488476ac 100644 --- a/backend/bin/kafkaConsumer/common/userFunctions.ts +++ b/backend/bin/kafkaConsumer/common/userFunctions.ts @@ -103,10 +103,10 @@ export const getExerciseCompletionsForCourses = async ({ }: GetExerciseCompletionsForCoursesArgs) => { // picks only one exercise completion per exercise/user: // the one with the latest timestamp and latest updated_at - const exercise_completions: ExerciseCompletionPart[] = await knex( + const exercise_completions = await knex( "exercise_completion as ec", ) - .select("course_id", "custom_id", "max_points", "exercise_id", "n_points") + .select("course_id", "custom_id", "max_points", "exercise_id", "n_points") .distinctOn("ec.exercise_id") .join("exercise as e", { "ec.exercise_id": "e.id" }) .whereIn("e.course_id", courseIds) @@ -170,7 +170,7 @@ export const pruneDuplicateExerciseCompletions = async ({ .returning("id")*/ // we probably can just delete all even if they have required actions - const deleted: Array> = await knex( + const deleted = await knex( "exercise_completion", ) .whereIn( @@ -207,8 +207,8 @@ export const pruneDuplicateExerciseCompletions = async ({ export const pruneOrphanedExerciseCompletionRequiredActions = async ({ context: { knex }, }: WithBaseContext) => { - const deleted: Array> = - await knex("exercise_completion_required_actions") + const deleted = + await knex("exercise_completion_required_actions") .whereNull("exercise_completion_id") .delete() .returning("id") diff --git a/backend/bin/sendAiStatistics.ts b/backend/bin/sendAiStatistics.ts index 847ce6231..0adfe7437 100644 --- a/backend/bin/sendAiStatistics.ts +++ b/backend/bin/sendAiStatistics.ts @@ -1,3 +1,4 @@ +import { Course } from "@prisma/client" import { AI_SLACK_URL } from "../config" import { languageInfo, LanguageInfo } from "../config/languageConfig" import prisma from "../prisma" @@ -80,8 +81,8 @@ const getDataByLanguage = async (langInfo: LanguageInfo) => { // } const getGlobalStats = async (): Promise => { - const course = await Knex.select("id") - .from("course") + const course = await Knex("course") + .select("id") .where({ slug: "elements-of-ai" }) const totalUsers = ( await Knex.countDistinct("user_id") @@ -105,8 +106,8 @@ const getGlobalStats = async (): Promise => { } const getGlobalStatsBAI = async (): Promise => { - const course = await Knex.select("id") - .from("course") + const course = await Knex("course") + .select("id") .where({ slug: "building-ai" }) const totalUsers = ( diff --git a/backend/bin/updateBAICompletionTiers.ts b/backend/bin/updateBAICompletionTiers.ts index aae504179..0469ca951 100644 --- a/backend/bin/updateBAICompletionTiers.ts +++ b/backend/bin/updateBAICompletionTiers.ts @@ -21,7 +21,7 @@ const updateBAICompletionTiers = async () => { logger.info("Getting completions") - const userIdsWithoutTiers = await knex[]>( + const userIdsWithoutTiers = await knex( "completion", ) .select("user_id") @@ -31,7 +31,7 @@ const updateBAICompletionTiers = async () => { .orderBy(["user_id", "course_id", { column: "created_at", order: "asc" }]) logger.info("Getting users") - const usersWithoutTiers = await knex("user") + const usersWithoutTiers = await knex("user") .select("*") .whereIn( "id", diff --git a/backend/graphql/Completion/model.ts b/backend/graphql/Completion/model.ts index 734846ffa..0174ecbc5 100644 --- a/backend/graphql/Completion/model.ts +++ b/backend/graphql/Completion/model.ts @@ -31,6 +31,20 @@ export const Completion = objectType({ t.model.tier() t.model.completion_registration_attempt_date() + t.id("course_instance_id") + t.field("course_instance", { + type: "Course", + // @ts-ignore: course_instance_id exists even if TS says it doesn't + resolve: async ({ course_instance_id }, _, ctx) => { + if (!course_instance_id) { + return null + } + + return ctx.prisma.course.findUnique({ + where: { id: course_instance_id }, + }) + } + }) // we're not querying completion course languages for now, and this was buggy /* t.field("course", { type: "Course", @@ -151,13 +165,14 @@ export const Completion = objectType({ t.field("certificate_availability", { type: "CertificateAvailability", - resolve: async ({ course_id, user_upstream_id }, _, ctx) => { + // @ts-ignore: course_instance_id exists even if TS says it doesn't + resolve: async ({ course_id, course_instance_id, user_upstream_id }, _, ctx) => { if (!course_id) { return null } const course = await ctx.prisma.course.findUnique({ - where: { id: course_id }, + where: { id: course_instance_id ?? course_id }, }) if (!course) { diff --git a/backend/graphql/Completion/queries.ts b/backend/graphql/Completion/queries.ts index d142b1317..eefd94188 100644 --- a/backend/graphql/Completion/queries.ts +++ b/backend/graphql/Completion/queries.ts @@ -8,7 +8,11 @@ import { findManyCursorConnection } from "@devoxa/prisma-relay-cursor-connection import { Prisma } from "@prisma/client" import { isAdmin, isOrganization, or } from "../../accessControl" -import { buildUserSearch, getCourseOrAlias } from "../../util/db-functions" +import { + buildUserSearch, + getCourseOrAlias, + mapCompletionsWithCourseInstanceId, +} from "../../util/db-functions" export const CompletionQueries = extendType({ type: "Query", @@ -55,7 +59,7 @@ export const CompletionQueries = extendType({ orderBy: { created_at: "asc" }, }) - return completions + return mapCompletionsWithCourseInstanceId(completions, course.id) }, }) @@ -100,12 +104,17 @@ export const CompletionQueries = extendType({ } return findManyCursorConnection( - (args) => - ctx.prisma.course - .findUnique({ - where: { id: course!.completions_handled_by_id ?? course!.id }, - }) - .completions(merge(baseArgs, args)), + async (args) => + mapCompletionsWithCourseInstanceId( + await ctx.prisma.course + .findUnique({ + where: { + id: course!.completions_handled_by_id ?? course!.id, + }, + }) + .completions(merge(baseArgs, args)), + course.id, + ), async () => { // TODO/FIXME: kludge as there is no distinct in prisma "count" or other aggregates // ctx.prisma.completion.count(baseArgs as any), // not really same type, so force it diff --git a/backend/graphql/Course/model.ts b/backend/graphql/Course/model.ts index 58b162f6d..fa78cfbd4 100644 --- a/backend/graphql/Course/model.ts +++ b/backend/graphql/Course/model.ts @@ -1,6 +1,7 @@ import { booleanArg, intArg, nullable, objectType, stringArg } from "nexus" import { isAdmin } from "../../accessControl" +import { mapCompletionsWithCourseInstanceId } from "../../util/db-functions" export const Course = objectType({ name: "Course", @@ -76,7 +77,11 @@ export const Course = objectType({ throw new Error("needs user_id or user_upstream_id") } - return ctx.prisma.course + // TODO: if we're querying from the parent course, obviously + // the course instance id is the same here -- depends on what + // we need? + return mapCompletionsWithCourseInstanceId( + await ctx.prisma.course .findUnique({ where: { id: parent.completions_handled_by_id ?? parent.id, @@ -91,7 +96,9 @@ export const Course = objectType({ }, distinct: ["user_id", "course_id"], orderBy: { updated_at: "desc" }, - }) + }), + parent.id + ) }, }) diff --git a/backend/graphql/User/model.ts b/backend/graphql/User/model.ts index eb2248acf..0f92583d3 100644 --- a/backend/graphql/User/model.ts +++ b/backend/graphql/User/model.ts @@ -3,7 +3,7 @@ import { booleanArg, idArg, nullable, objectType, stringArg } from "nexus" import { Course, Prisma } from "@prisma/client" -import { getCourseOrAlias } from "../../util/db-functions" +import { getCourseOrAlias, mapCompletionsWithCourseInstanceId } from "../../util/db-functions" import { getCourseOrCompletionHandlerCourse } from "../../util/graphql-functions" import { notEmpty } from "../../util/notEmpty" @@ -58,7 +58,7 @@ export const User = objectType({ } } - return ctx.prisma.user + const completions = await ctx.prisma.user .findUnique({ where: { id: parent.id }, }) @@ -73,6 +73,11 @@ export const User = objectType({ distinct: ["user_id", "course_id"], orderBy: { created_at: "asc" }, }) + + return mapCompletionsWithCourseInstanceId( + completions, + course_id, + ) }, }) diff --git a/backend/graphql/UserCourseSummary.ts b/backend/graphql/UserCourseSummary.ts index 5dda5b4fd..9f258dac2 100644 --- a/backend/graphql/UserCourseSummary.ts +++ b/backend/graphql/UserCourseSummary.ts @@ -1,6 +1,8 @@ import { UserInputError } from "apollo-server-express" import { booleanArg, objectType } from "nexus" +import { mapCompletionsWithCourseInstanceId } from "../util/db-functions" + export const UserCourseSummary = objectType({ name: "UserCourseSummary", definition(t) { @@ -32,19 +34,21 @@ export const UserCourseSummary = objectType({ if (!user_id || !course_id) { throw new UserInputError("need to specify user_id and course_id") } - const completions = await ctx.prisma.course - .findUnique({ - where: { id: completions_handled_by_id ?? course_id }, - }) - .completions({ - where: { - user_id, - }, - // TODO: completed: true? - orderBy: { created_at: "asc" }, - take: 1, - }) - + const completions = mapCompletionsWithCourseInstanceId( + await ctx.prisma.course + .findUnique({ + where: { id: completions_handled_by_id ?? course_id }, + }) + .completions({ + where: { + user_id, + }, + // TODO: completed: true? + orderBy: { created_at: "asc" }, + take: 1, + }), + course_id, + ) return completions?.[0] }, }) diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index a54d9f3c9..83fd9184f 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -48,6 +48,8 @@ model Completion { course Course? @relation(fields: [course_id], references: [id]) user User? @relation(fields: [user_id], references: [id]) completions_registered CompletionRegistered[] + course_instance_id String? + course_instance Course? @relation("completion_course_instance_to_course", fields: [course_instance_id], references: [id]) completion_date DateTime? tier Int? completion_registration_attempt_date DateTime? @@ -122,14 +124,15 @@ model Course { user_course_service_progresses UserCourseServiceProgress[] user_course_settings UserCourseSetting[] user_course_settings_visibilities UserCourseSettingsVisibility[] - services Service[] @relation("course_to_service", references: [id]) - study_modules StudyModule[] @relation("study_module_to_course", references: [id]) + services Service[] @relation("course_to_service") + study_modules StudyModule[] @relation("study_module_to_course") upcoming_active_link Boolean? tier Int? stored_data StoredData[] ownerships CourseOwnership[] course_stats_email_id String? course_stats_email EmailTemplate? @relation("courseTocourse_course_stats_email", fields: [course_stats_email_id], references: [id]) + instance_completions Completion[] @relation("completion_course_instance_to_course") @@map("course") } @@ -358,7 +361,7 @@ model Service { url String exercises Exercise[] user_course_service_progresses UserCourseServiceProgress[] - courses Course[] @relation("course_to_service", references: [id]) + courses Course[] @relation("course_to_service") @@map("service") } @@ -385,7 +388,7 @@ model StudyModule { slug String @unique updated_at DateTime? @updatedAt study_module_translations StudyModuleTranslation[] - courses Course[] @relation("study_module_to_course", references: [id]) + courses Course[] @relation("study_module_to_course") @@map("study_module") } diff --git a/backend/util/db-functions.ts b/backend/util/db-functions.ts index 66a16fb3e..ebe877d5e 100644 --- a/backend/util/db-functions.ts +++ b/backend/util/db-functions.ts @@ -9,6 +9,65 @@ import { BaseContext } from "../context" import { isNullOrUndefined } from "./isNullOrUndefined" import { notEmpty } from "./notEmpty" +type InferCompletionPayloadType = + Payload extends Prisma.CompletionGetPayload + ? Prisma.CompletionGetPayload + : Payload + +type Optional = T | null | undefined + +// gee, I hope there's enough overloads -_- +export function mapCompletionsWithCourseInstanceId< + T, + Payload extends InferCompletionPayloadType, +>( + completions: Array, + course_instance_id: null | undefined, +): typeof completions +export function mapCompletionsWithCourseInstanceId< + T, + Payload extends InferCompletionPayloadType, +>( + completions: null | undefined, + course_instance_id: Optional, +): typeof completions +export function mapCompletionsWithCourseInstanceId< + T, + Payload extends InferCompletionPayloadType, +>( + completions: Array, + course_instance_id: string, +): Array +export function mapCompletionsWithCourseInstanceId< + T, + Payload extends InferCompletionPayloadType, +>( + completions: Optional>, + course_instance_id: string, +): typeof completions | Array +export function mapCompletionsWithCourseInstanceId< + T, + Payload extends InferCompletionPayloadType, +>( + completions: Array, + course_instance_id: Optional, +): Array | Array +export function mapCompletionsWithCourseInstanceId< + T, + Payload extends InferCompletionPayloadType, +>(completions: Optional>, course_instance_id: Optional) { + if (!notEmpty(completions) || !notEmpty(course_instance_id)) { + return completions + } + + const res = completions?.map((completion) => ({ + ...completion, + course_instance_id, + })) + + return res +} + const flatten = (arr: T[]) => arr.reduce((acc, val) => acc.concat(val), []) const titleCase = (s?: string) => diff --git a/backend/util/notEmpty.ts b/backend/util/notEmpty.ts index fedda02ca..782f33352 100644 --- a/backend/util/notEmpty.ts +++ b/backend/util/notEmpty.ts @@ -1,5 +1,5 @@ export function notEmpty( value: TValue | null | undefined, ): value is TValue { - return value !== null && value !== undefined + return value !== null && typeof value !== "undefined" } diff --git a/backend/util/server-functions.ts b/backend/util/server-functions.ts index 96cc7de75..b83534143 100644 --- a/backend/util/server-functions.ts +++ b/backend/util/server-functions.ts @@ -104,16 +104,15 @@ export function getUser({ knex, logger }: BaseContext) { } let user = ( - await knex - .select("id") - .from("user") + await knex("user") + .select("*") .where("upstream_id", details.id) )?.[0] if (!user) { try { user = ( - await knex("user") + await knex("user") .insert({ upstream_id: details.id, administrator: details.administrator, @@ -127,9 +126,8 @@ export function getUser({ knex, logger }: BaseContext) { } catch { // race condition or something user = ( - await knex - .select("*") - .from("user") + await knex("user") + .select("*") .where("upstream_id", details.id) )?.[0] @@ -166,9 +164,8 @@ export function getOrganization({ knex }: BaseContext) { } const org = ( - await knex - .select("*") - .from("organization") + await knex("organization") + .select("*") .where({ secret_key }) .limit(1) )[0] diff --git a/frontend/components/CertificateButton.tsx b/frontend/components/CertificateButton.tsx index 1da5cec9b..2d7517915 100644 --- a/frontend/components/CertificateButton.tsx +++ b/frontend/components/CertificateButton.tsx @@ -26,7 +26,7 @@ import { CourseCoreFieldsFragment, } from "/graphql/generated" -const CERTIFICATES_URL = "https://certificates.mooc.fi" +const CERTIFICATES_URL = "http://localhost:9200" //"https://certificates.mooc.fi" const StyledButton = styled(Button)` margin: auto; diff --git a/frontend/lib/certificates.ts b/frontend/lib/certificates.ts index 79e681c19..1990ce0ce 100644 --- a/frontend/lib/certificates.ts +++ b/frontend/lib/certificates.ts @@ -2,7 +2,7 @@ import axios from "axios" import { getAccessToken } from "./authentication" -const SERVICE_URL = "https://certificates.mooc.fi" +const SERVICE_URL = "http://localhost:9200" //"https://certificates.mooc.fi" export const checkCertificate = async (courseSlug: string) => { const accessToken = getAccessToken(undefined) From b0b627c13d3ec2579fbc490ba895687bbcd140d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikko=20Pyykk=C3=B6?= Date: Wed, 28 Sep 2022 18:37:46 +0300 Subject: [PATCH 2/2] add course instance to schema, rename passed --- .../__test__/__snapshots__/api.test.ts.snap | 5 +++ backend/api/routes/completions.ts | 14 ++++---- backend/api/routes/progress.ts | 21 ++++-------- backend/api/routes/storedData.ts | 6 +--- .../bin/kafkaConsumer/common/userFunctions.ts | 28 +++++++++------- backend/bin/sendAiStatistics.ts | 1 + backend/bin/updateBAICompletionTiers.ts | 4 +-- backend/graphql/Completion/model.ts | 32 ++++++++++++++----- backend/graphql/Completion/mutations.ts | 7 ++++ backend/graphql/Course/model.ts | 30 ++++++++--------- backend/graphql/User/model.ts | 10 +++--- ...er-table-completion-add-course-instance.ts | 30 +++++++++++++++++ backend/util/db-functions.ts | 25 ++++++++------- backend/util/server-functions.ts | 8 ++--- frontend/components/CertificateButton.tsx | 2 +- frontend/lib/certificates.ts | 2 +- 16 files changed, 136 insertions(+), 89 deletions(-) create mode 100644 backend/migrations/20220928144940_alter-table-completion-add-course-instance.ts diff --git a/backend/api/routes/__test__/__snapshots__/api.test.ts.snap b/backend/api/routes/__test__/__snapshots__/api.test.ts.snap index 2ff8a5d5d..f6daa314d 100644 --- a/backend/api/routes/__test__/__snapshots__/api.test.ts.snap +++ b/backend/api/routes/__test__/__snapshots__/api.test.ts.snap @@ -9,6 +9,7 @@ Object { "completion_language": null, "completion_registration_attempt_date": "2022-01-01T08:00:00.000Z", "course_id": "00000000-0000-0000-0000-000000000001", + "course_instance_id": null, "created_at": "2000-01-01T08:00:00.000Z", "eligible_for_ects": true, "email": "e@mail.com", @@ -57,6 +58,7 @@ Object { "completion_language": null, "completion_registration_attempt_date": "2022-01-01T08:00:00.000Z", "course_id": "00000000-0000-0000-0000-000000000001", + "course_instance_id": null, "created_at": "2000-01-01T08:00:00.000Z", "eligible_for_ects": true, "email": "e@mail.com", @@ -115,6 +117,7 @@ Object { "completion_language": null, "completion_registration_attempt_date": "2022-01-01T08:00:00.000Z", "course_id": "00000000-0000-0000-0000-000000000001", + "course_instance_id": null, "created_at": "2000-01-01T08:00:00.000Z", "eligible_for_ects": true, "email": "e@mail.com", @@ -163,6 +166,7 @@ Object { "completion_language": null, "completion_registration_attempt_date": "2022-01-01T08:00:00.000Z", "course_id": "00000000-0000-0000-0000-000000000001", + "course_instance_id": null, "created_at": "2000-01-01T08:00:00.000Z", "eligible_for_ects": true, "email": "e@mail.com", @@ -249,6 +253,7 @@ Array [ "completion_language": null, "completion_registration_attempt_date": "2022-01-01T08:00:00.000Z", "course_id": "00000000-0000-0000-0000-000000000001", + "course_instance_id": null, "created_at": "2000-01-01T08:00:00.000Z", "eligible_for_ects": true, "email": "e@mail.com", diff --git a/backend/api/routes/completions.ts b/backend/api/routes/completions.ts index 534921fb8..d25d7a5d8 100644 --- a/backend/api/routes/completions.ts +++ b/backend/api/routes/completions.ts @@ -11,7 +11,6 @@ import { } from "@prisma/client" import { generateUserCourseProgress } from "../../bin/kafkaConsumer/common/userCourseProgress/generateUserCourseProgress" -import { mapCompletionsWithCourseInstanceId } from "../../util/db-functions" import { err } from "../../util/result" import { ApiContext, Controller } from "../types" @@ -55,9 +54,9 @@ export class CompletionController extends Controller { } const sql = knex("completion") - .select([ + .select([ "completion.*", - knex.raw(`'${course.id}' as course_instance_id`) + knex.raw(`'${course.id}' as passed_course_instance_id`), ]) .distinctOn("completion.user_id", "completion.course_id") .fullOuterJoin( @@ -148,7 +147,9 @@ export class CompletionController extends Controller { // TODO/FIXME: note - this now happily ignores completion_language and just gets the first one // - as it's now only used in BAI, shouldn't be a problem? const tiers = ( - await knex("open_university_registration_link") + await knex( + "open_university_registration_link", + ) .select("tiers") .where("course_id", course.id) )?.[0].tiers @@ -253,7 +254,7 @@ export class CompletionController extends Controller { } private getCompletion = async (course: Course, user: User) => { - return mapCompletionsWithCourseInstanceId( + return ( await this.ctx.prisma.user .findUnique({ where: { @@ -266,8 +267,7 @@ export class CompletionController extends Controller { }, orderBy: { created_at: "asc" }, take: 1, - }), - course.id, + }) )?.[0] } diff --git a/backend/api/routes/progress.ts b/backend/api/routes/progress.ts index dad77e091..8b8cd31f7 100644 --- a/backend/api/routes/progress.ts +++ b/backend/api/routes/progress.ts @@ -1,13 +1,12 @@ import { Request, Response } from "express" import { chunk, groupBy, omit } from "lodash" -import { Completion, ExerciseCompletion, User, UserCourseProgress } from "@prisma/client" +import { Completion, User, UserCourseProgress } from "@prisma/client" import { generateUserCourseProgress } from "../../bin/kafkaConsumer/common/userCourseProgress/generateUserCourseProgress" import { BAIParentCourse, BAItiers } from "../../config/courseConfig" import { notEmpty } from "../../util/notEmpty" import { ApiContext, Controller } from "../types" -import { mapCompletionsWithCourseInstanceId } from "../../util/db-functions" interface ExerciseCompletionResult { user_id: string @@ -66,7 +65,7 @@ export class ProgressController extends Controller { "section", "max_points", "completed", - "custom_id as quizzes_id" + "custom_id as quizzes_id", ) .distinctOn("ec.exercise_id") .join("exercise as e", { "ec.exercise_id": "e.id" }) @@ -153,16 +152,11 @@ export class ProgressController extends Controller { const exercise_completions = await exerciseCompletionQuery - const completions = mapCompletionsWithCourseInstanceId( - await knex(knex.ref("completion").as("c")) - .select( - "*", - ) + const completions = await knex(knex.ref("completion").as("c")) + .select("*") .where("course_id", course.completions_handled_by_id ?? course.id) .andWhere("user_id", user.id) - .orderBy("created_at", "asc"), - course.id - ) + .orderBy("created_at", "asc") const exercise_completions_map = (exercise_completions ?? []).reduce( (acc, curr) => ({ @@ -234,10 +228,7 @@ export class ProgressController extends Controller { } const data = await knex("user_course_progress") - .select( - "course_id", - "extra", - ) + .select("course_id", "extra") .where("user_course_progress.course_id", id) .andWhere("user_course_progress.user_id", user.id) .orderBy("created_at", "asc") diff --git a/backend/api/routes/storedData.ts b/backend/api/routes/storedData.ts index 9483a7956..692866e64 100644 --- a/backend/api/routes/storedData.ts +++ b/backend/api/routes/storedData.ts @@ -1,7 +1,6 @@ import { Request, Response } from "express-serve-static-core" import { omit } from "lodash" -import { mapCompletionsWithCourseInstanceId } from "../../util/db-functions" import { requireCourseOwnership } from "../../util/server-functions" import { ApiContext, Controller } from "../types" @@ -121,10 +120,7 @@ export class StoredDataController extends Controller { const mappedStoredData = storedData.map((data) => ({ user: omit(data.user, "completions"), - completions: mapCompletionsWithCourseInstanceId( - data.user?.completions, - course.id, - ), + completions: data.user?.completions, storedData: omit(data, "user"), })) diff --git a/backend/bin/kafkaConsumer/common/userFunctions.ts b/backend/bin/kafkaConsumer/common/userFunctions.ts index b488476ac..cb8078c71 100644 --- a/backend/bin/kafkaConsumer/common/userFunctions.ts +++ b/backend/bin/kafkaConsumer/common/userFunctions.ts @@ -103,10 +103,14 @@ export const getExerciseCompletionsForCourses = async ({ }: GetExerciseCompletionsForCoursesArgs) => { // picks only one exercise completion per exercise/user: // the one with the latest timestamp and latest updated_at - const exercise_completions = await knex( - "exercise_completion as ec", - ) - .select("course_id", "custom_id", "max_points", "exercise_id", "n_points") + const exercise_completions = await knex("exercise_completion as ec") + .select( + "course_id", + "custom_id", + "max_points", + "exercise_id", + "n_points", + ) .distinctOn("ec.exercise_id") .join("exercise as e", { "ec.exercise_id": "e.id" }) .whereIn("e.course_id", courseIds) @@ -170,9 +174,7 @@ export const pruneDuplicateExerciseCompletions = async ({ .returning("id")*/ // we probably can just delete all even if they have required actions - const deleted = await knex( - "exercise_completion", - ) + const deleted = await knex("exercise_completion") .whereIn( "id", knex @@ -207,11 +209,12 @@ export const pruneDuplicateExerciseCompletions = async ({ export const pruneOrphanedExerciseCompletionRequiredActions = async ({ context: { knex }, }: WithBaseContext) => { - const deleted = - await knex("exercise_completion_required_actions") - .whereNull("exercise_completion_id") - .delete() - .returning("id") + const deleted = await knex( + "exercise_completion_required_actions", + ) + .whereNull("exercise_completion_id") + .delete() + .returning("id") return deleted } @@ -356,6 +359,7 @@ export const createCompletion = async ({ const newCompletion = await prisma.completion.create({ data: { course: { connect: { id: handlerCourse.id } }, + course_instance: { connect: { id: course.id } }, email: user.email, user: { connect: { id: user.id } }, user_upstream_id: user.upstream_id, diff --git a/backend/bin/sendAiStatistics.ts b/backend/bin/sendAiStatistics.ts index 0adfe7437..3cac44ba7 100644 --- a/backend/bin/sendAiStatistics.ts +++ b/backend/bin/sendAiStatistics.ts @@ -1,4 +1,5 @@ import { Course } from "@prisma/client" + import { AI_SLACK_URL } from "../config" import { languageInfo, LanguageInfo } from "../config/languageConfig" import prisma from "../prisma" diff --git a/backend/bin/updateBAICompletionTiers.ts b/backend/bin/updateBAICompletionTiers.ts index 0469ca951..6e0b09813 100644 --- a/backend/bin/updateBAICompletionTiers.ts +++ b/backend/bin/updateBAICompletionTiers.ts @@ -21,9 +21,7 @@ const updateBAICompletionTiers = async () => { logger.info("Getting completions") - const userIdsWithoutTiers = await knex( - "completion", - ) + const userIdsWithoutTiers = await knex("completion") .select("user_id") .distinctOn("user_id", "course_id") .where("course_id", PARENT_COURSE_ID) diff --git a/backend/graphql/Completion/model.ts b/backend/graphql/Completion/model.ts index 0174ecbc5..03cb78411 100644 --- a/backend/graphql/Completion/model.ts +++ b/backend/graphql/Completion/model.ts @@ -30,20 +30,25 @@ export const Completion = objectType({ t.model.completion_date() t.model.tier() t.model.completion_registration_attempt_date() + t.model.course_instance_id() - t.id("course_instance_id") + t.id("passed_course_instance_id") t.field("course_instance", { type: "Course", - // @ts-ignore: course_instance_id exists even if TS says it doesn't - resolve: async ({ course_instance_id }, _, ctx) => { + resolve: async ( + // @ts-ignore: passed_course_instance_id exists even if TS says it doesn't + { course_instance_id, passed_course_instance_id }, + _, + ctx, + ) => { if (!course_instance_id) { return null } return ctx.prisma.course.findUnique({ - where: { id: course_instance_id }, + where: { id: course_instance_id ?? passed_course_instance_id }, }) - } + }, }) // we're not querying completion course languages for now, and this was buggy /* t.field("course", { @@ -165,14 +170,25 @@ export const Completion = objectType({ t.field("certificate_availability", { type: "CertificateAvailability", - // @ts-ignore: course_instance_id exists even if TS says it doesn't - resolve: async ({ course_id, course_instance_id, user_upstream_id }, _, ctx) => { + resolve: async ( + { + course_id, + course_instance_id, + // @ts-ignore: passed_course_instance_id exists even if TS says it doesn't + passed_course_instance_id, + user_upstream_id, + }, + _, + ctx, + ) => { if (!course_id) { return null } const course = await ctx.prisma.course.findUnique({ - where: { id: course_instance_id ?? course_id }, + where: { + id: course_instance_id ?? passed_course_instance_id ?? course_id, + }, }) if (!course) { diff --git a/backend/graphql/Completion/mutations.ts b/backend/graphql/Completion/mutations.ts index 7777a9787..db17657df 100644 --- a/backend/graphql/Completion/mutations.ts +++ b/backend/graphql/Completion/mutations.ts @@ -29,6 +29,7 @@ export const CompletionMutations = extendType({ student_number: stringArg(), user: nonNull(idArg()), course: nonNull(idArg()), + course_instance: idArg(), completion_language: stringArg(), tier: nullable(intArg()), }, @@ -40,6 +41,7 @@ export const CompletionMutations = extendType({ student_number, user, course, + course_instance, completion_language, tier, } = args @@ -47,6 +49,9 @@ export const CompletionMutations = extendType({ return ctx.prisma.completion.create({ data: { course: { connect: { id: course } }, + ...(course_instance && { + course_instance: { connect: { id: course_instance } }, + }), user: { connect: { id: user } }, email: email ?? "", student_number, @@ -98,6 +103,7 @@ export const CompletionMutations = extendType({ const newCompletions: Completion[] = completions.map((o) => { const databaseUser = databaseUsersByUpstreamId[o.user_id][0] + return { id: uuidv4(), created_at: new Date(), @@ -108,6 +114,7 @@ export const CompletionMutations = extendType({ databaseUser.real_student_number || databaseUser.student_number, completion_language: null, course_id: course.completions_handled_by_id ?? course_id, + course_instance_id: course_id, user_id: databaseUser.id, grade: o.grade ?? null, completion_date: o.completion_date, diff --git a/backend/graphql/Course/model.ts b/backend/graphql/Course/model.ts index fa78cfbd4..6055fc977 100644 --- a/backend/graphql/Course/model.ts +++ b/backend/graphql/Course/model.ts @@ -82,22 +82,22 @@ export const Course = objectType({ // we need? return mapCompletionsWithCourseInstanceId( await ctx.prisma.course - .findUnique({ - where: { - id: parent.completions_handled_by_id ?? parent.id, - }, - }) - .completions({ - where: { - user: { - id: user_id ?? undefined, - upstream_id: user_upstream_id ?? undefined, + .findUnique({ + where: { + id: parent.completions_handled_by_id ?? parent.id, + }, + }) + .completions({ + where: { + user: { + id: user_id ?? undefined, + upstream_id: user_upstream_id ?? undefined, + }, }, - }, - distinct: ["user_id", "course_id"], - orderBy: { updated_at: "desc" }, - }), - parent.id + distinct: ["user_id", "course_id"], + orderBy: { updated_at: "desc" }, + }), + parent.id, ) }, }) diff --git a/backend/graphql/User/model.ts b/backend/graphql/User/model.ts index 0f92583d3..487fa6443 100644 --- a/backend/graphql/User/model.ts +++ b/backend/graphql/User/model.ts @@ -3,7 +3,10 @@ import { booleanArg, idArg, nullable, objectType, stringArg } from "nexus" import { Course, Prisma } from "@prisma/client" -import { getCourseOrAlias, mapCompletionsWithCourseInstanceId } from "../../util/db-functions" +import { + getCourseOrAlias, + mapCompletionsWithCourseInstanceId, +} from "../../util/db-functions" import { getCourseOrCompletionHandlerCourse } from "../../util/graphql-functions" import { notEmpty } from "../../util/notEmpty" @@ -74,10 +77,7 @@ export const User = objectType({ orderBy: { created_at: "asc" }, }) - return mapCompletionsWithCourseInstanceId( - completions, - course_id, - ) + return mapCompletionsWithCourseInstanceId(completions, course_id) }, }) diff --git a/backend/migrations/20220928144940_alter-table-completion-add-course-instance.ts b/backend/migrations/20220928144940_alter-table-completion-add-course-instance.ts new file mode 100644 index 000000000..c3d85c2b6 --- /dev/null +++ b/backend/migrations/20220928144940_alter-table-completion-add-course-instance.ts @@ -0,0 +1,30 @@ +import { Knex } from "knex" + +export async function up(knex: Knex): Promise { + await knex.raw(` + ALTER TABLE completion + ADD COLUMN course_instance_id uuid; + `) + await knex.raw(` + ALTER TABLE completion + ADD CONSTRAINT completion_course_instance_fkey FOREIGN KEY (course_instance_id) REFERENCES course(id) ON DELETE SET NULL; + `) + await knex.raw(` + CREATE INDEX IF NOT EXISTS "completion.course_instance_id" + ON completion USING btree (course_instance_id); + `) +} + +export async function down(knex: Knex): Promise { + await knex.raw(` + DROP INDEX IF EXISTS "completion.course_instance_id"; + `) + await knex.raw(` + ALTER TABLE completion + DROP CONSTRAINT completion_course_instance_fkey; + `) + await knex.raw(` + ALTER TABLE completion + DROP COLUMN course_instance_id; + `) +} diff --git a/backend/util/db-functions.ts b/backend/util/db-functions.ts index ebe877d5e..b16be89e6 100644 --- a/backend/util/db-functions.ts +++ b/backend/util/db-functions.ts @@ -22,47 +22,50 @@ export function mapCompletionsWithCourseInstanceId< Payload extends InferCompletionPayloadType, >( completions: Array, - course_instance_id: null | undefined, + passed_course_instance_id: null | undefined, ): typeof completions export function mapCompletionsWithCourseInstanceId< T, Payload extends InferCompletionPayloadType, >( completions: null | undefined, - course_instance_id: Optional, + passed_course_instance_id: Optional, ): typeof completions export function mapCompletionsWithCourseInstanceId< T, Payload extends InferCompletionPayloadType, >( completions: Array, - course_instance_id: string, -): Array + passed_course_instance_id: string, +): Array export function mapCompletionsWithCourseInstanceId< T, Payload extends InferCompletionPayloadType, >( completions: Optional>, - course_instance_id: string, -): typeof completions | Array + passed_course_instance_id: string, +): typeof completions | Array export function mapCompletionsWithCourseInstanceId< T, Payload extends InferCompletionPayloadType, >( completions: Array, - course_instance_id: Optional, -): Array | Array + passed_course_instance_id: Optional, +): Array | Array export function mapCompletionsWithCourseInstanceId< T, Payload extends InferCompletionPayloadType, ->(completions: Optional>, course_instance_id: Optional) { - if (!notEmpty(completions) || !notEmpty(course_instance_id)) { +>( + completions: Optional>, + passed_course_instance_id: Optional, +) { + if (!notEmpty(completions) || !notEmpty(passed_course_instance_id)) { return completions } const res = completions?.map((completion) => ({ ...completion, - course_instance_id, + passed_course_instance_id, })) return res diff --git a/backend/util/server-functions.ts b/backend/util/server-functions.ts index b83534143..0b8afa49a 100644 --- a/backend/util/server-functions.ts +++ b/backend/util/server-functions.ts @@ -104,9 +104,7 @@ export function getUser({ knex, logger }: BaseContext) { } let user = ( - await knex("user") - .select("*") - .where("upstream_id", details.id) + await knex("user").select("*").where("upstream_id", details.id) )?.[0] if (!user) { @@ -126,9 +124,7 @@ export function getUser({ knex, logger }: BaseContext) { } catch { // race condition or something user = ( - await knex("user") - .select("*") - .where("upstream_id", details.id) + await knex("user").select("*").where("upstream_id", details.id) )?.[0] if (!user) { diff --git a/frontend/components/CertificateButton.tsx b/frontend/components/CertificateButton.tsx index 2d7517915..1da5cec9b 100644 --- a/frontend/components/CertificateButton.tsx +++ b/frontend/components/CertificateButton.tsx @@ -26,7 +26,7 @@ import { CourseCoreFieldsFragment, } from "/graphql/generated" -const CERTIFICATES_URL = "http://localhost:9200" //"https://certificates.mooc.fi" +const CERTIFICATES_URL = "https://certificates.mooc.fi" const StyledButton = styled(Button)` margin: auto; diff --git a/frontend/lib/certificates.ts b/frontend/lib/certificates.ts index 1990ce0ce..79e681c19 100644 --- a/frontend/lib/certificates.ts +++ b/frontend/lib/certificates.ts @@ -2,7 +2,7 @@ import axios from "axios" import { getAccessToken } from "./authentication" -const SERVICE_URL = "http://localhost:9200" //"https://certificates.mooc.fi" +const SERVICE_URL = "https://certificates.mooc.fi" export const checkCertificate = async (courseSlug: string) => { const accessToken = getAccessToken(undefined)