fix: return 404 when gettings tags for a non existing feature (#3560)

From the discussion here
#3496 (comment)

This PR checks the existence of the feature before trying to get tags
for the feature. Doing so by stealing the exists method from the feature
store, since that's what we need to know exists, and avoiding having the
feature store as a dependency to the featureTagStore seemed reasonable.

src/lib/db/feature-tag-store.ts

@@ -9,6 +9,7 @@ import {
     IFeatureTagStore,
 } from '../types/stores/feature-tag-store';
 import { Db } from './db';
+import NotFoundError from '../error/notfound-error';
 
 const COLUMNS = ['feature_name', 'tag_type', 'tag_value'];
 const TABLE = 'feature_tag';
@@ -95,12 +96,27 @@ class FeatureTagStore implements IFeatureTagStore {
 
     async getAllTagsForFeature(featureName: string): Promise<ITag[]> {
         const stopTimer = this.timer('getAllForFeature');
-        const rows = await this.db
-            .select(COLUMNS)
-            .from<FeatureTagTable>(TABLE)
-            .where({ feature_name: featureName });
-        stopTimer();
-        return rows.map(this.featureTagRowToTag);
+        if (await this.featureExists(featureName)) {
+            const rows = await this.db
+                .select(COLUMNS)
+                .from<FeatureTagTable>(TABLE)
+                .where({ feature_name: featureName });
+            stopTimer();
+            return rows.map(this.featureTagRowToTag);
+        } else {
+            throw new NotFoundError(
+                `Could not find feature with name ${featureName}`,
+            );
+        }
+    }
+
+    async featureExists(featureName: string): Promise<boolean> {
+        const result = await this.db.raw(
+            'SELECT EXISTS (SELECT 1 FROM features WHERE name = ?) AS present',
+            [featureName],
+        );
+        const { present } = result.rows[0];
+        return present;
     }
 
     async getAllByFeatures(features: string[]): Promise<IFeatureTag[]> {
@@ -187,13 +203,10 @@ class FeatureTagStore implements IFeatureTagStore {
     }
 
     featureTagRowToTag(row: FeatureTagTable): ITag {
-        if (row) {
-            return {
-                value: row.tag_value,
-                type: row.tag_type,
-            };
-        }
-        return null;
+        return {
+            value: row.tag_value,
+            type: row.tag_type,
+        };
     }
 
     rowToFeatureAndTag(row: FeatureTagTable): IFeatureAndTag {

src/lib/routes/admin-api/feature.ts

@@ -120,7 +120,7 @@ class FeatureController extends Controller {
                     operationId: 'listTags',
                     responses: {
                         200: createResponseSchema('tagsSchema'),
-                        ...getStandardResponses(401),
+                        ...getStandardResponses(401, 403, 404),
                     },
                 }),
             ],

src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap

@@ -6182,6 +6182,12 @@ If the provided project does not exist, the list of events will be empty.",
           "401": {
             "description": "Authorization information is missing or invalid. Provide a valid API token as the \`authorization\` header, e.g. \`authorization:*.*.my-admin-token\`.",
           },
+          "403": {
+            "description": "User credentials are valid but does not have enough privileges to execute this operation",
+          },
+          "404": {
+            "description": "The requested resource was not found.",
+          },
         },
         "summary": "Get all tags for a feature.",
         "tags": [

src/test/e2e/stores/feature-tag-store.e2e.test.ts

@@ -2,6 +2,7 @@ import { IFeatureTagStore } from 'lib/types/stores/feature-tag-store';
 import { IFeatureToggleStore } from 'lib/types/stores/feature-toggle-store';
 import dbInit from '../helpers/database-init';
 import getLogger from '../../fixtures/no-logger';
+import NotFoundError from '../../../lib/error/notfound-error';
 
 let stores;
 let db;
@@ -43,7 +44,7 @@ test('should tag feature', async () => {
     expect(featureTag.tagValue).toBe(tag.value);
 });
 
-test('feature tag exits', async () => {
+test('feature tag exists', async () => {
     await featureTagStore.tagFeature(featureName, tag);
     const exists = await featureTagStore.exists({
         featureName,
@@ -97,3 +98,20 @@ test('should import feature tags', async () => {
     const all = await featureTagStore.getAll();
     expect(all).toHaveLength(2);
 });
+
+test('should throw not found error if feature does not exist', async () => {
+    await expect(async () =>
+        featureTagStore.getAllTagsForFeature('non.existing.toggle'),
+    ).rejects.toThrow(
+        new NotFoundError(
+            `Could not find feature with name non.existing.toggle`,
+        ),
+    );
+});
+
+test('Returns empty tag list for existing feature with no tags', async () => {
+    const name = 'feature.with.no.tags';
+    await featureToggleStore.create('default', { name });
+    let tags = await featureTagStore.getAllTagsForFeature(name);
+    expect(tags).toHaveLength(0);
+});