From b59191195ba6843918a55984b5eede3e8712e61f Mon Sep 17 00:00:00 2001 From: Joyce Yuki <82857964+kathyavini@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:47:54 -0700 Subject: [PATCH 1/4] LF-3649 Clean up logical branches with hanging transactions --- packages/api/src/controllers/farmExpenseTypeController.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/api/src/controllers/farmExpenseTypeController.js b/packages/api/src/controllers/farmExpenseTypeController.js index 8f673d2565..949ad9577e 100644 --- a/packages/api/src/controllers/farmExpenseTypeController.js +++ b/packages/api/src/controllers/farmExpenseTypeController.js @@ -33,6 +33,7 @@ const farmExpenseTypeController = { // if not deleted, means it is a active expense type // throw conflict error if (record.deleted === false) { + await trx.rollback(); return res.status(409).send(); } else { // if its deleted, them make it active @@ -96,11 +97,13 @@ const farmExpenseTypeController = { return async (req, res) => { const trx = await transaction.start(Model.knex()); if (req.headers.farm_id == null) { + await trx.rollback(); res.sendStatus(403); } try { // do not allow operations to deleted records if (await this.isDeleted(req.params.expense_type_id)) { + await trx.rollback(); return res.status(404).send(); } @@ -136,16 +139,19 @@ const farmExpenseTypeController = { try { // do not allow updating of farm_id if (data.farm_id && data.farm_id !== farm_id) { + await trx.rollback(); return res.status(400).send(); } // do not allow update to deleted records if (await this.isDeleted(expense_type_id)) { + await trx.rollback(); return res.status(404).send(); } // if record exists then throw Conflict error if (await this.existsInFarm(farm_id, data.expense_name, expense_type_id)) { + await trx.rollback(); return res.status(409).send(); } From c1eca17b064ee8943a2de4309038f156739af438 Mon Sep 17 00:00:00 2001 From: Joyce Yuki <82857964+kathyavini@users.noreply.github.com> Date: Tue, 19 Sep 2023 12:34:12 -0700 Subject: [PATCH 2/4] LF-3649 Pass transaction object to abstracted queries --- .../src/controllers/farmExpenseTypeController.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/api/src/controllers/farmExpenseTypeController.js b/packages/api/src/controllers/farmExpenseTypeController.js index 949ad9577e..db72703466 100644 --- a/packages/api/src/controllers/farmExpenseTypeController.js +++ b/packages/api/src/controllers/farmExpenseTypeController.js @@ -27,7 +27,7 @@ const farmExpenseTypeController = { const data = req.body; data.expense_translation_key = baseController.formatTranslationKey(data.expense_name); - const record = await this.existsInFarm(farm_id, data.expense_name); + const record = await this.existsInFarm(farm_id, data.expense_name, trx); // if record exists in db if (record) { // if not deleted, means it is a active expense type @@ -102,7 +102,7 @@ const farmExpenseTypeController = { } try { // do not allow operations to deleted records - if (await this.isDeleted(req.params.expense_type_id)) { + if (await this.isDeleted(req.params.expense_type_id, trx)) { await trx.rollback(); return res.status(404).send(); } @@ -144,13 +144,13 @@ const farmExpenseTypeController = { } // do not allow update to deleted records - if (await this.isDeleted(expense_type_id)) { + if (await this.isDeleted(expense_type_id, trx)) { await trx.rollback(); return res.status(404).send(); } // if record exists then throw Conflict error - if (await this.existsInFarm(farm_id, data.expense_name, expense_type_id)) { + if (await this.existsInFarm(farm_id, data.expense_name, expense_type_id, trx)) { await trx.rollback(); return res.status(409).send(); } @@ -177,8 +177,8 @@ const farmExpenseTypeController = { * @async * @returns {Promise} - Object DB record promise */ - existsInFarm(farm_id, expense_name, expense_type_id = '') { - let query = ExpenseTypeModel.query().context({ showHidden: true }).where({ + existsInFarm(farm_id, expense_name, expense_type_id = '', trx) { + let query = ExpenseTypeModel.query(trx).context({ showHidden: true }).where({ expense_name, farm_id, }); @@ -196,8 +196,8 @@ const farmExpenseTypeController = { * @async * @returns {Boolean} - true or false */ - async isDeleted(expense_type_id) { - const expense = await ExpenseTypeModel.query() + async isDeleted(expense_type_id, trx) { + const expense = await ExpenseTypeModel.query(trx) .context({ showHidden: true }) .where({ expense_type_id, From 035a752322f1e4359b95cb1d4452026be612510d Mon Sep 17 00:00:00 2001 From: Joyce Yuki <82857964+kathyavini@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:03:09 -0700 Subject: [PATCH 3/4] LF-3649 Make trx first argument to existsInFarm as expense_type_id is optional --- packages/api/src/controllers/farmExpenseTypeController.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/api/src/controllers/farmExpenseTypeController.js b/packages/api/src/controllers/farmExpenseTypeController.js index db72703466..9b69e813f7 100644 --- a/packages/api/src/controllers/farmExpenseTypeController.js +++ b/packages/api/src/controllers/farmExpenseTypeController.js @@ -27,7 +27,7 @@ const farmExpenseTypeController = { const data = req.body; data.expense_translation_key = baseController.formatTranslationKey(data.expense_name); - const record = await this.existsInFarm(farm_id, data.expense_name, trx); + const record = await this.existsInFarm(trx, farm_id, data.expense_name); // if record exists in db if (record) { // if not deleted, means it is a active expense type @@ -150,7 +150,7 @@ const farmExpenseTypeController = { } // if record exists then throw Conflict error - if (await this.existsInFarm(farm_id, data.expense_name, expense_type_id, trx)) { + if (await this.existsInFarm(trx, farm_id, data.expense_name, expense_type_id)) { await trx.rollback(); return res.status(409).send(); } @@ -177,7 +177,7 @@ const farmExpenseTypeController = { * @async * @returns {Promise} - Object DB record promise */ - existsInFarm(farm_id, expense_name, expense_type_id = '', trx) { + existsInFarm(trx, farm_id, expense_name, expense_type_id = '') { let query = ExpenseTypeModel.query(trx).context({ showHidden: true }).where({ expense_name, farm_id, From 1d48b58e7053db8df84ea051a4b2b64d211003b6 Mon Sep 17 00:00:00 2001 From: Joyce Yuki <82857964+kathyavini@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:23:03 -0700 Subject: [PATCH 4/4] LF-3649 Update JSDoc --- packages/api/src/controllers/farmExpenseTypeController.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/api/src/controllers/farmExpenseTypeController.js b/packages/api/src/controllers/farmExpenseTypeController.js index 9b69e813f7..dd668281c5 100644 --- a/packages/api/src/controllers/farmExpenseTypeController.js +++ b/packages/api/src/controllers/farmExpenseTypeController.js @@ -171,6 +171,7 @@ const farmExpenseTypeController = { /** * Check if records exists in DB + * @param {Object} trx - transaction object * @param {number} farm_id * @param {String} expense_name * @param {number} expense_type_id - Expesnse type id to be excluded while checking records @@ -193,6 +194,7 @@ const farmExpenseTypeController = { /** * To check if record is deleted or not * @param {number} expense_type_id - Expesnse type id + * @param {Object} trx - transaction object * @async * @returns {Boolean} - true or false */