Merge pull request #1469 from danskernesdigitalebibliotek/DDFHER-89

DDFHER-89 - Ensure consistent sorting for loans

src/apps/dashboard/dashboard-notification-list/dashboard-notification-list.tsx

@@ -49,17 +49,12 @@ const DashboardNotificationList: FC<DashboardNotificationListProps> = ({
     }
   } = useReservations();
   const {
-    all: { loans, isLoading: isLoadingLoans },
+    all: { loans, soonOverdue, farFromOverdue, isLoading: isLoadingLoans },
     fbs: {
       overdue: loansOverduePhysical,
       soonOverdue: loansSoonOverduePhysical,
       farFromOverdue: loansFarFromOverduePhysical,
       isLoading: isLoadingLoansPhysical
-    },
-    publizon: {
-      soonOverdue: loansSoonOverdueDigital,
-      farFromOverdue: loansFarFromOverdueDigital,
-      isLoading: isLoadingLoansDigital
     }
   } = useLoans();
 
@@ -125,16 +120,12 @@ const DashboardNotificationList: FC<DashboardNotificationListProps> = ({
           break;
 
         case soon:
-          setLoansToDisplay(
-            loansSoonOverduePhysical.concat(loansSoonOverdueDigital)
-          );
+          setLoansToDisplay(soonOverdue);
           setModalHeader(t("loansSoonOverdueText"));
           break;
 
         case longer:
-          setLoansToDisplay(
-            loansFarFromOverduePhysical.concat(loansFarFromOverdueDigital)
-          );
+          setLoansToDisplay(farFromOverdue);
           setModalHeader(t("loansNotOverdueText"));
           break;
 
@@ -143,16 +134,7 @@ const DashboardNotificationList: FC<DashboardNotificationListProps> = ({
       }
       open(constructModalId(dueDateModal as string, [dueDateInput]));
     },
-    [
-      dueDateModal,
-      open,
-      loansFarFromOverduePhysical,
-      loansOverduePhysical,
-      loansSoonOverduePhysical,
-      loansSoonOverdueDigital,
-      loansFarFromOverdueDigital,
-      t
-    ]
+    [open, dueDateModal, loansOverduePhysical, t, soonOverdue, farFromOverdue]
   );
 
   const dashboardNotificationsLoan = [
@@ -169,8 +151,7 @@ const DashboardNotificationList: FC<DashboardNotificationListProps> = ({
           : openDueDateModal(yesterday)
     },
     {
-      listLength:
-        loansSoonOverduePhysical.length + loansSoonOverdueDigital.length,
+      listLength: soonOverdue.length,
       badge: t("statusBadgeWarningText"),
       header: t("loansSoonOverdueText"),
       color: "warning",
@@ -182,8 +163,7 @@ const DashboardNotificationList: FC<DashboardNotificationListProps> = ({
           : openDueDateModal(soon)
     },
     {
-      listLength:
-        loansFarFromOverduePhysical.length + loansFarFromOverdueDigital.length,
+      listLength: farFromOverdue.length,
       header: t("loansNotOverdueText"),
       dataCy: "loans-not-overdue",
       color: "neutral",
@@ -231,11 +211,7 @@ const DashboardNotificationList: FC<DashboardNotificationListProps> = ({
               materialsCount={loans.length}
               header={t("physicalLoansText")}
               emptyListText={t("noPhysicalLoansText")}
-              isLoading={
-                isLoadingLoans ||
-                isLoadingLoansPhysical ||
-                isLoadingLoansDigital
-              }
+              isLoading={isLoadingLoans || isLoadingLoansPhysical}
               linkText={t("dashboardLoansLinkText")}
               linkUrl={physicalLoansUrl}
             />
@@ -259,10 +235,7 @@ const DashboardNotificationList: FC<DashboardNotificationListProps> = ({
             ...dashboardNotificationsReservations
           ]}
           isLoading={
-            isLoadingLoans ||
-            isLoadingLoansPhysical ||
-            isLoadingLoansDigital ||
-            isLoadingReservations
+            isLoadingLoans || isLoadingLoansPhysical || isLoadingReservations
           }
         />
       )}

src/apps/loan-list/list/loan-list.tsx

@@ -3,7 +3,9 @@ import { useSelector } from "react-redux";
 import dayjs from "dayjs";
 import {
   getAmountOfRenewableLoans,
-  getScrollClass
+  getDueDatesLoan,
+  getScrollClass,
+  sortByDueDate
 } from "../../../core/utils/helpers/general";
 import { getUrlQueryParam } from "../../../core/utils/helpers/url";
 import { useText } from "../../../core/utils/text";
@@ -54,13 +56,12 @@ const LoanList: FC<LoanListProps> = ({ pageSize }) => {
   const [dueDate, setDueDate] = useState<string | null>(null);
   const [modalLoan, setModalLoan] = useState<LoanType | null>(null);
   const {
-    fbs: {
-      loans: loansPhysical,
-      stackedMaterialsDueDates: stackedMaterialsDueDatesFbs,
-      isLoading: isLoadingFbs
-    },
-    publizon: { loans: loansDigital, isLoading: isLoadingPublizon }
+    fbs: { loans: fbsLoans, isLoading: isLoadingFbs },
+    publizon: { loans: publizonLoans, isLoading: isLoadingPublizon }
   } = useLoans();
+  const loansPhysical = sortByDueDate(fbsLoans);
+  const loansDigital = sortByDueDate(publizonLoans);
+  const stackedMaterialsDueDatesFbs = getDueDatesLoan(loansPhysical);
   const openLoanDetailsModal = useCallback(
     (loan: LoanType) => {
       setModalLoan(loan);

src/components/GroupModal/GroupModalLoansList.tsx

@@ -1,4 +1,4 @@
-import React, { FC, useState, useEffect } from "react";
+import React, { FC, useState, useEffect, useMemo } from "react";
 import SelectableMaterial from "../../apps/loan-list/materials/selectable-material/selectable-material";
 import { useText } from "../../core/utils/text";
 import { isLoanType, LoanType } from "../../core/utils/types/loan-type";
@@ -8,6 +8,7 @@ import StatusBadge from "../../apps/loan-list/materials/utils/status-badge";
 import { formatDate } from "../../core/utils/helpers/date";
 import { ListType } from "../../core/utils/types/list-type";
 import { getLoanDeliveryDate } from "../../apps/loan-list/utils/helpers";
+import { sortLoansByIsRenewableThenDueDate } from "../../core/utils/helpers/general";
 
 export interface GroupModalLoansListProps {
   materials: LoanType[];
@@ -24,9 +25,11 @@ const GroupModalLoansList: FC<GroupModalLoansListProps> = ({
   selectMaterials,
   pageSize
 }) => {
-  // Show renewable materials first, then non-renewable
-  const groupedMaterials = materials.sort(
-    (a, b) => Number(!!b.isRenewable) - Number(!!a.isRenewable)
+  // Memoize groupedMaterials to prevent unnecessary re-computations
+  // Because groupedMaterials is a dependency of the useEffect hook
+  const groupedMaterials = useMemo(
+    () => sortLoansByIsRenewableThenDueDate(materials),
+    [materials]
   );
   const t = useText();
   const [displayedMaterials, setDisplayedMaterials] = useState<LoanType[]>([]);

src/core/utils/helpers/general.ts

@@ -1,6 +1,6 @@
 import { useEffect, useRef } from "react";
 import dayjs from "dayjs";
-import { first, uniq } from "lodash";
+import { first, orderBy, uniq } from "lodash";
 import { vi } from "vitest";
 import { CoverProps } from "../../../components/cover/cover";
 import { UseTextFunction } from "../text";
@@ -217,6 +217,35 @@ export const sortByReservationDate = (list: ReservationType[]) => {
   );
 };
 
+export const sortByDueDate = (list: LoanType[]) => {
+  //  We use orderBy from lodash to avoid mutating the original list
+  return orderBy(
+    list,
+    (item) => (item.dueDate ? dayjs(item.dueDate).valueOf() : Infinity),
+    "asc"
+  );
+};
+
+export const sortByRenewable = (list: LoanType[]) => {
+  return orderBy(list, (item) => Number(!!item.isRenewable), "desc");
+};
+
+export const sortLoansByIsRenewableThenDueDate = (list: LoanType[]) => {
+  const sortedByDueDate = sortByDueDate(list);
+  return sortByRenewable(sortedByDueDate);
+};
+
+export const getDueDatesLoan = (list: LoanType[]) => {
+  return Array.from(
+    new Set(
+      list
+        .filter(({ dueDate }) => dueDate !== undefined && dueDate !== null)
+        .map(({ dueDate }) => dueDate)
+        .sort()
+    )
+  ) as string[];
+};
+
 export const getDueDatesForModal = (list: LoanType[], date: string) => {
   return list.filter(({ dueDate }) => dueDate === date);
 };

src/core/utils/useLoans.tsx

@@ -33,27 +33,6 @@ const filterLoansSoonOverdue = (loans: LoanType[], warning: number) => {
     );
   });
 };
-//
-const getDueDatesLoan = (list: LoanType[]) => {
-  return Array.from(
-    new Set(
-      list
-        .filter(({ dueDate }) => dueDate !== (undefined || null))
-        .map(({ dueDate }) => dueDate)
-        .sort()
-    )
-  ) as string[];
-};
-
-const sortByDueDate = (list: LoanType[]) => {
-  // Todo figure out what to do if loan does not have loan date
-  // For now, its at the bottom of the list
-  return list.sort(
-    (a, b) =>
-      new Date(a.dueDate || new Date()).getTime() -
-      new Date(b.dueDate || new Date()).getTime()
-  );
-};
 
 type Loans = {
   loans: LoanType[];
@@ -62,7 +41,6 @@ type Loans = {
   farFromOverdue: LoanType[];
   isLoading: boolean;
   isError: boolean;
-  stackedMaterialsDueDates?: string[];
 };
 
 type UseLoansType = {
@@ -73,6 +51,10 @@ type UseLoansType = {
 
 type UseLoans = () => UseLoansType;
 
+// useLoans is a custom hook that fetches loans from both FBS and Publizon
+// and combines them into lists. The loans are then divided into three
+// categories: overdue, soon overdue, and far from overdue.
+// The hook is NOT responsible for any sorting of the loans.
 const useLoans: UseLoans = () => {
   const {
     data: loansFbs,
@@ -100,15 +82,12 @@ const useLoans: UseLoans = () => {
     : [];
 
   // Combine all loans from both FBS and Publizon
-  const loans = sortByDueDate([...mappedLoansFbs, ...mappedLoansPublizon]);
+  const loans = [...mappedLoansFbs, ...mappedLoansPublizon];
 
   // Combine "overdue loans" from both FBS and Publizon
   const loansOverdueFBS = filterLoansOverdue(mappedLoansFbs);
   const LoansOverduePublizon = filterLoansOverdue(mappedLoansPublizon);
-  const loansOverdue = sortByDueDate([
-    ...loansOverdueFBS,
-    ...LoansOverduePublizon
-  ]);
+  const loansOverdue = [...loansOverdueFBS, ...LoansOverduePublizon];
 
   // combine "soon overdue" loans from both FBS and Publizon
   const loansSoonOverdueFBS = filterLoansSoonOverdue(
@@ -119,10 +98,10 @@ const useLoans: UseLoans = () => {
     mappedLoansPublizon,
     threshold.warning
   );
-  const loansSoonOverdue = sortByDueDate([
+  const loansSoonOverdue = [
     ...loansSoonOverdueFBS,
     ...loansSoonOverduePublizon
-  ]);
+  ];
 
   // combine "far from overdue" loans from both FBS and Publizon
   const loansFarFromOverdueFBS = filterLoansNotOverdue(
@@ -133,19 +112,11 @@ const useLoans: UseLoans = () => {
     mappedLoansPublizon,
     threshold.warning
   );
-  const loansFarFromOverdue = sortByDueDate([
+  const loansFarFromOverdue = [
     ...loansFarFromOverdueFBS,
     ...loansFarFromOverduePublizon
-  ]);
-
-  // The due dates are used for the stacked materials
-  // The stacked materials view shows materials stacked by
-  // due date, and for this we need a unique list of due dates
-  const loansSortedByDateFbs = sortByDueDate(mappedLoansFbs);
-  const loansSortedByDatePublizon = sortByDueDate(mappedLoansPublizon);
+  ];
 
-  // list of all due dates used for the stacked materials
-  const stackedMaterialsDueDatesFbs = getDueDatesLoan(mappedLoansFbs);
   return {
     all: {
       loans,
@@ -156,16 +127,15 @@ const useLoans: UseLoans = () => {
       isError: loansIsError
     },
     fbs: {
-      loans: loansSortedByDateFbs,
+      loans: mappedLoansFbs,
       overdue: loansOverdueFBS,
       soonOverdue: loansSoonOverdueFBS,
       farFromOverdue: loansFarFromOverdueFBS,
-      stackedMaterialsDueDates: stackedMaterialsDueDatesFbs,
       isLoading: isLoadingFbs,
       isError: isErrorFbs
     },
     publizon: {
-      loans: loansSortedByDatePublizon,
+      loans: mappedLoansPublizon,
       overdue: LoansOverduePublizon,
       soonOverdue: loansSoonOverduePublizon,
       farFromOverdue: loansFarFromOverduePublizon,