fix(core): Resolve n+1 query issue in collection list

[biggamesmallworld]

Description

Addresses issue #4010. Removes the n+1 query behavior from the findAll method in the collection service. This was due to fetching productVariants one at a time. The solution is to provide a field resolver to batch this query into one, and to expose that count property to the request context. If the request context finds __productVariantCount on the collection in the request context, it we use getMany instead of getManyAndCount, because we have already run this query. This conditional prevents counting the same data twice in the same api call.

Breaking changes

N/A

Screenshots

I've benchmarked the n+1 query using this test:

/* eslint-disable @typescript-eslint/no-non-null-assertion */
import {
    DefaultJobQueuePlugin,
    facetValueCollectionFilter,
    Logger,
    TransactionalConnection,
    VendureLogger,
} from '@vendure/core';
import { createTestEnvironment, registerInitializer, SqljsInitializer } from '@vendure/testing';
import gql from 'graphql-tag';
import path from 'path';
import { Bench } from 'tinybench';
import { DataSource } from 'typeorm';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';

import { initialData } from '../../../e2e-common/e2e-initial-data';
import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';

import { ASSET_FRAGMENT } from './graphql/fragments';
import * as Codegen from './graphql/generated-e2e-admin-types';
import { CREATE_COLLECTION, GET_FACET_LIST } from './graphql/shared-definitions';
import { awaitRunningJobs } from './utils/await-running-jobs';

registerInitializer('sqljs', new SqljsInitializer(path.join(__dirname, '__data__'), 1000));

/**
 * Custom logger to count SQL queries
 */
class QueryCountingLogger implements VendureLogger {
    private queryCount = 0;
    private queries: string[] = [];
    private nPlusOneCount = 0;
    private isLogging = false;

    startLogging() {
        this.isLogging = true;
        this.queryCount = 0;
        this.queries = [];
        this.nPlusOneCount = 0;
    }

    stopLogging() {
        this.isLogging = false;
    }

    getQueryCount() {
        return this.queryCount;
    }

    getNPlusOneCount() {
        return this.nPlusOneCount;
    }

    getQueries() {
        return this.queries;
    }

    logQuery(query: string, parameters?: any[]) {
        if (!this.isLogging) return;

        this.queryCount++;
        this.queries.push(query);

        // Detect the specific N+1 pattern mentioned in the issue
        if (
            query.includes('SELECT DISTINCT "distinctAlias"."productvariant_id"') &&
            query.includes('FROM "product_variant" "productvariant"') &&
            query.includes('WHERE "product"."deletedAt" IS NULL') &&
            query.includes('AND "collection"."id" =')
        ) {
            this.nPlusOneCount++;
        }
    }

    logQueryError(error: string | Error, query: string, parameters?: any[]) {
        // no-op for benchmarking
    }

    logQuerySlow(time: number, query: string, parameters?: any[]) {
        // no-op for benchmarking
    }

    logSchemaBuild(message: string) {
        // no-op for benchmarking
    }

    logMigration(message: string) {
        // no-op for benchmarking
    }

    log(level: 'log' | 'info' | 'warn', message: any) {
        // no-op for benchmarking
    }
}

// GraphQL query that matches the dashboard query with productVariants
const GET_COLLECTION_LIST_WITH_VARIANTS = gql`
    query CollectionListWithVariants($options: CollectionListOptions) {
        collections(options: $options) {
            items {
                id
                createdAt
                updatedAt
                featuredAsset {
                    ...Asset
                }
                name
                slug
                description
                breadcrumbs {
                    id
                    name
                    slug
                }
                children {
                    id
                    name
                }
                position
                isPrivate
                parentId
                productVariants {
                    totalItems
                }
            }
            totalItems
        }
    }
    ${ASSET_FRAGMENT}
`;

const { server, adminClient } = createTestEnvironment(testConfig());

let queryLogger: QueryCountingLogger;
let connection: TransactionalConnection;
let dataSource: DataSource;

