From c76496dd9fc040a9dc7c8da41504c23fb2b088bb Mon Sep 17 00:00:00 2001 From: Jonathan Niles Date: Tue, 7 Jan 2025 13:54:28 -0600 Subject: [PATCH] fix(tax): rm race condition in TaxIPRConfigModal Fixes a race condition that caused buggy behavior in the Tax IPR Config Modal controller. Now, the submit button is disabled until all the data is available to use. --- .../ipr_tax/configuration/iprTaxConfig.js | 137 +++++++++++------- client/src/modules/ipr_tax/ipr_tax.routes.js | 4 +- client/src/modules/ipr_tax/ipr_tax.service.js | 6 +- .../modules/ipr_tax/ipr_tax_config.service.js | 18 ++- .../ipr_tax/modals/ipr_tax_config.modal.html | 2 +- .../ipr_tax/modals/ipr_tax_config.modal.js | 44 +++--- server/controllers/admin/iprTax.js | 38 ++--- 7 files changed, 136 insertions(+), 113 deletions(-) diff --git a/client/src/modules/ipr_tax/configuration/iprTaxConfig.js b/client/src/modules/ipr_tax/configuration/iprTaxConfig.js index 0df96f9b1c..860b233395 100644 --- a/client/src/modules/ipr_tax/configuration/iprTaxConfig.js +++ b/client/src/modules/ipr_tax/configuration/iprTaxConfig.js @@ -2,22 +2,19 @@ angular.module('bhima.controllers') .controller('IprTaxConfigurationController', IprTaxConfigurationController); IprTaxConfigurationController.$inject = [ - 'IprTaxConfigService', 'ModalService', - 'NotifyService', 'uiGridConstants', '$state', 'SessionService', + 'IprTaxConfigService', 'ModalService', 'NotifyService', 'uiGridConstants', '$state', ]; /** * IprTax Configuration Controller * - * This controller is about the IprTax configuration module in the admin zone - * It's responsible for creating, editing and updating a IprTax + * This controller is about the IprTax configuration module in the payroll modules. + * It's responsible for creating, editing and deleteing IprTax configurations. */ function IprTaxConfigurationController( - IprTaxes, ModalService, - Notify, uiGridConstants, $state, Session + IprTaxes, ModalService, Notify, uiGridConstants, $state, ) { - var vm = this; - var gridColumn; + const vm = this; // bind methods vm.deleteIprTax = deleteIprTax; @@ -29,49 +26,74 @@ function IprTaxConfigurationController( vm.gridApi = {}; vm.filterEnabled = false; - gridColumn = [ - { - field : 'rate', displayName : 'FORM.LABELS.RATE', headerCellFilter : 'translate', width : 60, cellFilter : 'percentage', - }, - { - field : 'tranche_annuelle_debut', displayName : 'FORM.LABELS.ANNUAL_TRANCH_FROM', headerCellFilter : 'translate', width : 170, cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'tranche_annuelle_fin', displayName : 'FORM.LABELS.ANNUAL_TRANCH_TO', headerCellFilter : 'translate', width : 170, cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'tranche_mensuelle_debut', displayName : 'FORM.LABELS.MONTH_TRANCH_FROM', headerCellFilter : 'translate', width : 170, cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'tranche_mensuelle_fin', displayName : 'FORM.LABELS.MONTH_TRANCH_TO', headerCellFilter : 'translate', width : 170, cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'ecart_annuel', displayName : 'FORM.LABELS.ANNUAL_ECART', headerCellFilter : 'translate', cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'ecart_mensuel', displayName : 'FORM.LABELS.MONTH_ECART', headerCellFilter : 'translate', cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'impot_annuel', displayName : 'FORM.LABELS.ANNUAL_IMPOT', headerCellFilter : 'translate', cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'impot_mensuel', displayName : 'FORM.LABELS.MONTH_IMPOT', headerCellFilter : 'translate', cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'cumul_annuel', displayName : 'FORM.LABELS.ANNUAL_CUMUL', headerCellFilter : 'translate', cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'cumul_mensuel', displayName : 'FORM.LABELS.MONTH_CUMUL', headerCellFilter : 'translate', cellFilter : 'currency:row.entity.currency_id', - }, - { - field : 'action', - width : 80, - displayName : '', - cellTemplate : '/modules/ipr_tax/templates/actionConfig.tmpl.html', - enableSorting : false, - enableFiltering : false, - }, - ]; + const gridColumn = [{ + field : 'rate', + displayName : 'FORM.LABELS.RATE', + headerCellFilter : 'translate', + width : 60, + cellFilter : 'percentage', + }, { + field : 'tranche_annuelle_debut', + displayName : 'FORM.LABELS.ANNUAL_TRANCH_FROM', + headerCellFilter : 'translate', + width : 170, + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'tranche_annuelle_fin', + displayName : 'FORM.LABELS.ANNUAL_TRANCH_TO', + headerCellFilter : 'translate', + width : 170, + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'tranche_mensuelle_debut', + displayName : 'FORM.LABELS.MONTH_TRANCH_FROM', + headerCellFilter : 'translate', + width : 170, + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'tranche_mensuelle_fin', + displayName : 'FORM.LABELS.MONTH_TRANCH_TO', + headerCellFilter : 'translate', + width : 170, + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'ecart_annuel', + displayName : 'FORM.LABELS.ANNUAL_ECART', + headerCellFilter : 'translate', + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'ecart_mensuel', + displayName : 'FORM.LABELS.MONTH_ECART', + headerCellFilter : 'translate', + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'impot_annuel', + displayName : 'FORM.LABELS.ANNUAL_IMPOT', + headerCellFilter : 'translate', + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'impot_mensuel', + displayName : 'FORM.LABELS.MONTH_IMPOT', + headerCellFilter : 'translate', + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'cumul_annuel', + displayName : 'FORM.LABELS.ANNUAL_CUMUL', + headerCellFilter : 'translate', + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'cumul_mensuel', + displayName : 'FORM.LABELS.MONTH_CUMUL', + headerCellFilter : 'translate', + cellFilter : 'currency:row.entity.currency_id', + }, { + field : 'action', + width : 80, + displayName : '', + cellTemplate : '/modules/ipr_tax/templates/actionConfig.tmpl.html', + enableSorting : false, + enableFiltering : false, + }]; // options for the UI grid vm.gridOptions = { @@ -97,6 +119,11 @@ function IprTaxConfigurationController( function iprScaleSelect(scaleId) { vm.taxIprId = scaleId; + // If taxIPrId is not defined, we can't load the taxes + // The user should select the year from the top of the grid to select + // the data to load. + // TODO(@jniles) - can we just reuse past IPR configurations? Do they change often + // enough to merit the user having to recreate it every year? if (vm.taxIprId) { loadIprTaxes(); } @@ -106,11 +133,11 @@ function IprTaxConfigurationController( vm.loading = true; IprTaxes.read(null, { taxe_ipr_id : vm.taxIprId }) - .then(function (data) { + .then((data) => { vm.gridOptions.data = data; }) .catch(Notify.handleError) - .finally(function () { + .finally(() => { vm.loading = false; }); } @@ -118,11 +145,11 @@ function IprTaxConfigurationController( // switch to delete warning mode function deleteIprTax(title) { ModalService.confirm('FORM.DIALOGS.CONFIRM_DELETE') - .then(function (bool) { + .then((bool) => { if (!bool) { return; } IprTaxes.delete(title.id) - .then(function () { + .then(() => { Notify.success('FORM.INFO.DELETE_SUCCESS'); loadIprTaxes(); }) diff --git a/client/src/modules/ipr_tax/ipr_tax.routes.js b/client/src/modules/ipr_tax/ipr_tax.routes.js index 3401b0ce3f..b2c2778998 100644 --- a/client/src/modules/ipr_tax/ipr_tax.routes.js +++ b/client/src/modules/ipr_tax/ipr_tax.routes.js @@ -23,7 +23,7 @@ angular.module('bhima.routes') }) .state('iprConfiguration.createConfig', { - url : '/:taxIprId/configuration/create', + url : '/:taxIprId/create', params : { isCreateState : { value : true }, taxIprId : { value : null }, @@ -42,7 +42,7 @@ angular.module('bhima.routes') }) .state('iprConfiguration.editConfig', { - url : '/:taxIprId/configuration/:id/edit', + url : '/:taxIprId/edit/:id', params : { taxIprId : { value : null }, id : { value : null }, diff --git a/client/src/modules/ipr_tax/ipr_tax.service.js b/client/src/modules/ipr_tax/ipr_tax.service.js index 16daa5393a..4b1d19d0a6 100644 --- a/client/src/modules/ipr_tax/ipr_tax.service.js +++ b/client/src/modules/ipr_tax/ipr_tax.service.js @@ -8,10 +8,8 @@ IprTaxService.$inject = ['PrototypeApiService']; * @extends PrototypeApiService * * @description - * Encapsulates common requests to the /iprTaxes/ URL. + * Encapsulates common requests to the /iprTax/ URL. */ function IprTaxService(Api) { - var service = new Api('/iprTax/'); - - return service; + return new Api('/iprTax/'); } diff --git a/client/src/modules/ipr_tax/ipr_tax_config.service.js b/client/src/modules/ipr_tax/ipr_tax_config.service.js index 87dc8a56b4..969c59eb48 100644 --- a/client/src/modules/ipr_tax/ipr_tax_config.service.js +++ b/client/src/modules/ipr_tax/ipr_tax_config.service.js @@ -1,7 +1,7 @@ angular.module('bhima.services') .service('IprTaxConfigService', IprTaxConfigService); -IprTaxConfigService.$inject = ['PrototypeApiService', '$uibModal']; +IprTaxConfigService.$inject = ['PrototypeApiService']; /** * @class IprTaxConfigService @@ -10,13 +10,13 @@ IprTaxConfigService.$inject = ['PrototypeApiService', '$uibModal']; * @description * Encapsulates common requests to the /iprTaxConfig/ URL. */ -function IprTaxConfigService(Api, Modal) { - var service = new Api('/iprTaxConfig/'); +function IprTaxConfigService(Api) { + const service = new Api('/iprTaxConfig/'); service.configData = configData; function configData(params, scales) { - var iprConfig = {}; - var cumul = 0; + const iprConfig = {}; + let cumul = 0; iprConfig.taxe_ipr_id = params.taxe_ipr_id; iprConfig.rate = params.rate; @@ -26,20 +26,24 @@ function IprTaxConfigService(Api, Modal) { iprConfig.tranche_mensuelle_debut = params.tranche_annuelle_debut / 12; iprConfig.tranche_mensuelle_fin = params.tranche_annuelle_fin / 12; - iprConfig.ecart_annuel = params.tranche_annuelle_fin - params.tranche_annuelle_debut; + iprConfig.ecart_annuel = params.tranche_annuelle_fin - params.tranche_annuelle_debut; iprConfig.ecart_mensuel = iprConfig.tranche_mensuelle_fin - iprConfig.tranche_mensuelle_debut; iprConfig.impot_annuel = iprConfig.ecart_annuel * (params.rate / 100); iprConfig.impot_mensuel = iprConfig.impot_annuel / 12; - scales.forEach(function (scale) { + // TODO(@jniles) - test this configData() function + scales.forEach((scale) => { if (scale.tranche_annuelle_fin === iprConfig.tranche_annuelle_debut) { cumul = iprConfig.impot_annuel + scale.cumul_annuel; } }); + // calculate the cumulative annual tax rate iprConfig.cumul_annuel = cumul; + // compute the monthly cumulative tax rate iprConfig.cumul_mensuel = iprConfig.cumul_annuel / 12; + return iprConfig; } diff --git a/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.html b/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.html index 089b5ae27f..e2514fd504 100644 --- a/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.html +++ b/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.html @@ -42,7 +42,7 @@ FORM.BUTTONS.CANCEL - + FORM.BUTTONS.SUBMIT diff --git a/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.js b/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.js index a5a2bf495a..b0467176ea 100644 --- a/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.js +++ b/client/src/modules/ipr_tax/modals/ipr_tax_config.modal.js @@ -2,10 +2,10 @@ angular.module('bhima.controllers') .controller('IprTaxConfigModalController', IprTaxConfigModalController); IprTaxConfigModalController.$inject = [ - '$state', 'IprTaxService', 'IprTaxConfigService', 'NotifyService', 'appcache', 'params', + '$state', '$q', 'IprTaxService', 'IprTaxConfigService', 'NotifyService', 'appcache', 'params', ]; -function IprTaxConfigModalController($state, IprTax, IprConfig, Notify, AppCache, params) { +function IprTaxConfigModalController($state, $q, IprTax, IprConfig, Notify, AppCache, params) { const vm = this; vm.iprTax = {}; @@ -23,38 +23,41 @@ function IprTaxConfigModalController($state, IprTax, IprConfig, Notify, AppCache // exposed methods vm.submit = submit; - if (vm.stateParams.taxIprId) { - IprTax.read(vm.stateParams.taxIprId) - .then((iprTax) => { + function startup() { + vm.loading = true; + + // both edit and create state + $q.all([ + IprTax.read(vm.stateParams.taxIprId), + IprConfig.read(null, { taxe_ipr_id : vm.stateParams.taxIprId }), + ]) + .then((iprTax, iprConfig) => { iprTax.taxe_ipr_id = iprTax.id; delete iprTax.id; vm.iprTax = iprTax; - }) - .catch(Notify.handleError); - - IprConfig.read(null, { taxe_ipr_id : vm.taxIprId }) - .then((iprConfig) => { vm.iprConfig = iprConfig; - }) - .catch(Notify.handleError); - } - if (!vm.isCreateState) { - IprConfig.read(vm.stateParams.id) - .then((iprTax) => { - vm.iprTax = iprTax; + // edit state only + if (!vm.isCreateState) { + return IprConfig.read(vm.stateParams.id) + .then(iprTaxData => { vm.iprTax = iprTaxData; }); + } + + return 0; }) - .catch(Notify.handleError); + .catch(Notify.handleError) + .finally(() => { vm.loading = false; }); } // submit the data to the server from all two forms (update, create) function submit(iprTaxForm) { - if (iprTaxForm.$invalid) { return 0; } + const iprConfigData = IprConfig.configData(vm.iprTax, vm.iprConfig); - const promise = (vm.isCreateState) ? IprConfig.create(iprConfigData) + const promise = (vm.isCreateState) + ? IprConfig.create(iprConfigData) : IprConfig.update(vm.iprTax.id, iprConfigData); return promise @@ -66,4 +69,5 @@ function IprTaxConfigModalController($state, IprTax, IprConfig, Notify, AppCache .catch(Notify.handleError); } + startup(); } diff --git a/server/controllers/admin/iprTax.js b/server/controllers/admin/iprTax.js index 7fbaf98de1..9c177125fb 100644 --- a/server/controllers/admin/iprTax.js +++ b/server/controllers/admin/iprTax.js @@ -5,7 +5,6 @@ */ const db = require('../../lib/db'); -const NotFound = require('../../lib/errors/NotFound'); const FilterParser = require('../../lib/filter'); // GET /IprTax @@ -64,20 +63,15 @@ function create(req, res, next) { } -// PUT /IprTax /:id -function update(req, res, next) { +// PUT /IprTax/:id +async function update(req, res, next) { const sql = `UPDATE taxe_ipr SET ? WHERE id = ?;`; - db.exec(sql, [req.body, req.params.id]) - .then(() => { - return lookupIprTax(req.params.id); - }) - .then((record) => { - // all updates completed successfull, return full object to client - res.status(200).json(record); - }) - .catch(next); - + try { + await db.exec(sql, [req.body, req.params.id]); + const record = await lookupIprTax(req.params.id); + res.status(200).json(record); + } catch (e) { next(e); } } // DELETE /IprTax/:id @@ -154,19 +148,15 @@ function createConfig(req, res, next) { } -// PUT /IprTaxConfig /:id -function updateConfig(req, res, next) { +// PUT /IprTaxConfig/:id +async function updateConfig(req, res, next) { const sql = `UPDATE taxe_ipr_configuration SET ? WHERE id = ?;`; - db.exec(sql, [req.body, req.params.id]) - .then(() => { - return lookupIprTaxConfig(req.params.id); - }) - .then((record) => { - // all updates completed successfull, return full object to client - res.status(200).json(record); - }) - .catch(next); + try { + await db.exec(sql, [req.body, req.params.id]); + const record = await lookupIprTaxConfig(req.params.id); + res.status(200).json(record); + } catch (e) { next(e); } }