wip: update membership system

Author: brage-andreas

apps/dashboard/src/app/(internal)/brukere/components/membership-form.tsx

@@ -8,13 +8,16 @@ import {
   MembershipWriteSchema,
   getMembershipTypeName,
   getSpecializationName,
+  getNextSemesterStart,
+  getCurrentSemesterStart,
 } from "@dotkomonline/types"
-import { getCurrentUTC } from "@dotkomonline/utils"
-import { addYears, isBefore } from "date-fns"
+import { isBefore } from "date-fns"
 import type { z } from "zod"
+import { createNumberInput } from "@/components/forms/NumberInput"
+import { Code, Stack, Text } from "@mantine/core"
 
 export const MembershipWriteFormSchema = MembershipWriteSchema.superRefine((data, ctx) => {
-  if (isBefore(data.end, data.start)) {
+  if (data.end && isBefore(data.end, data.start)) {
     ctx.addIssue({
       code: "custom",
       message: "Sluttdato må være etter startdato",
@@ -26,13 +29,14 @@ export const MembershipWriteFormSchema = MembershipWriteSchema.superRefine((data
 type MembershipWriteFormSchema = z.infer<typeof MembershipWriteFormSchema>
 
 const DEFAULT_VALUES: Partial<MembershipWriteFormSchema> = {
-  start: getCurrentUTC(),
-  end: addYears(getCurrentUTC(), 1),
+  start: getCurrentSemesterStart(),
+  end: getNextSemesterStart(),
   specialization: null,
+  semester: 0,
 }
 
 interface UseMembershipWriteFormProps {
-  onSubmit(data: z.infer<typeof MembershipWriteFormSchema>): void
+  onSubmit(data: MembershipWriteFormSchema): void
   defaultValues?: Partial<MembershipWrite>
   label?: string
 }
@@ -59,6 +63,7 @@ export const useMembershipWriteForm = ({
       }),
       specialization: createSelectInput({
         label: "Spesialisering",
+        description: "Masterspesialisering",
         required: false,
         clearable: true,
         placeholder: "Velg spesialisering",
@@ -68,6 +73,7 @@ export const useMembershipWriteForm = ({
             value: specialization,
             label: getSpecializationName(specialization) ?? specialization,
           })),
+        disabled: false,
       }),
       start: createDateTimeInput({
         label: "Startdato",
@@ -77,6 +83,40 @@ export const useMembershipWriteForm = ({
         label: "Sluttdato",
         required: true,
       }),
+      semester: createNumberInput({
+        label: "Semester",
+        description: (
+          <Stack gap="xs">
+            <Text size="xs" c="dimmed">
+              Hvilket semester medlemskapet innebærer. 0-indeksert.
+            </Text>
+            <Stack gap="0.25rem">
+              <Text size="xs" c="dimmed">
+                <Code>0</Code> → 1. semester (1. årstrinn)
+              </Text>
+              <Text size="xs" c="dimmed">
+                <Code>1</Code> → 2. semester (1. årstrinn)
+              </Text>
+              <Text size="xs" c="dimmed">
+                <Code>2</Code> → 3. semester (2. årstrinn)
+              </Text>
+              <Text size="xs" c="dimmed">
+                ...
+              </Text>
+              <Text size="xs" c="dimmed">
+                <Code>8</Code> → 9. semester (5. årstrinn)
+              </Text>
+              <Text size="xs" c="dimmed">
+                <Code>9</Code> → 10. semester (5. årstrinn)
+              </Text>
+            </Stack>
+          </Stack>
+        ),
+        required: false,
+        min: 0,
+        max: 9,
+        allowDecimal: false,
+      }),
     },
   })
 }

apps/rpc/src/modules/user/membership-service.ts

@@ -3,10 +3,14 @@ import { differenceInMonths, subYears } from "date-fns"
 import invariant from "tiny-invariant"
 import type { NTNUGroup } from "../feide/feide-groups-repository"
 import { getLogger } from "@dotkomonline/logger"
+import { getCurrentUTC } from "@dotkomonline/utils"
+import type { TZDate } from "@date-fns/tz"
 
 export interface MembershipService {
-  findMasterStartYearDelta(courses: NTNUGroup[]): number
-  findBachelorStartYearDelta(courses: NTNUGroup[]): number
+  findEstimatedStudyStart(
+    study: "BACHELOR" | "MASTER",
+    courses: NTNUGroup[]
+  ): { start: TZDate; semester: number; grade: number }
 }
 
 const BACHELOR_STUDY_PLAN = [
@@ -84,9 +88,9 @@ export function getMembershipService(): MembershipService {
   }
 
   /**
-   * Find the approximate start year based on a student's courses against a hard-coded set of courses.
+   * Find the approximate semester based on a student's courses against a hard-coded set of courses.
    */
-  function findApproximateStartYear(studentCourses: NTNUGroup[], courseSet: StudyPlanCourseSet): number {
+  function findEstimatedSemester(courseSet: StudyPlanCourseSet, studentCourses: NTNUGroup[]): number {
     logger.info("Searching for applicable membership based on courses %o and study plan %o", studentCourses, courseSet)
     let largestSemester = 0
     for (let i = 0; i < courseSet.length; i++) {
@@ -168,21 +172,37 @@ export function getMembershipService(): MembershipService {
       }
     }
 
-    // Give the value back in years (two school semesters in a year).
-    // We use Math#round because it will give us the correct year delta:
-    //   Year 1 fall (value 0)  : round(0 / 2) = 0 (Start year = current year)
-    //   Year 1 spring (value 1): round(1 / 2) = 1 (Start year = current - 1, since spring is in the next calendar year)
-    //   Year 2 fall (value 2)  : round(2 / 2) = 1 (Start year = current - 1)
-    //   Year 2 spring (value 3): round(3 / 2) = 2 (Start year = current - 2)
-    return Math.round(largestSemester / 2)
+    return largestSemester
   }
-  return {
-    findMasterStartYearDelta(courses) {
-      return findApproximateStartYear(courses, MASTER_STUDY_PLAN)
-    },
 
-    findBachelorStartYearDelta(courses) {
-      return findApproximateStartYear(courses, BACHELOR_STUDY_PLAN)
+  return {
+    findEstimatedStudyStart(study, courses) {
+      const studyPlan = study === "MASTER" ? MASTER_STUDY_PLAN : BACHELOR_STUDY_PLAN
+      const semester = findEstimatedSemester(studyPlan, courses)
+
+      // We use Math#round because it will give us the correct year delta:
+      //   Year 1 autumn (value 0)  : round(0 / 2) = 0 (Start year = current year)
+      //   Year 1 spring (value 1): round(1 / 2) = 1 (Start year = current - 1, since spring is in the next calendar year)
+      //   Year 2 autumn (value 2)  : round(2 / 2) = 1 (Start year = current - 1)
+      //   Year 2 spring (value 3): round(3 / 2) = 2 (Start year = current - 2)
+      //   ...
+      const yearOffset = Math.round(semester / 2)
+      const start = subYears(getAcademicStart(getCurrentUTC()), yearOffset)
+
+      // A school year consists of two semesters (Autumn and Spring). So this formula will give us the year:
+      //   Year 1 autumn (value 0): floor(0 / 2) + 1 = 1 (Year 1)
+      //   Year 1 spring (value 1): floor(1 / 2) + 1 = 1 (Year 1)
+      //   Year 2 autumn (value 2): floor(2 / 2) + 1 = 2 (Year 2)
+      //   Year 2 spring (value 3): floor(3 / 2) + 1 = 2 (Year 2)
+      //   Year 3 autumn (value 4): floor(4 / 2) + 1 = 3 (Year 3)
+      //   ...
+      const grade = Math.floor(semester / 2) + 1
+
+      return {
+        semester,
+        start,
+        grade,
+      }
     },
   }
 }

apps/rpc/src/modules/user/user-service.ts

@@ -4,10 +4,10 @@ import type { PresignedPost } from "@aws-sdk/s3-presigned-post"
 import type { DBHandle } from "@dotkomonline/db"
 import { getLogger } from "@dotkomonline/logger"
 import {
+  isMembershipActive,
   type Membership,
   type MembershipId,
   type MembershipSpecialization,
-  MembershipSpecializationSchema,
   type MembershipWrite,
   type User,
   type UserFilterQuery,
@@ -16,13 +16,13 @@ import {
   type UserWrite,
   UserWriteSchema,
   findActiveMembership,
-  getAcademicStart,
-  getNextAcademicStart,
+  getNextSemesterStart,
+  getCurrentSemesterStart,
 } from "@dotkomonline/types"
-import { createS3PresignedPost, getCurrentUTC, slugify } from "@dotkomonline/utils"
+import { createS3PresignedPost, slugify } from "@dotkomonline/utils"
 import { trace } from "@opentelemetry/api"
 import type { ManagementClient } from "auth0"
-import { isSameDay, subYears } from "date-fns"
+import { isSameDay } from "date-fns"
 import { AlreadyExistsError, IllegalStateError, InvalidArgumentError, NotFoundError } from "../../error"
 import type { Pageable } from "../../query"
 import type { FeideGroupsRepository, NTNUGroup } from "../feide/feide-groups-repository"
@@ -119,22 +119,36 @@ export function getUserService(
       return null
     }
 
-    // Master degree always takes precedence over bachelor every single time.
-    const isMasterStudent = masterProgramme !== undefined
-    const distanceFromStartInYears = isMasterStudent
-      ? membershipService.findMasterStartYearDelta(courses)
-      : membershipService.findBachelorStartYearDelta(courses)
-    const estimatedStudyStart = subYears(getAcademicStart(getCurrentUTC()), distanceFromStartInYears)
-
-    // NOTE: We grant memberships for at most one year at a time. If you are granted membership after new-years, you
-    // will only keep the membership until the start of the next school year.
-    const endDate = getNextAcademicStart()
-
-    // Master's programme takes precedence over bachelor's programme.
-    if (masterProgramme !== undefined) {
-      const code = MembershipSpecializationSchema.catch("UNKNOWN").parse(
-        getSpecializationFromCode(studySpecializations?.[0].code)
-      )
+    // Master degree always takes precedence over bachelor.
+    const study = masterProgramme !== undefined ? "MASTER" : "BACHELOR"
+    const estimatedStudyStart = membershipService.findEstimatedStudyStart(study, courses)
+
+    // We grant memberships for one semester at a time. This has some trade-offs, and natural alternative end dates are:
+    //   1. One semester (what we use)
+    //   2. School year (one or two semesters--until next Autumn semester, earlier referred to as the next
+    //      "academic start")
+    //   3. Entire degree (three years for Bachelor's and two years for Master's)
+    //
+    // The longer each membership lasts, the fewer times you need to calculate the grade and other information. This
+    // reduces the number of opportunities for wrong calculations, but also make the system less flexible. Sometimes
+    // students take a Bachelor's degree over a span of two years. Other times they change study. We choose the tradeoff
+    // where you have this flexibility, even though it costs us an increase in manual adjustments. You most often need
+    // to manually adjust someone's membership if someone:
+    //   a) Failed at least one of their courses a semester.
+    //   b) Has a very weird study plan due to previous studies.
+    //   c) Have been an exchange student and therefore not have done all their courses in the "correct" order
+    //      (according to our system anyway), where they have a "hole" in their course list exposed to us.
+    //
+    // We have decided it is best to manually adjust the membership in any nonlinear case, versus trying to correct for
+    // fairly common cases like exchange students automatically. We never want this heuristic to overestimate someone's
+    // grade. This is because we deem it generally less beneficial to be in a lower grade (because in practice the older
+    // students usually have priority for attendance), increasing their chances of reaching out to correct the error.
+    const endDate = getNextSemesterStart()
+    const startDate = getCurrentSemesterStart()
+
+    if (study === "MASTER") {
+      const code = getSpecializationFromCode(studySpecializations?.[0].code)
+
       // If we have a new code that we have not seen, or for some other reason the code catches and returns UNKNOWN, we
       // emit a trace for it.
       if (code === "UNKNOWN") {
@@ -148,18 +162,20 @@ export function getUserService(
       logger.info("Estimated end date for the master's programme to be %s", endDate.toUTCString())
       return {
         type: "MASTER_STUDENT",
-        start: estimatedStudyStart,
+        start: startDate,
         end: endDate,
         specialization: code,
+        semester: estimatedStudyStart.semester,
       }
     }
 
     logger.info("Estimated end date for the bachelor's programme to be %s", endDate.toUTCString())
     return {
       type: "BACHELOR_STUDENT",
-      start: estimatedStudyStart,
+      start: startDate,
       end: endDate,
       specialization: null,
+      semester: estimatedStudyStart.semester,
     }
   }
 
@@ -237,7 +253,9 @@ export function getUserService(
       if (existingUser !== null) {
         const membership = findActiveMembership(existingUser)
 
-        if (membership !== null) {
+        // If the best active membership is KNIGHT, we attempt to discover a new membership for the user, in case they
+        // can find a "better" membership this way.
+        if (membership !== null && membership.type !== "KNIGHT") {
           return existingUser
         }
 
@@ -351,11 +369,21 @@ export function getUserService(
 
                 // We can only replace memberships if there is a new applicable one for the user
                 if (
-                  shouldReplaceMembership(user.memberships, activeMembership, applicableMembership) &&
-                  applicableMembership !== null
+                  applicableMembership !== null &&
+                  shouldReplaceMembership(user.memberships, activeMembership, applicableMembership)
                 ) {
-                  logger.info("Discovered applicable membership for user %s: %o", user.id, applicableMembership)
-                  await userRepository.createMembership(handle, user.id, applicableMembership)
+                  // We make sure the membership is active before creating it. If it is not active, something has gone
+                  // wrong in our logic.
+                  if (isMembershipActive(applicableMembership)) {
+                    logger.info("Discovered applicable membership for user %s: %o", user.id, applicableMembership)
+                    await userRepository.createMembership(handle, user.id, applicableMembership)
+                  } else {
+                    logger.warn(
+                      "Discovered and discarded invalid membership for user %s: %o",
+                      user.id,
+                      applicableMembership
+                    )
+                  }
                 }
               }
             } finally {
@@ -412,6 +440,15 @@ export function getUserService(
   }
 }
 
+/**
+ * Determine if we should replace a previous membership with a new one.
+ *
+ * This is true if:
+ * - There is no previous membership.
+ * - The membership is not a duplicate of an existing membership (active or inactive).
+ * - The previous membership is not active, and the next one is active.
+ * - The next membership has a semester greater than or equal to the previous one.
+ */
 function shouldReplaceMembership(
   allMemberships: Membership[],
   previous: Membership | null,
@@ -430,16 +467,20 @@ function shouldReplaceMembership(
     return true
   }
 
-  if (previous.type === "BACHELOR_STUDENT" && next.type === "MASTER_STUDENT") {
+  if (!isMembershipActive(previous) && isMembershipActive(next)) {
     return true
   }
 
-  return previous.type === "SOCIAL_MEMBER"
+  // Returns true if the next semester is greater than or equal to the previous semester
+  return (next.semester ?? -Infinity) - (previous.semester ?? -Infinity) >= 0
 }
 
 function areMembershipsEqual(a: Membership, b: MembershipWrite) {
+  const isSameStart = isSameDay(a.start, b.start)
+  const isSameEnd = (a.end === null && b.end === null) || (a.end !== null && b.end !== null && isSameDay(a.end, b.end))
+
   return (
-    isSameDay(a.start, b.start) && isSameDay(a.end, b.end) && a.specialization === b.specialization && a.type === b.type
+    isSameStart && isSameEnd && a.specialization === b.specialization && a.type === b.type && a.semester === b.semester
   )
 }
 

apps/web/next-env.d.ts

@@ -1,5 +1,6 @@
 /// <reference types="next" />
 /// <reference types="next/image-types/global" />
+/// <reference path="./.next/types/routes.d.ts" />
 
 // NOTE: This file should not be edited
 // see https://nextjs.org/docs/app/api-reference/config/typescript for more information.

packages/db/prisma/migrations/20260125135233_add_semester_to_membership/migration.sql

@@ -0,0 +1,18 @@
+/*
+  Warnings:
+
+  - The values [OTHER] on the enum `membership_type` will be removed. If these variants are still used in the database, this will fail.
+
+*/
+-- AlterEnum
+BEGIN;
+CREATE TYPE "membership_type_new" AS ENUM ('BACHELOR_STUDENT', 'MASTER_STUDENT', 'PHD_STUDENT', 'KNIGHT', 'SOCIAL_MEMBER');
+ALTER TABLE "membership" ALTER COLUMN "type" TYPE "membership_type_new" USING ("type"::text::"membership_type_new");
+ALTER TYPE "membership_type" RENAME TO "membership_type_old";
+ALTER TYPE "membership_type_new" RENAME TO "membership_type";
+DROP TYPE "public"."membership_type_old";
+COMMIT;
+
+-- AlterTable
+ALTER TABLE "membership" ADD COLUMN     "semester" INTEGER,
+ALTER COLUMN "end" DROP NOT NULL;