Fix requirements double counting

src/components/Modals/NewCourse/NewCourseModal.vue

@@ -23,15 +23,16 @@
       data-cyId="newCourse-dropdown"
     />
     <div v-else class="selected-course" data-cyId="newCourse-selectedCourse">
-      {{ selectedCourse.subject }} {{ selectedCourse.catalogNbr }}: {{ selectedCourse.titleLong }}
+      {{ selectedCourse.subject }} {{ selectedCourse.catalogNbr }}:
+      {{ selectedCourse.titleLong }}
     </div>
     <div v-if="selectedCourse != null">
       <!-- if a course is selected -->
       <selected-requirement-editor
         :key="courseSelectorKey"
         :editMode="editMode"
         :selectedRequirementID="selectedRequirementID"
-        :requirementsThatAllowDoubleCounting="requirementsThatAllowDoubleCounting"
+        :automaticallyFulfilledRequirements="automaticallyFulfilledRequirements"
         :relatedRequirements="relatedRequirements"
         :potentialRequirements="selfCheckRequirements"
         @on-selected-change="onSelectedChange"
@@ -48,7 +49,10 @@ import TeleportModal from '@/components/Modals/TeleportModal.vue';
 import CourseSelector from '@/components/Modals/NewCourse/CourseSelector.vue';
 
 import store from '@/store';
