diff --git a/src/components/Modals/NewCourse/NewCourseModal.vue b/src/components/Modals/NewCourse/NewCourseModal.vue index 0265a45a8..69cabdb7f 100644 --- a/src/components/Modals/NewCourse/NewCourseModal.vue +++ b/src/components/Modals/NewCourse/NewCourseModal.vue @@ -54,8 +54,8 @@ export default defineComponent({ components: { CourseSelector, TeleportModal, SelectedRequirementEditor }, emits: { 'close-course-modal': () => true, - 'add-course': (course: CornellCourseRosterCourse, requirementID: string) => - typeof course === 'object' && typeof requirementID === 'string', + 'add-course': (course: CornellCourseRosterCourse, choice: FirestoreCourseOptInOptOutChoices) => + typeof course === 'object' && typeof choice === 'object', }, data() { return { @@ -78,9 +78,6 @@ export default defineComponent({ rightButtonText(): string { return this.editMode ? 'Next' : 'Add'; }, - selectableRequirementChoices(): AppSelectableRequirementChoices { - return store.state.selectableRequirementChoices; - }, }, methods: { selectCourse(result: CornellCourseRosterCourse) { @@ -98,7 +95,7 @@ export default defineComponent({ selectedCourse, store.state.groupedRequirementFulfillmentReport, store.state.toggleableRequirementChoices, - /* deprecated AppOverriddenFulfillmentChoices */ {} + store.state.overriddenFulfillmentChoices ); const requirementsThatAllowDoubleCounting: string[] = []; @@ -137,7 +134,17 @@ export default defineComponent({ }, addCourse() { if (this.selectedCourse == null) return; - this.$emit('add-course', this.selectedCourse, this.selectedRequirementID); + this.$emit('add-course', this.selectedCourse, { + // Only exclude the selected requirement from opt-out. + optOut: this.relatedRequirements + .filter(it => it.id !== this.selectedRequirementID) + .map(it => it.id), + // Only include the selected requirement from opt-in. + acknowledgedCheckerWarningOptIn: this.selfCheckRequirements + .filter(it => it.id === this.selectedRequirementID) + .map(it => it.id), + arbitraryOptIn: {}, + }); this.closeCurrentModal(); }, onSelectedChange(selected: string) { diff --git a/src/components/Requirements/IncompleteSelfCheck.vue b/src/components/Requirements/IncompleteSelfCheck.vue index d8046d625..5d0c8da79 100644 --- a/src/components/Requirements/IncompleteSelfCheck.vue +++ b/src/components/Requirements/IncompleteSelfCheck.vue @@ -41,9 +41,12 @@ import store from '@/store'; import { cornellCourseRosterCourseToFirebaseSemesterCourseWithGlobalData, addCourseToSemester, - addCourseToSelectableRequirements, + updateRequirementChoice, } from '@/global-firestore-data'; -import { canFulfillChecker } from '@/requirements/requirement-frontend-utils'; +import { + canFulfillChecker, + getAllEligibleRelatedRequirementIds, +} from '@/requirements/requirement-frontend-utils'; import NewSelfCheckCourseModal from '@/components/Modals/NewCourse/NewSelfCheckCourseModal.vue'; @@ -74,61 +77,46 @@ export default defineComponent({ // and courses that do not fulfill the requirement checker selfCheckCourses(): Record { const courses: Record = {}; - store.state.semesters.forEach(semester => { - semester.courses.forEach(course => { - const selectableRequirementCourses = - store.state.derivedSelectableRequirementData.requirementToCoursesMap[this.subReqId]; + store.state.semesters + .flatMap(it => it.courses) + .forEach(course => { + if ( + !canFulfillChecker( + store.state.userRequirementsMap, + store.state.toggleableRequirementChoices, + this.subReqId, + course.crseId + ) + ) { + // If the course can't help fulfill the checker, do not add to choices. + return; + } - // if course is mapped to another req(s), only allow it if all other reqs are double countable - let isAddable = true; - const otherReqsMappedTo = store.state.requirementFulfillmentGraph.getConnectedRequirementsFromCourse( + const currentlyMatchedRequirements = store.state.requirementFulfillmentGraph.getConnectedRequirementsFromCourse( { uniqueId: course.uniqueID } ); - - // true if all other requirements (if any) the course is assigned to are double countable, false otherwise - let allOtherReqsDoubleCountableIfAny = true; - - // true if this requirement is double countable, false otherwise. - let thisReqDoubleCountable = false; - - // loop through all reqs and determine if all other reqs this course is assigned to are - // double countable (if any exist) and whether or not this req itself is double countable - const collegesMajorsMinors = store.state.groupedRequirementFulfillmentReport; - collegesMajorsMinors.forEach(reqGroup => { - reqGroup.reqs.forEach(req => { - if ( - otherReqsMappedTo.includes(req.requirement.id) && - !req.requirement.allowCourseDoubleCounting - ) { - allOtherReqsDoubleCountableIfAny = false; - } else if ( - req.requirement.id === this.subReqId && - req.requirement.allowCourseDoubleCounting - ) { - thisReqDoubleCountable = true; - } - }); - }); - - // if neither the current req or all other assigned reqs are not double countable, restrict from adding - if (!(allOtherReqsDoubleCountableIfAny || thisReqDoubleCountable)) { - isAddable = false; + if (currentlyMatchedRequirements.includes(this.subReqId)) { + // If the course is already matched to the current requirement, do not add to choices. + return; } - const isAlreadyAddedToReq = - selectableRequirementCourses && selectableRequirementCourses.includes(course); - - // filter out courses that cannot fulfill the self-check, for self-checks with warnings - const canFulfillReq = canFulfillChecker( - store.state.userRequirementsMap, - store.state.toggleableRequirementChoices, - this.subReqId, - course.crseId - ); + const currentRequirementAllowDoubleCounting = + store.state.userRequirementsMap[this.subReqCourseId]?.allowCourseDoubleCounting; + const allOtherRequirementsAllowDoubleCounting = store.state.requirementFulfillmentGraph + .getConnectedRequirementsFromCourse({ uniqueId: course.uniqueID }) + .every(reqID => store.state.userRequirementsMap[reqID]?.allowCourseDoubleCounting); + if (!currentRequirementAllowDoubleCounting && !allOtherRequirementsAllowDoubleCounting) { + // At this point, we need to consider double counting issues. + // There are 2 ways we can add the course to the requirement without double counting violations: + // 1. This requirement allows double counting. + // 2. All the already matched requirements allow double counting. + // If both don't hold, we cannot add. + return; + } - if (!isAlreadyAddedToReq && isAddable && canFulfillReq) courses[course.code] = course; + // All pre-conditions have been checked, we can add it as choice! + courses[course.code] = course; }); - }); return courses; }, @@ -150,12 +138,45 @@ export default defineComponent({ }, addExistingCourse(option: string) { this.showDropdown = false; - addCourseToSelectableRequirements(this.selfCheckCourses[option].uniqueID, this.subReqId); + updateRequirementChoice(this.selfCheckCourses[option].uniqueID, choice => ({ + ...choice, + // Since we edit from a self-check requirement, + // we know it must be `acknowledgedCheckerWarningOptIn`. + acknowledgedCheckerWarningOptIn: Array.from( + new Set([...choice.acknowledgedCheckerWarningOptIn, this.subReqId]) + ), + // Keep existing behavior of keeping it connected to at most one requirement. + optOut: getAllEligibleRelatedRequirementIds( + this.selfCheckCourses[option].crseId, + store.state.groupedRequirementFulfillmentReport, + store.state.toggleableRequirementChoices + ), + })); }, addNewCourse(course: CornellCourseRosterCourse, season: FirestoreSemesterSeason, year: number) { this.showDropdown = false; const newCourse = cornellCourseRosterCourseToFirebaseSemesterCourseWithGlobalData(course); - addCourseToSemester(year, season, newCourse, this.subReqId, this.$gtag); + addCourseToSemester( + year, + season, + newCourse, + // Since the course is new, we know the old choice does not exist. + () => ({ + arbitraryOptIn: {}, + // Since we edit from a self-check requirement, + // we know it must be `acknowledgedCheckerWarningOptIn`. + acknowledgedCheckerWarningOptIn: [this.subReqId], + // We also need to opt-out of all requirements without warnings, + // because the user intention is clear that we only want to bind + // the course to this specific requirement. + optOut: getAllEligibleRelatedRequirementIds( + newCourse.crseId, + store.state.groupedRequirementFulfillmentReport, + store.state.toggleableRequirementChoices + ), + }), + this.$gtag + ); }, openCourseModal() { this.isCourseModalOpen = true; diff --git a/src/components/Requirements/RequirementSelfCheckSlots.vue b/src/components/Requirements/RequirementSelfCheckSlots.vue index 4286ef35f..868532000 100644 --- a/src/components/Requirements/RequirementSelfCheckSlots.vue +++ b/src/components/Requirements/RequirementSelfCheckSlots.vue @@ -21,7 +21,6 @@ import IncompleteSelfCheck from '@/components/Requirements/IncompleteSelfCheck.v import store from '@/store'; import { - convertFirestoreSemesterCourseToCourseTaken, getMatchedRequirementFulfillmentSpecification, courseIsAPIB, } from '@/requirements/requirement-frontend-utils'; @@ -39,13 +38,9 @@ export default defineComponent({ computed: { fulfilledSelfCheckCourses(): readonly CourseTaken[] { // selectedCourses are courses that fulfill the requirement based on user-choice - // they are taken from derivedSelectableRequirementData - const selectedFirestoreCourses = - store.state.derivedSelectableRequirementData.requirementToCoursesMap[ - this.requirementFulfillment.requirement.id - ] || []; - const selectedCourses = selectedFirestoreCourses.map( - convertFirestoreSemesterCourseToCourseTaken + // they are taken from requirement graph + const selectedCourses = store.state.requirementFulfillmentGraph.getConnectedCoursesFromRequirement( + this.requirementFulfillment.requirement.id ); // fulfillableCourses are the courses that can fulfill this requirement diff --git a/src/components/Semester/Semester.vue b/src/components/Semester/Semester.vue index 917a9ba39..69cd1fa18 100644 --- a/src/components/Semester/Semester.vue +++ b/src/components/Semester/Semester.vue @@ -139,9 +139,10 @@ import { addCourseToSemester, deleteCourseFromSemester, deleteAllCoursesFromSemester, - addCoursesToSelectableRequirements, + updateRequirementChoices, } from '@/global-firestore-data'; -import { updateSubjectColorData } from '@/store'; +import store, { updateSubjectColorData } from '@/store'; +import { getAllEligibleRelatedRequirementIds } from '@/requirements/requirement-frontend-utils'; type ComponentRef = { $el: HTMLDivElement }; @@ -229,6 +230,15 @@ export default defineComponent({ get(): readonly FirestoreSemesterCourse[] { return this.courses; }, + /** + * This function is called when a course is dragged into the semester. + * + * It can be a semester-to-semester drag-n-drop, which does not have `requirementID` + * and does not require update the requirement. + * It can also be a requirement-bar-to-semester drag-n-drop, which has a `requirementID` + * attached to the course. We need to check the presence of this field and update requirement + * choice accordingly. + */ set(newCourses: readonly AppFirestoreSemesterCourseWithRequirementID[]) { const courses = newCourses.map(({ requirementID: _, ...rest }) => rest); editSemester( @@ -239,11 +249,33 @@ export default defineComponent({ courses, }) ); - const newChoices: Record = {}; - newCourses.forEach(({ uniqueID, requirementID }) => { - if (requirementID) newChoices[uniqueID] = requirementID; + updateRequirementChoices(oldChoices => { + const choices = { ...oldChoices }; + newCourses.forEach(({ uniqueID, requirementID, crseId }) => { + if (requirementID == null) { + // In this case, it's not a course from requirement bar + return; + } + const choice = choices[uniqueID] || { + arbitraryOptIn: {}, + acknowledgedCheckerWarningOptIn: [], + optOut: [], + }; + // We know the requirement must be dragged from requirements without warnings, + // because only those courses provide suggested courses. + // As a result, `acknowledgedCheckerWarningOptIn` is irrelevant and we only need to update + // the `optOut` field. + // Below, we find all the requirements it can possibly match, + // and only remove the requirementID since that's the one we should keep. + const optOut = getAllEligibleRelatedRequirementIds( + crseId, + store.state.groupedRequirementFulfillmentReport, + store.state.toggleableRequirementChoices + ).filter(it => it !== requirementID); + choices[uniqueID] = { ...choice, optOut }; + }); + return choices; }); - addCoursesToSelectableRequirements(newChoices); }, }, // Add space for a course if there is a "shadow" of it, decrease if it is from the current sem @@ -330,9 +362,10 @@ export default defineComponent({ closeConfirmationModal() { this.isConfirmationOpen = false; }, - addCourse(data: CornellCourseRosterCourse, requirementID: string) { + addCourse(data: CornellCourseRosterCourse, choice: FirestoreCourseOptInOptOutChoices) { const newCourse = cornellCourseRosterCourseToFirebaseSemesterCourseWithGlobalData(data); - addCourseToSemester(this.year, this.season, newCourse, requirementID, this.$gtag); + // Since the course is new, we know the old choice does not exist. + addCourseToSemester(this.year, this.season, newCourse, () => choice, this.$gtag); const courseCode = `${data.subject} ${data.catalogNbr}`; this.openConfirmationModal(`Added ${courseCode} to ${this.season} ${this.year}`); diff --git a/src/firebase-admin-config.ts b/src/firebase-admin-config.ts index 2a19e6c64..a5de21702 100644 --- a/src/firebase-admin-config.ts +++ b/src/firebase-admin-config.ts @@ -36,10 +36,6 @@ export const toggleableRequirementChoicesCollection = db .collection('user-toggleable-requirement-choices') .withConverter(getTypedFirestoreDataConverter()); -export const selectableRequirementChoicesCollection = db - .collection('user-selectable-requirement-choices') - .withConverter(getTypedFirestoreDataConverter()); - export const overriddenFulfillmentChoicesCollection = db .collection('user-overridden-fulfillment-choices') .withConverter(getTypedFirestoreDataConverter()); diff --git a/src/firebase-frontend-config.ts b/src/firebase-frontend-config.ts index 49c529e30..b7f140e3d 100644 --- a/src/firebase-frontend-config.ts +++ b/src/firebase-frontend-config.ts @@ -52,10 +52,6 @@ export const toggleableRequirementChoicesCollection = db .collection('user-toggleable-requirement-choices') .withConverter(getTypedFirestoreDataConverter()); -export const selectableRequirementChoicesCollection = db - .collection('user-selectable-requirement-choices') - .withConverter(getTypedFirestoreDataConverter()); - export const overriddenFulfillmentChoicesCollection = db .collection('user-overridden-fulfillment-choices') .withConverter(getTypedFirestoreDataConverter()); diff --git a/src/global-firestore-data/index.ts b/src/global-firestore-data/index.ts index 2b1a94c38..0438ac6cf 100644 --- a/src/global-firestore-data/index.ts +++ b/src/global-firestore-data/index.ts @@ -26,7 +26,7 @@ export { } from './semesters'; export { default as chooseToggleableRequirementOption } from './toggleable-requirement-choices'; export { - addCourseToSelectableRequirements, - addCoursesToSelectableRequirements, - deleteCourseFromSelectableRequirements, -} from './selectable-requirement-choices'; + updateRequirementChoice, + updateRequirementChoices, + deleteCourseFromRequirementChoices, +} from './override-fulfillment-choices'; diff --git a/src/global-firestore-data/override-fulfillment-choices.ts b/src/global-firestore-data/override-fulfillment-choices.ts new file mode 100644 index 000000000..76e846124 --- /dev/null +++ b/src/global-firestore-data/override-fulfillment-choices.ts @@ -0,0 +1,46 @@ +import { overriddenFulfillmentChoicesCollection } from '../firebase-frontend-config'; +import store from '../store'; + +export const updateRequirementChoice = ( + courseUniqueID: string | number, + choiceUpdater: (choice: FirestoreCourseOptInOptOutChoices) => FirestoreCourseOptInOptOutChoices +): void => { + overriddenFulfillmentChoicesCollection.doc(store.state.currentFirebaseUser.email).set({ + ...store.state.overriddenFulfillmentChoices, + [courseUniqueID]: choiceUpdater( + store.state.overriddenFulfillmentChoices[courseUniqueID] || { + arbitraryOptIn: {}, + acknowledgedCheckerWarningOptIn: [], + optOut: [], + } + ), + }); +}; + +export const updateRequirementChoices = ( + updater: ( + oldChoices: FirestoreOverriddenFulfillmentChoices + ) => FirestoreOverriddenFulfillmentChoices +): void => { + overriddenFulfillmentChoicesCollection + .doc(store.state.currentFirebaseUser.email) + .set(updater(store.state.overriddenFulfillmentChoices)); +}; + +export const deleteCourseFromRequirementChoices = (courseUniqueID: string | number): void => + deleteCoursesFromRequirementChoices([courseUniqueID]); + +export const deleteCoursesFromRequirementChoices = ( + courseUniqueIds: readonly (string | number)[] +): void => { + const courseUniqueIdStrings = new Set(courseUniqueIds.map(uniqueId => uniqueId.toString())); + overriddenFulfillmentChoicesCollection + .doc(store.state.currentFirebaseUser.email) + .set( + Object.fromEntries( + Object.entries(store.state.overriddenFulfillmentChoices).filter( + ([uniqueId]) => !courseUniqueIdStrings.has(uniqueId) + ) + ) + ); +}; diff --git a/src/global-firestore-data/selectable-requirement-choices.ts b/src/global-firestore-data/selectable-requirement-choices.ts deleted file mode 100644 index bad382ee3..000000000 --- a/src/global-firestore-data/selectable-requirement-choices.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { getCourseCodesArray } from '../requirements/requirement-frontend-computation'; -import { getFirestoreCourseOptInOptOutChoicesBuilder } from '../requirements/requirement-graph-builder-from-user-data'; -import { - db, - selectableRequirementChoicesCollection, - overriddenFulfillmentChoicesCollection, -} from '../firebase-frontend-config'; -import store from '../store'; - -const chooseSelectableRequirementOption = ( - selectableRequirementChoices: AppSelectableRequirementChoices -): void => { - const courses = getCourseCodesArray(store.state.semesters, store.state.onboardingData); - const newFormatChoicesBuilder = getFirestoreCourseOptInOptOutChoicesBuilder( - courses, - store.state.onboardingData, - store.state.toggleableRequirementChoices, - selectableRequirementChoices, - /* deprecated AppOverriddenFulfillmentChoices */ {} - ); - const overriddenFulfillmentChoices = Object.fromEntries( - courses.map(course => [course.uniqueId, newFormatChoicesBuilder(course)]) - ); - - const batch = db.batch(); - batch.set( - selectableRequirementChoicesCollection.doc(store.state.currentFirebaseUser.email), - selectableRequirementChoices - ); - batch.set( - overriddenFulfillmentChoicesCollection.doc(store.state.currentFirebaseUser.email), - overriddenFulfillmentChoices - ); - batch.commit(); -}; - -export const addCourseToSelectableRequirements = ( - courseUniqueID: string | number, - requirementID: string | undefined -): void => { - // Even when there is no change, we set the old data anyways, - // so it can trigger a save of user choice in new format. - chooseSelectableRequirementOption( - requirementID - ? { - ...store.state.selectableRequirementChoices, - [courseUniqueID]: requirementID, - } - : store.state.selectableRequirementChoices - ); -}; - -export const addCoursesToSelectableRequirements = ( - newChoices: Readonly> -): void => { - // Even when there is no change, we set the old data anyways, - // so it can trigger a save of user choice in new format. - chooseSelectableRequirementOption({ - ...store.state.selectableRequirementChoices, - ...newChoices, - }); -}; - -export const deleteCourseFromSelectableRequirements = (courseUniqueID: string | number): void => { - deleteCoursesFromSelectableRequirements([courseUniqueID]); -}; - -export const deleteCoursesFromSelectableRequirements = ( - courseUniqueIds: (string | number)[] -): void => { - const courseUniqueIdStrings = new Set(courseUniqueIds.map(uniqueId => uniqueId.toString())); - chooseSelectableRequirementOption( - Object.assign( - {}, - ...Object.entries(store.state.selectableRequirementChoices) - .filter(([k]) => !courseUniqueIdStrings.has(k)) - .map(([k, v]) => ({ [k]: v })) - ) - ); -}; diff --git a/src/global-firestore-data/semesters.ts b/src/global-firestore-data/semesters.ts index 56cf57821..946f4bdb9 100644 --- a/src/global-firestore-data/semesters.ts +++ b/src/global-firestore-data/semesters.ts @@ -4,10 +4,10 @@ import { GTag, GTagEvent } from '../gtag'; import { sortedSemesters } from '../utilities'; import { - addCourseToSelectableRequirements, - deleteCourseFromSelectableRequirements, - deleteCoursesFromSelectableRequirements, -} from './selectable-requirement-choices'; + updateRequirementChoice, + deleteCourseFromRequirementChoices, + deleteCoursesFromRequirementChoices, +} from './override-fulfillment-choices'; export const editSemesters = ( updater: (oldSemesters: readonly FirestoreSemester[]) => readonly FirestoreSemester[] @@ -79,7 +79,7 @@ export const deleteSemester = ( GTagEvent(gtag, 'delete-semester'); const semester = store.state.semesters.find(sem => semesterEquals(sem, year, season)); if (semester) { - deleteCoursesFromSelectableRequirements(semester.courses.map(course => course.uniqueID)); + deleteCoursesFromRequirementChoices(semester.courses.map(course => course.uniqueID)); editSemesters(oldSemesters => oldSemesters.filter(sem => !semesterEquals(sem, year, season))); } }; @@ -88,7 +88,7 @@ export const addCourseToSemester = ( year: number, season: FirestoreSemesterSeason, newCourse: FirestoreSemesterCourse, - requirementID?: string, + choiceUpdater: (choice: FirestoreCourseOptInOptOutChoices) => FirestoreCourseOptInOptOutChoices, gtag?: GTag ): void => { GTagEvent(gtag, 'add-course'); @@ -104,9 +104,7 @@ export const addCourseToSemester = ( if (semesterFound) return newSemestersWithCourse; return [...oldSemesters, createSemester(year, season, [newCourse])]; }); - if (requirementID) { - addCourseToSelectableRequirements(newCourse.uniqueID, requirementID); - } + updateRequirementChoice(newCourse.uniqueID, choiceUpdater); }; export const deleteCourseFromSemester = ( @@ -118,7 +116,7 @@ export const deleteCourseFromSemester = ( GTagEvent(gtag, 'delete-course'); const semester = store.state.semesters.find(sem => semesterEquals(sem, year, season)); if (semester) { - deleteCourseFromSelectableRequirements(courseUniqueID); + deleteCourseFromRequirementChoices(courseUniqueID); editSemesters(oldSemesters => oldSemesters.map(sem => ({ ...sem, @@ -138,7 +136,7 @@ export const deleteAllCoursesFromSemester = ( GTagEvent(gtag, 'delete-semester-courses'); const semester = store.state.semesters.find(sem => semesterEquals(sem, year, season)); if (semester) { - deleteCoursesFromSelectableRequirements(semester.courses.map(course => course.uniqueID)); + deleteCoursesFromRequirementChoices(semester.courses.map(course => course.uniqueID)); editSemesters(oldSemesters => oldSemesters.map(sem => ({ ...sem, @@ -158,7 +156,7 @@ export const deleteCourseFromSemesters = (courseUniqueID: number, gtag?: GTag): return { ...semester, courses: coursesWithoutDeleted }; }) ); - deleteCourseFromSelectableRequirements(courseUniqueID); + deleteCourseFromRequirementChoices(courseUniqueID); }; // exposed for testing diff --git a/src/requirements/__test__/requirement-graph-builder.test.ts b/src/requirements/__test__/requirement-graph-builder.test.ts index 865034196..db2952b6e 100644 --- a/src/requirements/__test__/requirement-graph-builder.test.ts +++ b/src/requirements/__test__/requirement-graph-builder.test.ts @@ -22,18 +22,13 @@ const getAllCoursesThatCanPotentiallySatisfyRequirement = ( }; it('buildRequirementFulfillmentGraph phase 1 test 1', () => { - const graph = buildRequirementFulfillmentGraph( - { - requirements, - userCourses: [CS3410, CS3420, MATH4710], - userChoiceOnFulfillmentStrategy: {}, - userChoiceOnDoubleCountingElimination: {}, - userChoiceOnRequirementOverrides: {}, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: () => false, - }, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ true - ); + const graph = buildRequirementFulfillmentGraph({ + requirements, + userCourses: [CS3410, CS3420, MATH4710], + userChoiceOnFulfillmentStrategy: {}, + userChoiceOnRequirementOverrides: {}, + getAllCoursesThatCanPotentiallySatisfyRequirement, + }); expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3410, CS3420]); expect(graph.getConnectedCoursesFromRequirement('Probability')).toEqual([MATH4710]); @@ -43,18 +38,13 @@ it('buildRequirementFulfillmentGraph phase 1 test 1', () => { // This test ensures that we are actually using userCourses and drop any courses from pre-computed // course list that are not in userCourses. it('buildRequirementFulfillmentGraph phase 1 test 2', () => { - const graph = buildRequirementFulfillmentGraph( - { - requirements, - userCourses: [CS3410, MATH4710], - userChoiceOnFulfillmentStrategy: {}, - userChoiceOnDoubleCountingElimination: {}, - userChoiceOnRequirementOverrides: {}, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: () => false, - }, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ true - ); + const graph = buildRequirementFulfillmentGraph({ + requirements, + userCourses: [CS3410, MATH4710], + userChoiceOnFulfillmentStrategy: {}, + userChoiceOnRequirementOverrides: {}, + getAllCoursesThatCanPotentiallySatisfyRequirement, + }); expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3410]); expect(graph.getConnectedCoursesFromRequirement('Probability')).toEqual([MATH4710]); @@ -63,18 +53,13 @@ it('buildRequirementFulfillmentGraph phase 1 test 2', () => { // Following two tests test how we are removing edges depending on user choices on fulfillment strategy. it('buildRequirementFulfillmentGraph phase 2-1 test', () => { - const graph = buildRequirementFulfillmentGraph( - { - requirements, - userCourses: [CS3410, CS3420, MATH4710], - userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, - userChoiceOnDoubleCountingElimination: {}, - userChoiceOnRequirementOverrides: {}, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: () => false, - }, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ true - ); + const graph = buildRequirementFulfillmentGraph({ + requirements, + userCourses: [CS3410, CS3420, MATH4710], + userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, + userChoiceOnRequirementOverrides: {}, + getAllCoursesThatCanPotentiallySatisfyRequirement, + }); // In this case, 3420 is removed since user chooses strategy 1. expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3410]); @@ -83,18 +68,13 @@ it('buildRequirementFulfillmentGraph phase 2-1 test', () => { }); it('buildRequirementFulfillmentGraph phase 2-2 test', () => { - const graph = buildRequirementFulfillmentGraph( - { - requirements, - userCourses: [CS3410, CS3420, MATH4710], - userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3420.courseId] }, - userChoiceOnDoubleCountingElimination: {}, - userChoiceOnRequirementOverrides: {}, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: () => false, - }, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ true - ); + const graph = buildRequirementFulfillmentGraph({ + requirements, + userCourses: [CS3410, CS3420, MATH4710], + userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3420.courseId] }, + userChoiceOnRequirementOverrides: {}, + getAllCoursesThatCanPotentiallySatisfyRequirement, + }); // In this case, 3410 is removed since user chooses strategy 2. expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3420]); @@ -104,18 +84,15 @@ it('buildRequirementFulfillmentGraph phase 2-2 test', () => { // The following two tests test that we will remove edges incompatible with user supplied choices. it('buildRequirementFulfillmentGraph phase 3 test 1', () => { - const graph = buildRequirementFulfillmentGraph( - { - requirements, - userCourses: [CS3410, CS3420, MATH4710], - userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, - userChoiceOnDoubleCountingElimination: { [MATH4710.uniqueId]: 'Probability' }, - userChoiceOnRequirementOverrides: {}, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: () => false, + const graph = buildRequirementFulfillmentGraph({ + requirements, + userCourses: [CS3410, CS3420, MATH4710], + userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, + userChoiceOnRequirementOverrides: { + [MATH4710.uniqueId]: { optIn: [], optOut: ['Elective'] }, }, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ true - ); + getAllCoursesThatCanPotentiallySatisfyRequirement, + }); expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3410]); expect(graph.getConnectedCoursesFromRequirement('Probability')).toEqual([MATH4710]); @@ -123,81 +100,35 @@ it('buildRequirementFulfillmentGraph phase 3 test 1', () => { }); it('buildRequirementFulfillmentGraph phase 3 test 2', () => { - const graph = buildRequirementFulfillmentGraph( - { - requirements, - userCourses: [CS3410, CS3420, MATH4710], - userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, - userChoiceOnDoubleCountingElimination: { [MATH4710.uniqueId]: 'Elective' }, - userChoiceOnRequirementOverrides: {}, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: () => false, - }, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ true - ); - - expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3410]); - expect(graph.getConnectedCoursesFromRequirement('Probability')).toEqual([]); - expect(graph.getConnectedCoursesFromRequirement('Elective')).toEqual([CS3410, CS3420, MATH4710]); -}); - -// The following test ensures that we will remove edges when user makes no choice on courses. -it('buildRequirementFulfillmentGraph phase 3 test 3', () => { const graph = buildRequirementFulfillmentGraph({ requirements, userCourses: [CS3410, CS3420, MATH4710], userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, - userChoiceOnDoubleCountingElimination: { [MATH4710.uniqueId]: 'Probability' }, - userChoiceOnRequirementOverrides: {}, + userChoiceOnRequirementOverrides: { + [MATH4710.uniqueId]: { optIn: [], optOut: ['Probability'] }, + }, getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: () => false, }); - expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([]); - expect(graph.getConnectedCoursesFromRequirement('Probability')).toEqual([MATH4710]); - expect(graph.getConnectedCoursesFromRequirement('Elective')).toEqual([]); + expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3410]); + expect(graph.getConnectedCoursesFromRequirement('Probability')).toEqual([]); + expect(graph.getConnectedCoursesFromRequirement('Elective')).toEqual([CS3410, CS3420, MATH4710]); }); // The following test runs on a fully specified graph algorithm on complete user choices -it('buildRequirementFulfillmentGraph phase 3 test 4', () => { +it('buildRequirementFulfillmentGraph phase 3 test 3', () => { const graph = buildRequirementFulfillmentGraph({ requirements, userCourses: [CS3410, CS3420, MATH4710], userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, - userChoiceOnDoubleCountingElimination: { - [CS3410.uniqueId]: 'CS3410/CS3420', - [CS3420.uniqueId]: 'Elective', - [MATH4710.uniqueId]: 'Elective', + userChoiceOnRequirementOverrides: { + [CS3410.uniqueId]: { optIn: [], optOut: ['Elective'] }, + [MATH4710.uniqueId]: { optIn: [], optOut: ['Elective'] }, }, - userChoiceOnRequirementOverrides: {}, getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: r => r === 'Probability', }); expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([CS3410]); expect(graph.getConnectedCoursesFromRequirement('Probability')).toEqual([MATH4710]); - expect(graph.getConnectedCoursesFromRequirement('Elective')).toEqual([CS3420, MATH4710]); -}); - -// Normally, we will remove all edges when there is no user choice associated with a unique ID. -// If we do the same for AP/IB/swim courses, then all these courses will never be able to fulfill -// anything. -// The following test ensures that we don't regress again. -it('AP/IB/swim test course edge is not removed in step 3', () => { - // Test this for a lot of different unique ID less than - for (let uniqueId = -1; uniqueId >= -10; uniqueId -= 1) { - const graph = buildRequirementFulfillmentGraph({ - requirements, - userCourses: [{ courseId: CS3410.courseId, uniqueId }], // mock an AP/IB course - userChoiceOnFulfillmentStrategy: { 'CS3410/CS3420': [CS3410.courseId] }, - userChoiceOnDoubleCountingElimination: {}, - userChoiceOnRequirementOverrides: {}, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting: r => r === 'Probability', - }); - - expect(graph.getConnectedCoursesFromRequirement('CS3410/CS3420')).toEqual([ - { courseId: CS3410.courseId, uniqueId }, - ]); - } + expect(graph.getConnectedCoursesFromRequirement('Elective')).toEqual([CS3420]); }); diff --git a/src/requirements/admin/requirement-graph-admin-utils.ts b/src/requirements/admin/requirement-graph-admin-utils.ts index 1e4e35210..b37490a55 100644 --- a/src/requirements/admin/requirement-graph-admin-utils.ts +++ b/src/requirements/admin/requirement-graph-admin-utils.ts @@ -3,7 +3,7 @@ import { checkNotNull } from '../../utilities'; import { semestersCollection, toggleableRequirementChoicesCollection, - selectableRequirementChoicesCollection, + overriddenFulfillmentChoicesCollection, onboardingDataCollection, } from '../../firebase-admin-config'; import { getCourseCodesArray } from '../requirement-frontend-computation'; @@ -14,7 +14,7 @@ interface UserDataOnAdmin { readonly courses: readonly CourseTaken[]; readonly onboardingData: AppOnboardingData; readonly toggleableRequirementChoices: Readonly>; - readonly selectableRequirementChoices: Readonly>; + readonly overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices; } interface UserRequirementDataOnAdmin extends UserDataOnAdmin { @@ -26,7 +26,7 @@ export async function getUserDataOnAdmin(userEmail: string): Promise it.data() || {}), - selectableRequirementChoicesCollection + overriddenFulfillmentChoicesCollection .doc(userEmail) .get() .then(it => it.data() || {}), @@ -49,7 +49,7 @@ export async function getUserDataOnAdmin(userEmail: string): Promise>; readonly requirementFulfillmentGraph: RequirementFulfillmentGraph; @@ -198,7 +197,6 @@ export default function computeGroupedRequirementFulfillmentReports( coursesTaken, onboardingData, toggleableRequirementChoices, - selectableRequirementChoices, overriddenFulfillmentChoices ); diff --git a/src/requirements/requirement-frontend-utils.ts b/src/requirements/requirement-frontend-utils.ts index 8adb040ad..9a35c1899 100644 --- a/src/requirements/requirement-frontend-utils.ts +++ b/src/requirements/requirement-frontend-utils.ts @@ -177,6 +177,7 @@ export function getUserRequirements({ */ type MatchedRequirementFulfillmentSpecificationBase = { readonly fulfilledBy: 'courses' | 'credits'; + readonly hasRequirementCheckerWarning: boolean; readonly eligibleCourses: readonly (readonly number[])[]; readonly perSlotMinCount: readonly number[]; readonly slotNames: readonly string[]; @@ -226,6 +227,7 @@ export function getMatchedRequirementFulfillmentSpecification( name, { fulfilledBy: subRequirement.fulfilledBy, + hasRequirementCheckerWarning: false, eligibleCourses: subRequirement.courses, perSlotMinCount: subRequirement.perSlotMinCount, slotNames, @@ -235,12 +237,14 @@ export function getMatchedRequirementFulfillmentSpecification( }) ); + const hasRequirementCheckerWarning = requirement.checkerWarning != null; switch (requirement.fulfilledBy) { case 'self-check': return null; case 'courses': return { fulfilledBy: requirement.fulfilledBy, + hasRequirementCheckerWarning, eligibleCourses: requirement.courses, additionalRequirements: convertAdditionalRequirements(requirement.additionalRequirements), perSlotMinCount: requirement.perSlotMinCount, @@ -250,6 +254,7 @@ export function getMatchedRequirementFulfillmentSpecification( case 'credits': return { fulfilledBy: requirement.fulfilledBy, + hasRequirementCheckerWarning, eligibleCourses: requirement.courses, additionalRequirements: convertAdditionalRequirements(requirement.additionalRequirements), perSlotMinCount: requirement.perSlotMinCount, @@ -264,6 +269,7 @@ export function getMatchedRequirementFulfillmentSpecification( ]; return { fulfilledBy: option.counting, + hasRequirementCheckerWarning, eligibleCourses: option.courses, perSlotMinCount: option.perSlotMinCount, slotNames: option.counting === 'courses' ? option.slotNames : [], @@ -278,10 +284,11 @@ export function getMatchedRequirementFulfillmentSpecification( const computeFulfillmentStatistics = ( requirementName: string, coursesTaken: readonly CourseTaken[], - overriddenFulfillmentChoices: AppOverriddenFulfillmentChoices, + overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices, disallowTransferCredit: boolean, { fulfilledBy, + hasRequirementCheckerWarning, eligibleCourses, perSlotMinCount, slotNames, @@ -291,19 +298,33 @@ const computeFulfillmentStatistics = ( const coursesThatFulfilledSubRequirements: CourseTaken[][] = eligibleCourses.map(() => []); const subRequirementProgress: number[] = eligibleCourses.map(() => 0); coursesTaken.forEach(courseTaken => { - const overrideOptions = overriddenFulfillmentChoices[courseTaken.uniqueId]; - const optInSlotNames = (overrideOptions && overrideOptions.optIn[requirementName]) || null; - const optOutSlotNames = (overrideOptions && overrideOptions.optOut[requirementName]) || null; + const overrideOptions = overriddenFulfillmentChoices[courseTaken.uniqueId] || { + arbitraryOptIn: [], + acknowledgedCheckerWarningOptIn: [], + optOut: [], + }; + // If a requirement has checker warning, do not match it the course unless it's acknoledged. + if ( + overrideOptions.optOut.includes(requirementName) || + (hasRequirementCheckerWarning && + !overrideOptions.acknowledgedCheckerWarningOptIn.includes(requirementName)) + ) { + return; + } + const arbitraryOptInSlotNames = new Set(overrideOptions.arbitraryOptIn[requirementName] || []); // block AP/IB equivalent courses if disallowTransferCredit - if (!(disallowTransferCredit && courseIsAPIB(courseTaken)) || optInSlotNames) { + if ( + !(disallowTransferCredit && courseIsAPIB(courseTaken)) || + arbitraryOptInSlotNames.size > 0 + ) { for ( let subRequirementIndex = 0; subRequirementIndex < eligibleCourses.length; subRequirementIndex += 1 ) { const slotName = fulfilledBy === 'courses' ? slotNames[subRequirementIndex] : 'Course'; - if (optInSlotNames && (fulfilledBy === 'credits' || optInSlotNames.has(slotName))) { + if (arbitraryOptInSlotNames.has(slotName)) { // the user wants to use this course to override this sub-requirement coursesThatFulfilledSubRequirements[subRequirementIndex].push(courseTaken); subRequirementProgress[subRequirementIndex] += @@ -311,8 +332,7 @@ const computeFulfillmentStatistics = ( // don't break, in case the user wants to override more sub-requirements with the same course } else if ( eligibleCourses[subRequirementIndex].includes(courseTaken.courseId) && - subRequirementProgress[subRequirementIndex] < perSlotMinCount[subRequirementIndex] && - !(optOutSlotNames && (fulfilledBy === 'credits' || optOutSlotNames.has(slotName))) + subRequirementProgress[subRequirementIndex] < perSlotMinCount[subRequirementIndex] ) { // this course is eligible to fulfill this sub-requirement, and the user did not opt out coursesThatFulfilledSubRequirements[subRequirementIndex].push(courseTaken); @@ -357,7 +377,7 @@ export function computeFulfillmentCoursesAndStatistics( requirement: RequirementWithIDSourceType, coursesTaken: readonly CourseTaken[], toggleableRequirementChoices: AppToggleableRequirementChoices, - overriddenFulfillmentChoices: AppOverriddenFulfillmentChoices + overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices ): RequirementFulfillmentStatisticsWithCoursesWithAdditionalRequirements { const spec = getMatchedRequirementFulfillmentSpecification( requirement, @@ -393,6 +413,27 @@ export function computeFulfillmentCoursesAndStatistics( }; } +export function getAllEligibleRelatedRequirementIds( + courseId: number, + groupedRequirements: readonly GroupedRequirementFulfillmentReport[], + toggleableRequirementChoices: AppToggleableRequirementChoices +): readonly string[] { + return groupedRequirements + .flatMap(it => it.reqs) + .flatMap(({ requirement }) => { + const spec = getMatchedRequirementFulfillmentSpecification( + requirement, + toggleableRequirementChoices + ); + if (spec == null) return []; + const allEligibleCourses = spec.eligibleCourses.flat(); + if (allEligibleCourses.includes(courseId) && requirement.checkerWarning == null) { + return [requirement.id]; + } + return []; + }); +} + export function getRelatedUnfulfilledRequirements( { crseId: courseId, @@ -402,7 +443,7 @@ export function getRelatedUnfulfilledRequirements( }: CornellCourseRosterCourse, groupedRequirements: readonly GroupedRequirementFulfillmentReport[], toggleableRequirementChoices: AppToggleableRequirementChoices, - overriddenFulfillmentChoices: AppOverriddenFulfillmentChoices + overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices ): { readonly directlyRelatedRequirements: readonly RequirementWithIDSourceType[]; readonly selfCheckRequirements: readonly RequirementWithIDSourceType[]; @@ -436,7 +477,16 @@ export function getRelatedUnfulfilledRequirements( subRequirement, [...existingCourses, { uniqueId: -1, courseId, code, credits }], toggleableRequirementChoices, - overriddenFulfillmentChoices + { + ...overriddenFulfillmentChoices, + // Very loose choice to make the course count towards fulfillment + // Careful computation of choice is done during actual adding time. + [-1]: { + acknowledgedCheckerWarningOptIn: [subRequirement.id], + optOut: [], + arbitraryOptIn: {}, + }, + } ); if (fulfillmentStatisticsWithNewCourse.minCountFulfilled > existingMinCountFulfilled) { if (subRequirement.checkerWarning == null) { diff --git a/src/requirements/requirement-graph-builder-from-user-data.ts b/src/requirements/requirement-graph-builder-from-user-data.ts index 3d134c8bb..b8f8f73b8 100644 --- a/src/requirements/requirement-graph-builder-from-user-data.ts +++ b/src/requirements/requirement-graph-builder-from-user-data.ts @@ -1,5 +1,5 @@ import { CREDITS_COURSE_ID } from './data/constants'; -import { courseIsAPIB, getUserRequirements } from './requirement-frontend-utils'; +import { getUserRequirements } from './requirement-frontend-utils'; import RequirementFulfillmentGraph from './requirement-graph'; import buildRequirementFulfillmentGraph, { BuildRequirementFulfillmentGraphParameters, @@ -32,10 +32,7 @@ export default function buildRequirementFulfillmentGraphFromUserData( coursesTaken: readonly CourseTaken[], onboardingData: AppOnboardingData, toggleableRequirementChoices: AppToggleableRequirementChoices, - selectableRequirementChoices: AppSelectableRequirementChoices, - overriddenFulfillmentChoices: AppOverriddenFulfillmentChoices, - /** A flag for data migration. Prod code should never use this */ - keepCoursesWithoutDoubleCountingEliminationChoice = false + overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices ): { readonly userRequirements: readonly RequirementWithIDSourceType[]; readonly userRequirementsMap: Readonly>; @@ -62,30 +59,32 @@ export default function buildRequirementFulfillmentGraphFromUserData( }) .filter((it): it is [string, number[]] => it != null) ), - userChoiceOnDoubleCountingElimination: Object.fromEntries( - Object.entries(selectableRequirementChoices).map(([uniqueIDString, requirementID]) => { - const uniqueId = Number.isNaN(uniqueIDString) - ? uniqueIDString - : parseInt(uniqueIDString, 10); - return [uniqueId, requirementID] as const; + userChoiceOnRequirementOverrides: Object.fromEntries( + coursesTaken.flatMap(course => { + const uniqueId = course.uniqueId.toString(); + const choice = overriddenFulfillmentChoices[uniqueId]; + if (choice == null) return []; + return [ + [ + uniqueId, + { + optIn: Array.from( + new Set([ + ...Object.keys(choice.arbitraryOptIn), + ...choice.acknowledgedCheckerWarningOptIn, + ]) + ), + optOut: choice.optOut, + }, + ], + ]; }) ), - userChoiceOnRequirementOverrides: { - ...Object.fromEntries( - coursesTaken - .map(course => { - const uniqueId = course.uniqueId.toString(); - if (!(uniqueId in overriddenFulfillmentChoices)) return null; - const overriddenFulfillments = new Set( - Object.keys(overriddenFulfillmentChoices[uniqueId].optIn) - ); - return [uniqueId, overriddenFulfillments]; - }) - .filter((it): it is [string, Set] => it != null) - ), - }, getAllCoursesThatCanPotentiallySatisfyRequirement: requirementID => { const requirement = userRequirementsMap[requirementID]; + // When a requirement has checker warning, we do not add those edges in phase 1. + // All edges will be explictly opt-in only from stage 3. + if (requirement.checkerWarning != null) return []; let eligibleCoursesList: readonly (readonly number[])[]; switch (requirement.fulfilledBy) { case 'self-check': @@ -104,115 +103,10 @@ export default function buildRequirementFulfillmentGraphFromUserData( } return eligibleCoursesList.flat(); }, - allowDoubleCounting: requirementID => - userRequirementsMap[requirementID].allowCourseDoubleCounting || false, }; const requirementFulfillmentGraph = buildRequirementFulfillmentGraph( - requirementGraphBuilderParameters, - keepCoursesWithoutDoubleCountingEliminationChoice + requirementGraphBuilderParameters ); return { userRequirements, userRequirementsMap, requirementFulfillmentGraph }; } - -export type FirestoreCourseOptInOptOutChoicesBuilder = ( - c: CourseTaken -) => FirestoreCourseOptInOptOutChoices; - -/** - * @returns a function that given a course, return a `FirestoreCourseOptInOptOutChoices`. - * - * Frontend code can use the returned function to compute the equivalent opt-out data, - * and the data migration code can use the returned function to compute opt-out choice for all courses. - * This function should be deleted once we fully finish the migration. - */ -export function getFirestoreCourseOptInOptOutChoicesBuilder( - coursesTaken: readonly CourseTaken[], - onboardingData: AppOnboardingData, - toggleableRequirementChoices: AppToggleableRequirementChoices, - selectableRequirementChoices: AppSelectableRequirementChoices, - overriddenFulfillmentChoices: AppOverriddenFulfillmentChoices -): FirestoreCourseOptInOptOutChoicesBuilder { - // In this graph, each course is connected to all requirements that course can be used to satisfy. - const { - requirementFulfillmentGraph: graphWithoutDoubleCountingAccounted, - userRequirementsMap, - } = buildRequirementFulfillmentGraphFromUserData( - coursesTaken, - onboardingData, - toggleableRequirementChoices, - {}, // Provide no double counting choices, so all the edges will be kept - overriddenFulfillmentChoices, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ true - ); - - // In this graph, each course is only connected to the selected requirement and the requirements - // that allow double counting. - const graphWithDoubleCountingAccounted = buildRequirementFulfillmentGraphFromUserData( - coursesTaken, - onboardingData, - toggleableRequirementChoices, - selectableRequirementChoices, - overriddenFulfillmentChoices, - /* keepCoursesWithoutDoubleCountingEliminationChoice */ false - ).requirementFulfillmentGraph; - - /** - * The table below summerizes the type of all possible requirement-course edges before - * double-counting elimination, and where they will go in the new format. - * - * ------------------------------------------------------------------------------------------------- - * | Requirement Type | edge exists after double-counting elim | where is it in the new format | - * | ---------------- | -------------------------------------- | --------------------------------- | - * | selected | True | implicit (connected by default) | - * | (no warning) | | | - * | | | | - * | selected | True | `acknowledgedCheckerWarningOptIn` | - * | (has warning) | | | - * | | | | - * | allow double | True | implicit (connected by default) | - * | counting | | | - * | | | | - * | not connected | False | `optOut` | - * | (no warning) | | | - * | | | | - * | not connected | False | implicit (unconnected by default) | - * | (has warning) | | | - */ - return function builder(course) { - const requirementsWithDoubleCountingRemoved = graphWithDoubleCountingAccounted.getConnectedRequirementsFromCourse( - course - ); - const allRelevantRequirements = graphWithoutDoubleCountingAccounted.getConnectedRequirementsFromCourse( - course - ); - /** - * complementary == All unconnected requirement after double counting elimination - * - * It's true that - * ``` - * union( - * selectableRequirementChoices[course], - * requirements that allow double counting and the given course can be used to satisfy, - * complementary, - * ) == all requirements that course can be used to satisfy - * ``` - */ - const complementary = new Set(allRelevantRequirements); - requirementsWithDoubleCountingRemoved.forEach(r => complementary.delete(r)); - - // We only need to explicitly opt-out of requirements without checker warnings, since requirement - // with checker warnings need to be explicitly opt-in. - const optOut = Array.from(complementary).filter( - it => userRequirementsMap[it].checkerWarning == null - ); - // Find requirements with checker warnings that needs to be explictly opt-in. - const acknowledgedCheckerWarningOptIn = courseIsAPIB(course) - ? [] - : requirementsWithDoubleCountingRemoved.filter( - it => userRequirementsMap[it].checkerWarning != null - ); - - return { optOut, arbitraryOptIn: {}, acknowledgedCheckerWarningOptIn }; - }; -} diff --git a/src/requirements/requirement-graph-builder.ts b/src/requirements/requirement-graph-builder.ts index 3a9c3a0bb..908f79bb7 100644 --- a/src/requirements/requirement-graph-builder.ts +++ b/src/requirements/requirement-graph-builder.ts @@ -24,22 +24,21 @@ export type BuildRequirementFulfillmentGraphParameters< */ readonly userChoiceOnFulfillmentStrategy: Readonly>; /** - * The mapping from course's unique ID to requirement. - * It describes how the user decides how a course is used to satisfy requirements. - * Suppose a course with unique ID c can be used to satisfy both r1 and r2, but the - * requirements do not allow double counting, then a mapping `c => r1` means that we should keep the - * (r1, c) edge in the graph and drop the (r2, c) edge. - */ - readonly userChoiceOnDoubleCountingElimination: Readonly>; - /** - * The mapping from course's unique ID to requirement override (opt-in) options. + * The mapping from course's unique ID to requirement override opt-in and opt-out options. * It describes how the user wants to use a course to override requirements. - * This handles AP/IB overrides, as well as general overrides. + * This handles AP/IB overrides, as well as general overrides. This can encode double counting + * elimination as opt-out. * The granularity of optIn/optOut being slot-specific requires the actual * slot computation to be handled in the frontend computation. When building the - * graph, only optIn choices are relevant (to add the extra edges). + * graph, only optIn/optOut choices are relevant (to add the extra edges). */ - readonly userChoiceOnRequirementOverrides: Readonly>>; + readonly userChoiceOnRequirementOverrides: { + readonly [uniqueId: string]: { + // Including both acknowledged and arbitrary opt-in. + readonly optIn: readonly Requirement[]; + readonly optOut: readonly Requirement[]; + }; + }; /** * Naively give a list of courses ID that can satisfy a requirement. Most of the time this function * should just return the pre-computed eligible course id list. For requirements have multiple @@ -48,29 +47,21 @@ export type BuildRequirementFulfillmentGraphParameters< readonly getAllCoursesThatCanPotentiallySatisfyRequirement: ( requirement: Requirement ) => readonly number[]; - /** - * Report whether a requirement allows a course connected to it to also be used to fulfill some - * other requirement. - */ - readonly allowDoubleCounting: (requirement: Requirement) => boolean; }; const buildRequirementFulfillmentGraph = < Requirement extends string, Course extends CourseForRequirementGraph ->( - { - requirements, - userCourses, - userChoiceOnFulfillmentStrategy, - userChoiceOnDoubleCountingElimination, - userChoiceOnRequirementOverrides, - getAllCoursesThatCanPotentiallySatisfyRequirement, - allowDoubleCounting, - }: BuildRequirementFulfillmentGraphParameters, - /** A flag for testing. Prod code should never use this */ - keepCoursesWithoutDoubleCountingEliminationChoice = false -): RequirementFulfillmentGraph => { +>({ + requirements, + userCourses, + userChoiceOnFulfillmentStrategy, + userChoiceOnRequirementOverrides, + getAllCoursesThatCanPotentiallySatisfyRequirement, +}: BuildRequirementFulfillmentGraphParameters): RequirementFulfillmentGraph< + Requirement, + Course +> => { const graph = new RequirementFulfillmentGraph(); const userCourseCourseIDToCourseMap = new Map(); userCourses.forEach(course => { @@ -110,34 +101,22 @@ const buildRequirementFulfillmentGraph = < } ); - // Phase 3: Respect user's choices on double-counted courses. - userCourses.forEach(({ uniqueId }) => { + // 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 chosenRequirement = userChoiceOnDoubleCountingElimination[uniqueId]; - graph.getConnectedRequirementsFromCourse({ uniqueId }).forEach(connectedRequirement => { - if (allowDoubleCounting(connectedRequirement)) return; - // keepCoursesWithoutDoubleCountingEliminationChoice is used to avoid removing edges when - // the user choice is null on the course. It is used to test phase-1 and phase-2 output - // independently. - if (keepCoursesWithoutDoubleCountingEliminationChoice && chosenRequirement == null) return; - if (connectedRequirement !== chosenRequirement) { - graph.removeEdge(connectedRequirement, { uniqueId }); - } + const userChoiceOnOptInOptOutCourse = userChoiceOnRequirementOverrides[uniqueId]; + if (userChoiceOnOptInOptOutCourse == null) return; + userChoiceOnOptInOptOutCourse.optIn.forEach(optedInRequirement => { + graph.addEdge(optedInRequirement, course); + }); + userChoiceOnOptInOptOutCourse.optOut.forEach(optedInRequirement => { + graph.removeEdge(optedInRequirement, course); }); - }); - - // Phase 4: Respect user's choices on overrides (optIn only). - userCourses.forEach(course => { - const { uniqueId } = course; - if (uniqueId in userChoiceOnRequirementOverrides) { - userChoiceOnRequirementOverrides[uniqueId].forEach(requirement => { - graph.addEdge(requirement, course); - }); - } }); // Phase MAX_INT: PROFIT! diff --git a/src/store.ts b/src/store.ts index 5bcd199e7..b1faced90 100644 --- a/src/store.ts +++ b/src/store.ts @@ -28,16 +28,6 @@ type DerivedCoursesData = { readonly courseToSemesterMap: Readonly>; }; -/** - * Some course data that can be derived from selectable requirement choices, but added to the global store - * for efficiency and ease of access. - * This should be used for self-check requirements and are not used in the requirement graph. - */ -type DerivedSelectableRequirementData = { - // Mapping from requirement ID to the user-selected courses that fulfill the requirement. - readonly requirementToCoursesMap: Readonly>; -}; - /** * Some AP/IB equivalent course data that can be derived from onboarding data, but added to the global store * for efficiency and ease of access. @@ -56,10 +46,8 @@ export type VuexStoreState = { semesters: readonly FirestoreSemester[]; orderByNewest: boolean; derivedCoursesData: DerivedCoursesData; - derivedSelectableRequirementData: DerivedSelectableRequirementData; derivedAPIBEquivalentCourseData: DerivedAPIBEquivalentCourseData; toggleableRequirementChoices: AppToggleableRequirementChoices; - selectableRequirementChoices: AppSelectableRequirementChoices; overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices; userRequirementsMap: Readonly>; requirementFulfillmentGraph: RequirementFulfillmentGraph; @@ -97,15 +85,11 @@ const store: TypedVuexStore = new TypedVuexStore({ courseMap: {}, courseToSemesterMap: {}, }, - derivedSelectableRequirementData: { - requirementToCoursesMap: {}, - }, derivedAPIBEquivalentCourseData: { examToUniqueIdsMap: {}, uniqueIdToExamMap: {}, }, toggleableRequirementChoices: {}, - selectableRequirementChoices: {}, overriddenFulfillmentChoices: {}, userRequirementsMap: {}, // It won't be null once the app loads. @@ -136,12 +120,6 @@ const store: TypedVuexStore = new TypedVuexStore({ setDerivedCourseData(state: VuexStoreState, data: DerivedCoursesData) { state.derivedCoursesData = data; }, - setDerivedSelectableRequirementData( - state: VuexStoreState, - data: DerivedSelectableRequirementData - ) { - state.derivedSelectableRequirementData = data; - }, setDerivedAPIBEquivalentCourseData( state: VuexStoreState, data: DerivedAPIBEquivalentCourseData @@ -154,12 +132,6 @@ const store: TypedVuexStore = new TypedVuexStore({ ) { state.toggleableRequirementChoices = toggleableRequirementChoices; }, - setSelectableRequirementChoices( - state: VuexStoreState, - selectableRequirementChoices: AppSelectableRequirementChoices - ) { - state.selectableRequirementChoices = selectableRequirementChoices; - }, setOverriddenFulfillmentChoices( state: VuexStoreState, overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices @@ -221,21 +193,6 @@ const autoRecomputeDerivedData = (): (() => void) => }; store.commit('setDerivedCourseData', derivedCourseData); } - if (payload.type === 'setSelectableRequirementChoices') { - const requirementToCoursesMap: Record = {}; - Object.entries(state.selectableRequirementChoices) - .sort((a, b) => parseInt(a[1], 10) - parseInt(b[1], 10)) - .forEach(([courseUniqueId, reqId]) => { - const course: FirestoreSemesterCourse = - state.derivedCoursesData.courseMap[parseInt(courseUniqueId, 10)]; - if (course) - requirementToCoursesMap[reqId] = [...(requirementToCoursesMap[reqId] || []), course]; - }); - const derivedSelectableRequirementData: DerivedSelectableRequirementData = { - requirementToCoursesMap, - }; - store.commit('setDerivedSelectableRequirementData', derivedSelectableRequirementData); - } if (payload.type === 'setOnboardingData') { const examToUniqueIdsMap: Record> = {}; const uniqueIdToExamMap: Record = {}; @@ -262,7 +219,6 @@ const autoRecomputeDerivedData = (): (() => void) => payload.type === 'setOnboardingData' || payload.type === 'setSemesters' || payload.type === 'setToggleableRequirementChoices' || - payload.type === 'setSelectableRequirementChoices' || payload.type === 'setOverriddenFulfillmentChoices' ) { if (state.onboardingData.college !== '') { @@ -272,8 +228,7 @@ const autoRecomputeDerivedData = (): (() => void) => state.semesters, state.onboardingData, state.toggleableRequirementChoices, - state.selectableRequirementChoices, - /* deprecated AppOverriddenFulfillmentChoices */ {} + state.overriddenFulfillmentChoices ) ); } @@ -288,7 +243,6 @@ export const initializeFirestoreListeners = (onLoad: () => void): (() => void) = let semestersInitialLoadFinished = false; let orderByNewestInitialLoadFinished = false; let toggleableRequirementChoiceInitialLoadFinished = false; - let selectableRequirementChoiceInitialLoadFinished = false; let overriddenFulfillmentChoiceInitialLoadFinished = false; let subjectColorInitialLoadFinished = false; let uniqueIncrementerInitialLoadFinished = false; @@ -302,7 +256,6 @@ export const initializeFirestoreListeners = (onLoad: () => void): (() => void) = semestersInitialLoadFinished && orderByNewestInitialLoadFinished && toggleableRequirementChoiceInitialLoadFinished && - selectableRequirementChoiceInitialLoadFinished && overriddenFulfillmentChoiceInitialLoadFinished && subjectColorInitialLoadFinished && uniqueIncrementerInitialLoadFinished && @@ -370,14 +323,6 @@ export const initializeFirestoreListeners = (onLoad: () => void): (() => void) = toggleableRequirementChoiceInitialLoadFinished = true; emitOnLoadWhenLoaded(); }); - const selectableRequirementChoiceUnsubscriber = fb.selectableRequirementChoicesCollection - .doc(simplifiedUser.email) - .onSnapshot(snapshot => { - const selectableRequirementChoices = snapshot.data() || {}; - store.commit('setSelectableRequirementChoices', selectableRequirementChoices); - selectableRequirementChoiceInitialLoadFinished = true; - emitOnLoadWhenLoaded(); - }); const overriddenFulfillmentChoiceUnsubscriber = fb.overriddenFulfillmentChoicesCollection .doc(simplifiedUser.email) .onSnapshot(snapshot => { @@ -412,7 +357,6 @@ export const initializeFirestoreListeners = (onLoad: () => void): (() => void) = userNameUnsubscriber(); onboardingDataUnsubscriber(); toggleableRequirementChoiceUnsubscriber(); - selectableRequirementChoiceUnsubscriber(); overriddenFulfillmentChoiceUnsubscriber(); uniqueIncrementerUnsubscriber(); derivedDataComputationUnsubscriber(); diff --git a/src/user-data.d.ts b/src/user-data.d.ts index a3fcb4878..7f08a84cb 100644 --- a/src/user-data.d.ts +++ b/src/user-data.d.ts @@ -46,8 +46,6 @@ type FirestoreTransferExam = { readonly examType: TransferExamType; readonly score: number; readonly subject: string; - readonly optIn?: FirestoreAPIBOverriddenFulfillments; - readonly optOut?: FirestoreAPIBOverriddenFulfillments; }; type FirestoreCollegeMajorMinorOrGrad = { readonly acronym: string }; @@ -200,21 +198,3 @@ type AppBottomBarCourse = { /** Map from requirement ID to option chosen */ type AppToggleableRequirementChoices = Readonly>; - -/** Map from course's unique ID to requirement ID */ -type AppSelectableRequirementChoices = Readonly>; - -/** - * @deprecated replaced by `FirestoreOverriddenFulfillmentChoices` - * - * Map from course's unique ID to override options. - */ -type AppOverriddenFulfillmentChoices = Readonly< - Record< - string, - { - readonly optIn: Record>; - readonly optOut: Record>; - } - > ->;