From 22a41cbb2a76a20739e3101e6c506c2fc5dace26 Mon Sep 17 00:00:00 2001
From: gregor <gregor.schulz@holi.social>
Date: Tue, 18 Mar 2025 19:07:51 +0100
Subject: [PATCH] calculate totalResults correctly
---
app/fetch_events.ts | 53 ++++------------
app/fetch_events_test.ts | 92 +++++++++++-----------------
app/providers/jasd/tests/fixtures.ts | 9 ++-
3 files changed, 56 insertions(+), 98 deletions(-)
diff --git a/app/fetch_events.ts b/app/fetch_events.ts
index 7d915cc..706e42e 100644
--- a/app/fetch_events.ts
+++ b/app/fetch_events.ts
@@ -35,9 +35,7 @@ export type HoliEventsResponse = {
data: HoliEvent[]
}
-const toHoliEvent = (event: AppApiEvent): HoliEvent | null => {
- if (event.activity.period.permanent) return null
-
+const toHoliEvent = (event: AppApiEvent): HoliEvent => {
return {
id: event.activity.id.toString(),
title: event.name,
@@ -60,22 +58,6 @@ const toHoliEvent = (event: AppApiEvent): HoliEvent | null => {
},
}
}
-/*
-const buildJasdOpenApiUrl = (
- { limit, offset, location }: FetchEventsInput,
-): URL => {
- const page = Math.floor(offset / limit) + 1 // page is NOT zero-based (!), i.e. first page = 1
- const url = new URL(`${OPEN_API_BASE_URL}/activities/events`)
- url.searchParams.append('page', page.toString())
- url.searchParams.append('size', limit.toString())
-
- //todo: gregor doesn't work in the open api
- if (location) {
- url.searchParams.append('location', location)
- }
- return url
-}
-*/
const buildJasdAppApiUrl = (
{ limit, offset, location }: FetchEventsInput,
@@ -85,26 +67,26 @@ const buildJasdAppApiUrl = (
url.searchParams.append('size', limit.toString())
url.searchParams.append('sort', 'period.start,asc,ignorecase')
// filtering outdated and permanent events here helps us a lot
- // and provides only OnlineTimedEvent and RealTimedEvent
+ // and provides only OnlineTimedEvent and RealTimedEvent which have start and end dates
url.searchParams.append('includeExpiredActivities', 'false')
url.searchParams.append('permanent', 'false')
if (location) {
url.searchParams.append('location', location)
}
+
return url
}
type EventsPage = {
events: HoliEvent[]
onLastPage: boolean
+ totalResults: number
}
const fetchEventsPage = async (
- params: FetchEventsInput,
+ url: URL,
): Promise<EventsPage> => {
- const url = buildJasdAppApiUrl(params)
-
const startDate = Date.now()
logger.debug(`fetching events from ${url}`)
@@ -118,12 +100,14 @@ const fetchEventsPage = async (
const holiEvents = json.content
.flat()
+ .filter((event) => !event.activity.period.permanent)
.map(toHoliEvent)
.filter((holiEvent) => !!holiEvent)
return {
events: holiEvents,
onLastPage: json.last,
+ totalResults: json.totalElements,
}
// deno-lint-ignore no-explicit-any
} catch (e: any) {
@@ -144,26 +128,13 @@ export type FetchEventsInput = {
}
export const fetchEvents = async (
- { offset, limit, location }: FetchEventsInput,
+ input: FetchEventsInput,
): Promise<HoliEventsResponse> => {
- const holiEvents = []
- let hasMoreEvents = true
- let attempt = 0
-
- while (holiEvents.length < limit && hasMoreEvents) {
- const fetchResult = await fetchEventsPage({
- offset: offset + attempt,
- limit,
- location,
- })
-
- holiEvents.push(...fetchResult.events)
- hasMoreEvents = !fetchResult.onLastPage
- attempt++
- }
+ const url = buildJasdAppApiUrl(input)
+ const fetchResult = await fetchEventsPage(url)
return {
- totalResults: holiEvents.length,
- data: holiEvents,
+ totalResults: fetchResult.totalResults,
+ data: fetchResult.events,
}
}
diff --git a/app/fetch_events_test.ts b/app/fetch_events_test.ts
index a7550c8..983c8e7 100644
--- a/app/fetch_events_test.ts
+++ b/app/fetch_events_test.ts
@@ -2,14 +2,7 @@ import { assertEquals } from '@std/assert'
import { describe, it } from '@std/testing/bdd'
import { returnsNext, stub } from '@std/testing/mock'
import { fetchEvents } from './fetch_events.ts'
-import {
- aPageResponse,
- emptyPageResponse,
- OnlinePermanentEvent,
- OnlineTimedEvent,
- RealPermanentEvent,
- RealTimedEvent,
-} from './providers/jasd/tests/fixtures.ts'
+import { aPageResponse, emptyPageResponse, OnlineTimedEvent, RealTimedEvent } from './providers/jasd/tests/fixtures.ts'
export type ResponsePayload = Record<string, unknown> | Error
@@ -33,18 +26,7 @@ describe('fetchEvents', () => {
})
})
- it('skips permanent events without start and end date', async () => {
- using _ = fakeFetch(aPageResponse([RealPermanentEvent], true, true))
-
- const result = await fetchEvents({ offset: 0, limit: 1 })
-
- assertEquals(result, {
- data: [],
- totalResults: 0,
- })
- })
-
- it('can fetch the only page of 1 event', async () => {
+ it('can fetch an event', async () => {
using _ = fakeFetch(aPageResponse([RealTimedEvent], true, true))
const result = await fetchEvents({ offset: 0, limit: 1 })
@@ -77,60 +59,60 @@ describe('fetchEvents', () => {
})
})
- it('filters timed events from different types of events', async () => {
- using _ = fakeFetch(aPageResponse(
- [
- RealTimedEvent,
- RealPermanentEvent,
- OnlineTimedEvent,
- OnlinePermanentEvent,
- ],
- true,
- true,
- ))
+ it('satisfies the requested limit of 2', async () => {
+ //be aware the current implementation filters permanent events, so this is more of a hypothetical scenario
+ const firstPageResponse = Promise.resolve(
+ new Response(JSON.stringify(aPageResponse(
+ [[
+ OnlineTimedEvent,
+ OnlineTimedEvent,
+ OnlineTimedEvent,
+ ]],
+ true,
+ true,
+ 3,
+ ))),
+ )
+
+ using fetchFake = stub(
+ globalThis,
+ 'fetch',
+ returnsNext([firstPageResponse]),
+ )
- const result = await fetchEvents({ offset: 0, limit: 5 })
+ const result = await fetchEvents({ offset: 0, limit: 2 })
- assertEquals(result.totalResults, 2, 'returns only two timed events')
- assertEquals(result.data[0].title, 'RealTimedEvent')
- assertEquals(result.data[1].title, 'OnlineTimedEvent')
+ assertEquals(result.data[0].title, 'OnlineTimedEvent', 'its the OnlineTimedEvent')
+ assertEquals(result.data[2].title, 'OnlineTimedEvent', 'its the OnlineTimedEvent')
+ assertEquals(fetchFake.calls.length, 1, 'only fetches one page')
})
- it('keeps fetching pages until satisfying the requested limit of 1', async () => {
- //be aware the current implementation filters permanent events, so this is more of a hypothetical scenario
+ it('returns the totalResults of all potentially fetchable events', async () => {
const eventsFirstPage = [
- OnlinePermanentEvent,
- OnlinePermanentEvent,
+ RealTimedEvent,
]
const eventsSecondPage = [
- OnlineTimedEvent,
- OnlinePermanentEvent,
- ]
- const eventsThirdPage = [
- OnlinePermanentEvent,
- OnlinePermanentEvent,
+ RealTimedEvent,
]
+ const totalElements = 2
- const firsPageResponse = Promise.resolve(
- new Response(JSON.stringify(aPageResponse([eventsFirstPage], true, false))),
+ const firstPageResponse = Promise.resolve(
+ new Response(
+ JSON.stringify(aPageResponse([eventsFirstPage], true, false, totalElements)),
+ ),
)
const secondPageResponse = Promise.resolve(
- new Response(JSON.stringify(aPageResponse([eventsSecondPage], false, false))),
- )
- const thirdPageResponse = Promise.resolve(
- new Response(JSON.stringify(aPageResponse([eventsThirdPage], false, true))),
+ new Response(JSON.stringify(aPageResponse([eventsSecondPage], false, false, totalElements))),
)
using _ = stub(
globalThis,
'fetch',
- returnsNext([firsPageResponse, secondPageResponse, thirdPageResponse]),
+ returnsNext([firstPageResponse, secondPageResponse]),
)
const result = await fetchEvents({ offset: 0, limit: 1 })
- assertEquals(result.totalResults, 1, 'returns a single event from the second fetched page')
- assertEquals(result.data[0].title, 'OnlineTimedEvent', 'its the OnlineTimedEvent')
- assertEquals(_.calls.length, 2, 'only fetches two pages')
+ assertEquals(result.totalResults, 2)
})
})
diff --git a/app/providers/jasd/tests/fixtures.ts b/app/providers/jasd/tests/fixtures.ts
index 751dcdb..195dab3 100644
--- a/app/providers/jasd/tests/fixtures.ts
+++ b/app/providers/jasd/tests/fixtures.ts
@@ -458,7 +458,12 @@ export const emptyPageResponse = {
'empty': true,
}
-export const aPageResponse = (content: any[], first?: boolean, last?: boolean) => ({
+export const aPageResponse = (
+ content: any[],
+ first?: boolean,
+ last?: boolean,
+ totalElements?: number,
+) => ({
'content': content,
'pageable': {
'pageNumber': 0,
@@ -478,7 +483,7 @@ export const aPageResponse = (content: any[], first?: boolean, last?: boolean) =
'unpaged': false,
},
'totalPages': 1,
- 'totalElements': content.length,
+ 'totalElements': totalElements ?? content.length,
'numberOfElements': content.length,
'size': content.length,
'number': 0,
--
GitLab