beforeAll(async () => {
    await server.init({
        initialData,
        productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-collections.csv'),
        customerCount: 1,
    });
    await adminClient.asSuperAdmin();

    connection = server.app.get(TransactionalConnection);
    dataSource = connection.rawConnection;

    // Create multiple collections with products to simulate real-world scenario
    const facetValuesResult = await adminClient.query<Codegen.GetFacetListQuery>(GET_FACET_LIST);
    const electronicsId = facetValuesResult.facets.items
        .find(f => f.code === 'category')
        ?.values.find(v => v.code === 'electronics')?.id;

    if (electronicsId) {
        // Create 10 test collections to demonstrate the N+1 issue
        for (let i = 0; i < 10; i++) {
            await adminClient.query<
                Codegen.CreateCollectionMutation,
                Codegen.CreateCollectionMutationVariables
            >(CREATE_COLLECTION, {
                input: {
                    filters: [
                        {
                            code: facetValueCollectionFilter.code,
                            arguments: [
                                {
                                    name: 'facetValueIds',
                                    value: `["${electronicsId}"]`,
                                },
                                {
                                    name: 'containsAny',
                                    value: 'false',
                                },
                            ],
                        },
                    ],
                    translations: [
                        {
                            languageCode: Codegen.LanguageCode.en,
                            name: `Test Collection ${i}`,
                            description: `Test collection ${i} for N+1 benchmark`,
                            slug: `test-collection-${i}`,
                        },
                    ],
                },
            });
        }
    }

    await awaitRunningJobs(adminClient);

    // Set up custom query logger
    queryLogger = new QueryCountingLogger();
    dataSource.setOptions({
        ...dataSource.options,
        logging: true,
        logger: queryLogger,
    });
}, TEST_SETUP_TIMEOUT_MS);

afterAll(async () => {
    await server.destroy();
});

const isDevelopment = process.env.NODE_ENV === 'development';
describe.skipIf(isDevelopment)('Collection N+1 Query - Benchmark', () => {
    it('measures query count for CollectionList with productVariants', async () => {
        // Reset and start logging
        queryLogger.startLogging();

        // Execute the query
        const result = await adminClient.query<
            Codegen.GetCollectionListQuery,
            Codegen.GetCollectionListQueryVariables
        >(GET_COLLECTION_LIST_WITH_VARIANTS, {
            options: {
                take: 20,
            },
        });

        // Stop logging
        queryLogger.stopLogging();

        const totalQueries = queryLogger.getQueryCount();
        const nPlusOneQueries = queryLogger.getNPlusOneCount();
        const collectionCount = result.collections.items.length;

        console.log('\n=== Collection N+1 Query Benchmark Results ===');
        console.log(`Collections returned: ${collectionCount}`);
        console.log(`Total SQL queries executed: ${totalQueries}`);
        console.log(`N+1 queries (productVariant count): ${nPlusOneQueries}`);
        console.log('=============================================\n');

        // Expected behavior: N+1 queries should equal the number of collections (one per collection)
        // After fix: N+1 queries should be 0 (all data fetched in bulk)
        expect(result.collections.items.length).toBeGreaterThan(0);
        expect(totalQueries).toBeGreaterThan(0);

        // Store baseline metrics for comparison
        // Current (broken) state: nPlusOneQueries ≈ collectionCount
        // Target (fixed) state: nPlusOneQueries = 0
        console.log(`Baseline: ${nPlusOneQueries} N+1 queries detected for ${collectionCount} collections`);
    });

    it('benchmarks execution time for CollectionList query', async () => {
        const bench = new Bench({
            warmupTime: 0,
            warmupIterations: 2,
            time: 0,
            iterations: 50,
        });

        bench.add('CollectionList with productVariants', async () => {
            await adminClient.query<
                Codegen.GetCollectionListQuery,
                Codegen.GetCollectionListQueryVariables
            >(GET_COLLECTION_LIST_WITH_VARIANTS, {
                options: {
                    take: 20,
                },
            });
        });

        const tasks = await bench.run();

        console.log('\n=== Performance Benchmark Results ===');
        tasks.forEach(task => {
            if (task.result?.mean) {
                console.log(`Average execution time: ${task.result.mean.toFixed(2)}ms`);
                console.log(`Operations per second: ${(1000 / task.result.mean).toFixed(2)}`);
                console.log(`Standard deviation: ${task.result.sd?.toFixed(2)}ms`);
                console.log(`Min: ${task.result.min?.toFixed(2)}ms`);
                console.log(`Max: ${task.result.max?.toFixed(2)}ms`);
            }
        });
        console.log('=====================================\n');

        // Performance assertion - adjust this threshold as needed
        tasks.forEach(task => {
            expect(task.result?.mean).toBeDefined();
            if (task.result?.mean) {
                // Current baseline - this should improve significantly after the fix
                console.log(`Current performance baseline: ${task.result.mean.toFixed(2)}ms per query`);
            }
        });
    });

    it('provides detailed query analysis', async () => {
        queryLogger.startLogging();

        await adminClient.query<
            Codegen.GetCollectionListQuery,
            Codegen.GetCollectionListQueryVariables
        >(GET_COLLECTION_LIST_WITH_VARIANTS, {
            options: {
                take: 5,
            },
        });

        queryLogger.stopLogging();

        const queries = queryLogger.getQueries();
        const uniqueQueryPatterns = new Set(
            queries.map(q =>
                q
                    .replace(/\$\d+/g, '$N') // Normalize parameters
                    .replace(/= '[^']*'/g, "= 'X'") // Normalize string values
                    .replace(/= \d+/g, '= N'), // Normalize numeric values
            ),
        );

        console.log('\n=== Query Analysis ===');
        console.log(`Total queries: ${queries.length}`);
        console.log(`Unique query patterns: ${uniqueQueryPatterns.size}`);
        console.log('\nRepeated Query Patterns:');

        const patternCounts = new Map<string, number>();
        queries.forEach(q => {
            const normalized = q
                .replace(/\$\d+/g, '$N')
                .replace(/= '[^']*'/g, "= 'X'")
                .replace(/= \d+/g, '= N');
            patternCounts.set(normalized, (patternCounts.get(normalized) || 0) + 1);
        });

        // Show patterns that are repeated (potential N+1)
        Array.from(patternCounts.entries())
            .filter(([, count]) => count > 1)
            .sort((a, b) => b[1] - a[1])
            .forEach(([pattern, count]) => {
                console.log(`  - Executed ${count} times: ${pattern.substring(0, 100)}...`);
            });

        console.log('======================\n');
    });
});

