remove transaction/rollback/commit on single table service calls.

Signed-off-by: Jason Sherman <[email protected]>
usingtechnology · Jun 21, 2024 · 1f0703e · 1f0703e
1 parent 450f712
commit 1f0703e
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 90 deletions.
diff --git a/app/src/forms/admin/service.js b/app/src/forms/admin/service.js
@@ -135,31 +135,23 @@ const service = {
       .modify('orderDefault');
   },
   updateExternalAPI: async (id, data) => {
-    let trx;
-    try {
-      await ExternalAPI.query().findById(id).throwIfNotFound();
-      trx = await ExternalAPI.startTransaction();
-      // admins only change the status code and allow send user token
-      const upd = {
-        code: data.code,
-        allowSendUserToken: data.allowSendUserToken,
-        updatedBy: 'ADMIN',
-      };
-      // if we are not allowing sending user token, ensure any user token fields are cleared out
-      if (!data.allowSendUserToken) {
-        upd['sendUserToken'] = false;
-        upd['userTokenHeader'] = null;
-        upd['userTokenBearer'] = false;
-      }
+    await ExternalAPI.query().findById(id).throwIfNotFound();
+    // admins only change the status code and allow send user token
+    const upd = {
+      code: data.code,
+      allowSendUserToken: data.allowSendUserToken,
+      updatedBy: 'ADMIN',
+    };
+    // if we are not allowing sending user token, ensure any user token fields are cleared out
+    if (!data.allowSendUserToken) {
+      upd['sendUserToken'] = false;
+      upd['userTokenHeader'] = null;
+      upd['userTokenBearer'] = false;
+    }
 
-      await ExternalAPI.query(trx).patchAndFetchById(id, upd);
+    await ExternalAPI.query().patchAndFetchById(id, upd);
 
-      await trx.commit();
-      return ExternalAPI.query().findById(id);
-    } catch (err) {
-      if (trx) await trx.rollback();
-      throw err;
-    }
+    return ExternalAPI.query().findById(id);
   },
   getExternalAPIStatusCodes: async () => {
     return ExternalAPIStatusCode.query();

diff --git a/app/src/forms/form/externalApi/service.js b/app/src/forms/form/externalApi/service.js
@@ -63,64 +63,41 @@ const service = {
   createExternalAPI: async (formId, data, currentUser) => {
     service.validateExternalAPI(data);
 
-    let trx;
-    try {
-      trx = await ExternalAPI.startTransaction();
-      data.id = uuidv4();
-      // set status to SUBMITTED
-      data.code = ExternalAPIStatuses.SUBMITTED;
-      // ensure that new records don't send user tokens.
-      service.checkAllowSendUserToken(data, false);
-      await ExternalAPI.query(trx).insert({
-        ...data,
-        createdBy: currentUser.usernameIdp,
-      });
-
-      await trx.commit();
-      return ExternalAPI.query().findById(data.id);
-    } catch (err) {
-      if (trx) await trx.rollback();
-      throw err;
-    }
+    data.id = uuidv4();
+    // set status to SUBMITTED
+    data.code = ExternalAPIStatuses.SUBMITTED;
+    // ensure that new records don't send user tokens.
+    service.checkAllowSendUserToken(data, false);
+    await ExternalAPI.query().insert({
+      ...data,
+      createdBy: currentUser.usernameIdp,
+    });
+
+    return ExternalAPI.query().findById(data.id);
   },
 
   updateExternalAPI: async (formId, externalAPIId, data, currentUser) => {
     service.validateExternalAPI(data);
 
-    let trx;
-    try {
-      const existing = await ExternalAPI.query().modify('findByIdAndFormId', externalAPIId, formId).first().throwIfNotFound();
-      trx = await ExternalAPI.startTransaction();
-      // let's use a different method for the administrators to update status code and allow send user token
-      // this method should not change the status code.
-      data.code = existing.code;
-      service.checkAllowSendUserToken(data, existing.allowSendUserToken);
-      await ExternalAPI.query(trx)
-        .modify('findByIdAndFormId', externalAPIId, formId)
-        .update({
-          ...data,
-          updatedBy: currentUser.usernameIdp,
-        });
-
-      await trx.commit();
-      return ExternalAPI.query().findById(externalAPIId);
-    } catch (err) {
-      if (trx) await trx.rollback();
-      throw err;
-    }
+    const existing = await ExternalAPI.query().modify('findByIdAndFormId', externalAPIId, formId).first().throwIfNotFound();
+
+    // let's use a different method for the administrators to update status code and allow send user token
+    // this method should not change the status code.
+    data.code = existing.code;
+    service.checkAllowSendUserToken(data, existing.allowSendUserToken);
+    await ExternalAPI.query()
+      .modify('findByIdAndFormId', externalAPIId, formId)
+      .update({
+        ...data,
+        updatedBy: currentUser.usernameIdp,
+      });
+
+    return ExternalAPI.query().findById(externalAPIId);
   },
 
   deleteExternalAPI: async (formId, externalAPIId) => {
-    let trx;
-    try {
-      await ExternalAPI.query().modify('findByIdAndFormId', externalAPIId, formId).first().throwIfNotFound();
-      trx = await ExternalAPI.startTransaction();
-      await ExternalAPI.query().deleteById(externalAPIId);
-      await trx.commit();
-    } catch (err) {
-      if (trx) await trx.rollback();
-      throw err;
-    }
+    await ExternalAPI.query().modify('findByIdAndFormId', externalAPIId, formId).first().throwIfNotFound();
+    await ExternalAPI.query().deleteById(externalAPIId);
   },
 };
 

diff --git a/app/tests/unit/forms/form/externalApi/service.spec.js b/app/tests/unit/forms/form/externalApi/service.spec.js
@@ -164,15 +164,11 @@ describe('createExternalAPI', () => {
       code: ExternalAPIStatuses.SUBMITTED,
       ...validData,
     });
-    expect(MockTransaction.commit).toBeCalledTimes(1);
   });
 
