From 54569cfc53f91f3da6924d53c30b2ef45ec0b8f4 Mon Sep 17 00:00:00 2001 From: Joseph Connolly Date: Sun, 22 Oct 2023 15:42:26 -0700 Subject: [PATCH 1/2] Bug fix: Page is not being passed to queryPage from Cards.search paging test was comparing the last card to the first card and giving a false positive --- src/api/Cards.ts | 10 +++++++++- src/api/Migrations.ts | 2 +- src/tests/Main.ts | 2 +- src/util/MagicQuerier.ts | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/api/Cards.ts b/src/api/Cards.ts index 007da2a..fc7f9e6 100644 --- a/src/api/Cards.ts +++ b/src/api/Cards.ts @@ -635,7 +635,15 @@ class Cards extends MagicQuerier { const emitter = new MagicEmitter() .map(Card.construct); - this.queryPage(emitter, "cards/search", { q: query, ...typeof options === "number" ? { page: options } : options }) + let page = 1; + if (typeof options === "number") { + page = options; + options = {}; + } else if (options) { + page = options.page ?? 1; + } + + this.queryPage(emitter, "cards/search", { q: query, ...options }, page) .catch(err => emitter.emit("error", err)); return emitter; diff --git a/src/api/Migrations.ts b/src/api/Migrations.ts index 0613ea4..1771c6b 100644 --- a/src/api/Migrations.ts +++ b/src/api/Migrations.ts @@ -23,7 +23,7 @@ export interface Migration { } class Migrations extends MagicQuerier { - public all (page?: number) { + public all (page = 1) { const emitter = new MagicEmitter(); this.queryPage(emitter, "migrations", {}, page) diff --git a/src/tests/Main.ts b/src/tests/Main.ts index 13391f8..ce163ea 100644 --- a/src/tests/Main.ts +++ b/src/tests/Main.ts @@ -168,7 +168,7 @@ describe("Scry", function () { }), new Promise((resolve, reject) => { const emitter = Scry.Cards.search("type:creature", 2).cancelAfterPage(); - emitter.on("data", card => secondPageCard = card) + emitter.on("data", card => (secondPageCard = card, emitter.cancel())) .on("end", () => reject(new Error("Did not expect to reach this point"))) .on("cancel", resolve) .on("error", reject); diff --git a/src/util/MagicQuerier.ts b/src/util/MagicQuerier.ts index e842a61..cb49f95 100644 --- a/src/util/MagicQuerier.ts +++ b/src/util/MagicQuerier.ts @@ -88,7 +88,7 @@ export default class MagicQuerier { return result; } - protected async queryPage (emitter: MagicEmitter, apiPath: string, query: any, page = 1): Promise { + protected async queryPage (emitter: MagicEmitter, apiPath: string, query: any, page: number): Promise { let error: SearchError | undefined; const results = await this.query>(apiPath, { ...query, page }) .catch(err => error = err); From c489edc43bed0a4c4d245d5fdcfe85faa181f1b0 Mon Sep 17 00:00:00 2001 From: Chiri Vulpes <6081834+ChiriVulpes@users.noreply.github.com> Date: Mon, 23 Oct 2023 12:13:53 +1300 Subject: [PATCH 2/2] Tweak this bugfix and the test --- src/api/Cards.ts | 19 ++++++++++--------- src/tests/Main.ts | 29 +++++++++++------------------ src/util/MagicQuerier.ts | 2 +- 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/api/Cards.ts b/src/api/Cards.ts index fc7f9e6..856ac74 100644 --- a/src/api/Cards.ts +++ b/src/api/Cards.ts @@ -631,19 +631,20 @@ class Cards extends MagicQuerier { /** * Returns a MagicEmitter of every card in the Scryfall database that matches the given query. */ + public search (query: string, options?: SearchOptions): MagicEmitter; + /** + * Returns a MagicEmitter of every card in the Scryfall database that matches the given query. + */ + public search (query: string, page?: number): MagicEmitter; + /** + * Returns a MagicEmitter of every card in the Scryfall database that matches the given query. + */ + public search (query: string, options?: SearchOptions | number): MagicEmitter; public search (query: string, options?: SearchOptions | number) { const emitter = new MagicEmitter() .map(Card.construct); - let page = 1; - if (typeof options === "number") { - page = options; - options = {}; - } else if (options) { - page = options.page ?? 1; - } - - this.queryPage(emitter, "cards/search", { q: query, ...options }, page) + this.queryPage(emitter, "cards/search", { q: query, ...typeof options === "number" ? { page: options } : options }) .catch(err => emitter.emit("error", err)); return emitter; diff --git a/src/tests/Main.ts b/src/tests/Main.ts index ce163ea..5bc8877 100644 --- a/src/tests/Main.ts +++ b/src/tests/Main.ts @@ -155,24 +155,17 @@ describe("Scry", function () { }).timeout(15000); it("should support pagination of searches", async () => { - let firstPageCard: Scry.Card; - let secondPageCard: Scry.Card; - - await Promise.all([ - new Promise((resolve, reject) => { - const emitter = Scry.Cards.search("type:creature"); - emitter.on("data", card => (firstPageCard = card, emitter.cancel())) - .on("end", () => reject(new Error("Did not expect to reach this point"))) - .on("cancel", resolve) - .on("error", reject); - }), - new Promise((resolve, reject) => { - const emitter = Scry.Cards.search("type:creature", 2).cancelAfterPage(); - emitter.on("data", card => (secondPageCard = card, emitter.cancel())) - .on("end", () => reject(new Error("Did not expect to reach this point"))) - .on("cancel", resolve) - .on("error", reject); - }), + const [firstPageCard, secondPageCard] = await Promise.all([ + new Promise((resolve, reject) => Scry.Cards.search("type:creature") + .cancelAfterPage() + .waitForAll() + .then(cards => cards[0]) + .then(resolve, reject)), + new Promise((resolve, reject) => Scry.Cards.search("type:creature", 2) + .cancelAfterPage() + .waitForAll() + .then(cards => cards[0]) + .then(resolve, reject)), ]); expect(firstPageCard!.id).not.eq(secondPageCard!.id); diff --git a/src/util/MagicQuerier.ts b/src/util/MagicQuerier.ts index cb49f95..034545d 100644 --- a/src/util/MagicQuerier.ts +++ b/src/util/MagicQuerier.ts @@ -88,7 +88,7 @@ export default class MagicQuerier { return result; } - protected async queryPage (emitter: MagicEmitter, apiPath: string, query: any, page: number): Promise { + protected async queryPage (emitter: MagicEmitter, apiPath: string, query: any, page = query?.page as number | undefined ?? 1): Promise { let error: SearchError | undefined; const results = await this.query>(apiPath, { ...query, page }) .catch(err => error = err);