-import { getRelatedUnfulfilledRequirements } from '@/requirements/requirement-frontend-utils';
+import {
+  getRelatedRequirementIdsForCourseOptOut,
+  getRelatedUnfulfilledRequirements,
+} from '@/requirements/requirement-frontend-utils';
 
 export default defineComponent({
   components: { CourseSelector, TeleportModal, SelectedRequirementEditor },
@@ -61,7 +65,7 @@ export default defineComponent({
     return {
       selectedCourse: null as CornellCourseRosterCourse | null,
       selectedRequirementID: '',
-      requirementsThatAllowDoubleCounting: [] as readonly string[],
+      automaticallyFulfilledRequirements: [] as readonly string[],
       // relatedRequirements : the requirements that don't allow double counting
       relatedRequirements: [] as readonly RequirementWithIDSourceType[],
       selfCheckRequirements: [] as readonly RequirementWithIDSourceType[],
@@ -89,38 +93,31 @@ export default defineComponent({
     },
     getReqsRelatedToCourse(selectedCourse: CornellCourseRosterCourse) {
       const {
-        directlyRelatedRequirements,
+        relatedRequirements,
         selfCheckRequirements,
+        automaticallyFulfilledRequirements,
       } = getRelatedUnfulfilledRequirements(
         selectedCourse,
         store.state.groupedRequirementFulfillmentReport,
         store.state.toggleableRequirementChoices,
-        store.state.overriddenFulfillmentChoices
+        store.state.overriddenFulfillmentChoices,
+        store.state.userRequirementsMap
+      );
+      const automaticallyFulfilledRequirementIds = new Set(
+        automaticallyFulfilledRequirements.map(({ id }) => id)
       );
 
-      const requirementsThatAllowDoubleCounting: string[] = [];
-      const relatedRequirements: RequirementWithIDSourceType[] = [];
-      directlyRelatedRequirements.forEach(it => {
-        if (it.allowCourseDoubleCounting) {
-          requirementsThatAllowDoubleCounting.push(it.name);
-        } else {
-          relatedRequirements.push(it);
-        }
-      });
-      const selfCheckRequirementsThatDoesNotAllowDoubleCounting: RequirementWithIDSourceType[] = [];
-      selfCheckRequirements.forEach(it => {
-        if (it.allowCourseDoubleCounting) {
-          requirementsThatAllowDoubleCounting.push(it.name);
-        } else {
-          selfCheckRequirementsThatDoesNotAllowDoubleCounting.push(it);
-        }
-      });
-
-      this.requirementsThatAllowDoubleCounting = requirementsThatAllowDoubleCounting;
-      this.relatedRequirements = relatedRequirements;
-      this.selfCheckRequirements = selfCheckRequirementsThatDoesNotAllowDoubleCounting;
-      if (relatedRequirements.length > 0) {
-        this.selectedRequirementID = relatedRequirements[0].id;
+      this.automaticallyFulfilledRequirements = automaticallyFulfilledRequirements.map(
+        ({ name }) => name
+      );
+      this.relatedRequirements = relatedRequirements.filter(
+        req => !automaticallyFulfilledRequirementIds.has(req.id)
+      );
+      this.selfCheckRequirements = selfCheckRequirements.filter(
+        req => !automaticallyFulfilledRequirementIds.has(req.id)
+      );
+      if (this.relatedRequirements.length > 0) {
+        this.selectedRequirementID = this.relatedRequirements[0].id;
       } else {
         this.selectedRequirementID = '';
       }
@@ -135,10 +132,13 @@ export default defineComponent({
     addCourse() {
       if (this.selectedCourse == null) return;
       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),
+        optOut: getRelatedRequirementIdsForCourseOptOut(
+          this.selectedCourse.crseId,
+          this.selectedRequirementID,
+          store.state.groupedRequirementFulfillmentReport,
+          store.state.toggleableRequirementChoices,
+          store.state.userRequirementsMap
+        ),
         // Only include the selected requirement from opt-in.
         acknowledgedCheckerWarningOptIn: this.selfCheckRequirements
           .filter(it => it.id === this.selectedRequirementID)

src/components/Modals/NewCourse/SelectedRequirementEditor.vue

@@ -2,15 +2,15 @@
   <div>
     <div
       v-if="
-        editMode ? requirementsThatAllowDoubleCounting.length > 0 : chosenRequirementText.length > 0
+        editMode ? automaticallyFulfilledRequirements.length > 0 : chosenRequirementText.length > 0
       "
     >
       <div class="newCourse-title">
         This class automatically fulfills the following requirement(s):
       </div>
       <div class="newCourse-requirements-container">
         <div class="newCourse-requirements" data-cyId="newCourse-requirements">
-          {{ editMode ? requirementsThatAllowDoubleCounting.join(', ') : chosenRequirementText }}
+          {{ editMode ? automaticallyFulfilledRequirements.join(', ') : chosenRequirementText }}
         </div>
       </div>
     </div>
@@ -82,7 +82,7 @@ export default defineComponent({
   props: {
     editMode: { type: Boolean, required: true },
     selectedRequirementID: { type: String, required: true },
-    requirementsThatAllowDoubleCounting: {
+    automaticallyFulfilledRequirements: {
       type: Array as PropType<readonly string[]>,
       required: true,
     },
@@ -104,12 +104,12 @@ export default defineComponent({
   computed: {
     chosenRequirementText(): string {
       if (this.selectedRequirementID === '') {
-        return this.requirementsThatAllowDoubleCounting.join(', ');
+        return this.automaticallyFulfilledRequirements.join(', ');
       }
       const chosenRequirementNames = [...this.relatedRequirements, ...this.potentialRequirements]
         .filter(it => it.id === this.selectedRequirementID)
         .map(it => it.name);
-      return [...this.requirementsThatAllowDoubleCounting, ...chosenRequirementNames].join(', ');
+      return [...this.automaticallyFulfilledRequirements, ...chosenRequirementNames].join(', ');
     },
     // nonAutoRequirements = relatedRequirements + potentialRequirements
     // All the requirements that are not automatically associated with the course

src/components/Requirements/IncompleteSelfCheck.vue

@@ -45,7 +45,7 @@ import {
 } from '@/global-firestore-data';
 import {
   canFulfillChecker,
-  getAllEligibleRelatedRequirementIds,
+  getRelatedRequirementIdsForCourseOptOut,
 } from '@/requirements/requirement-frontend-utils';
 
 import NewSelfCheckCourseModal from '@/components/Modals/NewCourse/NewSelfCheckCourseModal.vue';
@@ -101,6 +101,8 @@ export default defineComponent({
             return;
           }
 
+          /* TODO @bshen fix .allowCourseDoubleCounting flag
+             we should allow the constraint violation to be broken, i.e. don't return early */
           const currentRequirementAllowDoubleCounting =
             store.state.userRequirementsMap[this.subReqCourseId]?.allowCourseDoubleCounting;
           const allOtherRequirementsAllowDoubleCounting = store.state.safeRequirementFulfillmentGraph
@@ -139,18 +141,21 @@ export default defineComponent({
     },
     addExistingCourse(option: string) {
       this.showDropdown = false;
-      updateRequirementChoice(this.selfCheckCourses[option].uniqueID, choice => ({
+      const { uniqueID, crseId } = this.selfCheckCourses[option];
+      updateRequirementChoice(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,
+        optOut: getRelatedRequirementIdsForCourseOptOut(
+          crseId,
+          this.subReqId,
           store.state.groupedRequirementFulfillmentReport,
-          store.state.toggleableRequirementChoices
+          store.state.toggleableRequirementChoices,
+          store.state.userRequirementsMap
         ),
       }));
     },
@@ -170,10 +175,12 @@ export default defineComponent({
           // 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(
+          optOut: getRelatedRequirementIdsForCourseOptOut(
             newCourse.crseId,
+            this.subReqId,
             store.state.groupedRequirementFulfillmentReport,
-            store.state.toggleableRequirementChoices
+            store.state.toggleableRequirementChoices,
+            store.state.userRequirementsMap
           ),
         }),
         this.$gtag

src/components/Semester/Semester.vue

@@ -154,7 +154,7 @@ import {
   updateRequirementChoices,
 } from '@/global-firestore-data';
 import store, { updateSubjectColorData } from '@/store';
-import { getAllEligibleRelatedRequirementIds } from '@/requirements/requirement-frontend-utils';
+import { getRelatedRequirementIdsForCourseOptOut } from '@/requirements/requirement-frontend-utils';
 
 type ComponentRef = { $el: HTMLDivElement };
 
@@ -280,11 +280,13 @@ export default defineComponent({
             // 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(
+            const optOut = getRelatedRequirementIdsForCourseOptOut(
               crseId,
+              requirementID,
               store.state.groupedRequirementFulfillmentReport,
-              store.state.toggleableRequirementChoices
-            ).filter(it => it !== requirementID);
+              store.state.toggleableRequirementChoices,
+              store.state.userRequirementsMap
+            );
             choices[uniqueID] = { ...choice, optOut };
           });
           return choices;

src/requirements/requirement-frontend-utils.ts

@@ -1,6 +1,7 @@
 import requirementJson from './typed-requirement-json';
 import specialized from './specialize';
 import { examCourseIds } from './requirement-exam-mapping';
+import { getConstraintViolationsForSingleCourse } from './requirement-constraints-utils';
 
 /**
  * A collection of helper functions
@@ -444,10 +445,12 @@ export function computeFulfillmentCoursesAndStatistics(
   };
 }
 
-export function getAllEligibleRelatedRequirementIds(
+export function getRelatedRequirementIdsForCourseOptOut(
   courseId: number,
+  associatedRequirementId: string,
   groupedRequirements: readonly GroupedRequirementFulfillmentReport[],
-  toggleableRequirementChoices: AppToggleableRequirementChoices
+  toggleableRequirementChoices: AppToggleableRequirementChoices,
+  userRequirementsMap: Readonly<Record<string, RequirementWithIDSourceType>>
 ): readonly string[] {
   const requirements = groupedRequirements
     .flatMap(it => it.reqs)
@@ -458,12 +461,33 @@ export function getAllEligibleRelatedRequirementIds(
       );
       if (spec == null) return [];
       const allEligibleCourses = spec.eligibleCourses.flat();
-      if (allEligibleCourses.includes(courseId) && requirement.checkerWarning == null) {
+      if (
+        requirement.id === associatedRequirementId ||
+        (allEligibleCourses.includes(courseId) && requirement.checkerWarning == null)
+      ) {
         return [requirement.id];
       }
       return [];
     });
-  return requirements;
+  // only return the requirements that are in a constraint violation
+  const uniqueId = -1; // dummy unique id
+  const { courseToRequirementsInConstraintViolations } = getConstraintViolationsForSingleCourse(
+    { uniqueId },
+    requirements,
+    (reqA, reqB) =>
+      allowCourseDoubleCountingBetweenRequirements(
+        userRequirementsMap[reqA],
+        userRequirementsMap[reqB]
+      )
+  );
+  const optOut = new Set<string>();
+  courseToRequirementsInConstraintViolations.get(uniqueId)?.forEach(requirementGroup => {
+    if (requirementGroup.includes(associatedRequirementId)) {
+      requirementGroup.forEach(requirement => optOut.add(requirement));
+    }
+  });
+  // order does not need to be preserved
+  return Array.from(optOut).filter(it => it !== associatedRequirementId);
 }
 
 export function getRelatedUnfulfilledRequirements(
@@ -475,13 +499,15 @@ export function getRelatedUnfulfilledRequirements(
   }: CornellCourseRosterCourse,
   groupedRequirements: readonly GroupedRequirementFulfillmentReport[],
   toggleableRequirementChoices: AppToggleableRequirementChoices,
-  overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices
+  overriddenFulfillmentChoices: FirestoreOverriddenFulfillmentChoices,
+  userRequirementsMap: Readonly<Record<string, RequirementWithIDSourceType>>
 ): {
-  readonly directlyRelatedRequirements: readonly RequirementWithIDSourceType[];
+  readonly relatedRequirements: readonly RequirementWithIDSourceType[];
   readonly selfCheckRequirements: readonly RequirementWithIDSourceType[];
+  readonly automaticallyFulfilledRequirements: readonly RequirementWithIDSourceType[];
 } {
   const code = `${subject} ${catalogNbr}`;
-  const directlyRelatedRequirements: RequirementWithIDSourceType[] = [];
+  const relatedRequirements: RequirementWithIDSourceType[] = [];
   const selfCheckRequirements: RequirementWithIDSourceType[] = [];
   for (let i = 0; i < groupedRequirements.length; i += 1) {
     const subreqs = groupedRequirements[i].reqs.filter(
@@ -501,9 +527,6 @@ export function getRelatedUnfulfilledRequirements(
         toggleableRequirementChoices
       );
       // potential self-check requirements
-      if (requirementSpec == null && !subRequirement.allowCourseDoubleCounting) {
-        selfCheckRequirements.push(subRequirement);
-      }
       if (requirementSpec != null) {
         const allEligibleCourses = requirementSpec.eligibleCourses.flat();
         if (allEligibleCourses.includes(courseId)) {
@@ -524,14 +547,30 @@ export function getRelatedUnfulfilledRequirements(
           );
           if (fulfillmentStatisticsWithNewCourse.minCountFulfilled > existingMinCountFulfilled) {
             if (subRequirement.checkerWarning == null) {
-              directlyRelatedRequirements.push(subRequirement);
+              relatedRequirements.push(subRequirement);
             } else {
               selfCheckRequirements.push(subRequirement);
             }
           }
         }
+      } else {
+        selfCheckRequirements.push(subRequirement);
       }
     }
   }
-  return { directlyRelatedRequirements, selfCheckRequirements };
+  const allRequirements = [...relatedRequirements, ...selfCheckRequirements];
+  const { requirementsThatDoNotAllowDoubleCounting } = getConstraintViolationsForSingleCourse(
+    { uniqueId: -1 },
+    allRequirements.map(({ id }) => id),
+    (reqA, reqB) =>
+      allowCourseDoubleCountingBetweenRequirements(
+        userRequirementsMap[reqA],
+        userRequirementsMap[reqB]
+      )
+  );
+  const automaticallyFulfilledRequirements = relatedRequirements.filter(
+    ({ id }) => !requirementsThatDoNotAllowDoubleCounting.has(id)
+  );
+
+  return { relatedRequirements, selfCheckRequirements, automaticallyFulfilledRequirements };
 }