diff --git a/src/resources/tickets.ts b/src/resources/tickets.ts index b5644d8..e2782c1 100644 --- a/src/resources/tickets.ts +++ b/src/resources/tickets.ts @@ -36,7 +36,7 @@ export class TicketsResource { */ async list(params?: TicketListParams): Promise { return this.httpClient.request('/Tickets', { - params: this.buildListParams(params), + params: this.buildTicketListParams(params), }); } @@ -48,7 +48,7 @@ export class TicketsResource { this.httpClient, '/Tickets', 'tickets', - this.buildListParams(params) + this.buildTicketListParams(params) ); } @@ -108,7 +108,7 @@ export class TicketsResource { */ async actions(id: number, params?: ActionListParams): Promise { return this.httpClient.request(`/Tickets/${id}/Actions`, { - params: this.buildListParams(params), + params: sharedBuildListParams(params), }); } @@ -150,9 +150,35 @@ export class TicketsResource { } /** - * Build query parameters from list params + * Build query parameters for `list()`/`listAll()`. + * + * `dateoccurred_start`/`dateoccurred_end` are not real HaloPSA query + * parameters -- sending them literally (as the shared camelCase→snake_case + * converter would, since they're already snake_case) is accepted and + * silently ignored by the API, with no error and no filtering applied. + * The actual mechanism (confirmed against HaloPSA's own + * `/api/swagger/v2/swagger.json`) is a generic `datesearch=` plus + * `startdate`/`enddate` pair; `dateoccured` (missing the second 'r') is + * HaloPSA's own misspelling of the "date opened" field, not ours. + * + * Only ticket list params carry this pair (`actions()` uses the shared + * `buildListParams` directly), so this stays properly typed rather than + * the generic `` every other resource's private helper + * uses -- there's no other caller to support here. */ - private buildListParams(params?: T): Record { - return sharedBuildListParams(params); + private buildTicketListParams( + params?: TicketListParams + ): Record { + if (params?.dateoccurred_start === undefined && params?.dateoccurred_end === undefined) { + return sharedBuildListParams(params); + } + + const { dateoccurred_start, dateoccurred_end, ...rest } = params; + return sharedBuildListParams({ + ...rest, + datesearch: 'dateoccured', + startdate: dateoccurred_start, + enddate: dateoccurred_end, + }); } } diff --git a/src/resources/utils.ts b/src/resources/utils.ts index cb40ccc..6039310 100644 --- a/src/resources/utils.ts +++ b/src/resources/utils.ts @@ -30,12 +30,26 @@ export function unwrapSingle( * HaloPSA silently ignores `page_size`/`page_no` unless `pageinate=true` * (their typo, not ours) is sent alongside. Mutates the params object in * place when paging is requested. + * + * Also defaults `page_no` to `1` whenever `page_size` is set without it: + * HaloPSA only honors a caller's `page_size` when `page_no` is *also* + * present on the same request. Send `page_size` alone (as every single-page + * `.list({ pageSize })` call did before this fix) and the API silently + * falls back to its own default page size (50) for that implicit first + * page — `page_size` is accepted but ignored, with no error, and the + * records between the truncated first page and an explicit `page_no: 2` + * request are never returned by any call. `record_count` is affected the + * same way: it only reports the true total once pagination is genuinely + * active on every request, which requires `page_no` to be explicit too. */ export function addPageinate( params: Record ): Record { if (params.page_size !== undefined || params.page_no !== undefined) { params.pageinate = true; + if (params.page_no === undefined) { + params.page_no = 1; + } } return params; } diff --git a/src/types/tickets.ts b/src/types/tickets.ts index 81dabe7..2b42093 100644 --- a/src/types/tickets.ts +++ b/src/types/tickets.ts @@ -64,9 +64,9 @@ export interface TicketListParams extends BaseListParams { tickettype_id?: number; /** Filter by category */ category_1?: string; - /** Filter by date occurred start */ + /** Filter by date occurred start (translated to HaloPSA's datesearch/startdate pair) */ dateoccurred_start?: string; - /** Filter by date occurred end */ + /** Filter by date occurred end (translated to HaloPSA's datesearch/enddate pair) */ dateoccurred_end?: string; /** Show only open tickets */ open_only?: boolean; diff --git a/tests/integration/tickets.test.ts b/tests/integration/tickets.test.ts index 0d50f08..42eb335 100644 --- a/tests/integration/tickets.test.ts +++ b/tests/integration/tickets.test.ts @@ -30,6 +30,76 @@ describe('TicketsResource', () => { expect(response.tickets).toHaveLength(1); expect(response.tickets[0]?.summary).toBe('Printer not working'); }); + + // Regression: a page_size-only call (no page_no) was sent as + // pageinate=true&page_size=100 with no page_no -- HaloPSA silently + // ignored page_size on that implicit first page and fell back to its + // own default (50), leaving a hole between it and an explicit + // page_no=2 request. Every list() call must send page_no explicitly. + it('sends an explicit page_no so HaloPSA honors page_size on the first page', async () => { + const { server } = await import('../mocks/server.js'); + const { http, HttpResponse } = await import('msw'); + let capturedParams: URLSearchParams | undefined; + server.use( + http.get('https://testcompany.halopsa.com/api/Tickets', ({ request }) => { + capturedParams = new URL(request.url).searchParams; + return HttpResponse.json({ record_count: 385, tickets: [] }); + }) + ); + + const client = createClient(); + await client.tickets.list({ pageSize: 100 }); + + expect(capturedParams?.get('page_size')).toBe('100'); + expect(capturedParams?.get('page_no')).toBe('1'); + expect(capturedParams?.get('pageinate')).toBe('true'); + }); + + // Regression: dateoccurred_start/dateoccurred_end are not real HaloPSA + // query parameters -- sent literally, the API accepted and silently + // ignored them with no filtering applied and no error. HaloPSA expects + // datesearch= plus startdate/enddate instead. + it('translates dateoccurred_start/dateoccurred_end into datesearch+startdate+enddate', async () => { + const { server } = await import('../mocks/server.js'); + const { http, HttpResponse } = await import('msw'); + let capturedParams: URLSearchParams | undefined; + server.use( + http.get('https://testcompany.halopsa.com/api/Tickets', ({ request }) => { + capturedParams = new URL(request.url).searchParams; + return HttpResponse.json({ record_count: 0, tickets: [] }); + }) + ); + + const client = createClient(); + await client.tickets.list({ + dateoccurred_start: '2025-08-01T00:00:00Z', + dateoccurred_end: '2026-05-01T00:00:00Z', + }); + + expect(capturedParams?.get('datesearch')).toBe('dateoccured'); + expect(capturedParams?.get('startdate')).toBe('2025-08-01T00:00:00Z'); + expect(capturedParams?.get('enddate')).toBe('2026-05-01T00:00:00Z'); + expect(capturedParams?.has('dateoccurred_start')).toBe(false); + expect(capturedParams?.has('dateoccurred_end')).toBe(false); + }); + + it('leaves other filters untouched when no date range is given', async () => { + const { server } = await import('../mocks/server.js'); + const { http, HttpResponse } = await import('msw'); + let capturedParams: URLSearchParams | undefined; + server.use( + http.get('https://testcompany.halopsa.com/api/Tickets', ({ request }) => { + capturedParams = new URL(request.url).searchParams; + return HttpResponse.json({ record_count: 0, tickets: [] }); + }) + ); + + const client = createClient(); + await client.tickets.list({ client_id: 467 }); + + expect(capturedParams?.get('client_id')).toBe('467'); + expect(capturedParams?.has('datesearch')).toBe(false); + }); }); describe('listAll', () => { diff --git a/tests/unit/utils.test.ts b/tests/unit/utils.test.ts index 37bd33f..3aca632 100644 --- a/tests/unit/utils.test.ts +++ b/tests/unit/utils.test.ts @@ -36,11 +36,23 @@ describe('unwrapSingle', () => { }); describe('addPageinate', () => { - it('adds pageinate=true when page_size is set', () => { - expect(addPageinate({ page_size: 10 })).toEqual({ page_size: 10, pageinate: true }); + // Regression: HaloPSA only honors a caller's page_size when page_no is + // ALSO present on the same request. page_size alone (pageinate=true, no + // page_no) is accepted but silently ignored -- the API falls back to its + // own default page size (50) for that implicit first page, with no error. + it('defaults page_no to 1 when page_size is set without it', () => { + expect(addPageinate({ page_size: 10 })).toEqual({ page_size: 10, page_no: 1, pageinate: true }); }); - it('adds pageinate=true when page_no is set', () => { + it('does not override an explicit page_no', () => { + expect(addPageinate({ page_size: 10, page_no: 2 })).toEqual({ + page_size: 10, + page_no: 2, + pageinate: true, + }); + }); + + it('adds pageinate=true when only page_no is set', () => { expect(addPageinate({ page_no: 2 })).toEqual({ page_no: 2, pageinate: true }); }); @@ -53,6 +65,7 @@ describe('buildListParams', () => { it('camelCase → snake_case', () => { expect(buildListParams({ pageSize: 25, openOnly: true })).toEqual({ page_size: 25, + page_no: 1, open_only: true, pageinate: true, });