From e8792d5686ca81c739253352ff523c83324711ea Mon Sep 17 00:00:00 2001 From: joan Date: Thu, 9 Sep 2021 14:43:18 +0200 Subject: [PATCH 1/2] refactor(client): added restriction to change the credit amount from zero set by a manager Refs: 3087 --- loopback/locale/es.json | 4 +- modules/client/back/models/client.js | 84 ++++++++++--------- .../client/back/models/specs/client.spec.js | 72 ++++++++++++++-- 3 files changed, 112 insertions(+), 48 deletions(-) diff --git a/loopback/locale/es.json b/loopback/locale/es.json index c4c8db4d2..7a670d2ed 100644 --- a/loopback/locale/es.json +++ b/loopback/locale/es.json @@ -207,5 +207,7 @@ "A ticket with a negative base can't be invoiced": "No se puede facturar un ticket con una base negativa", "Global invoicing failed": "[Facturación global] No se han podido facturar algunos clientes", "Wasn't able to invoice the following clients": "No se han podido facturar los siguientes clientes", - "Can't verify data unless the client has a business type": "No se puede verificar datos de un cliente que no tiene tipo de negocio" + "Can't verify data unless the client has a business type": "No se puede verificar datos de un cliente que no tiene tipo de negocio", + "You don't have enough privileges to set this credit amount": "No tienes suficientes privilegios para establecer esta cantidad de crédito", + "You can't change the credit set to zero from a manager": "No puedes cambiar el cŕedito establecido a cero por un gerente" } \ No newline at end of file diff --git a/modules/client/back/models/client.js b/modules/client/back/models/client.js index a4332290d..be9df48fe 100644 --- a/modules/client/back/models/client.js +++ b/modules/client/back/models/client.js @@ -217,19 +217,9 @@ module.exports = Self => { if (isTaxDataCheckedChanged && !orgData.businessTypeFk) throw new UserError(`Can't verify data unless the client has a business type`); } - - if (changes.credit !== undefined) { - await validateCreditChange(ctx, finalState); - let filter = {fields: ['id'], where: {userFk: ctx.options.accessToken.userId}}; - let worker = await Self.app.models.Worker.findOne(filter); - - let newCredit = { - amount: changes.credit, - clientFk: finalState.id, - workerFk: worker ? worker.id : null - }; - await Self.app.models.ClientCredit.create(newCredit); - } + // Credit changes + if (changes.credit !== undefined) + await Self.changeCredit(ctx, finalState, changes); }); Self.observe('after save', async ctx => { @@ -332,42 +322,54 @@ module.exports = Self => { await models.Chat.send(httpCtx, `@${currentWorker.user}`, message); }; - async function validateCreditChange(ctx, finalState) { - let models = Self.app.models; - let userId = ctx.options.accessToken.userId; + // Credit change validations + Self.changeCredit = async function changeCredit(ctx, finalState, changes) { + const models = Self.app.models; + const userId = ctx.options.accessToken.userId; - let currentUserIsManager = await models.Account.hasRole(userId, 'manager'); - if (currentUserIsManager) - return; + const isManager = await models.Account.hasRole(userId, 'manager', ctx.options); + if (!isManager) { + const lastCredit = await models.ClientCredit.findOne({ + where: { + clientFk: finalState.id + }, + order: 'id DESC' + }, ctx.options); - let filter = { - fields: ['roleFk'], - where: { - maxAmount: {gt: ctx.data.credit} - } - }; + const lastAmount = lastCredit.amount; + const lastWorkerId = lastCredit.workerFk; + const lastWorkerIsManager = await models.Account.hasRole(lastWorkerId, 'manager', ctx.options); - let limits = await models.ClientCreditLimit.find(filter); + if (lastAmount == 0 && lastWorkerIsManager) + throw new UserError(`You can't change the credit set to zero from a manager`); - if (limits.length == 0) - throw new UserError('Credit limits not found'); + const creditLimits = await models.ClientCreditLimit.find({ + fields: ['roleFk'], + where: { + maxAmount: {gte: changes.credit} + } + }, ctx.options); - // Si el usuario no tiene alguno de los roles no continua + const requiredRoles = []; + for (limit of creditLimits) + requiredRoles.push(limit.roleFk); - let requiredRoles = []; - for (limit of limits) - requiredRoles.push(limit.roleFk); + const userRequiredRoles = await models.RoleMapping.count({ + roleId: {inq: requiredRoles}, + principalType: 'USER', + principalId: userId + }, ctx.options); - let where = { - roleId: {inq: requiredRoles}, - principalType: 'USER', - principalId: userId - }; - let count = await models.RoleMapping.count(where); + if (userRequiredRoles <= 0) + throw new UserError(`You don't have enough privileges to set this credit amount`); + } - if (count <= 0) - throw new UserError('The role cannot set this credit amount'); - } + await models.ClientCredit.create({ + amount: changes.credit, + clientFk: finalState.id, + workerFk: userId + }, ctx.options); + }; const app = require('vn-loopback/server/server'); app.on('started', function() { diff --git a/modules/client/back/models/specs/client.spec.js b/modules/client/back/models/specs/client.spec.js index 0406cfc21..d1a6d77cb 100644 --- a/modules/client/back/models/specs/client.spec.js +++ b/modules/client/back/models/specs/client.spec.js @@ -1,4 +1,4 @@ -const app = require('vn-loopback/server/server'); +const models = require('vn-loopback/server/server').models; const LoopBackContext = require('loopback-context'); describe('Client Model', () => { @@ -14,8 +14,8 @@ describe('Client Model', () => { } }; const ctx = {req: activeCtx}; - const chatModel = app.models.Chat; - const client = {id: 1101, name: 'Bruce Banner'}; + const chatModel = models.Chat; + const instance = {id: 1101, name: 'Bruce Banner'}; const previousWorkerId = 1106; // DavidCharlesHaller const currentWorkerId = 1107; // HankPym @@ -29,7 +29,7 @@ describe('Client Model', () => { it('should call to the Chat send() method for both workers', async() => { spyOn(chatModel, 'send').and.callThrough(); - await app.models.Client.notifyAssignment(client, previousWorkerId, currentWorkerId); + await models.Client.notifyAssignment(instance, previousWorkerId, currentWorkerId); expect(chatModel.send).toHaveBeenCalledWith(ctx, '@DavidCharlesHaller', `Client assignment has changed`); expect(chatModel.send).toHaveBeenCalledWith(ctx, '@HankPym', `Client assignment has changed`); @@ -38,7 +38,7 @@ describe('Client Model', () => { it('should call to the Chat send() method for the previous worker', async() => { spyOn(chatModel, 'send').and.callThrough(); - await app.models.Client.notifyAssignment(client, null, currentWorkerId); + await models.Client.notifyAssignment(instance, null, currentWorkerId); expect(chatModel.send).toHaveBeenCalledWith(ctx, '@HankPym', `Client assignment has changed`); }); @@ -46,9 +46,69 @@ describe('Client Model', () => { it('should call to the Chat send() method for the current worker', async() => { spyOn(chatModel, 'send').and.callThrough(); - await app.models.Client.notifyAssignment(client, previousWorkerId, null); + await models.Client.notifyAssignment(instance, previousWorkerId, null); expect(chatModel.send).toHaveBeenCalledWith(ctx, '@DavidCharlesHaller', `Client assignment has changed`); }); }); + + describe('changeCredit()', () => { + it('should fail to change the credit as a salesAssistant set to zero by a manager', async() => { + const tx = await models.Client.beginTransaction({}); + + let error; + + try { + const options = {transaction: tx}; + const context = {options}; + + // Set credit to zero by a manager + const financialBoss = await models.Account.findOne({ + where: {name: 'financialBoss'} + }, options); + context.options.accessToken = {userId: financialBoss.id}; + + await models.Client.changeCredit(context, instance, {credit: 0}); + + const salesAssistant = await models.Account.findOne({ + where: {name: 'salesAssistant'} + }, options); + context.options.accessToken = {userId: salesAssistant.id}; + + await models.Client.changeCredit(context, instance, {credit: 300}); + + await tx.rollback(); + } catch (e) { + error = e; + await tx.rollback(); + } + + expect(error.message).toEqual(`You can't change the credit set to zero from a manager`); + }); + + it('should fail to change to a high credit amount as a salesAssistant', async() => { + const tx = await models.Client.beginTransaction({}); + + let error; + + try { + const options = {transaction: tx}; + const context = {options}; + + const salesAssistant = await models.Account.findOne({ + where: {name: 'salesAssistant'} + }, options); + context.options.accessToken = {userId: salesAssistant.id}; + + await models.Client.changeCredit(context, instance, {credit: 99999}); + + await tx.rollback(); + } catch (e) { + error = e; + await tx.rollback(); + } + + expect(error.message).toEqual(`You don't have enough privileges to set this credit amount`); + }); + }); }); From b0d291e49c15ff12899062f5f364a9d8b3c17767 Mon Sep 17 00:00:00 2001 From: joan Date: Thu, 9 Sep 2021 16:44:15 +0200 Subject: [PATCH 2/2] Fixed error for clients without a previous credit --- modules/client/back/models/client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/client/back/models/client.js b/modules/client/back/models/client.js index be9df48fe..6519cb979 100644 --- a/modules/client/back/models/client.js +++ b/modules/client/back/models/client.js @@ -336,8 +336,8 @@ module.exports = Self => { order: 'id DESC' }, ctx.options); - const lastAmount = lastCredit.amount; - const lastWorkerId = lastCredit.workerFk; + const lastAmount = lastCredit && lastCredit.amount; + const lastWorkerId = lastCredit && lastCredit.workerFk; const lastWorkerIsManager = await models.Account.hasRole(lastWorkerId, 'manager', ctx.options); if (lastAmount == 0 && lastWorkerIsManager)