fix(pr): pr review comments

bitbeckers · bitbeckers · commit de397b15ea3d · 2025-04-08T20:01:45.000+02:00
diff --git a/src/lib/db/queryModifiers/applyPagination.ts b/src/lib/db/queryModifiers/applyPagination.ts
@@ -1,5 +1,5 @@
 import { SelectQueryBuilder, Selectable } from "kysely";
-import { SupportedDatabases } from "../../../services/database/strategies/QueryStrategy.js";
+import { SupportedDatabase } from "../../../services/database/strategies/QueryStrategy.js";
 
 /**
  * Type definition for pagination parameters
@@ -36,7 +36,7 @@ type PaginationArgs = {
  * ```
  */
 export function applyPagination<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   Args extends PaginationArgs,
 >(
diff --git a/src/lib/db/queryModifiers/applySort.ts b/src/lib/db/queryModifiers/applySort.ts
@@ -1,6 +1,6 @@
 import { SelectQueryBuilder, Selectable } from "kysely";
 import { SortOrder } from "../../../graphql/schemas/enums/sortEnums.js";
-import { SupportedDatabases } from "../../../services/database/strategies/QueryStrategy.js";
+import { SupportedDatabase } from "../../../services/database/strategies/QueryStrategy.js";
 
 /**
  * Applies sorting to a query based on the provided arguments.
@@ -35,7 +35,7 @@ import { SupportedDatabases } from "../../../services/database/strategies/QueryS
  * ```
  */
 export function applySort<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   Args extends {
     sortBy?: { [K in keyof DB[T]]?: SortOrder | null | undefined };
diff --git a/src/lib/db/queryModifiers/applyWhere.ts b/src/lib/db/queryModifiers/applyWhere.ts
@@ -1,5 +1,5 @@
 import { expressionBuilder, SelectQueryBuilder, Selectable } from "kysely";
-import { SupportedDatabases } from "../../../services/database/strategies/QueryStrategy.js";
+import { SupportedDatabase } from "../../../services/database/strategies/QueryStrategy.js";
 import { BaseQueryArgsType } from "../../graphql/BaseQueryArgs.js";
 import { SortOrder } from "../../../graphql/schemas/enums/sortEnums.js";
 import {
@@ -40,7 +40,7 @@ import {
  * ```
  */
 export function applyWhere<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   // TODO: cleaner typing than object, object. We'd need to have a general where input type
   Args extends BaseQueryArgsType<
diff --git a/src/lib/db/queryModifiers/buildWhereCondition.ts b/src/lib/db/queryModifiers/buildWhereCondition.ts
@@ -1,5 +1,5 @@
 import { Expression, ExpressionBuilder, sql, SqlBool } from "kysely";
-import { SupportedDatabases } from "../../../services/database/strategies/QueryStrategy.js";
+import { SupportedDatabase } from "../../../services/database/strategies/QueryStrategy.js";
 import { getRelation, hasRelation } from "./tableRelations.js";
 
 // Define more specific types for our filter values
@@ -195,7 +195,7 @@ const filterBuilders: Record<FilterOperator, FilterBuilder> = {
  * - Undefined values in filter conditions are ignored
  */
 export function buildWhereCondition<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB,
 >(
   tableName: T,
diff --git a/src/lib/db/queryModifiers/queryModifiers.ts b/src/lib/db/queryModifiers/queryModifiers.ts
@@ -1,6 +1,6 @@
 import { SelectQueryBuilder, Selectable } from "kysely";
 import { SortOrder } from "../../../graphql/schemas/enums/sortEnums.js";
-import { SupportedDatabases } from "../../../services/database/strategies/QueryStrategy.js";
+import { SupportedDatabase } from "../../../services/database/strategies/QueryStrategy.js";
 import { BaseQueryArgsType } from "../../graphql/BaseQueryArgs.js";
 import { applyPagination } from "./applyPagination.js";
 import { applySort } from "./applySort.js";
@@ -27,7 +27,7 @@ import { applyWhere } from "./applyWhere.js";
  * ```
  */
 export type QueryModifier<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   Args,
 > = (
@@ -64,7 +64,7 @@ export type QueryModifier<
  * ```
  */
 export function composeQueryModifiers<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   Args,
 >(...modifiers: QueryModifier<DB, T, Args>[]) {
@@ -105,7 +105,7 @@ export function composeQueryModifiers<
  * ```
  */
 export function createStandardQueryModifier<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   Args extends BaseQueryArgsType<
     object,
diff --git a/src/services/database/entities/EntityServiceFactory.ts b/src/services/database/entities/EntityServiceFactory.ts
@@ -8,7 +8,7 @@ import {
 import { BaseQueryArgsType } from "../../../lib/graphql/BaseQueryArgs.js";
 import {
   QueryStrategy,
-  SupportedDatabases,
+  SupportedDatabase,
 } from "../strategies/QueryStrategy.js";
 import { QueryStrategyFactory } from "../strategies/QueryStrategyFactory.js";
 
@@ -45,7 +45,7 @@ export interface EntityService<TEntity, TArgs> {
  * @throws {Error} If the strategy for the table cannot be found
  */
 export function createEntityService<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   Args extends BaseQueryArgsType<
     Record<string, unknown>,
diff --git a/src/services/database/strategies/QueryStrategy.ts b/src/services/database/strategies/QueryStrategy.ts
@@ -3,7 +3,7 @@ import { Kysely, Selectable, SelectQueryBuilder } from "kysely";
 import type { CachingDatabase } from "../../../types/kyselySupabaseCaching.js";
 import type { DataDatabase } from "../../../types/kyselySupabaseData.js";
 
-export type SupportedDatabases = CachingDatabase | DataDatabase;
+export type SupportedDatabase = CachingDatabase | DataDatabase;
 
 /**
  * Abstract base class for building database queries with a consistent interface.
@@ -38,7 +38,7 @@ export type SupportedDatabases = CachingDatabase | DataDatabase;
  * }
  */
 export abstract class QueryStrategy<
-  DB extends SupportedDatabases,
+  DB extends SupportedDatabase,
   T extends keyof DB & string,
   Args = void,
   Selection = Selectable<DB[T]>,
diff --git a/src/services/database/strategies/QueryStrategyFactory.ts b/src/services/database/strategies/QueryStrategyFactory.ts
@@ -9,7 +9,7 @@ import { FractionsQueryStrategy } from "./FractionsQueryStrategy.js";
 import { HyperboardsQueryStrategy } from "./HyperboardsQueryStrategy.js";
 import { MarketplaceOrdersQueryStrategy } from "./MarketplaceOrdersQueryStrategy.js";
 import { MetadataQueryStrategy } from "./MetadataQueryStrategy.js";
-import { QueryStrategy, SupportedDatabases } from "./QueryStrategy.js";
+import { QueryStrategy, SupportedDatabase } from "./QueryStrategy.js";
 import { SalesQueryStrategy } from "./SalesQueryStrategy.js";
 import { SignatureRequestsQueryStrategy } from "./SignatureRequestsQueryStrategy.js";
 import { SupportedSchemasQueryStrategy } from "./SupportedSchemasQueryStrategy.js";
@@ -29,7 +29,7 @@ type QueryArgs = BaseQueryArgsType<
  * Type for strategy constructors to ensure they match the QueryStrategy interface
  */
 type QueryStrategyConstructor<
-  DB extends SupportedDatabases = SupportedDatabases,
+  DB extends SupportedDatabase = SupportedDatabase,
   T extends keyof DB & string = keyof DB & string,
   Args extends QueryArgs = QueryArgs,
 > = new () => QueryStrategy<DB, T, Args>;
@@ -38,8 +38,8 @@ type QueryStrategyConstructor<
  * Type for the strategy registry mapping table names to their constructors
  */
 type StrategyRegistry = {
-  [K in keyof SupportedDatabases & string]: QueryStrategyConstructor<
-    SupportedDatabases,
+  [K in keyof SupportedDatabase & string]: QueryStrategyConstructor<
+    SupportedDatabase,
     K
   >;
 };
@@ -48,10 +48,7 @@ type StrategyRegistry = {
  * Type for the strategy cache mapping table names to their instances
  */
 type StrategyCache = {
-  [K in keyof SupportedDatabases & string]?: QueryStrategy<
-    SupportedDatabases,
-    K
-  >;
+  [K in keyof SupportedDatabase & string]?: QueryStrategy<SupportedDatabase, K>;
 };
 
 /**
@@ -91,13 +88,13 @@ export class QueryStrategyFactory {
    * Cache of strategy instances
    * @private
    */
-  private static strategies: StrategyCache = new Proxy<StrategyCache>(
+  private static strategies = new Proxy<StrategyCache>(
     {},
     {
-      get<K extends keyof SupportedDatabases & string>(
+      get<K extends keyof SupportedDatabase & string>(
         target: StrategyCache,
         prop: K | string | symbol,
-      ): QueryStrategy<SupportedDatabases, K> | undefined {
+      ): QueryStrategy<SupportedDatabase, K> | undefined {
         if (typeof prop !== "string") {
           return undefined;
         }
@@ -106,7 +103,7 @@ export class QueryStrategyFactory {
 
         // Check if we already have a cached instance
         if (key in target && target[key]) {
-          return target[key] as QueryStrategy<SupportedDatabases, K>;
+          return target[key] as QueryStrategy<SupportedDatabase, K>;
         }
 
         // Get the constructor from the registry
@@ -121,10 +118,10 @@ export class QueryStrategyFactory {
 
         // Create and cache a new instance
         const strategy = new Constructor() as QueryStrategy<
-          SupportedDatabases,
+          SupportedDatabase,
           K
         >;
-        (target as Record<K, QueryStrategy<SupportedDatabases, K>>)[key] =
+        (target as Record<K, QueryStrategy<SupportedDatabase, K>>)[key] =
           strategy;
         return strategy;
       },
@@ -140,18 +137,17 @@ export class QueryStrategyFactory {
    * @throws Error if no strategy is registered for the table
    */
   static getStrategy<
-    DB extends SupportedDatabases,
+    DB extends SupportedDatabase,
     T extends keyof DB & string,
     Args extends QueryArgs = QueryArgs,
   >(tableName: T): QueryStrategy<DB, T, Args> {
-    const strategy = (this.strategies as Record<T, QueryStrategy<DB, T, Args>>)[
-      tableName
-    ];
+    const strategy =
+      this.strategies[tableName as keyof SupportedDatabase & string];
     if (!strategy) {
       throw new Error(
         `Failed to get strategy for table "${tableName}". This might be a type mismatch or the strategy is not properly registered.`,
       );
     }
-    return strategy;
+    return strategy as QueryStrategy<DB, T, Args>;
   }
 }
diff --git a/src/services/graphql/resolvers/collectionResolver.ts b/src/services/graphql/resolvers/collectionResolver.ts
@@ -10,7 +10,7 @@ import { User } from "../../../graphql/schemas/typeDefs/userTypeDefs.js";
 
 import { inject, injectable } from "tsyringe";
 import { CollectionService } from "../../database/entities/CollectionEntityService.js";
-import { Hypercert } from "../../../graphql/schemas/typeDefs/hypercertTypeDefs.js";
+import { GetHypercertsResponse } from "../../../graphql/schemas/typeDefs/hypercertTypeDefs.js";
 
 /**
  * GraphQL resolver for Collection operations.
@@ -113,7 +113,7 @@ class CollectionResolver {
    * }
    * ```
    */
-  @FieldResolver(() => [Hypercert])
+  @FieldResolver(() => GetHypercertsResponse)
   async hypercerts(@Root() collection: Collection) {
     if (!collection.id) {
       console.error(
diff --git a/src/types/argTypes.ts b/src/types/argTypes.ts
@@ -9,17 +9,6 @@ import {
   SignatureRequestStatusSearchOptions,
 } from "../graphql/schemas/inputs/searchOptions.js";
 
-export type SearchOptionType = {
-  string: typeof StringSearchOptions;
-  number: typeof NumberSearchOptions;
-  bigint: typeof BigIntSearchOptions;
-  id: typeof IdSearchOptions;
-  boolean: typeof BooleanSearchOptions;
-  stringArray: typeof StringArraySearchOptions;
-  numberArray: typeof NumberArraySearchOptions;
-  enum: typeof SignatureRequestStatusSearchOptions;
-};
-
 export const SearchOptionMap = {
   string: StringSearchOptions,
   number: NumberSearchOptions,
diff --git a/src/utils/addPriceInUSDToOrder.ts b/src/utils/addPriceInUSDToOrder.ts
@@ -15,8 +15,16 @@ export const addPriceInUsdToOrder = async (
     throw new Error(`Token price not found for ${currency}`);
   }
 
-  if (!tokenPrice.decimals || !tokenPrice.price) {
-    throw new Error(`Token price data incomplete for ${currency}`);
+  if (!tokenPrice.decimals) {
+    throw new Error(
+      `Token price data incomplete for ${currency}: decimals missing`,
+    );
+  }
+
+  if (!tokenPrice.price) {
+    throw new Error(
+      `Token price data incomplete for ${currency}: price missing`,
+    );
   }
 
   const unitsInPercentage = BigInt(unitsInHypercerts) / BigInt(100);
diff --git a/test/graphql/schemas/args/hypercertsArgs.test.ts b/test/graphql/schemas/args/hypercertsArgs.test.ts
@@ -48,8 +48,6 @@ describe("HypercertsArgs", () => {
     const sortInstance = new HypercertSortOptions();
     const sortFields = Object.keys(sortInstance);
 
-    console.log("Available sort fields:", sortFields); // Debug line
-
     // Basic fields that should be sortable
     expect(sortFields).toContain("id");
     expect(sortFields).toContain("creation_block_timestamp");
diff --git a/test/lib/db/queryModifiers/applyPagination.test.ts b/test/lib/db/queryModifiers/applyPagination.test.ts
@@ -96,11 +96,14 @@ describe("applyPagination", () => {
 
     it("should handle large values correctly", () => {
       const baseQuery = db.selectFrom("test_users").selectAll() as any;
-      const result = applyPagination(baseQuery, { first: 1000, offset: 5000 });
+      const result = applyPagination(baseQuery, {
+        first: 1000,
+        offset: Number.MAX_SAFE_INTEGER,
+      });
 
       const { sql, parameters } = result.compile();
       expect(sql).toMatch(/limit \$1 offset \$2/);
-      expect(parameters).toEqual([1000, 5000]);
+      expect(parameters).toEqual([1000, Number.MAX_SAFE_INTEGER]);
     });
   });
 
diff --git a/test/lib/db/queryModifiers/applySort.test.ts b/test/lib/db/queryModifiers/applySort.test.ts
@@ -50,6 +50,8 @@ describe("applySort", () => {
       const baseQuery = db.selectFrom("test_users").selectAll() as any;
       const result = applySort<TestDatabase, "test_users", any>(baseQuery, {});
 
+      expect(result).toBe(baseQuery);
+
       const { sql, parameters } = result.compile();
       expect(sql).not.toContain("order by");
       expect(parameters).toEqual([]);
@@ -131,6 +133,8 @@ describe("applySort", () => {
         },
       });
 
+      expect(result).toBe(baseQuery);
+
       const { sql } = result.compile();
       expect(sql).not.toContain("order by");
     });
@@ -185,42 +189,4 @@ describe("applySort", () => {
       expect(parameters).toContain(20);
     });
   });
-
-  describe("data validation", () => {
-    it("should correctly sort numeric values", async () => {
-      const result = await db
-        .selectFrom("test_users")
-        .selectAll()
-        .orderBy("score", "desc")
-        .execute();
-
-      expect(result[0].score).toBe(100);
-      expect(result[1].score).toBe(95);
-      expect(result[2].score).toBe(85);
-    });
-
-    it("should correctly sort text values", async () => {
-      const result = await db
-        .selectFrom("test_users")
-        .selectAll()
-        .orderBy("name", "asc")
-        .execute();
-
-      expect(result[0].name).toBe("Alice");
-      expect(result[1].name).toBe("Bob");
-      expect(result[2].name).toBe("Charlie");
-    });
-
-    it("should correctly sort dates", async () => {
-      const result = await db
-        .selectFrom("test_users")
-        .selectAll()
-        .orderBy("created_at", "asc")
-        .execute();
-
-      expect(result[0].name).toBe("Alice"); // 2024-01-01
-      expect(result[1].name).toBe("Bob"); // 2024-01-02
-      expect(result[2].name).toBe("Charlie"); // 2024-01-03
-    });
-  });
 });
diff --git a/test/lib/db/queryModifiers/applyWhere.test.ts b/test/lib/db/queryModifiers/applyWhere.test.ts
@@ -45,6 +45,8 @@ describe("applyWhere", () => {
         {},
       );
 
+      expect(result).toBe(baseQuery);
+
       const { sql, parameters } = result.compile();
       expect(sql).not.toContain("where");
       expect(parameters).toEqual([]);
diff --git a/test/lib/graphql/createEntitySortArgs.test.ts b/test/lib/graphql/createEntitySortArgs.test.ts
diff --git a/test/services/database/strategies/QueryStrategyFactory.test.ts b/test/services/database/strategies/QueryStrategyFactory.test.ts
