Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions src/resources/tickets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class TicketsResource {
*/
async list(params?: TicketListParams): Promise<TicketListResponse> {
return this.httpClient.request<TicketListResponse>('/Tickets', {
params: this.buildListParams(params),
params: this.buildTicketListParams(params),
});
}

Expand All @@ -48,7 +48,7 @@ export class TicketsResource {
this.httpClient,
'/Tickets',
'tickets',
this.buildListParams(params)
this.buildTicketListParams(params)
);
}

Expand Down Expand Up @@ -108,7 +108,7 @@ export class TicketsResource {
*/
async actions(id: number, params?: ActionListParams): Promise<ActionListResponse> {
return this.httpClient.request<ActionListResponse>(`/Tickets/${id}/Actions`, {
params: this.buildListParams(params),
params: sharedBuildListParams(params),
});
}

Expand Down Expand Up @@ -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=<field>` 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 `<T extends object>` every other resource's private helper
* uses -- there's no other caller to support here.
*/
private buildListParams<T extends object>(params?: T): Record<string, string | number | boolean | undefined> {
return sharedBuildListParams(params);
private buildTicketListParams(
params?: TicketListParams
): Record<string, string | number | boolean | undefined> {
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,
});
}
}
14 changes: 14 additions & 0 deletions src/resources/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,26 @@ export function unwrapSingle<T>(
* 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<string, string | number | boolean | undefined>
): Record<string, string | number | boolean | undefined> {
if (params.page_size !== undefined || params.page_no !== undefined) {
params.pageinate = true;
if (params.page_no === undefined) {
params.page_no = 1;
}
}
return params;
}
Expand Down
4 changes: 2 additions & 2 deletions src/types/tickets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
70 changes: 70 additions & 0 deletions tests/integration/tickets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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=<field> 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', () => {
Expand Down
19 changes: 16 additions & 3 deletions tests/unit/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});

Expand All @@ -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,
});
Expand Down