From 757093bc2fe299ade920a1bfddaec51ed309124b Mon Sep 17 00:00:00 2001
From: Debjeet Biswas <detj@users.noreply.github.com>
Date: Wed, 6 Nov 2024 20:11:31 +0530
Subject: [PATCH] fix(frontend): better pagination in session detail (#1491)

- move to limit/offset based pagination
- fix couple of react key dupication violation
- refactor session detail and remove old code

Signed-off-by: detj <detj@users.noreply.github.com>
---
 .../dashboard/app/[teamId]/sessions/page.tsx  | 46 +++++++------------
 frontend/dashboard/app/api/api_calls.ts       | 27 ++++++-----
 .../app/components/dropdown_select.tsx        |  4 +-
 3 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/frontend/dashboard/app/[teamId]/sessions/page.tsx b/frontend/dashboard/app/[teamId]/sessions/page.tsx
index b96bae91a..1d5bf6df7 100644
--- a/frontend/dashboard/app/[teamId]/sessions/page.tsx
+++ b/frontend/dashboard/app/[teamId]/sessions/page.tsx
@@ -3,7 +3,7 @@
 import { emptySessionsOverviewResponse, SessionsOverviewApiStatus, fetchSessionsOverviewFromServer, FiltersApiType } from '@/app/api/api_calls';
 import Filters, { AppVersionsInitialSelectionType, defaultFilters } from '@/app/components/filters';
 import LoadingBar from '@/app/components/loading_bar';
-import Paginator, { PaginationDirection } from '@/app/components/paginator';
+import Paginator from '@/app/components/paginator';
 import SessionsOverviewPlot from '@/app/components/sessions_overview_plot';
 import { formatDateToHumanReadableDate, formatDateToHumanReadableTime, formatMillisToHumanReadable } from '@/app/utils/time_utils';
 import Link from 'next/link';
@@ -17,38 +17,19 @@ export default function SessionsOverview({ params }: { params: { teamId: string
     const [filters, setFilters] = useState(defaultFilters);
 
     const [sessionsOverview, setSessionsOverview] = useState(emptySessionsOverviewResponse);
-    const paginationOffset = 5
-    const [paginationIndex, setPaginationIndex] = useState(0)
-    const [paginationDirection, setPaginationDirection] = useState(PaginationDirection.None)
+    const paginationLimit = 5
+    const [paginationOffset, setPaginationOffset] = useState(0)
 
     const getSessionsOverview = async () => {
         setSessionsOverviewApiStatus(SessionsOverviewApiStatus.Loading)
 
-        // Set key id if user has paginated. Last index of current list if forward navigation, first index if backward
-        var keyId = null
-        if (sessionsOverview.results !== null && sessionsOverview.results.length > 0) {
-            if (paginationDirection === PaginationDirection.Forward) {
-                keyId = sessionsOverview.results[sessionsOverview.results.length - 1].session_id
-            } else if (paginationDirection === PaginationDirection.Backward) {
-                keyId = sessionsOverview.results[0].session_id
-            }
-        }
-
-        // Invert limit if paginating backward
-        var limit = paginationOffset
-        if (paginationDirection === PaginationDirection.Backward) {
-            limit = - limit
-        }
-
-        const result = await fetchSessionsOverviewFromServer(filters, keyId, limit, router)
+        const result = await fetchSessionsOverviewFromServer(filters, null, null, paginationLimit, paginationOffset, router)
 
         switch (result.status) {
             case SessionsOverviewApiStatus.Error:
-                setPaginationDirection(PaginationDirection.None) // Reset pagination direction to None after API call so that a change in any filters does not cause keyId to be added to the next API call
                 setSessionsOverviewApiStatus(SessionsOverviewApiStatus.Error)
                 break
             case SessionsOverviewApiStatus.Success:
-                setPaginationDirection(PaginationDirection.None) // Reset pagination direction to None after API call so that a change in any filters does not cause keyId to be added to the next API call
                 setSessionsOverviewApiStatus(SessionsOverviewApiStatus.Success)
                 setSessionsOverview(result.data)
                 break
@@ -61,7 +42,14 @@ export default function SessionsOverview({ params }: { params: { teamId: string
         }
 
         getSessionsOverview()
-    }, [paginationIndex, filters]);
+    }, [paginationOffset, filters]);
+
+    useEffect(() => {
+        if (!filters.ready) {
+            return
+        }
+        setPaginationOffset(0)
+    }, [filters])
 
     return (
         <div className="flex flex-col selection:bg-yellow-200/75 items-start p-24 pt-8">
@@ -109,12 +97,10 @@ export default function SessionsOverview({ params }: { params: { teamId: string
                     <div className='self-end'>
                         <Paginator prevEnabled={sessionsOverviewApiStatus === SessionsOverviewApiStatus.Loading ? false : sessionsOverview.meta.previous} nextEnabled={sessionsOverviewApiStatus === SessionsOverviewApiStatus.Loading ? false : sessionsOverview.meta.next} displayText=''
                             onNext={() => {
-                                setPaginationDirection(PaginationDirection.Forward)
-                                setPaginationIndex(paginationIndex + 1)
+                                setPaginationOffset(paginationOffset + paginationLimit)
                             }}
                             onPrev={() => {
-                                setPaginationDirection(PaginationDirection.Backward)
-                                setPaginationIndex(paginationIndex - 1)
+                                setPaginationOffset(paginationOffset - paginationLimit)
                             }} />
                     </div>
                     <div className={`py-1 w-full ${sessionsOverviewApiStatus === SessionsOverviewApiStatus.Loading ? 'visible' : 'invisible'}`}>
@@ -129,8 +115,8 @@ export default function SessionsOverview({ params }: { params: { teamId: string
                             </div>
                         </div>
                         <div className="table-row-group font-sans">
-                            {sessionsOverview.results.map(({ session_id, app_id, first_event_time, duration, matched_free_text, attribute }) => (
-                                <Link key={session_id} href={`/${params.teamId}/sessions/${app_id}/${session_id}`} className="table-row border-b-2 border-black hover:bg-yellow-200 focus:bg-yellow-200 active:bg-yellow-300 ">
+                            {sessionsOverview.results.map(({ session_id, app_id, first_event_time, duration, matched_free_text, attribute }, idx) => (
+                                <Link key={`${idx}-${session_id}`} href={`/${params.teamId}/sessions/${app_id}/${session_id}`} className="table-row border-b-2 border-black hover:bg-yellow-200 focus:bg-yellow-200 active:bg-yellow-300 ">
                                     <div className="table-cell p-4">
                                         <p className='truncate'>{session_id}</p>
                                         <div className='py-1' />
diff --git a/frontend/dashboard/app/api/api_calls.ts b/frontend/dashboard/app/api/api_calls.ts
index 477551cf5..649523f05 100644
--- a/frontend/dashboard/app/api/api_calls.ts
+++ b/frontend/dashboard/app/api/api_calls.ts
@@ -722,7 +722,7 @@ export const saveListFiltersToServer = async (filters: Filters) => {
     }
 }
 
-async function applyGenericFiltersToUrl(url: string, filters: Filters, keyId: string | null, keyTimestamp: string | null, limit: number | null) {
+async function applyGenericFiltersToUrl(url: string, filters: Filters, keyId: string | null, keyTimestamp: string | null, limit: number | null, offset: number | null) {
     const serverFormattedStartDate = formatUserInputDateToServerFormat(filters.startDate)
     const serverFormattedEndDate = formatUserInputDateToServerFormat(filters.endDate)
     const timezone = getTimeZoneForServer()
@@ -770,6 +770,11 @@ async function applyGenericFiltersToUrl(url: string, filters: Filters, keyId: st
         searchParams.append('limit', String(limit))
     }
 
+    // Append offset if present
+    if (offset !== null) {
+        searchParams.append('offset', String(offset))
+    }
+
     u.search = searchParams.toString()
 
     return u.toString()
@@ -872,7 +877,7 @@ export const fetchJourneyFromServer = async (journeyType: JourneyType, exception
     // Append bidirectional value
     url = url + `bigraph=${bidirectional ? '1&' : '0&'}`
 
-    url = await applyGenericFiltersToUrl(url, filters, null, null, null)
+    url = await applyGenericFiltersToUrl(url, filters, null, null, null, null)
 
     try {
         const res = await fetchMeasure(url);
@@ -895,7 +900,7 @@ export const fetchMetricsFromServer = async (filters: Filters, router: AppRouter
 
     let url = `${origin}/apps/${filters.app.id}/metrics?`
 
-    url = await applyGenericFiltersToUrl(url, filters, null, null, null)
+    url = await applyGenericFiltersToUrl(url, filters, null, null, null, null)
 
     try {
         const res = await fetchMeasure(url);
@@ -913,12 +918,12 @@ export const fetchMetricsFromServer = async (filters: Filters, router: AppRouter
     }
 }
 
-export const fetchSessionsOverviewFromServer = async (filters: Filters, keyId: string | null, limit: number, router: AppRouterInstance) => {
+export const fetchSessionsOverviewFromServer = async (filters: Filters, keyId: string | null, keyTimestamp: string | null, limit: number, offset: number, router: AppRouterInstance) => {
     const origin = process.env.NEXT_PUBLIC_API_BASE_URL
 
     var url = `${origin}/apps/${filters.app.id}/sessions?`
 
-    url = await applyGenericFiltersToUrl(url, filters, keyId, null, limit)
+    url = await applyGenericFiltersToUrl(url, filters, keyId, keyTimestamp, limit, offset)
 
     try {
         const res = await fetchMeasure(url);
@@ -941,7 +946,7 @@ export const fetchSessionsOverviewPlotFromServer = async (filters: Filters, rout
 
     var url = `${origin}/apps/${filters.app.id}/sessions/plots/instances?`
 
-    url = await applyGenericFiltersToUrl(url, filters, null, null, null)
+    url = await applyGenericFiltersToUrl(url, filters, null, null, null, null)
 
     try {
         const res = await fetchMeasure(url);
@@ -973,7 +978,7 @@ export const fetchExceptionsOverviewFromServer = async (exceptionsType: Exceptio
         url = `${origin}/apps/${filters.app.id}/anrGroups?`
     }
 
-    url = await applyGenericFiltersToUrl(url, filters, keyId, null, limit)
+    url = await applyGenericFiltersToUrl(url, filters, keyId, null, limit, null)
 
     try {
         const res = await fetchMeasure(url);
@@ -1002,7 +1007,7 @@ export const fetchExceptionsDetailsFromServer = async (exceptionsType: Exception
         url = `${origin}/apps/${filters.app.id}/anrGroups/${exceptionsGroupdId}/anrs?`
     }
 
-    url = await applyGenericFiltersToUrl(url, filters, keyId, keyTimestamp, limit)
+    url = await applyGenericFiltersToUrl(url, filters, keyId, keyTimestamp, limit, null)
 
     try {
         const res = await fetchMeasure(url);
@@ -1031,7 +1036,7 @@ export const fetchExceptionsOverviewPlotFromServer = async (exceptionsType: Exce
         url = `${origin}/apps/${filters.app.id}/anrGroups/plots/instances?`
     }
 
-    url = await applyGenericFiltersToUrl(url, filters, null, null, null)
+    url = await applyGenericFiltersToUrl(url, filters, null, null, null, null)
 
     try {
         const res = await fetchMeasure(url);
@@ -1064,7 +1069,7 @@ export const fetchExceptionsDetailsPlotFromServer = async (exceptionsType: Excep
         url = `${origin}/apps/${filters.app.id}/anrGroups/${exceptionsGroupdId}/plots/instances?`
     }
 
-    url = await applyGenericFiltersToUrl(url, filters, null, null, null)
+    url = await applyGenericFiltersToUrl(url, filters, null, null, null, null)
 
     try {
         const res = await fetchMeasure(url);
@@ -1096,7 +1101,7 @@ export const fetchExceptionsDistributionPlotFromServer = async (exceptionsType:
         url = `${origin}/apps/${filters.app.id}/anrGroups/${exceptionsGroupdId}/plots/distribution?`
     }
 
-    url = await applyGenericFiltersToUrl(url, filters, null, null, null)
+    url = await applyGenericFiltersToUrl(url, filters, null, null, null, null)
 
     try {
         const res = await fetchMeasure(url);
diff --git a/frontend/dashboard/app/components/dropdown_select.tsx b/frontend/dashboard/app/components/dropdown_select.tsx
index bd2e087fd..220453361 100644
--- a/frontend/dashboard/app/components/dropdown_select.tsx
+++ b/frontend/dashboard/app/components/dropdown_select.tsx
@@ -298,8 +298,8 @@ const DropdownSelect: React.FC<DropdownSelectProps> = ({ title, type, items, ini
                     All
                   </button>
                 </div>}
-                {items.filter((item) => (item as OsVersion).displayName.toLocaleLowerCase().includes(searchText.toLocaleLowerCase())).map((item) => (
-                  <div key={item as string} className={checkboxContainerStyle} role="menuitem">
+                {items.filter((item) => (item as OsVersion).displayName.toLocaleLowerCase().includes(searchText.toLocaleLowerCase())).map((item, idx) => (
+                  <div key={`${idx}-${item as string}`} className={checkboxContainerStyle} role="menuitem">
                     <input
                       type="checkbox"
                       className={checkboxInputStyle}