-  it('should rollback on error', async () => {
+  it('should raise errors', async () => {
     MockModel.insert = jest.fn().mockRejectedValueOnce(new Error('SQL Error'));
-
     await expect(service.createExternalAPI(validData.formId, validData, user)).rejects.toThrow();
-
-    expect(MockTransaction.rollback).toBeCalledTimes(1);
   });
 });
 
@@ -221,7 +217,6 @@ describe('updateExternalAPI', () => {
       code: ExternalAPIStatuses.SUBMITTED,
       ...validData,
     });
-    expect(MockTransaction.commit).toBeCalledTimes(1);
   });
 
   it('should update user token fields when allowed', async () => {
@@ -239,7 +234,6 @@ describe('updateExternalAPI', () => {
       code: ExternalAPIStatuses.SUBMITTED,
       ...validData,
     });
-    expect(MockTransaction.commit).toBeCalledTimes(1);
   });
 
   it('should blank out user token fields when not allowed', async () => {
@@ -260,23 +254,15 @@ describe('updateExternalAPI', () => {
       userTokenBearer: false,
       ...validData,
     });
-    expect(MockTransaction.commit).toBeCalledTimes(1);
   });
 
-  it('should not commit when not found', async () => {
+  it('should throw error when not found', async () => {
     MockModel.throwIfNotFound = jest.fn().mockRejectedValueOnce(new Error('SQL Error'));
-
     await expect(service.updateExternalAPI(validData.formId, validData.id, validData, user)).rejects.toThrow();
-    // shouldn't start the transaction
-    expect(MockModel.startTransaction).toBeCalledTimes(0);
-    expect(MockTransaction.commit).toBeCalledTimes(0);
   });
 
-  it('should rollback on error', async () => {
+  it('should raise errors', async () => {
     MockModel.update = jest.fn().mockRejectedValueOnce(new Error('SQL Error'));
-
     await expect(service.updateExternalAPI(validData.formId, validData.id, validData, user)).rejects.toThrow();
-
-    expect(MockTransaction.rollback).toBeCalledTimes(1);
   });
 });