From 1d086a1f184c430f9bc74bee0a0f989181952130 Mon Sep 17 00:00:00 2001 From: iuliandanea Date: Fri, 25 Oct 2024 12:07:54 +0300 Subject: [PATCH] PER-9866 [back-end] Endpoint for admins to update environment configuration New endpoint to update a feature-flag. There are validations on the globallyEnabled and description fields Unit and functional tests are covering all scenarios --- .../api/docs/present/paths/feature_flags.yaml | 1 - .../api/src/feature_flag/controller.test.ts | 178 +++++++++++++++++- packages/api/src/feature_flag/controller.ts | 31 ++- packages/api/src/feature_flag/models.ts | 7 + .../queries/update_feature_flag.sql | 14 ++ .../api/src/feature_flag/service/update.ts | 32 ++++ .../api/src/feature_flag/validators.test.ts | 122 +++++++++++- packages/api/src/feature_flag/validators.ts | 34 +++- 8 files changed, 413 insertions(+), 6 deletions(-) create mode 100644 packages/api/src/feature_flag/queries/update_feature_flag.sql create mode 100644 packages/api/src/feature_flag/service/update.ts diff --git a/packages/api/docs/present/paths/feature_flags.yaml b/packages/api/docs/present/paths/feature_flags.yaml index c80fc71..6cf48a9 100644 --- a/packages/api/docs/present/paths/feature_flags.yaml +++ b/packages/api/docs/present/paths/feature_flags.yaml @@ -93,7 +93,6 @@ feature-flags/{id}: schema: type: object required: - - description - globallyEnabled properties: description: diff --git a/packages/api/src/feature_flag/controller.test.ts b/packages/api/src/feature_flag/controller.test.ts index a88def9..142c2d7 100644 --- a/packages/api/src/feature_flag/controller.test.ts +++ b/packages/api/src/feature_flag/controller.test.ts @@ -8,7 +8,11 @@ import { extractUserIsAdminFromAuthToken, verifyAdminAuthentication, } from "../middleware"; -import type { CreateFeatureFlagRequest, FeatureFlagRequest } from "./models"; +import type { + CreateFeatureFlagRequest, + UpdateFeatureFlagRequest, + FeatureFlagRequest, +} from "./models"; jest.mock("../database"); jest.mock("../middleware"); @@ -22,6 +26,8 @@ const clearDatabase = async (): Promise => { await db.query("TRUNCATE feature_flag CASCADE"); }; +const notExistingFeatureFlagId = "1bdf2da6-026b-4e8e-9b57-a86b1817be4d"; + describe("GET /feature-flags", () => { const agent = request(app); @@ -283,3 +289,173 @@ describe("POST /feature-flag", () => { expect(logger.error).toHaveBeenCalled(); }); }); + +describe("PUT /feature-flag/:featureFlagId", () => { + const agent = request(app); + beforeEach(async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (req: Request, __, next: NextFunction) => { + (req.body as UpdateFeatureFlagRequest).emailFromAuthToken = + "test@permanent.org"; + (req.body as UpdateFeatureFlagRequest).adminSubjectFromAuthToken = + "6b640c73-4963-47de-a096-4a05ff8dc5f5"; + next(); + } + ); + jest.restoreAllMocks(); + jest.clearAllMocks(); + await clearDatabase(); + await loadFixtures(); + }); + + afterEach(async () => { + jest.restoreAllMocks(); + jest.clearAllMocks(); + await clearDatabase(); + }); + + test("should respond with a 200 status code", async () => { + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "a description", + globallyEnabled: false, + }) + .expect(200); + }); + + test("should respond with 401 status code if lacking admin authentication", async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (_: Request, __, next: NextFunction) => { + next(new createError.Unauthorized("You aren't logged in")); + } + ); + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .expect(401); + }); + + test("should respond with 400 status code if missing emailFromAuthToken", async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (req: Request, __, next: NextFunction) => { + (req.body as UpdateFeatureFlagRequest).adminSubjectFromAuthToken = + "6b640c73-4963-47de-a096-4a05ff8dc5f5"; + next(); + } + ); + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "description", + globallyEnabled: true, + }) + .expect(400); + }); + + test("should respond with 400 status code if emailFromAuthToken is not a string", async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (req: Request, __, next: NextFunction) => { + (req.body as { emailFromAuthToken: number }).emailFromAuthToken = 123; + (req.body as UpdateFeatureFlagRequest).adminSubjectFromAuthToken = + "6b640c73-4963-47de-a096-4a05ff8dc5f5"; + next(); + } + ); + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "description", + globallyEnabled: true, + }) + .expect(400); + }); + + test("should respond with 400 status if globallyEnabled is missing", async () => { + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "description", + }) + .expect(400); + }); + + test("should respond with 400 status code if globallyEnabled is not a boolean", async () => { + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "description", + globallyEnabled: "a string", + }) + .expect(400); + }); + + test("should respond with 400 status code if description is not a text", async () => { + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: 1, + globallyEnabled: true, + }) + .expect(400); + }); + + test("should update the feature flag in the database", async () => { + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "a description", + globallyEnabled: false, + }) + .expect(200); + const result = await db.query( + `SELECT + description, + globally_enabled::boolean as "globallyEnabled" + FROM + feature_flag + WHERE + id = '1bdf2da6-026b-4e8e-9b57-a86b1817be5d'` + ); + expect(result.rows.length).toBe(1); + expect(result.rows[0]).toEqual({ + description: "a description", + globallyEnabled: false, + }); + }); + + test("should respond with 500 if the database call fails", async () => { + jest.spyOn(db, "sql").mockImplementation(() => { + throw new Error("SQL error"); + }); + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "description", + globallyEnabled: true, + }) + .expect(500); + }); + + test("should log the error if the database call fails", async () => { + const testError = new Error("SQL error"); + jest.spyOn(db, "sql").mockRejectedValueOnce(testError); + await agent + .put("/api/v2/feature-flags/1bdf2da6-026b-4e8e-9b57-a86b1817be5d") + .send({ + description: "description", + globallyEnabled: true, + }) + .expect(500); + expect(logger.error).toHaveBeenCalled(); + }); + + test("should respond with 404 status code if feature flag does not exist", async () => { + await agent + .put(`/api/v2/feature-flags/${notExistingFeatureFlagId}`) + .send({ + description: "description", + globallyEnabled: true, + }) + .expect(404); + }); +}); diff --git a/packages/api/src/feature_flag/controller.ts b/packages/api/src/feature_flag/controller.ts index 009e7e3..7c003c4 100644 --- a/packages/api/src/feature_flag/controller.ts +++ b/packages/api/src/feature_flag/controller.ts @@ -4,13 +4,18 @@ import type { Request, Response, NextFunction } from "express"; import { logger } from "@stela/logger"; import { featureService } from "./service"; import { createFeatureService } from "./service/create"; +import { updateFeatureService } from "./service/update"; import { extractUserIsAdminFromAuthToken, verifyAdminAuthentication, } from "../middleware"; -import { validateCreateFeatureFlagRequest } from "./validators"; -import { validateIsAdminFromAuthentication } from "../validators/shared"; import { isValidationError } from "../validators/validator_util"; +import { + validateCreateFeatureFlagRequest, + validateUpdateFeatureFlagRequest, + validateFeatureFlagParams, +} from "./validators"; +import { validateIsAdminFromAuthentication } from "../validators/shared"; export const featureController = Router(); @@ -52,3 +57,25 @@ featureController.post( } } ); + +featureController.put( + "/:featureId", + verifyAdminAuthentication, + async (req: Request, res: Response, next: NextFunction) => { + try { + validateUpdateFeatureFlagRequest(req.body); + validateFeatureFlagParams(req.params); + const featureFlag = await updateFeatureService.updateFeatureFlag( + req.params.featureId, + req.body + ); + res.status(200).send({ data: featureFlag }); + } catch (err) { + if (isValidationError(err)) { + res.status(400).json({ error: err.message }); + return; + } + next(err); + } + } +); diff --git a/packages/api/src/feature_flag/models.ts b/packages/api/src/feature_flag/models.ts index 08234ba..5972781 100644 --- a/packages/api/src/feature_flag/models.ts +++ b/packages/api/src/feature_flag/models.ts @@ -21,3 +21,10 @@ export interface CreateFeatureFlagRequest { export interface FeatureFlagRequest { admin: boolean; } + +export interface UpdateFeatureFlagRequest { + emailFromAuthToken: string; + adminSubjectFromAuthToken: string; + description: string; + globallyEnabled: boolean; +} diff --git a/packages/api/src/feature_flag/queries/update_feature_flag.sql b/packages/api/src/feature_flag/queries/update_feature_flag.sql new file mode 100644 index 0000000..461e640 --- /dev/null +++ b/packages/api/src/feature_flag/queries/update_feature_flag.sql @@ -0,0 +1,14 @@ +UPDATE feature_flag +SET + description = :description, + globally_enabled = :globally_enabled, + updated_at = CURRENT_TIMESTAMP +WHERE ( + id = :id +) RETURNING +id, +name, +description, +globally_enabled AS "globallyEnabled", +created_at AS "createdAt", +updated_at AS "updatedAt"; diff --git a/packages/api/src/feature_flag/service/update.ts b/packages/api/src/feature_flag/service/update.ts new file mode 100644 index 0000000..a2a7536 --- /dev/null +++ b/packages/api/src/feature_flag/service/update.ts @@ -0,0 +1,32 @@ +import { logger } from "@stela/logger"; +import createError from "http-errors"; +import type { FeatureFlagRow, UpdateFeatureFlagRequest } from "../models"; +import { db } from "../../database"; + +export const updateFeatureFlag = async ( + featureFlagId: string, + featureFlagData: UpdateFeatureFlagRequest +): Promise => { + const result = await db + .sql("feature_flag.queries.update_feature_flag", { + id: featureFlagId, + description: featureFlagData.description, + globally_enabled: featureFlagData.globallyEnabled, + }) + .catch((err) => { + logger.error(err); + throw new createError.InternalServerError( + "Failed to update feature flag" + ); + }); + + if (result.rows[0] === undefined) { + throw new createError.NotFound("Feature flag not found"); + } + + return result.rows[0]; +}; + +export const updateFeatureService = { + updateFeatureFlag, +}; diff --git a/packages/api/src/feature_flag/validators.test.ts b/packages/api/src/feature_flag/validators.test.ts index 439910f..31671ae 100644 --- a/packages/api/src/feature_flag/validators.test.ts +++ b/packages/api/src/feature_flag/validators.test.ts @@ -1,4 +1,8 @@ -import { validateCreateFeatureFlagRequest } from "./validators"; +import { + validateCreateFeatureFlagRequest, + validateUpdateFeatureFlagRequest, + validateFeatureFlagParams, +} from "./validators"; describe("validateCreateFeatureFlagRequest", () => { test("should find no errors in a valid request", () => { @@ -76,3 +80,119 @@ describe("validateCreateFeatureFlagRequest", () => { } }); }); + +describe("validateUpdateFeatureFlagRequest", () => { + test("should find no errors in a valid request", () => { + let error = null; + try { + validateUpdateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + description: "feature flag description", + globallyEnabled: false, + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + test("should not raise an error when optional fields are missing", () => { + let error = null; + try { + validateUpdateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + globallyEnabled: false, + description: "feature flag description", + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + test("should raise an error if globallyEnabled is missing", () => { + let error = null; + try { + validateUpdateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + description: "feature flag description", + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + test("should raise an error if globallyEnabled is wrong type", () => { + let error = null; + try { + validateUpdateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + name: 1, + description: "feature flag description", + globallyEnabled: "test", + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should raise an error if description is wrong type", () => { + let error = null; + try { + validateUpdateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + description: 123, + globallyEnabled: false, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); +}); + +describe("validateFeatureFlagParams", () => { + test("should find no errors in a valid request", () => { + let error = null; + try { + validateFeatureFlagParams({ + featureId: "bdefb1b1-2a2d-410a-b9a2-d41f15dfb5ce", + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + test("should raise an error if id is missing", () => { + let error = null; + try { + validateFeatureFlagParams({}); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + test("should raise an error if id is wrong type", () => { + let error = null; + try { + validateFeatureFlagParams({ + featureId: 3, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); +}); diff --git a/packages/api/src/feature_flag/validators.ts b/packages/api/src/feature_flag/validators.ts index 9a3fff1..36b4eff 100644 --- a/packages/api/src/feature_flag/validators.ts +++ b/packages/api/src/feature_flag/validators.ts @@ -1,5 +1,8 @@ import Joi from "joi"; -import type { CreateFeatureFlagRequest } from "./models"; +import type { + CreateFeatureFlagRequest, + UpdateFeatureFlagRequest, +} from "./models"; import { fieldsFromAdminAuthentication } from "../validators"; export function validateCreateFeatureFlagRequest( @@ -17,3 +20,32 @@ export function validateCreateFeatureFlagRequest( throw validation.error; } } + +export function validateUpdateFeatureFlagRequest( + data: unknown +): asserts data is UpdateFeatureFlagRequest { + const validation = Joi.object() + .keys({ + ...fieldsFromAdminAuthentication, + description: Joi.string(), + globallyEnabled: Joi.boolean().required(), + }) + .validate(data); + + if (validation.error) { + throw validation.error; + } +} + +export function validateFeatureFlagParams( + data: unknown +): asserts data is { featureId: string } { + const validation = Joi.object() + .keys({ + featureId: Joi.string().uuid().required(), + }) + .validate(data); + if (validation.error) { + throw validation.error; + } +}