Fix swim test bugs

src/components/Modals/DeleteCourseModal.vue

@@ -9,9 +9,9 @@
     @modal-closed="closeCurrentModal"
     :rightButtonIsDisabled="false"
   >
-    <div v-if="isTestReq" class="text-width">
+    <div v-if="isTransferCredit" class="text-width">
       Are you sure you want to remove "{{ reqName }}" for this requirement? This will delete the
-      selected transfer credit from your schedule.
+      selected transfer credit.
       <br />
       Transfer credits can be re-added in your Profile.
     </div>
@@ -29,7 +29,7 @@ export default defineComponent({
   components: { TeleportModal },
   props: {
     reqName: { type: String, required: true },
-    isTestReq: { type: Boolean, required: true },
+    isTransferCredit: { type: Boolean, required: true },
   },
   emits: {
     'close-delete-course-modal': (value: boolean) => typeof value === 'boolean',

src/components/Modals/Onboarding/Onboarding.vue

@@ -108,7 +108,7 @@ import { PropType, defineComponent } from 'vue';
 import OnboardingBasic from '@/components/Modals/Onboarding/OnboardingBasic.vue';
 import OnboardingTransfer from '@/components/Modals/Onboarding/OnboardingTransfer.vue';
 import OnboardingReview from '@/components/Modals/Onboarding/OnboardingReview.vue';
-import { setOnboardingData, populateSemesters } from '@/global-firestore-data';
+import { setAppOnboardingData, populateSemesters } from '@/global-firestore-data';
 import { getMajorFullName, getMinorFullName, getGradFullName } from '@/utilities';
 import timeline1Text from '@/assets/images/timeline1text.svg';
 import timeline2Text from '@/assets/images/timeline2text.svg';
@@ -211,7 +211,7 @@ export default defineComponent({
   methods: {
     submitOnboarding() {
       this.clearTransferCreditIfGraduate();
-      setOnboardingData(this.name, this.onboarding);
+      setAppOnboardingData(this.name, this.onboarding);
       // indicates first time user onboarding
       if (!this.isEditingProfile) populateSemesters(this.onboarding);
       this.$emit('onboard');

src/components/Requirements/CompletedSubReqCourse.vue

@@ -1,7 +1,7 @@
 <template>
   <div class="completedsubreqcourse">
     <delete-course-modal
-      :isTestReq="isTransferCredit"
+      :isTransferCredit="isTransferCredit"
       :reqName="courseTaken.code"
       v-if="deleteModalVisible"
       @close-delete-course-modal="onDeleteCourseModalClose"
@@ -43,8 +43,7 @@ import ReqCourse from '@/components/Requirements/ReqCourse.vue';
 import SlotMenu from '@/components/Modals/SlotMenu.vue';
 import DeleteCourseModal from '@/components/Modals/DeleteCourseModal.vue';
 import store from '@/store';
-import { deleteCourseFromSemesters } from '@/global-firestore-data';
-import { onboardingDataCollection } from '@/firebase-frontend-config';
+import { deleteCourseFromSemesters, deleteTransferCredit } from '@/global-firestore-data';
 import { getCurrentSeason, getCurrentYear, clickOutside } from '@/utilities';
 
 const transferCreditColor = 'DA4A4A'; // Arbitrary color for transfer credit
@@ -102,14 +101,7 @@ export default defineComponent({
 
       if (isDelete) {
         if (this.isTransferCredit) {
-          const type = this.courseTaken.code.substr(0, 2);
-          const name = this.courseTaken.code.substr(3);
-
-          const onBoardingData = store.state.onboardingData;
-
-          onboardingDataCollection.doc(store.state.currentFirebaseUser.email).update({
-            exam: onBoardingData.exam.filter(e => !(e.type === type && e.subject === name)),
-          });
+          deleteTransferCredit(this.courseTaken.code);
         } else {
           const { uniqueId } = this.courseTaken;
           if (typeof uniqueId === 'number') deleteCourseFromSemesters(uniqueId, this.$gtag);

src/global-firestore-data/index.ts

@@ -12,7 +12,7 @@ export const cornellCourseRosterCourseToFirebaseSemesterCourseWithGlobalData = (
   cornellCourseRosterCourseToFirebaseSemesterCourse(course, store, incrementUniqueID);
 
 export { incrementUniqueID };
-export { default as setOnboardingData } from './onboarding-data';
+export { setAppOnboardingData, deleteTransferCredit } from './onboarding-data';
 export {
   editSemesters,
   editSemester,

src/global-firestore-data/onboarding-data.ts

@@ -1,8 +1,12 @@
+import { SWIM_TEST_CODE } from '@/requirements/data/constants';
 import { onboardingDataCollection } from '../firebase-frontend-config';
 import store from '../store';
 import setUsernameData from './username-data';
 
-const setOnboardingData = (name: FirestoreUserName, onboarding: AppOnboardingData): void => {
+export const setAppOnboardingData = (
+  name: FirestoreUserName,
+  onboarding: AppOnboardingData
+): void => {
   setUsernameData(name);
   onboardingDataCollection.doc(store.state.currentFirebaseUser.email).set({
     gradYear: onboarding.gradYear,
@@ -16,4 +20,25 @@ const setOnboardingData = (name: FirestoreUserName, onboarding: AppOnboardingDat
   });
 };
 
-export default setOnboardingData;
+const setTookSwim = (tookSwim: 'yes' | 'no'): void => {
+  onboardingDataCollection.doc(store.state.currentFirebaseUser.email).update({
+    tookSwim,
+  });
+};
+
+const setExams = (exam: FirestoreAPIBExam[]) => {
+  onboardingDataCollection.doc(store.state.currentFirebaseUser.email).update({
+    exam,
+  });
+};
+
+export const deleteTransferCredit = (code: string): void => {
+  if (code === SWIM_TEST_CODE) {
+    setTookSwim('no');
+    return;
+  }
+  const [type, subject] = code.split(/ (.*)/);
+  setExams(
+    store.state.onboardingData.exam.filter(e => !(e.type === type && e.subject === subject))
+  );
+};

src/requirement-types.d.ts

@@ -104,7 +104,7 @@ type DecoratedCollegeOrMajorRequirement = RequirementCommon &
 type CourseTaken = {
   /** The course ID from course roster, or our dummy id to denote special courses like FWS equiv. */
   readonly courseId: number;
-  /** Using the unique ID of firestore course for real course, -1 for swim test, and string for AP/IB. */
+  /** Using the unique ID of firestore course for real course, string for swim test and AP/IB. */
   readonly uniqueId: string | number;
   /**
    * Course code like 'CS 2112', 'AP CS'.

src/requirements/__test__/__snapshots__/requirement-data-id.test.ts.snap

@@ -47,6 +47,7 @@ Array [
   "College-IL-Labor History Electives",
   "College-IL-Second Year Core Requirements",
   "College-UNI-Physical Education",
+  "College-UNI-Swim Test",
   "Grad-MPA-Additional Foundation Coursework",
   "Grad-MPA-Colloquium Coursework",
   "Grad-MPA-Concentration Coursework",

src/requirements/data/constants.ts

@@ -4,11 +4,21 @@ export const NO_FULFILLMENTS_COURSE_ID = 10;
 /** Equivalent course ID for special course that satisfies FWS requirement. */
 export const FWS_COURSE_ID = 11;
 
+/** Equivalent course ID for special course that satisfies swim test requirement. */
+export const SWIM_TEST_COURSE_ID = 13;
+
+/** The unique ID of the special course that fulfills swim test. */
+export const SWIM_TEST_UNIQUE_ID = 'Swim Test';
+
+/** The code of the special course that fulfills swim test. */
+export const SWIM_TEST_CODE = 'Swim Test';
+
 /** List of special course IDs */
 export const SPECIAL_COURSES = {
   NO_FULFILLMENTS: NO_FULFILLMENTS_COURSE_ID,
   FWS: FWS_COURSE_ID,
+  SWIM_TEST: SWIM_TEST_COURSE_ID,
 };
 
 /** The real course ID of the course that fulfills swim test. */
-export const SWIM_TEST_COURSE_ID = 350002;
+export const SWIM_TEST_REAL_COURSE_ID = 350002;

src/requirements/data/university/index.ts

@@ -1,12 +1,27 @@
 import { Course, CollegeOrMajorRequirement } from '../../types';
+import { SWIM_TEST_COURSE_ID, SWIM_TEST_REAL_COURSE_ID } from '../constants';
 
 const universityRequirements: readonly CollegeOrMajorRequirement[] = [
+  {
+    name: 'Swim Test',
+    description:
+      'The Faculty Advisory Committee on Athletics and Physical Education has established a basic swimming ' +
+      'and water safety competency requirement for all entering first-year undergraduate students.',
+    source: 'http://courses.cornell.edu/content.php?catoid=41&navoid=11637#swim_test',
+    checker: [
+      (course: Course): boolean =>
+        [SWIM_TEST_COURSE_ID, SWIM_TEST_REAL_COURSE_ID].includes(course.crseId),
+    ],
+    fulfilledBy: 'courses',
+    perSlotMinCount: [1],
+    slotNames: ['Course'],
+  },
   {
     name: 'Physical Education',
     description:
       'All incoming freshmen are required to take two credits (two courses) of Physical Education, ' +
       'one credit each semester of the first year on campus.',
-    source: 'http://courses.cornell.edu/content.php?catoid=41&navoid=11637',
+    source: 'http://courses.cornell.edu/content.php?catoid=41&navoid=11637#physical',
     allowCourseDoubleCounting: true,
     checker: [(course: Course): boolean => 'PE'.includes(course.subject)],
     fulfilledBy: 'courses',

src/requirements/requirement-frontend-computation.ts

@@ -1,5 +1,5 @@
 import { isPlaceholderCourse } from '../utilities';
-import { SWIM_TEST_COURSE_ID } from './data/constants';
+import { SWIM_TEST_CODE, SWIM_TEST_COURSE_ID, SWIM_TEST_UNIQUE_ID } from './data/constants';
 import userDataToExamCourses from './requirement-exam-utils';
 import {
   courseIsAPIB,
@@ -125,61 +125,27 @@ const getTotalCreditsFulfillmentStatistics = (
   };
 };
 
-const getSwimTestFulfillmentStatistics = (
-  college: string,
-  courses: readonly CourseTaken[],
-  tookSwimTest: boolean
-): RequirementFulfillment => {
-  const requirement: RequirementWithIDSourceType = {
-    id: 'College-UNI-SwimTest',
-    sourceType: 'College',
-    sourceSpecificName: college,
-    name: 'Swim Test',
-    description:
-      'The Faculty Advisory Committee on Athletics and Physical Education has established a basic swimming ' +
-      'and water safety competency requirement for all entering first-year undergraduate students.',
-    source: 'http://courses.cornell.edu/content.php?catoid=41&navoid=11637',
-    courses: [[SWIM_TEST_COURSE_ID]],
-    fulfilledBy: 'courses',
-    perSlotMinCount: [1],
-    slotNames: ['Course'],
-  };
-  const swimClasses = courses.filter(it => it.courseId === SWIM_TEST_COURSE_ID);
-  if (tookSwimTest) {
-    swimClasses.push({
-      courseId: SWIM_TEST_COURSE_ID,
-      uniqueId: -1,
-      code: 'Swim Test',
-      credits: 0,
-    });
-  }
-  const minCountFulfilled = swimClasses.length > 0 ? 1 : 0;
-  return {
-    requirement,
-    fulfillment: {
-      fulfilledBy: 'courses',
-      safeCourses: [swimClasses],
-      dangerousCourses: [swimClasses],
-      safeMinCountFulfilled: minCountFulfilled,
-      dangerousMinCountFulfilled: minCountFulfilled,
-      minCountRequired: 1,
-    },
-  };
-};
-
 export function getCourseCodesArray(
   semesters: readonly FirestoreSemester[],
   onboardingData: AppOnboardingData
 ): readonly CourseTaken[] {
   const courses: CourseTaken[] = [];
+  if (onboardingData.tookSwim === 'yes') {
+    courses.push({
+      courseId: SWIM_TEST_COURSE_ID,
+      uniqueId: SWIM_TEST_UNIQUE_ID,
+      code: SWIM_TEST_CODE,
+      credits: 0,
+    });
+  }
+  courses.push(...userDataToExamCourses(onboardingData));
   semesters.forEach(semester => {
     semester.courses.forEach(course => {
       if (!isPlaceholderCourse(course)) {
         courses.push(convertFirestoreSemesterCourseToCourseTaken(course));
       }
     });
   });
-  courses.push(...userDataToExamCourses(onboardingData));
   return courses;
 }
 
@@ -263,10 +229,6 @@ export default function computeGroupedRequirementFulfillmentReports(
   if (totalCreditsFulfillmentStatistics != null) {
     collegeFulfillmentStatistics.push(totalCreditsFulfillmentStatistics);
   }
-  if (college)
-    collegeFulfillmentStatistics.push(
-      getSwimTestFulfillmentStatistics(college, coursesTaken, onboardingData.tookSwim === 'yes')
-    );
   const majorFulfillmentStatisticsMap = new Map<string, RequirementFulfillment[]>();
   const minorFulfillmentStatisticsMap = new Map<string, RequirementFulfillment[]>();
   const gradFulfillmentStatisticsMap = new Map<string, RequirementFulfillment[]>();

src/requirements/requirement-graph-builder.ts

@@ -103,13 +103,7 @@ export const buildRequirementFulfillmentGraph = <
 
   // Phase 3: Respect user's choices on opt-in/opt-out.
   userCourses.forEach(course => {
-    const { uniqueId } = course;
-    // typeof uniqueId === 'string' means it's AP/IB equivalent course.
-    // uniqueId < 0 means it's swim test.
-    // User never gets to make a choice about these courses, so it will never appear in the choices.
-    // Therefore, removing those edges will nullify all these credits.
-    if (typeof uniqueId === 'string' || uniqueId < 0) return;
-    const userChoiceOnOptInOptOutCourse = userChoiceOnRequirementOverrides[uniqueId];
+    const userChoiceOnOptInOptOutCourse = userChoiceOnRequirementOverrides[course.uniqueId];
     if (userChoiceOnOptInOptOutCourse == null) return;
     userChoiceOnOptInOptOutCourse.optIn.forEach(optedInRequirement => {
       graph.addEdge(optedInRequirement, course);