Merge pull request #688 from namecheap/fix/app_filtering_query_fix

m2broth · web-flow · commit 2a0c28f734fe · 2026-01-26T17:58:17.000+03:00
fix: resolve SQL ambiguous column reference in apps API filtering
diff --git a/registry/server/apps/repositories/AppsRepository.ts b/registry/server/apps/repositories/AppsRepository.ts
@@ -50,7 +50,7 @@ export class AppsRepository {
             return;
         }
         if (filters.id || filters.name) {
-            query.whereIn('name', [...(filters.id ?? filters.name ?? [])]);
+            query.whereIn(`${Tables.Apps}.name`, [...(filters.id ?? filters.name ?? [])]);
         }
     }
 
@@ -62,14 +62,14 @@ export class AppsRepository {
         if (kind.length === 0) {
             return;
         }
-        query.whereIn('kind', kind);
+        query.whereIn(`${Tables.Apps}.kind`, kind);
     }
 
     private addFilterByQuery(query: Knex.QueryBuilder, filters: AppsGetListFilters) {
         if (!filters.q) {
             return;
         }
-        query.where('name', 'like', `%${filters.q}%`);
+        query.where(`${Tables.Apps}.name`, 'like', `%${filters.q}%`);
     }
 
     private addFilterByDomainId(query: Knex.QueryBuilder, filters: AppsGetListFilters) {
@@ -92,8 +92,12 @@ export class AppsRepository {
             query
                 .leftJoin(Tables.RouteSlots, `${Tables.RouteSlots}.appName`, `${Tables.Apps}.name`)
                 .leftJoin(Tables.Routes, `${Tables.Routes}.id`, `${Tables.RouteSlots}.routeId`)
-                .where(`${Tables.Routes}.domainId`, filters.domainId)
-                .orWhere(`${Tables.Apps}.enforceDomain`, filters.domainId)
+                .where(function () {
+                    this.where(`${Tables.Routes}.domainId`, filters.domainId).orWhere(
+                        `${Tables.Apps}.enforceDomain`,
+                        filters.domainId,
+                    );
+                })
                 .groupBy(`${Tables.Apps}.name`)
                 .groupBy('v.versionId');
         }
diff --git a/registry/tests/apps.spec.ts b/registry/tests/apps.spec.ts
@@ -550,6 +550,60 @@ describe(`Tests ${example.url}`, () => {
             }
         });
 
+        it('should successfully return records filtered by name search (q) and domainId combined', async () => {
+            const teardownFns: (() => Promise<void>)[] = [];
+
+            try {
+                for (const app of example.appsList) {
+                    await req.post(example.url).send(app).expect(200);
+                }
+                teardownFns.unshift(async () => {
+                    for (const app of example.appsList) {
+                        await req.delete(example.url + app.name).expect(204);
+                    }
+                });
+
+                const template = {
+                    name: 'hello500',
+                    content: '<html><head></head><body class="custom">hello500</body></html>',
+                };
+                await req.post('/api/v1/template').send(template).expect(200);
+                teardownFns.unshift(async () => {
+                    await req.delete('/api/v1/template/' + template.name).expect(204);
+                });
+
+                const domain = { domainName: 'example.com', template500: template.name };
+                const {
+                    body: { id: routerDomainId },
+                } = await req.post('/api/v1/router_domains').send(domain).expect(200);
+                teardownFns.unshift(async () => {
+                    await req.delete('/api/v1/router_domains/' + routerDomainId).expect(204);
+                });
+
+                const route = {
+                    next: false,
+                    slots: { myslot: { appName: example.appsList[1].name, kind: 'primary' } },
+                    route: 'myroute',
+                    domainId: routerDomainId,
+                };
+                const {
+                    body: { id: routeId },
+                } = await req.post('/api/v1/route').send(route).expect(200);
+                teardownFns.unshift(async () => {
+                    await req.delete('/api/v1/route/' + routeId).expect(204);
+                });
+
+                const query = makeFilterQuery({ q: 'app1', domainId: routerDomainId });
+                const response = await req.get(`${example.url}?${query}`).expect(200);
+
+                expectAppsListEqual(response.body, [example.appsList[1]]);
+            } finally {
+                for (const fn of teardownFns) {
+                    await fn();
+                }
+            }
+        });
+
         it('should successfully filter by name array (single name)', async () => {
             try {
                 for (const app of example.appsList) {

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ export class AppsRepository {`
`50`	`50`	`return;`
`51`	`51`	`}`
`52`	`52`	`if (filters.id \|\| filters.name) {`
`53`		`- query.whereIn('name', [...(filters.id ?? filters.name ?? [])]);`
	`53`	+ query.whereIn(`${Tables.Apps}.name`, [...(filters.id ?? filters.name ?? [])]);
`54`	`54`	`}`
`55`	`55`	`}`
`56`	`56`
`@@ -62,14 +62,14 @@ export class AppsRepository {`
`62`	`62`	`if (kind.length === 0) {`
`63`	`63`	`return;`
`64`	`64`	`}`
`65`		`- query.whereIn('kind', kind);`
	`65`	+ query.whereIn(`${Tables.Apps}.kind`, kind);
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`private addFilterByQuery(query: Knex.QueryBuilder, filters: AppsGetListFilters) {`
`69`	`69`	`if (!filters.q) {`
`70`	`70`	`return;`
`71`	`71`	`}`
`72`		- query.where('name', 'like', `%${filters.q}%`);
	`72`	+ query.where(`${Tables.Apps}.name`, 'like', `%${filters.q}%`);
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`private addFilterByDomainId(query: Knex.QueryBuilder, filters: AppsGetListFilters) {`
`@@ -92,8 +92,12 @@ export class AppsRepository {`
`92`	`92`	`query`
`93`	`93`	.leftJoin(Tables.RouteSlots, `${Tables.RouteSlots}.appName`, `${Tables.Apps}.name`)
`94`	`94`	.leftJoin(Tables.Routes, `${Tables.Routes}.id`, `${Tables.RouteSlots}.routeId`)
`95`		- .where(`${Tables.Routes}.domainId`, filters.domainId)
`96`		- .orWhere(`${Tables.Apps}.enforceDomain`, filters.domainId)
	`95`	`+ .where(function () {`
	`96`	+ this.where(`${Tables.Routes}.domainId`, filters.domainId).orWhere(
	`97`	+ `${Tables.Apps}.enforceDomain`,
	`98`	`+ filters.domainId,`
	`99`	`+ );`
	`100`	`+ })`
`97`	`101`	.groupBy(`${Tables.Apps}.name`)
`98`	`102`	`.groupBy('v.versionId');`
`99`	`103`	`}`