fix: Paging was not working due to invalid URL created for additional…

… pages.

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);

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();

src/lib/clientLib/FavroClient.ts

@@ -25,8 +25,10 @@ const favroApiBaseUrl = 'https://favro.com/api/v1';
 
 function createFavroApiUrl(url: string, params?: Record<string, string>) {
   // 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<Data, this>(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

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;
   }
 

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<Default>(
   string: Nullable<string>,
-  options?: { defaultIfNullish?: number; customError?: typeof BravoError },
+  options?: { defaultIfNullish?: Default; customError?: typeof BravoError },
 ) {
   if (isTruthyString(string)) {
     if (isNaN(Number(string))) {

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();

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);
+    }
+  }
+}