Fix membership calculation

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

@@ -5,11 +5,11 @@ import type { NTNUGroup } from "../feide/feide-groups-repository"
 import { getLogger } from "@dotkomonline/logger"
 
 export interface MembershipService {
-  findApproximateMasterStartYear(courses: NTNUGroup[]): number
-  findApproximateBachelorStartYear(courses: NTNUGroup[]): number
+  findMasterStartYearDelta(courses: NTNUGroup[]): number
+  findBachelorStartYearDelta(courses: NTNUGroup[]): number
 }
 
-const BACHELOR_STUDY_PLAN_COURSES = [
+const BACHELOR_STUDY_PLAN = [
   {
     semester: 0,
     courses: ["IT2805", "MA0001", "TDT4109"],
@@ -28,7 +28,7 @@ const BACHELOR_STUDY_PLAN_COURSES = [
   },
   {
     semester: 4,
-    // NOTE: Semester 1 in Year 3 are all elective courses, so we do not use any of them to determine which year somebody
+    // Semester 1 in year 3 are all elective courses, so we do not use any of them to determine which year somebody
     // started studying
     courses: [],
   },
@@ -57,13 +57,13 @@ const MASTER_STUDY_PLAN = [
   },
 ] as const
 
-type StudyPlanCourseSet = typeof BACHELOR_STUDY_PLAN_COURSES | typeof MASTER_STUDY_PLAN
+type StudyPlanCourseSet = typeof BACHELOR_STUDY_PLAN | typeof MASTER_STUDY_PLAN
 
 export function getMembershipService(): MembershipService {
   const logger = getLogger("membership-service")
   // The study plan course set makes some assumptions for the approximation code to work as expected. In order to make
   // it easier for future dotkom developers, the invariants are checked here.
-  validateStudyPlanCourseSet(BACHELOR_STUDY_PLAN_COURSES)
+  validateStudyPlanCourseSet(BACHELOR_STUDY_PLAN)
   validateStudyPlanCourseSet(MASTER_STUDY_PLAN)
 
   function validateStudyPlanCourseSet(courseSet: StudyPlanCourseSet) {
@@ -143,7 +143,9 @@ export function getMembershipService(): MembershipService {
 
           // Take the mean distance for this semester
           const sum = previousSemesterDistances.reduce((acc, curr) => acc + curr, 0)
-          largestLocalSemester = Math.ceil(sum / previousSemesterDistances.length)
+          const currentSemesterEstimate = Math.ceil(sum / previousSemesterDistances.length)
+
+          largestLocalSemester = Math.max(largestLocalSemester, currentSemesterEstimate)
         }
 
         largestSemester = Math.max(largestSemester, largestLocalSemester)
@@ -167,15 +169,20 @@ export function getMembershipService(): MembershipService {
     }
 
     // Give the value back in years (two school semesters in a year).
-    return Math.floor(largestSemester / 2)
+    // 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 {
-    findApproximateMasterStartYear(courses) {
+    findMasterStartYearDelta(courses) {
       return findApproximateStartYear(courses, MASTER_STUDY_PLAN)
     },
 
-    findApproximateBachelorStartYear(courses) {
-      return findApproximateStartYear(courses, BACHELOR_STUDY_PLAN_COURSES)
+    findBachelorStartYearDelta(courses) {
+      return findApproximateStartYear(courses, BACHELOR_STUDY_PLAN)
     },
   }
 }

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

@@ -122,8 +122,8 @@ export function getUserService(
     // Master degree always takes precedence over bachelor every single time.
     const isMasterStudent = masterProgramme !== undefined
     const distanceFromStartInYears = isMasterStudent
-      ? membershipService.findApproximateMasterStartYear(courses)
-      : membershipService.findApproximateBachelorStartYear(courses)
+      ? 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
@@ -215,7 +215,7 @@ export function getUserService(
     },
 
     async register(handle, userId) {
-      // NOTE: The registrer function here has a few responsibilities because of our data strategy:
+      // NOTE: The register function here has a few responsibilities because of our data strategy:
       //
       // 1. The database is the source of truth, and is ALWAYS intended to be as such.
       // 2. Unfortunately, there was a period in time where Auth0 was the source of truth, most notably right after we

packages/types/src/user.ts

@@ -1,7 +1,7 @@
 import type { TZDate } from "@date-fns/tz"
 import { schemas } from "@dotkomonline/db/schemas"
 import { getCurrentUTC, slugify } from "@dotkomonline/utils"
-import { addYears, differenceInYears, isAfter, isBefore, setMonth, startOfMonth } from "date-fns"
+import { addYears, isAfter, isBefore, setMonth, startOfMonth } from "date-fns"
 import { z } from "zod"
 import { buildSearchFilter } from "./filters"
 
@@ -31,7 +31,7 @@ export type UserId = User["id"]
 export type UserProfileSlug = User["profileSlug"]
 
 export const NAME_REGEX = /^[\p{L}\p{M}\s'-]+$/u
-export const PHONE_REGEX = /^[0-9+-\s]*$/
+export const PHONE_REGEX = /^[0-9-+\s]*$/
 export const PROFILE_SLUG_REGEX = /^[a-z0-9-]+$/
 
 // These max and min values are arbitrary
@@ -93,25 +93,30 @@ export function findActiveMembership(user: User): Membership | null {
 }
 
 export function getMembershipGrade(membership: Membership): 1 | 2 | 3 | 4 | 5 | null {
-  // Take the difference, and add one because if `startYear == currentYear` they are in their first year
-  const delta = differenceInYears(getAcademicStart(getCurrentUTC()), getAcademicStart(membership.start)) + 1
+  const now = getCurrentUTC()
+
+  // Make sure we clamp the value to a minimum of 1
+  const delta = Math.max(1, getAcademicYearDelta(membership.start, now))
 
   switch (membership.type) {
     case "KNIGHT":
     case "PHD_STUDENT":
       return 5
+
     case "SOCIAL_MEMBER":
       return 1
+
     case "BACHELOR_STUDENT": {
       // Bachelor students are clamped at 1-3, regardless of how many years they used to take the degree.
-      return Math.max(1, Math.min(3, delta)) as 1 | 2 | 3
+      return Math.min(3, delta) as 1 | 2 | 3
     }
+
     case "MASTER_STUDENT": {
-      // Master students must be clamped at 4-5 because they can only be in their first or second year, but are always
-      // considered to have a bachelor's degree from beforehand.
-      const yearsGivenBachelors = delta + 3
-      return Math.max(4, Math.min(5, yearsGivenBachelors)) as 4 | 5
+      // Master students are clamped at 4-5, and are always considered to have a bachelor's degree from beforehand.
+      const yearsWithBachelors = delta + 3
+      return Math.min(5, yearsWithBachelors) as 4 | 5
     }
+
     case "OTHER":
       return null
   }
@@ -161,3 +166,25 @@ export function getNextAcademicStart(): TZDate {
   const isBeforeAugust = isBefore(now, firstAugust)
   return isBeforeAugust ? firstAugust : addYears(firstAugust, 1)
 }
+
+/**
+ * Calculates how many academic years have passed since the start date.
+ * If start is "last August" (current academic year), returns 1.
+ * If start was the August before that, returns 2.
+ */
+function getAcademicYearDelta(startDate: Date | TZDate, now: Date | TZDate = getCurrentUTC()): number {
+  const currentYear = now.getFullYear()
+  const currentMonth = now.getMonth() // 0-indexed (Jan=0, Aug=7)
+
+  // If we are in Jan-July (0-6), the academic year started in the PREVIOUS calendar year
+  // If we are in Aug-Dec (7-11), the academic year started in THIS calendar year
+  const academicYearCurrent = currentMonth >= 7 ? currentYear : currentYear - 1
+
+  // We do the same normalization for the membership start date
+  // (Handling cases where a member might join in Jan/Feb)
+  const startYear = startDate.getFullYear()
+  const startMonth = startDate.getMonth()
+  const academicYearStart = startMonth >= 7 ? startYear : startYear - 1
+
+  return academicYearCurrent - academicYearStart + 1
+}