Before 11 n+1 queries:

This repeated query matches the query reported in the original issue:

SELECT DISTINCT "distinctAlias"."productvariant_id" AS "ids_productvariant_id" FROM (SELECT "product...

After:

Checklist

📌 Always:

• I have set a clear title
• My PR is small and contains a single feature
• I have checked my own PR

👍 Most of the time:

• I have added or updated test cases
• I have updated the README if needed

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
docs	Ready	Preview, Comment	Jan 8, 2026 3:28pm
vendure-storybook	Ready	Preview, Comment	Jan 8, 2026 3:28pm

[coderabbitai]

📝 Walkthrough

Walkthrough

Injects ProductVariantService into CollectionService and eagerly loads per-collection variant totals in findAll, storing them on each collection as \_\_productVariantCount. Adds ProductVariantService.getVariantCountsByCollectionIds to fetch counts for multiple collection IDs in one query. CollectionEntityResolver.productVariants now passes the cached \_\_productVariantCount to ProductVariantService.getVariantsByCollectionId to enable a cache-aware fast path while preserving existing behavior when no cached count is present.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: resolving an N+1 query issue in the collection list, which is the primary objective of this PR.
Description check	✅ Passed	The description covers the issue being fixed, explains the solution approach, includes benchmark results with screenshots showing the improvement, and addresses breaking changes. However, test cases were not added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @packages/core/src/api/resolvers/entity/collection-entity.resolver.ts:
- Around line 73-88: The current early-return treats requests missing explicit
pagination as count-only; change isCountOnlyRequest so it only becomes true when
an explicit count-only intent is provided (e.g. options.countOnly === true)
instead of when skip/take/filter/sort are absent; if cachedCount exists but no
explicit count-only flag, do NOT return items:[]—invoke
productVariantService.getVariantsByCollectionId(ctx, collection.id, options,
relations) to fetch items (and if possible avoid an extra count by passing the
cachedCount as a total override or by assigning cachedCount to the
result.totalItems after fetching); update the isCountOnlyRequest check and use
cachedCount only for true count-only short-circuit or as a totalItems
optimization after fetching items.

In @packages/core/src/service/services/collection.service.ts:
- Around line 219-245: The eager-loading variant count query in
CollectionService duplicates
ProductVariantService.getVariantCountsByCollectionIds; inject
ProductVariantService into the CollectionService constructor and replace the
inline query (the block that builds a QueryBuilder on ProductVariant and
computes collectionId→count) with a call to
ProductVariantService.getVariantCountsByCollectionIds(collectionIds), then build
the same countMap and assign __productVariantCount on each Collection just as
before; ensure you update the constructor signature to accept
ProductVariantService and adjust any DI/container wiring so CollectionService
uses the shared service method.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1364d09 and d63a89e.

📒 Files selected for processing (3)

• packages/core/src/api/resolvers/entity/collection-entity.resolver.ts
• packages/core/src/service/services/collection.service.ts
• packages/core/src/service/services/product-variant.service.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: e2e tests (20.x, postgres)
• GitHub Check: e2e tests (24.x, mariadb)
• GitHub Check: e2e tests (20.x, mysql)
• GitHub Check: e2e tests (22.x, mysql)
• GitHub Check: e2e tests (22.x, mariadb)
• GitHub Check: e2e tests (22.x, sqljs)
• GitHub Check: e2e tests (24.x, mysql)
• GitHub Check: e2e tests (24.x, postgres)
• GitHub Check: e2e tests (24.x, sqljs)
• GitHub Check: e2e tests (22.x, postgres)
• GitHub Check: e2e tests (20.x, mariadb)
• GitHub Check: e2e tests (20.x, sqljs)
• GitHub Check: publish_install (windows-latest, 24.x)
• GitHub Check: publish_install (windows-latest, 20.x)
• GitHub Check: publish_install (ubuntu-latest, 20.x)
• GitHub Check: publish_install (macos-latest, 22.x)
• GitHub Check: publish_install (windows-latest, 22.x)
• GitHub Check: publish_install (macos-latest, 20.x)
• GitHub Check: publish_install (ubuntu-latest, 22.x)
• GitHub Check: publish_install (ubuntu-latest, 24.x)

🔇 Additional comments (2)

packages/core/src/service/services/collection.service.ts (1)

70-72: LGTM!

The interface definition is appropriate for typing collections with preloaded variant counts. The __ prefix convention clearly indicates this is an internal property.

packages/core/src/service/services/product-variant.service.ts (1)

240-274: LGTM! Well-designed method for batching variant counts.

The implementation correctly:

• Handles empty input gracefully by returning an empty Map
• Filters out soft-deleted variants and products
• Initializes all requested collection IDs with a count of 0 (ensuring collections with no variants are represented)
• Safely parses count values as integers

This effectively solves the N+1 query problem for variant counts across multiple collections.

Note: This method is currently not being used in CollectionService.findAll(), where the same query logic is duplicated. See my review comment on collection.service.ts lines 219-245 for the refactoring suggestion.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @packages/core/src/service/services/collection.service.ts:
- Around line 222-236: Wrap the call to
productVariantService.getVariantCountsByCollectionIds in a try-catch inside the
items.length > 0 block (the eager-loading in the CollectionService findAll flow)
so a DB failure won't break the whole request; on error call the service logger
(e.g., this.logger.error) with context including the collectionIds and the
caught error, and as a fallback set (collection as
CollectionWithVariantCount).__productVariantCount = 0 for each collection so
resolvers still return sane defaults.

🧹 Nitpick comments (1)

packages/core/src/service/services/collection.service.ts (1)

71-73: Consider adding documentation for the interface.

The interface correctly extends Translated<Collection> with the cached count property. The __ prefix appropriately signals internal state, but adding JSDoc would help future maintainers understand its purpose and usage pattern.

📝 Suggested documentation

+/**
+ * @internal
+ * Collection with eagerly-loaded product variant count to prevent N+1 queries
+ * in resolver when accessing productVariants.totalItems
+ */
 interface CollectionWithVariantCount extends Translated<Collection> {
     __productVariantCount?: number;
 }

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d63a89e and bcaf8ef.

📒 Files selected for processing (2)

• packages/core/src/api/resolvers/entity/collection-entity.resolver.ts
• packages/core/src/service/services/collection.service.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/core/src/api/resolvers/entity/collection-entity.resolver.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: e2e tests (22.x, mariadb)
• GitHub Check: e2e tests (22.x, postgres)
• GitHub Check: e2e tests (20.x, postgres)
• GitHub Check: e2e tests (20.x, mariadb)
• GitHub Check: codegen / codegen
• GitHub Check: build (24.x)
• GitHub Check: unit tests (20.x)
• GitHub Check: build (22.x)
• GitHub Check: build (20.x)
• GitHub Check: unit tests (22.x)
• GitHub Check: publish_install (windows-latest, 22.x)
• GitHub Check: publish_install (macos-latest, 22.x)
• GitHub Check: publish_install (macos-latest, 20.x)
• GitHub Check: publish_install (windows-latest, 20.x)
• GitHub Check: publish_install (ubuntu-latest, 24.x)
• GitHub Check: publish_install (macos-latest, 24.x)
• GitHub Check: publish_install (windows-latest, 24.x)
• GitHub Check: publish_install (ubuntu-latest, 22.x)
• GitHub Check: publish_install (ubuntu-latest, 20.x)
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (2)

packages/core/src/service/services/collection.service.ts (2)

56-56: LGTM: Import supports N+1 query fix.

The ProductVariantService import is correctly added to support batched variant count retrieval.

---

103-103: LGTM: Dependency injection follows NestJS patterns.

The ProductVariantService injection is correctly implemented using NestJS constructor-based dependency injection.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find ProductVariantService file
fd "product-variant.service" --type f

Repository: vendurehq/vendure

Length of output: 193

---

🏁 Script executed:

# Search for the getVariantCountsByCollectionIds method
rg -n "getVariantCountsByCollectionIds" --type=ts -A 10 -B 2

Repository: vendurehq/vendure

Length of output: 2724

---

🏁 Script executed:

# Search for resolver usage of __productVariantCount
rg -n "__productVariantCount" --type=ts -B 3 -A 3

Repository: vendurehq/vendure

Length of output: 2287

---

🏁 Script executed:

# Get more context around the eager loading block to see if there's error handling
sed -n '200,240p' packages/core/src/service/services/collection.service.ts

Repository: vendurehq/vendure

Length of output: 1795

---

🏁 Script executed:

# Check if getVariantCountsByCollectionIds has any error handling or throws
sed -n '240,280p' packages/core/src/service/services/product-variant.service.ts

Repository: vendurehq/vendure

Length of output: 1500

---

🏁 Script executed:

# Check common error handling patterns in the codebase for similar database operations
rg -n "try\s*{" packages/core/src/service/services/collection.service.ts -A 5 | head -40

Repository: vendurehq/vendure

Length of output: 138

---

🏁 Script executed:

# Check how the resolver actually uses the cached count and what happens if it's missing
sed -n '60,90p' packages/core/src/api/resolvers/entity/collection-entity.resolver.ts

Repository: vendurehq/vendure

Length of output: 1371

---

🏁 Script executed:

# Search for error handling patterns in service files (try-catch blocks)
rg -n "try {" packages/core/src/service/services/collection.service.ts -A 5 -B 2

Repository: vendurehq/vendure

Length of output: 134

---

🏁 Script executed:

# Check if there are other async batch operations and how they handle errors
rg -n "async.*Ids" packages/core/src/service/services/collection.service.ts -A 10 | head -50

Repository: vendurehq/vendure

Length of output: 1258

---

🏁 Script executed:

# Check how errors are typically handled in the collection service
rg -n "Logger.error\|Logger.warn\|catch" packages/core/src/service/services/collection.service.ts -B 2 -A 2

Repository: vendurehq/vendure

Length of output: 43

---

🏁 Script executed:

# Check if global error handling is used (middleware/interceptors)
rg -n "catch\|error.*handler" packages/core/src/api/resolvers/entity/collection-entity.resolver.ts -B 2 -A 3

Repository: vendurehq/vendure

Length of output: 43

---

🏁 Script executed:

# Verify if the current implementation without error handling has been in use and working
git log --oneline -n 20 -- packages/core/src/service/services/collection.service.ts

Repository: vendurehq/vendure

Length of output: 134

---

🏁 Script executed:

# Check error handling patterns in service files more broadly
rg -n "catch.*error\|throw\|Logger" packages/core/src/service/services/collection.service.ts | head -20

Repository: vendurehq/vendure

Length of output: 43

---

🏁 Script executed:

# Check how other batch operations in product-variant.service handle errors
rg -n "async get.*Ids\|async.*Collections\|catch" packages/core/src/service/services/product-variant.service.ts -B 2 -A 5 | head -60

Repository: vendurehq/vendure

Length of output: 43

---

🏁 Script executed:

# Check if there's a pattern of wrapping service calls in resolvers with error handling
rg -n "this\.[a-zA-Z]+Service\." packages/core/src/api/resolvers/entity/collection-entity.resolver.ts -B 1 -A 3 | head -40

Repository: vendurehq/vendure

Length of output: 1277

---

🏁 Script executed:

# Check if there's a global error handling mechanism (middleware/interceptor)
fd "error" packages/core/src -type f | grep -E "middleware|interceptor|filter" | head -10

Repository: vendurehq/vendure

Length of output: 231

---

🏁 Script executed:

# Check the actual commit that added this eager loading to understand intent
git show bcaf8efd7 --stat | head -20

Repository: vendurehq/vendure

Length of output: 1129

---

🏁 Script executed:

# Check if getVariantCountsByCollectionIds is used elsewhere and whether those places have error handling
rg -n "getVariantCountsByCollectionIds" packages/core/src -B 2 -A 5

Repository: vendurehq/vendure

Length of output: 1717

---

Add error handling for batch variant count query.

The eager loading correctly prevents N+1 queries, but getVariantCountsByCollectionIds is an unguarded database operation. If it fails, the entire findAll breaks. Add try-catch with logging fallback:

            if (items.length > 0) {
                const collectionIds = items.map(c => c.id);
-               const countMap = await this.productVariantService.getVariantCountsByCollectionIds(
-                   ctx,
-                   collectionIds,
-               );
-
-               // Store counts on collection objects so the resolver can use them
-               items.forEach(collection => {
-                   (collection as CollectionWithVariantCount).__productVariantCount =
-                       countMap.get(collection.id) ?? 0;
-               });
+               try {
+                   const countMap = await this.productVariantService.getVariantCountsByCollectionIds(
+                       ctx,
+                       collectionIds,
+                   );
+                   items.forEach(collection => {
+                       (collection as CollectionWithVariantCount).__productVariantCount =
+                           countMap.get(collection.id) ?? 0;
+                   });
+               } catch (error) {
+                   Logger.error('Failed to eagerly load product variant counts', undefined, error);
+               }
            }

🤖 Prompt for AI Agents

In @packages/core/src/service/services/collection.service.ts around lines 222 -
236, Wrap the call to productVariantService.getVariantCountsByCollectionIds in a
try-catch inside the items.length > 0 block (the eager-loading in the
CollectionService findAll flow) so a DB failure won't break the whole request;
on error call the service logger (e.g., this.logger.error) with context
including the collectionIds and the caught error, and as a fallback set
(collection as CollectionWithVariantCount).__productVariantCount = 0 for each
collection so resolvers still return sane defaults.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @packages/core/src/service/services/product-variant.service.ts:
- Around line 256-286: The batch count query in getVariantCountsByCollectionIds
is missing the same channel filtering used by getVariantsByCollectionId, causing
inflated counts in multi-channel setups; update getVariantCountsByCollectionIds
to join the variant.channels relation (e.g., innerJoin('variant.channels',
'channel')) and add a filter for the current channel id (use ctx.channelId as a
query parameter, e.g., 'channel.id = :channelId') so the count only includes
variants available in the active channel (also mirror any deletedAt checks on
the channel if present in getVariantsByCollectionId).

🧹 Nitpick comments (1)

packages/core/src/api/resolvers/entity/collection-entity.resolver.ts (1)

73-85: The cached count optimization looks correct.

The implementation properly reads the pre-loaded __productVariantCount and passes it to the service method. Since cachedCount will be undefined when the property doesn't exist, the service correctly falls back to the count query path.

One minor consideration: the type assertion (collection as Collection & { __productVariantCount: number }) could mask runtime type issues. Consider using optional chaining for safer access:

♻️ Optional: Slightly safer property access

-        const cachedCount = (collection as Collection & { __productVariantCount: number })
-            .__productVariantCount;
+        const cachedCount = (collection as Collection & { __productVariantCount?: number })
+            ?.__productVariantCount;

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bcaf8ef and 10cf314.

📒 Files selected for processing (2)

• packages/core/src/api/resolvers/entity/collection-entity.resolver.ts
• packages/core/src/service/services/product-variant.service.ts

🧰 Additional context used

🧬 Code graph analysis (1)

packages/core/src/service/services/product-variant.service.ts (2)

packages/core/src/api/common/request-context.ts (1)

• RequestContext (179-447)

packages/common/src/shared-types.ts (1)

• ID (79-79)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: e2e tests (24.x, postgres)
• GitHub Check: e2e tests (24.x, mariadb)
• GitHub Check: e2e tests (20.x, mysql)
• GitHub Check: e2e tests (22.x, sqljs)
• GitHub Check: e2e tests (20.x, postgres)
• GitHub Check: e2e tests (20.x, sqljs)
• GitHub Check: codegen / codegen
• GitHub Check: build (22.x)
• GitHub Check: unit tests (24.x)
• GitHub Check: build (24.x)
• GitHub Check: unit tests (20.x)
• GitHub Check: publish_install (macos-latest, 20.x)
• GitHub Check: publish_install (windows-latest, 24.x)
• GitHub Check: publish_install (ubuntu-latest, 20.x)
• GitHub Check: publish_install (windows-latest, 20.x)
• GitHub Check: publish_install (ubuntu-latest, 22.x)
• GitHub Check: publish_install (ubuntu-latest, 24.x)
• GitHub Check: publish_install (macos-latest, 22.x)
• GitHub Check: publish_install (macos-latest, 24.x)
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (2)

packages/core/src/service/services/product-variant.service.ts (2)

214-215: Backward-compatible API extension.

Adding the optional cachedTotalItems parameter at the end maintains backward compatibility with existing callers. The signature change is appropriate.

---

277-283: Good defensive initialization pattern.

Pre-populating the map with zeros for all requested IDs (line 279) before applying query results ensures callers always get a count for each requested collection, even if the collection has no variants. This prevents potential undefined access issues downstream.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Missing channel filter in count query may cause incorrect totalItems in multi-channel setups.

The getVariantsByCollectionId method filters variants by channelId: ctx.channelId (line 219), but this batch count query doesn't apply the same channel filter. In multi-channel deployments, the cached count could be higher than the actual number of items returned, leading to incorrect pagination metadata.

🔧 Proposed fix: Add channel filtering to match the variant query

     async getVariantCountsByCollectionIds(
         ctx: RequestContext,
         collectionIds: ID[],
     ): Promise<Map<ID, number>> {
         if (collectionIds.length === 0) {
             return new Map();
         }

         const counts = await this.connection
             .getRepository(ctx, ProductVariant)
             .createQueryBuilder('variant')
             .select('collection.id', 'collectionId')
             .addSelect('COUNT(DISTINCT variant.id)', 'count')
             .innerJoin('variant.collections', 'collection')
             .innerJoin('variant.product', 'product')
+            .innerJoin('variant.channels', 'channel')
             .where('collection.id IN (:...ids)', { ids: collectionIds })
             .andWhere('variant.deletedAt IS NULL')
             .andWhere('product.deletedAt IS NULL')
+            .andWhere('channel.id = :channelId', { channelId: ctx.channelId })
             .groupBy('collection.id')
             .getRawMany();

🤖 Prompt for AI Agents

In @packages/core/src/service/services/product-variant.service.ts around lines
256 - 286, The batch count query in getVariantCountsByCollectionIds is missing
the same channel filtering used by getVariantsByCollectionId, causing inflated
counts in multi-channel setups; update getVariantCountsByCollectionIds to join
the variant.channels relation (e.g., innerJoin('variant.channels', 'channel'))
and add a filter for the current channel id (use ctx.channelId as a query
parameter, e.g., 'channel.id = :channelId') so the count only includes variants
available in the active channel (also mirror any deletedAt checks on the channel
if present in getVariantsByCollectionId).