From aff8ee28ea0dd67a862407b2e1abae6af71383c7 Mon Sep 17 00:00:00 2001 From: jorgep Date: Wed, 7 Feb 2024 10:16:47 +0100 Subject: [PATCH] fix: refs #6272 back & create test --- .../00-aclUpdateFiscalData.sql | 3 + loopback/locale/en.json | 5 +- loopback/locale/es.json | 7 +- .../supplier/specs/updateFiscalData.spec.js | 202 +++++++++++------- .../back/methods/supplier/updateFiscalData.js | 91 ++++---- 5 files changed, 186 insertions(+), 122 deletions(-) create mode 100644 db/versions/10872-pinkLilium/00-aclUpdateFiscalData.sql diff --git a/db/versions/10872-pinkLilium/00-aclUpdateFiscalData.sql b/db/versions/10872-pinkLilium/00-aclUpdateFiscalData.sql new file mode 100644 index 000000000..24f5346a8 --- /dev/null +++ b/db/versions/10872-pinkLilium/00-aclUpdateFiscalData.sql @@ -0,0 +1,3 @@ +INSERT INTO salix.ACL (model, property, accessType, permission, principalType, principalId) + VALUES ('Supplier', 'updateAllFiscalData', 'WRITE', 'ALLOW', 'ROLE', 'administrative'), + ('Supplier', 'updateFiscalData', 'WRITE', 'ALLOW', 'ROLE', 'buyer'); \ No newline at end of file diff --git a/loopback/locale/en.json b/loopback/locale/en.json index 7e010e1b5..419775d1b 100644 --- a/loopback/locale/en.json +++ b/loopback/locale/en.json @@ -206,6 +206,5 @@ "Incorrect pin": "Incorrect pin.", "The notification subscription of this worker cant be modified": "The notification subscription of this worker cant be modified", "Name should be uppercase": "Name should be uppercase", - "Fecha fuera de rango": "Fecha fuera de rango", - "There is no zone for these parameters 34": "There is no zone for these parameters 34" -} + "You cannot update these fields": "You cannot update these fields" +} \ No newline at end of file diff --git a/loopback/locale/es.json b/loopback/locale/es.json index 1dd70b633..dac839085 100644 --- a/loopback/locale/es.json +++ b/loopback/locale/es.json @@ -338,6 +338,7 @@ "The alias cant be modified": "Este alias de correo no puede ser modificado", "No tickets to invoice": "No hay tickets para facturar", "Name should be uppercase": "El nombre debe ir en mayúscula", - "Bank entity must be specified": "La entidad bancaria es obligatoria", - "An email is necessary": "Es necesario un email" -} + "Bank entity must be specified": "La entidad bancaria es obligatoria", + "An email is necessary": "Es necesario un email", + "You cannot update these fields": "No puedes actualizar estos campos" +} \ No newline at end of file diff --git a/modules/supplier/back/methods/supplier/specs/updateFiscalData.spec.js b/modules/supplier/back/methods/supplier/specs/updateFiscalData.spec.js index 56030a894..7cb95f840 100644 --- a/modules/supplier/back/methods/supplier/specs/updateFiscalData.spec.js +++ b/modules/supplier/back/methods/supplier/specs/updateFiscalData.spec.js @@ -1,92 +1,142 @@ -const app = require('vn-loopback/server/server'); +const models = require('vn-loopback/server/server').models; const LoopBackContext = require('loopback-context'); -describe('Supplier updateFiscalData', () => { +describe('Supplier updateFiscalData()', () => { const supplierId = 1; const administrativeId = 5; - const employeeId = 1; - const defaultData = { - name: 'PLANTS SL', - nif: '06089160W', - account: '4100000001', - sageTaxTypeFk: 4, - sageWithholdingFk: 1, - sageTransactionTypeFk: 1, - postCode: '15214', - city: 'PONTEVEDRA', - provinceFk: 1, - countryFk: 1, - }; + const buyerId = 35; - it('should return an error if the user is not administrative', async() => { - const ctx = {req: {accessToken: {userId: employeeId}}}; - ctx.args = {}; + const name = 'NEW PLANTS'; + const city = 'PONTEVEDRA'; + const nif = 'A68446004'; + const account = '4000000005'; + const sageTaxTypeFk = 5; + const sageWithholdingFk = 2; + const sageTransactionTypeFk = 2; + const postCode = '46460'; + const phone = 456129367; + const street = ' Fake address 12 3 flat'; + const provinceFk = 2; + const countryFk = 1; + const supplierActivityFk = 'animals'; + const healthRegister = '400664487H'; - let error; - await app.models.Supplier.updateFiscalData(ctx, supplierId) - .catch(e => { - error = e; - }); + let ctx; + let options; + let tx; - expect(error.message).toBeDefined(); - }); - - it('should check that the supplier fiscal data is untainted', async() => { - const supplier = await app.models.Supplier.findById(supplierId); - - expect(supplier.name).toEqual(defaultData.name); - expect(supplier.nif).toEqual(defaultData.nif); - expect(supplier.account).toEqual(defaultData.account); - expect(supplier.sageTaxTypeFk).toEqual(defaultData.sageTaxTypeFk); - expect(supplier.sageWithholdingFk).toEqual(defaultData.sageWithholdingFk); - expect(supplier.sageTransactionTypeFk).toEqual(defaultData.sageTransactionTypeFk); - expect(supplier.postCode).toEqual(defaultData.postCode); - expect(supplier.city).toEqual(defaultData.city); - expect(supplier.provinceFk).toEqual(defaultData.provinceFk); - expect(supplier.countryFk).toEqual(defaultData.countryFk); - }); - - it('should update the supplier fiscal data and return the count if changes made', async() => { - const activeCtx = { - accessToken: {userId: administrativeId}, + beforeEach(async() => { + ctx = { + req: { + accessToken: {userId: buyerId}, + headers: {origin: 'http://localhost'}, + __: value => value + }, + args: {} }; - const ctx = {req: activeCtx}; + spyOn(LoopBackContext, 'getCurrentContext').and.returnValue({ - active: activeCtx + active: ctx.req }); - ctx.args = { - name: 'WEAPON DEALER', - nif: 'A68446004', - account: '4000000005', - sageTaxTypeFk: 5, - sageWithholdingFk: 2, - sageTransactionTypeFk: 2, - postCode: '46460', - city: 'VALENCIA', - provinceFk: 2, - countryFk: 1, - supplierActivityFk: 'animals', - healthRegister: '400664487H' - }; + options = {transaction: tx}; + tx = await models.Sale.beginTransaction({}); + options.transaction = tx; + }); - const result = await app.models.Supplier.updateFiscalData(ctx, supplierId); + afterEach(async() => { + await tx.rollback(); + }); - expect(result.name).toEqual('WEAPON DEALER'); - expect(result.nif).toEqual('A68446004'); - expect(result.account).toEqual('4000000005'); - expect(result.sageTaxTypeFk).toEqual(5); - expect(result.sageWithholdingFk).toEqual(2); - expect(result.sageTransactionTypeFk).toEqual(2); - expect(result.postCode).toEqual('46460'); - expect(result.city).toEqual('VALENCIA'); - expect(result.provinceFk).toEqual(2); - expect(result.countryFk).toEqual(1); - expect(result.supplierActivityFk).toEqual('animals'); - expect(result.healthRegister).toEqual('400664487H'); + it('should throw an error if it is a buyer and tries to update forbidden fiscal data', async() => { + try { + await models.Supplier.updateFiscalData(ctx, + supplierId, + name, + nif, + account, + undefined, + sageTaxTypeFk, + undefined, + sageTransactionTypeFk, + undefined, + undefined, + undefined, + provinceFk, + countryFk, + supplierActivityFk, + healthRegister, + undefined, + undefined, + options); + } catch (e) { + expect(e.message).toEqual('You cannot update these fields'); + } + }); - // Restores - ctx.args = defaultData; - await app.models.Supplier.updateFiscalData(ctx, supplierId); + it('should update the granted fiscal data if it is a buyer', async() => { + const supplier = await models.Supplier.updateFiscalData(ctx, + supplierId, + undefined, + undefined, + account, + phone, + undefined, + undefined, + undefined, + postCode, + street, + city, + provinceFk, + undefined, + undefined, + undefined, + undefined, + undefined, + options); + + expect(supplier.account).toEqual(account); + expect(supplier.phone).toEqual(phone); + expect(supplier.postCode).toEqual(postCode); + expect(supplier.account).toEqual(account); + expect(supplier.city).toEqual(city); + expect(supplier.provinceFk).toEqual(provinceFk); + }); + + it('should update all fiscalData if it is an administative', async() => { + const supplier = await models.Supplier.updateFiscalData(ctx, + supplierId, + name, + nif, + account, + phone, + sageTaxTypeFk, + sageWithholdingFk, + sageTransactionTypeFk, + postCode, + street, + city, + provinceFk, + countryFk, + supplierActivityFk, + healthRegister, + undefined, + undefined, + options); + + expect(supplier.name).toEqual(name); + expect(supplier.nif).toEqual(nif); + expect(supplier.account).toEqual(account); + expect(supplier.phone).toEqual(phone); + expect(supplier.sageTaxTypeFk).toEqual(sageTaxTypeFk); + expect(supplier.sageWithholdingFk).toEqual(sageWithholdingFk); + expect(supplier.sageTransactionTypeFk).toEqual(sageTransactionTypeFk); + expect(supplier.postCode).toEqual(postCode); + expect(supplier.street).toEqual(street); + expect(supplier.city).toEqual(city); + expect(supplier.provinceFk).toEqual(provinceFk); + expect(supplier.countryFk).toEqual(countryFk); + expect(supplier.supplierActivityFk).toEqual(supplierActivityFk); + expect(supplier.healthRegister).toEqual(healthRegister); }); }); diff --git a/modules/supplier/back/methods/supplier/updateFiscalData.js b/modules/supplier/back/methods/supplier/updateFiscalData.js index 271ed8769..c0b860983 100644 --- a/modules/supplier/back/methods/supplier/updateFiscalData.js +++ b/modules/supplier/back/methods/supplier/updateFiscalData.js @@ -1,75 +1,59 @@ +const UserError = require('vn-loopback/util/user-error'); module.exports = Self => { - Self.remoteMethod('updateFiscalData', { + Self.remoteMethodCtx('updateFiscalData', { description: 'Updates fiscal data of a supplier', accessType: 'WRITE', accepts: [{ - arg: 'ctx', - type: 'Object', - http: {source: 'context'} - }, - { arg: 'id', type: 'Number', description: 'The supplier id', http: {source: 'path'} - }, - { + }, { arg: 'name', type: 'string' - }, - { + }, { arg: 'nif', type: 'string' - }, - { + }, { arg: 'account', type: 'any' - }, - { + }, { + arg: 'phone', + type: 'string' + }, { arg: 'sageTaxTypeFk', type: 'any' - }, - { + }, { arg: 'sageWithholdingFk', type: 'any' - }, - { + }, { arg: 'sageTransactionTypeFk', type: 'any' - }, - { + }, { arg: 'postCode', type: 'any' - }, - { + }, { arg: 'street', type: 'any' - }, - { + }, { arg: 'city', type: 'string' - }, - { + }, { arg: 'provinceFk', type: 'any' - }, - { + }, { arg: 'countryFk', type: 'any' - }, - { + }, { arg: 'supplierActivityFk', type: 'string' - }, - { + }, { arg: 'healthRegister', type: 'string' - }, - { + }, { arg: 'isVies', type: 'boolean' - }, - { + }, { arg: 'isTrucker', type: 'boolean' }], @@ -84,15 +68,42 @@ module.exports = Self => { } }); - Self.updateFiscalData = async(ctx, supplierId) => { + Self.updateFiscalData = async(ctx, supplierId, name, nif, account, phone, sageTaxTypeFk, sageWithholdingFk, sageTransactionTypeFk, postCode, street, city, provinceFk, countryFk, supplierActivityFk, healthRegister, isVies, isTrucker, options) => { const models = Self.app.models; - const args = ctx.args; + const {args} = ctx; + const myOptions = {}; const supplier = await models.Supplier.findById(supplierId); - // Remove unwanted properties + if (typeof options == 'object') Object.assign(myOptions, options); + delete args.ctx; delete args.id; - return supplier.updateAttributes(args); + const updateAllFiscalData = await models.ACL.checkAccessAcl(ctx, 'Supplier', 'updateAllFiscalData', 'WRITE'); + if (!updateAllFiscalData) { + for (const arg in args) { + if (args[arg] && !['street', 'postCode', 'city', 'provinceFk', 'phone'].includes(arg)) + throw new UserError('You cannot update these fields'); + } + } + + return supplier.updateAttributes({ + name, + nif, + account, + phone, + sageTaxTypeFk, + sageWithholdingFk, + sageTransactionTypeFk, + postCode, + street, + city, + provinceFk, + countryFk, + supplierActivityFk, + healthRegister, + isVies, + isTrucker + }, myOptions); }; };