refactor: make CourseModal less dependent on the catalog listing stru…

…cture
coursetable · May 12, 2024 · 9142d17 · 9142d17
1 parent 7c9005a
commit 9142d17
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 55 deletions.
diff --git a/frontend/src/components/CourseModal/CourseModal.tsx b/frontend/src/components/CourseModal/CourseModal.tsx
@@ -10,8 +10,12 @@ import { toast } from 'react-toastify';
 import { CUR_YEAR } from '../../config';
 import { useFerry } from '../../contexts/ferryContext';
 import { useUser } from '../../contexts/userContext';
-import type { Listing } from '../../queries/api';
-import type { Season, Crn } from '../../queries/graphql-types';
+import type {
+  Season,
+  Crn,
+  ExtraInfo,
+  TimesByDay,
+} from '../../queries/graphql-types';
 import { extraInfo } from '../../utilities/constants';
 import { toSeasonString, truncatedText } from '../../utilities/course';
 import { suspended, useCourseModalLink } from '../../utilities/display';
@@ -20,7 +24,23 @@ import { TextComponent } from '../Typography';
 import WorksheetToggleButton from '../Worksheet/WorksheetToggleButton';
 import styles from './CourseModal.module.css';
 
-function ShareButton({ listing }: { readonly listing: Listing }) {
+export type CourseModalHeaderData = {
+  readonly season_code: Season;
+  readonly crn: Crn;
+  readonly title: string;
+  readonly course_code: string;
+  readonly all_course_codes: string[];
+  readonly section: string;
+  readonly skills: string[];
+  readonly areas: string[];
+  readonly extra_info: ExtraInfo;
+  readonly description: string;
+  readonly times_by_day: TimesByDay;
+  readonly same_course_id: number;
+  readonly professor_ids: string[];
+};
+
+function ShareButton({ listing }: { readonly listing: CourseModalHeaderData }) {
   const copyToClipboard = () => {
     const textToCopy = `${listing.course_code} -- CourseTable: ${window.location.href}`;
     navigator.clipboard.writeText(textToCopy).then(
@@ -49,7 +69,7 @@ function MoreButton({
   listing,
   hide,
 }: {
-  readonly listing: Listing;
+  readonly listing: CourseModalHeaderData;
   readonly hide: () => void;
 }) {
   return (
@@ -140,7 +160,7 @@ function CourseModal() {
 
   const [view, setView] = useState<'overview' | 'evals'>('overview');
   // Stack for listings that the user has viewed
-  const [history, setHistory] = useState<Listing[]>([]);
+  const [history, setHistory] = useState<CourseModalHeaderData[]>([]);
   useEffect(() => {
     if (history.length !== 0) return;
     const courseModal = searchParams.get('course-modal');
@@ -260,7 +280,7 @@ function CourseModal() {
                 return;
               setHistory([...history, l]);
             }}
-            listing={listing}
+            header={listing}
           />
         ) : (
           <CourseModalEvaluations

diff --git a/frontend/src/components/CourseModal/CourseModalOverview.tsx b/frontend/src/components/CourseModal/CourseModalOverview.tsx
@@ -1,5 +1,6 @@
 import { Row, Col, Modal } from 'react-bootstrap';
 
+import type { CourseModalHeaderData } from './CourseModal';
 import OverviewInfo from './OverviewInfo';
 import OverviewRatings from './OverviewRatings';
 
@@ -8,50 +9,58 @@ import {
   useSameCourseOrProfOfferingsQuery,
   type SameCourseOrProfOfferingsQuery,
 } from '../../generated/graphql';
-import type { Listing } from '../../queries/api';
 import Spinner from '../Spinner';
 import './react-multi-toggle-override.css';
 
+export type ListingInfo = SameCourseOrProfOfferingsQuery['self'][number];
+
 export type RelatedListingInfo =
-  SameCourseOrProfOfferingsQuery['computed_listing_info'][number];
+  SameCourseOrProfOfferingsQuery['others'][number];
 
 function CourseModalOverview({
   gotoCourse,
-  listing,
+  header,
 }: {
-  readonly gotoCourse: (x: Listing) => void;
-  readonly listing: Listing;
+  readonly gotoCourse: (x: CourseModalHeaderData) => void;
+  readonly header: CourseModalHeaderData;
 }) {
   const { user } = useUser();
 
   const { data, loading, error } = useSameCourseOrProfOfferingsQuery({
     variables: {
+      seasonCode: header.season_code,
+      crn: header.crn,
       hasEval: Boolean(user.hasEvals),
-      same_course_id: listing.same_course_id,
-      professor_ids: listing.professor_ids,
+      same_course_id: header.same_course_id,
+      professor_ids: header.professor_ids,
     },
   });
 
   // Wait until data is fetched
-  if (loading || error) {
+  if (loading || error || !data) {
     return (
       <Modal.Body>
         <Spinner />
       </Modal.Body>
     );
   }
 
+  const {
+    self: [listing],
+    others,
+  } = data;
+
   return (
     <Modal.Body>
       <Row className="m-auto">
         <Col md={7} className="px-0 mt-0 mb-3">
-          <OverviewInfo listing={listing} data={data} />
+          <OverviewInfo listing={listing!} others={others} />
         </Col>
         <Col md={5} className="px-0 my-0">
           <OverviewRatings
             gotoCourse={gotoCourse}
-            data={data}
-            listing={listing}
+            listing={listing!}
+            others={others}
           />
         </Col>
       </Row>

diff --git a/frontend/src/components/CourseModal/OverviewInfo.tsx b/frontend/src/components/CourseModal/OverviewInfo.tsx
@@ -15,11 +15,10 @@ import { MdExpandMore, MdExpandLess } from 'react-icons/md';
 import LinesEllipsis from 'react-lines-ellipsis';
 import responsiveHOC from 'react-lines-ellipsis/lib/responsiveHOC';
 
-import type { RelatedListingInfo } from './CourseModalOverview';
+import type { ListingInfo, RelatedListingInfo } from './CourseModalOverview';
 
 import { CUR_SEASON } from '../../config';
 import { useSearch } from '../../contexts/searchContext';
-import type { SameCourseOrProfOfferingsQuery } from '../../generated/graphql';
 import type { Listing } from '../../queries/api';
 import type { Weekdays } from '../../queries/graphql-types';
 import { ratingColormap } from '../../utilities/constants';
@@ -193,17 +192,16 @@ function DataField({
 }
 
 function Syllabus({
-  data,
   listing,
+  others,
 }: {
-  readonly data: SameCourseOrProfOfferingsQuery | undefined;
-  readonly listing: Listing;
+  readonly listing: ListingInfo;
+  readonly others: RelatedListingInfo[];
 }) {
   const pastSyllabi = useMemo(() => {
-    if (!data) return [];
     // Remove duplicates by syllabus URL
     const courseBySyllabus = new Map<string, RelatedListingInfo>();
-    for (const course of data.computed_listing_info) {
+    for (const course of others) {
       if (
         course.same_course_id !== listing.same_course_id ||
         !course.syllabus_url
@@ -217,7 +215,7 @@ function Syllabus({
         b.season_code.localeCompare(a.season_code, 'en-US') ||
         parseInt(a.section, 10) - parseInt(b.section, 10),
     );
-  }, [data, listing.same_course_id]);
+  }, [others, listing.same_course_id]);
 
   return (
     <div className="mt-2">
@@ -266,11 +264,11 @@ function Syllabus({
 }
 
 function Professors({
-  data,
   listing,
+  others,
 }: {
-  readonly data: SameCourseOrProfOfferingsQuery | undefined;
-  readonly listing: Listing;
+  readonly listing: ListingInfo;
+  readonly others: RelatedListingInfo[];
 }) {
   const profInfo = useMemo(() => {
     const profInfo = new Map(
@@ -281,8 +279,7 @@ function Professors({
     );
     // Only count cross-listed courses once per season
     const countedCourses = new Set<string>();
-    if (!data) return profInfo;
-    for (const season of data.computed_listing_info) {
+    for (const season of others) {
       if (countedCourses.has(`${season.season_code}-${season.course_code}`))
         continue;
       if (!season.professor_info) continue;
@@ -299,7 +296,7 @@ function Professors({
       });
     }
     return profInfo;
-  }, [data, listing]);
+  }, [others, listing]);
 
   return (
     <DataField
@@ -325,7 +322,7 @@ function Professors({
   );
 }
 
-function TimeLocation({ listing }: { readonly listing: Listing }) {
+function TimeLocation({ listing }: { readonly listing: ListingInfo }) {
   const locations = new Map<string, string>();
   const times = new Map<string, Set<Weekdays>>();
   for (const [day, info] of Object.entries(listing.times_by_day)) {
@@ -385,10 +382,10 @@ function TimeLocation({ listing }: { readonly listing: Listing }) {
 
 function OverviewInfo({
   listing,
-  data,
+  others,
 }: {
-  readonly listing: Listing;
-  readonly data: SameCourseOrProfOfferingsQuery | undefined;
+  readonly listing: ListingInfo;
+  readonly others: RelatedListingInfo[];
 }) {
   const { numFriends } = useSearch();
   const alsoTaking = [
@@ -400,8 +397,8 @@ function OverviewInfo({
       {listing.requirements && (
         <div className={styles.requirements}>{listing.requirements}</div>
       )}
-      <Syllabus data={data} listing={listing} />
-      <Professors data={data} listing={listing} />
+      <Syllabus listing={listing} others={others} />
+      <Professors listing={listing} others={others} />
       <TimeLocation listing={listing} />
       <DataField name="Section" value={listing.section} />
       <DataField

diff --git a/frontend/src/components/CourseModal/OverviewRatings.tsx b/frontend/src/components/CourseModal/OverviewRatings.tsx
@@ -4,9 +4,10 @@ import clsx from 'clsx';
 import { Row, Col, OverlayTrigger, Tooltip } from 'react-bootstrap';
 import MultiToggle from 'react-multi-toggle';
 
+import type { ListingInfo, RelatedListingInfo } from './CourseModalOverview';
+
 import { CUR_YEAR } from '../../config';
 import { useUser } from '../../contexts/userContext';
-import type { SameCourseOrProfOfferingsQuery } from '../../generated/graphql';
 import type { Listing } from '../../queries/api';
 import { generateRandomColor } from '../../utilities/common';
 import { ratingColormap, workloadColormap } from '../../utilities/constants';
@@ -143,19 +144,18 @@ function CourseLink({
 function OverviewRatings({
   gotoCourse,
   listing,
-  data,
+  others,
 }: {
-  readonly gotoCourse: (x: Listing) => void;
-  readonly listing: Listing;
-  readonly data: SameCourseOrProfOfferingsQuery | undefined;
+  readonly gotoCourse: (x: ListingInfo) => void;
+  readonly listing: ListingInfo;
+  readonly others: RelatedListingInfo[];
 }) {
   const { user } = useUser();
   const overlapSections = useMemo(() => {
     const overlapSections: {
       [filter in Filter]: CourseOffering[];
     } = { both: [], course: [], professor: [] };
-    if (!data) return overlapSections;
-    data.computed_listing_info
+    others
       // Discussion sections have no ratings, nothing to show
       .filter((course) => !isDiscussionSection(course))
       .map((course): CourseOffering => {
@@ -183,8 +183,9 @@ function OverviewRatings({
       .forEach((offering) => {
         // Skip listings in the current and future seasons that have no evals
         if (CUR_YEAR.includes(offering.listing.season_code)) return;
-        const overlappingProfs = listing.professor_names.reduce(
-          (cnt, prof) => cnt + (offering.professor.includes(prof) ? 1 : 0),
+        const overlappingProfs = listing.professor_ids.reduce(
+          (cnt, prof) =>
+            cnt + (offering.listing.professor_ids.includes(prof) ? 1 : 0),
           0,
         );
         // TODO: this whole logic is not ideal. We need to systematically
@@ -197,7 +198,7 @@ function OverviewRatings({
         const isBothOverlap =
           isCourseOverlap &&
           overlappingProfs === offering.professor.length &&
-          overlappingProfs === listing.professor_names.length;
+          overlappingProfs === listing.professor_ids.length;
         if (isBothOverlap) overlapSections.both.push(offering);
         if (isCourseOverlap) overlapSections.course.push(offering);
         if (isProfOverlap) overlapSections.professor.push(offering);
@@ -209,7 +210,7 @@ function OverviewRatings({
         // reconsideration of course relationships needed...
       });
     return overlapSections;
-  }, [data, listing]);
+  }, [others, listing]);
   const options = [
     {
       displayName: `Course (${overlapSections.course.length})`,

diff --git a/frontend/src/components/Worksheet/WorksheetToggleButton.tsx b/frontend/src/components/Worksheet/WorksheetToggleButton.tsx
@@ -24,7 +24,7 @@ function CourseConflictIcon({
   modal,
   worksheetNumber,
 }: {
-  readonly listing: Listing;
+  readonly listing: Pick<Listing, 'season_code' | 'crn' | 'times_by_day'>;
   readonly inWorksheet: boolean;
   readonly modal: boolean;
   readonly worksheetNumber: number;
@@ -45,7 +45,6 @@ function CourseConflictIcon({
         return 'This will add to a worksheet of a semester that has already ended.';
       return undefined;
     }
-    if (listing.times_summary === 'TBA') return undefined;
     const conflicts = checkConflict(data, listing);
     if (conflicts.length > 0)
       return `Conflicts with: ${conflicts.map((x) => x.course_code).join(', ')}`;
@@ -79,7 +78,7 @@ function WorksheetToggleButton({
   modal,
   inWorksheet: inWorksheetProp,
 }: {
-  readonly listing: Listing;
+  readonly listing: Pick<Listing, 'season_code' | 'crn' | 'times_by_day'>;
   readonly modal: boolean;
   readonly inWorksheet?: boolean;
 }) {