refactor(create-config): move unique violation error handling to error-handler

packages/backend/src/controllers/api/v1/admin/apps/create-auth-client.ee.test.js

@@ -23,7 +23,7 @@ describe('POST /api/v1/admin/apps/:appKey/auth-clients', () => {
 
   it('should return created response for valid app config', async () => {
     await createAppConfig({
-      key: 'gitlab'
+      key: 'gitlab',
     });
 
     const appAuthClient = {
@@ -35,7 +35,7 @@ describe('POST /api/v1/admin/apps/:appKey/auth-clients', () => {
         clientSecret: 'sample client secret',
         instanceUrl: 'https://gitlab.com',
         oAuthRedirectUrl: 'http://localhost:3001/app/gitlab/connection/add',
-      }
+      },
     };
 
     const response = await request(app)
@@ -58,7 +58,7 @@ describe('POST /api/v1/admin/apps/:appKey/auth-clients', () => {
         clientSecret: 'sample client secret',
         instanceUrl: 'https://gitlab.com',
         oAuthRedirectUrl: 'http://localhost:3001/app/gitlab/connection/add',
-      }
+      },
     };
 
     await request(app)
@@ -70,7 +70,7 @@ describe('POST /api/v1/admin/apps/:appKey/auth-clients', () => {
 
   it('should return bad request response for missing required fields', async () => {
     await createAppConfig({
-      key: 'gitlab'
+      key: 'gitlab',
     });
 
     const appAuthClient = {
@@ -86,7 +86,9 @@ describe('POST /api/v1/admin/apps/:appKey/auth-clients', () => {
     expect(response.body.meta.type).toEqual('ModelValidation');
     expect(response.body.errors).toMatchObject({
       name: ["must have required property 'name'"],
-      formattedAuthDefaults: ["must have required property 'formattedAuthDefaults'"]
+      formattedAuthDefaults: [
+        "must have required property 'formattedAuthDefaults'",
+      ],
     });
   });
 });

packages/backend/src/controllers/api/v1/admin/apps/create-config.ee.js

@@ -1,19 +1,10 @@
-import { renderError, renderObject } from '../../../../../helpers/renderer.js';
+import { renderObject } from '../../../../../helpers/renderer.js';
 import AppConfig from '../../../../../models/app-config.js';
 
 export default async (request, response) => {
-  const appKey = request.params.appKey;
+  const createdAppConfig = await AppConfig.query().insertAndFetch(
-
+    appConfigParams(request)
-  const appConfig = await AppConfig.query()
+  );
-    .findOne({ key: appKey });
-
-  if (appConfig) {
-    return renderError(response, [{ general: ['App config already exists.'] }], 409);
-  }
-
-  const createdAppConfig = await AppConfig
-    .query()
-    .insertAndFetch(appConfigParams(request));
 
   renderObject(response, createdAppConfig, { status: 201 });
 };

packages/backend/src/controllers/api/v1/admin/apps/create-config.ee.test.js

@@ -23,7 +23,6 @@ describe('POST /api/v1/admin/apps/:appKey/config', () => {
 
   it('should return created app config', async () => {
     const appConfig = {
-      key: 'gitlab',
       allowCustomConnection: true,
       shared: true,
       disabled: false,
@@ -35,11 +34,14 @@ describe('POST /api/v1/admin/apps/:appKey/config', () => {
       .send(appConfig)
       .expect(201);
 
-    const expectedPayload = createAppConfigMock(appConfig);
+    const expectedPayload = createAppConfigMock({
+      ...appConfig,
+      key: 'gitlab',
+    });
     expect(response.body).toMatchObject(expectedPayload);
   });
 
-  it('should return HTTP 409 for already existing app config', async () => {
+  it('should return HTTP 422 for already existing app config', async () => {
     const appConfig = {
       key: 'gitlab',
       allowCustomConnection: true,
@@ -49,10 +51,17 @@ describe('POST /api/v1/admin/apps/:appKey/config', () => {
 
     await createAppConfig(appConfig);
 
-    await request(app)
+    const response = await request(app)
       .post('/api/v1/admin/apps/gitlab/config')
       .set('Authorization', token)
-      .send(appConfig)
+      .send({
-      .expect(409);
+        disabled: false,
+      })
+      .expect(422);
+
+    expect(response.body.meta.type).toEqual('UniqueViolationError');
+    expect(response.body.errors).toMatchObject({
+      key: ["'key' must be unique."],
+    });
   });
 });

packages/backend/src/helpers/error-handler.js

@@ -1,9 +1,13 @@
 import logger from './logger.js';
 import objection from 'objection';
 import * as Sentry from './sentry.ee.js';
-const { NotFoundError, DataError, ValidationError } = objection;
+const { NotFoundError, DataError, ValidationError, UniqueViolationError } =
+  objection;
 import HttpError from '../errors/http.js';
-import { renderObjectionError } from './renderer.js';
+import {
+  renderObjectionError,
+  renderUniqueViolationError,
+} from './renderer.js';
 
 // Do not remove `next` argument as the function signature will not fit for an error handler middleware
 // eslint-disable-next-line no-unused-vars
@@ -20,6 +24,10 @@ const errorHandler = (error, request, response, next) => {
     renderObjectionError(response, error, 422);
   }
 
+  if (error instanceof UniqueViolationError) {
+    renderUniqueViolationError(response, error);
+  }
+
   if (error instanceof DataError) {
     response.status(400).end();
   }

packages/backend/src/helpers/renderer.js

@@ -64,16 +64,31 @@ const renderError = (response, errors, status, type) => {
   return response.status(errorStatus).send(payload);
 };
 
+const renderUniqueViolationError = (response, error) => {
+  const errors = error.columns.map((column) => ({
+    [column]: [`'${column}' must be unique.`],
+  }));
+
+  return renderError(response, errors, 422, 'UniqueViolationError');
+};
+
 const renderObjectionError = (response, error, status) => {
   const { statusCode, type, data = {} } = error;
 
   const computedStatusCode = status || statusCode;
 
-  const computedErrors = Object.entries(data).map(([fieldName, fieldErrors]) => ({
+  const computedErrors = Object.entries(data).map(
-    [fieldName]: fieldErrors.map(({ message }) => message)
+    ([fieldName, fieldErrors]) => ({
-  }));
+      [fieldName]: fieldErrors.map(({ message }) => message),
+    })
+  );
 
   return renderError(response, computedErrors, computedStatusCode, type);
 };
 
-export { renderObject, renderError, renderObjectionError };
+export {
+  renderObject,
+  renderError,
+  renderObjectionError,
+  renderUniqueViolationError,
+};