From 36d6b2ce88743626fd7e6add6049b37c892b85f1 Mon Sep 17 00:00:00 2001 From: Adam Coster Date: Thu, 7 Oct 2021 19:41:42 -0500 Subject: [PATCH] fix: Paging was not working due to invalid URL created for additional pages. --- src/lib/Logger.ts | 5 ++- src/lib/clientLib/BravoResponse.ts | 3 ++ src/lib/clientLib/FavroClient.ts | 18 ++++---- src/lib/clientLib/FavroResponse.ts | 12 +++++- src/lib/utility.ts | 4 +- src/test/client.test.ts | 22 +++++++++- src/test/utility/populateTestOrg.ts | 66 +++++++++++++++++++++++++++++ 7 files changed, 116 insertions(+), 14 deletions(-) create mode 100644 src/test/utility/populateTestOrg.ts diff --git a/src/lib/Logger.ts b/src/lib/Logger.ts index d42d2fb..808e5eb 100644 --- a/src/lib/Logger.ts +++ b/src/lib/Logger.ts @@ -33,6 +33,9 @@ export class Logger { bodies: null, stats: null, }, + paging: { + next: null, + }, }, } as const; } @@ -71,7 +74,7 @@ export class Logger { Logger._utility.error.bind(Logger._utility)(...args); } - static getDebugLogger(path: DebugPath) { + static debug(path: DebugPath) { this._debuggers[path] ||= debug(path); this._debuggers[path].log = Logger.log; this._debuggerPaths.add(path); diff --git a/src/lib/clientLib/BravoResponse.ts b/src/lib/clientLib/BravoResponse.ts index 7dfb448..d1c885d 100644 --- a/src/lib/clientLib/BravoResponse.ts +++ b/src/lib/clientLib/BravoResponse.ts @@ -5,6 +5,7 @@ import type { BravoWidget } from '$entities/BravoWidget.js'; import type { BravoCustomFieldDefinition } from '../entities/BravoCustomField.js'; import type { FavroApi } from '$types/FavroApi.js'; import type { BravoTagDefinition } from '../entities/BravoTag.js'; +import { Logger } from '../Logger.js'; export type BravoResponseWidgets = BravoResponseEntities< FavroApi.Widget.Model, @@ -178,8 +179,10 @@ export class BravoResponseEntities< */ private async fetchNextPage() { // Ensure the prior page got added! + Logger.debug('bravo:paging:next')(`fetching`); await this.ensureEntitiesAreHydrated(); if (!this._latestPage || (await this._latestPage.isLastPage())) { + Logger.debug('bravo:paging:next')(`cancelled`); return; } this._latestPage = await this._latestPage.getNextPageResponse(); diff --git a/src/lib/clientLib/FavroClient.ts b/src/lib/clientLib/FavroClient.ts index e4c7c42..44413b7 100644 --- a/src/lib/clientLib/FavroClient.ts +++ b/src/lib/clientLib/FavroClient.ts @@ -25,8 +25,10 @@ const favroApiBaseUrl = 'https://favro.com/api/v1'; function createFavroApiUrl(url: string, params?: Record) { // Ensure initial slash - url = url.startsWith('/') ? url : `/${url}`; - url = `${favroApiBaseUrl}${url}`; + if (!url.startsWith('https://favro.com')) { + url = url.startsWith('/') ? url : `/${url}`; + url = `${favroApiBaseUrl}${url}`; + } const fullUrl = new URL(url); if (params) { for (const param of Object.keys(params)) { @@ -182,10 +184,10 @@ export class FavroClient { customFetch?: Fetcher, ) { const debugBase = `bravo:http:`; - const debugBasic = Logger.getDebugLogger(`${debugBase}basic`); - const debugHeaders = Logger.getDebugLogger(`${debugBase}headers`); - const debugBodies = Logger.getDebugLogger(`${debugBase}bodies`); - const debugStats = Logger.getDebugLogger(`${debugBase}stats`); + const debugBasic = Logger.debug(`${debugBase}basic`); + const debugHeaders = Logger.debug(`${debugBase}headers`); + const debugBodies = Logger.debug(`${debugBase}bodies`); + const debugStats = Logger.debug(`${debugBase}stats`); this.assert( this._organizationId || !options?.requireOrganizationId, 'An organizationId must be set for this request', @@ -228,8 +230,8 @@ export class FavroClient { const res = new FavroResponse(this, rawResponse); this._backendId = res.backendId || this._backendId; this._limitResetsAt = res.limitResetsAt; - this._requestsLimit = res.limit; - this._requestsRemaining = res.requestsRemaining; + this._requestsLimit = res.limit ?? this._requestsRemaining; + this._requestsRemaining = res.requestsRemaining ?? this._requestsRemaining; if (this._requestsRemaining < 1 || res.status == 429) { // TODO: Set an interval before allowing requests to go through again, OR SOMETHING diff --git a/src/lib/clientLib/FavroResponse.ts b/src/lib/clientLib/FavroResponse.ts index 943909a..d5dabe6 100644 --- a/src/lib/clientLib/FavroResponse.ts +++ b/src/lib/clientLib/FavroResponse.ts @@ -4,6 +4,7 @@ import type { FavroApi } from '$types/FavroApi.js'; import { BravoError } from '../errors.js'; import type { FavroClient } from './FavroClient.js'; import { stringToDate, stringToNumber } from '../utility.js'; +import { Logger } from '../Logger.js'; function dataIsNull(data: any): data is null { return data === null; @@ -57,7 +58,7 @@ export class FavroResponse< get requestsRemaining() { return stringToNumber(this._response.headers.get('X-RateLimit-Remaining'), { - defaultIfNullish: Infinity, + defaultIfNullish: null, customError: this._client.error, }); } @@ -71,7 +72,7 @@ export class FavroResponse< get limit() { return stringToNumber(this._response.headers.get('X-RateLimit-Limit'), { - defaultIfNullish: Infinity, + defaultIfNullish: null, customError: this._client.error, }); } @@ -109,11 +110,18 @@ export class FavroResponse< async isLastPage() { const body = await this.getParsedBody(); if (!dataIsPagedEntity(body)) { + Logger.debug('bravo:paging:next')(`NOT_PAGED %O`, body); return true; } if (body.page >= body.pages - 1) { + Logger.debug('bravo:paging:next')( + `ON_LAST_PAGE:${body.page}/${body.pages}`, + ); return true; } + Logger.debug('bravo:paging:next')( + `MORE_PAGES_EXIST:${body.page}/${body.pages}`, + ); return false; } diff --git a/src/lib/utility.ts b/src/lib/utility.ts index ef67bc8..e50f097 100644 --- a/src/lib/utility.ts +++ b/src/lib/utility.ts @@ -311,9 +311,9 @@ export function stringToDate( * Convert a numeric string to a number. If the string argument is * nullish, throw an error unless `defaultIfNullish` is provided. */ -export function stringToNumber( +export function stringToNumber( string: Nullable, - options?: { defaultIfNullish?: number; customError?: typeof BravoError }, + options?: { defaultIfNullish?: Default; customError?: typeof BravoError }, ) { if (isTruthyString(string)) { if (isNaN(Number(string))) { diff --git a/src/test/client.test.ts b/src/test/client.test.ts index 845ff69..cf40866 100644 --- a/src/test/client.test.ts +++ b/src/test/client.test.ts @@ -6,6 +6,8 @@ * for full testing. Therefore the developer must do some * manual setup work in a Favro Organization created for testing: * + * ## TESTING SETUP + * * - Create a separate Favro Organization for testing * (to keep your production Favro data safe!) * - A root .env file must be added and include vars: @@ -47,6 +49,11 @@ import type { BravoTagDefinition } from '$/lib/entities/BravoTag.js'; import type { FavroApi } from '$types/FavroApi.js'; import { BravoWebhookDefinition } from '$/lib/entities/BravoWebhook.js'; import type { BravoGroup } from '$/types/Bravo.js'; +import { + pagingTestMinCards, + pagingTestWidgetName, + populateTestOrg, +} from './utility/populateTestOrg.js'; /** * @remarks A root .env file must be populated with the required @@ -170,6 +177,7 @@ describe('BravoClient', function () { // are low) the tests become dependent on the outcomes of prior tests. before(async function () { + client.disableDebugLogging(); // Clean up any leftover remote testing content // (Since names aren't required to be unique, there could be quite a mess!) // NOTE: @@ -190,8 +198,8 @@ describe('BravoClient', function () { await collection.delete(); } testWebhookSecret = await generateRandomString(32, 'base64'); + await populateTestOrg(client); client.clearCache(); - client.disableDebugLogging(); }); it('utility functions behave', async function () { @@ -240,6 +248,18 @@ describe('BravoClient', function () { } }); + it('can page multi-page responses', async function () { + const pagingTestWidget = await client.findWidgetByName( + pagingTestWidgetName, + ); + assertBravoTestClaim(pagingTestWidget, 'Paging test widget not found'); + const cardsList = await client.listCardInstances({ + widgetCommonId: pagingTestWidget.widgetCommonId, + }); + const allCards = await cardsList.getAllEntities(); + expect(allCards.length).to.be.greaterThanOrEqual(pagingTestMinCards); + }); + it('can list organizations', async function () { canSkip(this); const organizations = await client.listOrganizations(); diff --git a/src/test/utility/populateTestOrg.ts b/src/test/utility/populateTestOrg.ts new file mode 100644 index 0000000..fb87816 --- /dev/null +++ b/src/test/utility/populateTestOrg.ts @@ -0,0 +1,66 @@ +/** + * @file + * + * 💥⚠️ **WARNING** ⚠️💥 + * + * Use with caution! When run, this will populate the org + * whose ID is in the root .env file with a huge batch + * of cards and custom fields. This is to allow testing + * of paging, since Favro API pages are 100 items long. + */ + +import type { BravoClient } from '../../index.js'; + +export const pagingTestCollectionName = '__BRAVO_POP'; +export const pagingTestWidgetName = '__BRAVO_POP_WIDGET'; +export const pagingTestCardPrefix = '__BRAVO_POP_CARD'; +export const pagingTestColumnName = '__BRAVO_POP_COLUMN'; +export const pagingTestMinCards = 110; + +export async function populateTestOrg(client: BravoClient) { + // Add a Collection, so that at least things can be kept separate + const testCollection = + (await client.findCollectionByName(pagingTestCollectionName)) || + (await client.createCollection(pagingTestCollectionName)); + + const testWidget = + (await client.findWidgetByName( + pagingTestWidgetName, + testCollection.collectionId, + )) || + (await client.createWidget( + testCollection.collectionId, + pagingTestWidgetName, + )); + + const testColumn = + (await client.findColumn( + testWidget.widgetCommonId, + (col) => col.name == pagingTestColumnName, + )) || + (await client.createColumn( + testWidget.widgetCommonId, + pagingTestColumnName, + )); + + const testCards = await ( + await client.listCardInstances({ + widgetCommonId: testWidget.widgetCommonId, + columnId: testColumn.columnId, + }) + ).getAllEntities(); + + const testCardCount = testCards.length; + if (testCardCount < pagingTestMinCards) { + const newCount = pagingTestMinCards - testCardCount; + console.log('Creating', newCount, 'cards'); + for (const i of Array(newCount).keys()) { + const newField = await client.createCard({ + widgetCommonId: testWidget.widgetCommonId, + name: `${pagingTestCardPrefix}_${i + testCardCount}`, + columnId: testColumn.columnId, + }); + testCards.push(newField); + } + } +}