From aff8ee28ea0dd67a862407b2e1abae6af71383c7 Mon Sep 17 00:00:00 2001 From: jorgep Date: Wed, 7 Feb 2024 10:16:47 +0100 Subject: [PATCH 1/3] 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); }; }; From f18191d6e9222bb147331a687b5e613fa685d859 Mon Sep 17 00:00:00 2001 From: Juan Ferrer Toribio Date: Fri, 9 Feb 2024 09:23:00 +0100 Subject: [PATCH 2/3] refactor: refs#6706 root dir clean, minimist dep removed --- Dockerfile => back/Dockerfile | 0 docker-compose.yml | 3 ++- e2e/helpers/tests.js | 14 +++++++++----- fileMock.js => front/jest-mock.js | 0 jest-front.js => front/jest-setup.js | 28 ++++++++++++++-------------- gulpfile.js | 4 ---- jest.front.config.js | 4 ++-- package.json | 1 - pnpm-lock.yaml | 3 --- 9 files changed, 27 insertions(+), 30 deletions(-) rename Dockerfile => back/Dockerfile (100%) rename fileMock.js => front/jest-mock.js (100%) rename jest-front.js => front/jest-setup.js (59%) diff --git a/Dockerfile b/back/Dockerfile similarity index 100% rename from Dockerfile rename to back/Dockerfile diff --git a/docker-compose.yml b/docker-compose.yml index 8391a5e23..43a528a8b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -19,10 +19,11 @@ services: back: image: registry.verdnatura.es/salix-back:${VERSION:?} build: . + dockerfile: back/Dockerfile environment: + - TZ - NODE_ENV - DEBUG - - TZ ports: - 3000 configs: diff --git a/e2e/helpers/tests.js b/e2e/helpers/tests.js index a4a059622..7dd299e0b 100644 --- a/e2e/helpers/tests.js +++ b/e2e/helpers/tests.js @@ -1,7 +1,9 @@ +/* eslint-disable no-console */ require('@babel/register')({presets: ['@babel/env']}); require('core-js/stable'); require('regenerator-runtime/runtime'); require('vn-loopback/server/boot/date')(); +const getopts = require('getopts'); const path = require('path'); const Myt = require('@verdnatura/myt/myt'); @@ -18,12 +20,16 @@ process.on('warning', warning => { }); async function test() { - if (process.argv[2] === 'show') - process.env.E2E_SHOW = true; + const opts = getopts(process.argv.slice(2), { + boolean: ['show'] + }); + process.env.E2E_SHOW = opts.show; + console.log('Building and running DB container.'); const myt = new Myt(); await myt.init({workspace: path.join(__dirname, '../..')}); await myt.run(Run); + await myt.deinit(); const Jasmine = require('jasmine'); const jasmine = new Jasmine(); @@ -70,12 +76,10 @@ async function test() { jasmine.jasmine.DEFAULT_TIMEOUT_INTERVAL = 30000; await jasmine.execute(); - - await myt.deinit(); } async function backendStatus() { - log('Awaiting backend connection...'); + log('Awaiting backend connection.'); const milliseconds = 1000; const maxAttempts = 10; diff --git a/fileMock.js b/front/jest-mock.js similarity index 100% rename from fileMock.js rename to front/jest-mock.js diff --git a/jest-front.js b/front/jest-setup.js similarity index 59% rename from jest-front.js rename to front/jest-setup.js index eabda9110..eebd99bbd 100644 --- a/jest-front.js +++ b/front/jest-setup.js @@ -1,19 +1,19 @@ import 'angular'; import 'angular-mocks'; -import core from './front/core/module.js'; -import './front/salix/components/app/app.js'; -import './modules/zone/front/module.js'; -import './modules/claim/front/module.js'; -import './modules/client/front/module.js'; -import './modules/invoiceOut/front/module.js'; -import './modules/invoiceIn/front/module.js'; -import './modules/item/front/module.js'; -import './modules/order/front/module.js'; -import './modules/route/front/module.js'; -import './modules/ticket/front/module.js'; -import './modules/travel/front/module.js'; -import './modules/worker/front/module.js'; -import './modules/shelving/front/module.js'; +import core from './core/module.js'; +import './salix/components/app/app.js'; +import '../modules/zone/front/module.js'; +import '../modules/claim/front/module.js'; +import '../modules/client/front/module.js'; +import '../modules/invoiceOut/front/module.js'; +import '../modules/invoiceIn/front/module.js'; +import '../modules/item/front/module.js'; +import '../modules/order/front/module.js'; +import '../modules/route/front/module.js'; +import '../modules/ticket/front/module.js'; +import '../modules/travel/front/module.js'; +import '../modules/worker/front/module.js'; +import '../modules/shelving/front/module.js'; import 'vn-loopback/server/boot/date'; // Set NODE_ENV diff --git a/gulpfile.js b/gulpfile.js index 255b1ee05..054a65c1c 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -2,7 +2,6 @@ require('require-yaml'); const gulp = require('gulp'); const PluginError = require('plugin-error'); -const argv = require('minimist')(process.argv.slice(2)); const log = require('fancy-log'); const Myt = require('@verdnatura/myt/myt'); const Run = require('@verdnatura/myt/myt-run'); @@ -12,9 +11,6 @@ const Start = require('@verdnatura/myt/myt-start'); let isWindows = /^win/.test(process.platform); -if (argv.NODE_ENV) - process.env.NODE_ENV = argv.NODE_ENV; - let langs = ['es', 'en']; let srcDir = './front'; let modulesDir = './modules'; diff --git a/jest.front.config.js b/jest.front.config.js index 3289df8bb..a843832ea 100644 --- a/jest.front.config.js +++ b/jest.front.config.js @@ -10,7 +10,7 @@ module.exports = { }, testEnvironment: 'jsdom', setupFilesAfterEnv: [ - './jest-front.js' + './front/jest-setup.js' ], testMatch: [ '**/front/**/*.spec.js', @@ -37,7 +37,7 @@ module.exports = { ], moduleNameMapper: { '\\.(css|scss)$': 'identity-obj-proxy', - '\\.(jpg|ico|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': '/fileMock.js', + '\\.(jpg|ico|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': '/front/jest-mock.js', }, testURL: 'http://localhost', verbose: false, diff --git a/package.json b/package.json index 197c1094d..9ae8b276b 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,6 @@ "js-yaml": "^4.1.0", "json-loader": "^0.5.7", "merge-stream": "^1.0.1", - "minimist": "^1.2.5", "node-sass": "^9.0.0", "nodemon": "^2.0.16", "plugin-error": "^1.0.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 19f57caf9..221008dd9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -232,9 +232,6 @@ devDependencies: merge-stream: specifier: ^1.0.1 version: 1.0.1 - minimist: - specifier: ^1.2.5 - version: 1.2.8 node-sass: specifier: ^9.0.0 version: 9.0.0 From 26d4dbbe347f4ea5218c5f26d64326e876d21289 Mon Sep 17 00:00:00 2001 From: Juan Ferrer Toribio Date: Fri, 9 Feb 2024 09:26:46 +0100 Subject: [PATCH 3/3] fix: refs#6706 docker-compose.yml fixes --- docker-compose.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 43a528a8b..5bb168093 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,8 +18,9 @@ services: memory: 1G back: image: registry.verdnatura.es/salix-back:${VERSION:?} - build: . - dockerfile: back/Dockerfile + build: + context: . + dockerfile: back/Dockerfile environment: - TZ - NODE_ENV