From fe8ae164eb503f4025b927aa137c992a95dce6ed Mon Sep 17 00:00:00 2001 From: alexm Date: Tue, 4 Oct 2022 13:22:11 +0200 Subject: [PATCH 01/14] refactor(ticket): isEditable, canEdit --- back/methods/account/aclFunc.js | 33 +++++++++++++++++++ back/models/account.js | 3 +- db/dump/fixtures.sql | 6 ++++ modules/ticket/back/methods/sale/canEdit.js | 24 ++++++++++---- .../ticket/back/methods/sale/deleteSales.js | 3 +- .../back/methods/sale/recalculatePrice.js | 7 ++-- modules/ticket/back/methods/sale/reserve.js | 3 +- .../back/methods/sale/specs/canEdit.spec.js | 6 ++-- .../ticket/back/methods/ticket/isEditable.js | 24 ++++++-------- .../back/methods/ticket/isRoleAdvanced.js | 32 ++++++++++++++++++ modules/ticket/back/model-config.json | 3 ++ modules/ticket/back/models/sale-cloned.json | 26 +++++++++++++++ modules/ticket/back/models/ticket.js | 1 + 13 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 back/methods/account/aclFunc.js create mode 100644 modules/ticket/back/methods/ticket/isRoleAdvanced.js create mode 100644 modules/ticket/back/models/sale-cloned.json diff --git a/back/methods/account/aclFunc.js b/back/methods/account/aclFunc.js new file mode 100644 index 0000000000..47dcd6f7a8 --- /dev/null +++ b/back/methods/account/aclFunc.js @@ -0,0 +1,33 @@ +module.exports = Self => { + Self.remoteMethodCtx('aclFunc', { + description: 'Get the user information and permissions', + accepts: [ + { + arg: 'property', + type: 'String', + description: 'The user name or email', + required: true + } + ], + returns: { + type: 'Object', + root: true + }, + http: { + path: `/aclFunc`, + verb: 'GET' + } + }); + + Self.aclFunc = async function(ctx, property) { + const userId = ctx.req.accessToken.userId; + const models = Self.app.models; + + const [acl] = await Self.rawSql( + `SELECT a.principalId + FROM salix.ACL a + WHERE a.property = ?`, [property]); + + return await models.Account.hasRole(userId, acl.principalId); + }; +}; diff --git a/back/models/account.js b/back/models/account.js index ba703c68d7..5b101eef76 100644 --- a/back/models/account.js +++ b/back/models/account.js @@ -7,6 +7,7 @@ module.exports = Self => { require('../methods/account/change-password')(Self); require('../methods/account/set-password')(Self); require('../methods/account/validate-token')(Self); + require('../methods/account/aclFunc')(Self); // Validations @@ -77,7 +78,7 @@ module.exports = Self => { `SELECT r.name FROM account.user u JOIN account.roleRole rr ON rr.role = u.role - JOIN account.role r ON r.id = rr.inheritsFrom + JOIN account.role r ON r.id = rr.inheritsFrom WHERE u.id = ?`, [userId], options); let roles = []; diff --git a/db/dump/fixtures.sql b/db/dump/fixtures.sql index 1f66a53cfc..fd78a5401d 100644 --- a/db/dump/fixtures.sql +++ b/db/dump/fixtures.sql @@ -2651,3 +2651,9 @@ INSERT INTO `vn`.`collection` (`id`, `created`, `workerFk`, `stateFk`, `itemPack INSERT INTO `vn`.`ticketCollection` (`ticketFk`, `collectionFk`, `created`, `level`, `wagon`, `smartTagFk`, `usedShelves`, `itemCount`, `liters`) VALUES (9, 3, util.VN_NOW(), NULL, 0, NULL, NULL, NULL, NULL); + +INSERT INTO salix.ACL +(model, property, accessType, permission, principalType, principalId) +VALUES +('Sale', 'editTracked', 'WRITE', 'ALLOW', 'ROLE', 'production'), +('Sale', 'editCloned', 'WRITE', 'ALLOW', 'ROLE', 'production'); diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index 4e0fc5f8bf..f359b19e41 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -1,3 +1,5 @@ +const UserError = require('vn-loopback/util/user-error'); + module.exports = Self => { Self.remoteMethodCtx('canEdit', { description: 'Check if all the received sales are aditable', @@ -25,16 +27,26 @@ module.exports = Self => { if (typeof options == 'object') Object.assign(myOptions, options); - const idsCollection = sales.map(sale => sale.id); - - const saleTracking = await models.SaleTracking.find({where: {saleFk: {inq: idsCollection}}}, myOptions); + /* const firstSale = await models.Sale.findById(sales[0], null, myOptions); + const isTicketEditable = await models.Ticket.isEditable(ctx, firstSale.ticketFk, myOptions); + if (!isTicketEditable) + throw new UserError(`The sales of this ticket can't be modified`);*/ + const saleTracking = await models.SaleTracking.find({where: {saleFk: {inq: sales}}}, myOptions); const hasSaleTracking = saleTracking.length; - const isProductionRole = await models.Account.hasRole(userId, 'production', myOptions); + const saleCloned = await models.SaleCloned.find({where: {saleClonedFk: {inq: sales}}}, myOptions); + const hasSaleCloned = saleCloned.length; - const canEdit = (isProductionRole || !hasSaleTracking); + /* const isProductionRole = await models.Account.hasRole(userId, 'production', myOptions); + const canEdit = (isProductionRole || !hasSaleTracking);// && (isRole || !hasSaleCloned); - return canEdit; + const isRole = await models.Account.hasRole(userId, 'developer', myOptions);*/ + const editTracked = await models.Account.aclFunc(ctx, 'editTracked'); + const editCloned = await models.Account.aclFunc(ctx, 'editCloned'); + console.log(editTracked); + const canEdit = (editTracked || !hasSaleTracking) && (editCloned || !hasSaleCloned); + + return canEdit;// && isTicketEditable; }; }; diff --git a/modules/ticket/back/methods/sale/deleteSales.js b/modules/ticket/back/methods/sale/deleteSales.js index c1359569d7..7036821e9c 100644 --- a/modules/ticket/back/methods/sale/deleteSales.js +++ b/modules/ticket/back/methods/sale/deleteSales.js @@ -41,7 +41,8 @@ module.exports = Self => { } try { - const canEditSales = await models.Sale.canEdit(ctx, sales, myOptions); + const saleIds = sales.map(sale => sale.id); + const canEditSales = await models.Sale.canEdit(ctx, saleIds, myOptions); const ticket = await models.Ticket.findById(ticketId, { include: { diff --git a/modules/ticket/back/methods/sale/recalculatePrice.js b/modules/ticket/back/methods/sale/recalculatePrice.js index 59c7d3535c..cbe59cad2d 100644 --- a/modules/ticket/back/methods/sale/recalculatePrice.js +++ b/modules/ticket/back/methods/sale/recalculatePrice.js @@ -1,4 +1,5 @@ const UserError = require('vn-loopback/util/user-error'); + module.exports = Self => { Self.remoteMethodCtx('recalculatePrice', { description: 'Calculates the price of sales and its components', @@ -34,15 +35,13 @@ module.exports = Self => { } try { - const salesIds = []; - for (let sale of sales) - salesIds.push(sale.id); + const salesIds = sales.map(sale => sale.id); const isEditable = await models.Ticket.isEditable(ctx, sales[0].ticketFk, myOptions); if (!isEditable) throw new UserError(`The sales of this ticket can't be modified`); - const canEditSale = await models.Sale.canEdit(ctx, sales, myOptions); + const canEditSale = await models.Sale.canEdit(ctx, salesIds, myOptions); if (!canEditSale) throw new UserError(`Sale(s) blocked, please contact production`); diff --git a/modules/ticket/back/methods/sale/reserve.js b/modules/ticket/back/methods/sale/reserve.js index b368f6fc3d..c3e9ac30f2 100644 --- a/modules/ticket/back/methods/sale/reserve.js +++ b/modules/ticket/back/methods/sale/reserve.js @@ -53,7 +53,8 @@ module.exports = Self => { if (!isTicketEditable) throw new UserError(`The sales of this ticket can't be modified`); - const canEditSale = await models.Sale.canEdit(ctx, sales, myOptions); + const salesIds = sales.map(sale => sale.id); + const canEditSale = await models.Sale.canEdit(ctx, salesIds, myOptions); if (!canEditSale) throw new UserError(`Sale(s) blocked, please contact production`); diff --git a/modules/ticket/back/methods/sale/specs/canEdit.spec.js b/modules/ticket/back/methods/sale/specs/canEdit.spec.js index 4f6747257e..667ce98da3 100644 --- a/modules/ticket/back/methods/sale/specs/canEdit.spec.js +++ b/modules/ticket/back/methods/sale/specs/canEdit.spec.js @@ -10,7 +10,7 @@ describe('sale canEdit()', () => { const productionUserID = 49; const ctx = {req: {accessToken: {userId: productionUserID}}}; - const sales = [{id: 3}]; + const sales = [3]; const result = await models.Sale.canEdit(ctx, sales, options); @@ -32,7 +32,7 @@ describe('sale canEdit()', () => { const salesPersonUserID = 18; const ctx = {req: {accessToken: {userId: salesPersonUserID}}}; - const sales = [{id: 10}]; + const sales = [10]; const result = await models.Sale.canEdit(ctx, sales, options); @@ -54,7 +54,7 @@ describe('sale canEdit()', () => { const salesPersonUserID = 18; const ctx = {req: {accessToken: {userId: salesPersonUserID}}}; - const sales = [{id: 3}]; + const sales = [3]; const result = await models.Sale.canEdit(ctx, sales, options); diff --git a/modules/ticket/back/methods/ticket/isEditable.js b/modules/ticket/back/methods/ticket/isEditable.js index 5b9a397a1b..bdd18d7de4 100644 --- a/modules/ticket/back/methods/ticket/isEditable.js +++ b/modules/ticket/back/methods/ticket/isEditable.js @@ -20,24 +20,20 @@ module.exports = Self => { }); Self.isEditable = async(ctx, id, options) => { - const userId = ctx.req.accessToken.userId; + const models = Self.app.models; const myOptions = {}; if (typeof options == 'object') Object.assign(myOptions, options); - let state = await Self.app.models.TicketState.findOne({ + const state = await models.TicketState.findOne({ where: {ticketFk: id} }, myOptions); - const isSalesAssistant = await Self.app.models.Account.hasRole(userId, 'salesAssistant', myOptions); - const isDeliveryBoss = await Self.app.models.Account.hasRole(userId, 'deliveryBoss', myOptions); - const isBuyer = await Self.app.models.Account.hasRole(userId, 'buyer', myOptions); + const isRoleAdvanced = await models.Ticket.isRoleAdvanced(ctx, myOptions); - const isValidRole = isSalesAssistant || isDeliveryBoss || isBuyer; - - let alertLevel = state ? state.alertLevel : null; - let ticket = await Self.app.models.Ticket.findById(id, { + const alertLevel = state ? state.alertLevel : null; + const ticket = await models.Ticket.findById(id, { fields: ['clientFk'], include: [{ relation: 'client', @@ -48,15 +44,15 @@ module.exports = Self => { } }] }, myOptions); - const isLocked = await Self.app.models.Ticket.isLocked(id, myOptions); + const isLocked = await models.Ticket.isLocked(id, myOptions); const alertLevelGreaterThanZero = (alertLevel && alertLevel > 0); const isNormalClient = ticket && ticket.client().type().code == 'normal'; - const validAlertAndRoleNormalClient = (alertLevelGreaterThanZero && isNormalClient && !isValidRole); + const isEditable = !(alertLevelGreaterThanZero && isNormalClient); - if (!ticket || validAlertAndRoleNormalClient || isLocked) - return false; + if (ticket && (isEditable || isRoleAdvanced) && !isLocked) + return true; - return true; + return false; }; }; diff --git a/modules/ticket/back/methods/ticket/isRoleAdvanced.js b/modules/ticket/back/methods/ticket/isRoleAdvanced.js new file mode 100644 index 0000000000..7c5c8ed86f --- /dev/null +++ b/modules/ticket/back/methods/ticket/isRoleAdvanced.js @@ -0,0 +1,32 @@ +module.exports = Self => { + Self.remoteMethodCtx('isRoleAdvanced', { + description: 'Check if a ticket is editable', + accessType: 'READ', + returns: { + type: 'boolean', + root: true + }, + http: { + path: `/isRoleAdvanced`, + verb: 'GET' + } + }); + + Self.isRoleAdvanced = async(ctx, options) => { + const models = Self.app.models; + const userId = ctx.req.accessToken.userId; + const myOptions = {}; + + if (typeof options == 'object') + Object.assign(myOptions, options); + + const isSalesAssistant = await models.Account.hasRole(userId, 'salesAssistant', myOptions); + const isDeliveryBoss = await models.Account.hasRole(userId, 'deliveryBoss', myOptions); + const isBuyer = await models.Account.hasRole(userId, 'buyer', myOptions); + const isClaimManager = await models.Account.hasRole(userId, 'claimManager', myOptions); + + const isRoleAdvanced = isSalesAssistant || isDeliveryBoss || isBuyer || isClaimManager; + + return isRoleAdvanced; + }; +}; diff --git a/modules/ticket/back/model-config.json b/modules/ticket/back/model-config.json index 8a6ac0c004..032cc88949 100644 --- a/modules/ticket/back/model-config.json +++ b/modules/ticket/back/model-config.json @@ -29,6 +29,9 @@ "SaleChecked": { "dataSource": "vn" }, + "SaleCloned": { + "dataSource": "vn" + }, "SaleComponent": { "dataSource": "vn" }, diff --git a/modules/ticket/back/models/sale-cloned.json b/modules/ticket/back/models/sale-cloned.json new file mode 100644 index 0000000000..eb0641684e --- /dev/null +++ b/modules/ticket/back/models/sale-cloned.json @@ -0,0 +1,26 @@ +{ + "name": "SaleCloned", + "base": "VnModel", + "options": { + "mysql": { + "table": "saleCloned" + } + }, + "properties": { + "saleClonedFk": { + "id": true + } + }, + "relations": { + "saleOriginal": { + "type": "belongsTo", + "model": "Sale", + "foreignKey": "saleOriginalFk" + }, + "saleCloned": { + "type": "belongsTo", + "model": "Sale", + "foreignKey": "saleClonedFk" + } + } +} diff --git a/modules/ticket/back/models/ticket.js b/modules/ticket/back/models/ticket.js index 47d105824e..7f5984f504 100644 --- a/modules/ticket/back/models/ticket.js +++ b/modules/ticket/back/models/ticket.js @@ -28,6 +28,7 @@ module.exports = Self => { require('../methods/ticket/freightCost')(Self); require('../methods/ticket/getComponentsSum')(Self); require('../methods/ticket/refund')(Self); + require('../methods/ticket/isRoleAdvanced')(Self); Self.observe('before save', async function(ctx) { const loopBackContext = LoopBackContext.getCurrentContext(); From 5f39249edb68aa45c37ec55722c717a47ff50758 Mon Sep 17 00:00:00 2001 From: alexm Date: Tue, 4 Oct 2022 15:02:00 +0200 Subject: [PATCH 02/14] fix(ticket): canEdit use aclFunc --- modules/ticket/back/methods/sale/canEdit.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index f359b19e41..94c2fc19a9 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -14,23 +14,22 @@ module.exports = Self => { root: true }, http: { - path: `/isEditable`, + path: `/canEdit`, verb: 'get' } }); Self.canEdit = async(ctx, sales, options) => { const models = Self.app.models; - const userId = ctx.req.accessToken.userId; const myOptions = {}; if (typeof options == 'object') Object.assign(myOptions, options); - /* const firstSale = await models.Sale.findById(sales[0], null, myOptions); + const firstSale = await models.Sale.findById(sales[0], null, myOptions); const isTicketEditable = await models.Ticket.isEditable(ctx, firstSale.ticketFk, myOptions); if (!isTicketEditable) - throw new UserError(`The sales of this ticket can't be modified`);*/ + throw new UserError(`The sales of this ticket can't be modified`); const saleTracking = await models.SaleTracking.find({where: {saleFk: {inq: sales}}}, myOptions); const hasSaleTracking = saleTracking.length; @@ -38,15 +37,11 @@ module.exports = Self => { const saleCloned = await models.SaleCloned.find({where: {saleClonedFk: {inq: sales}}}, myOptions); const hasSaleCloned = saleCloned.length; - /* const isProductionRole = await models.Account.hasRole(userId, 'production', myOptions); - const canEdit = (isProductionRole || !hasSaleTracking);// && (isRole || !hasSaleCloned); + const canEditTracked = await models.Account.aclFunc(ctx, 'editTracked'); + const canEditCloned = await models.Account.aclFunc(ctx, 'editCloned'); - const isRole = await models.Account.hasRole(userId, 'developer', myOptions);*/ - const editTracked = await models.Account.aclFunc(ctx, 'editTracked'); - const editCloned = await models.Account.aclFunc(ctx, 'editCloned'); - console.log(editTracked); - const canEdit = (editTracked || !hasSaleTracking) && (editCloned || !hasSaleCloned); + const canEdit = (canEditTracked || !hasSaleTracking) && (canEditCloned || !hasSaleCloned); - return canEdit;// && isTicketEditable; + return canEdit; }; }; From 9d482f7dfdd6d7193e42f99140d757aeb5f6dcb4 Mon Sep 17 00:00:00 2001 From: alexm Date: Wed, 5 Oct 2022 15:13:36 +0200 Subject: [PATCH 03/14] test(ticket): fix tests and add funcionalityAcl --- back/methods/account/aclFunc.js | 33 ---------- back/methods/account/funcionalityAcl.js | 40 +++++++++++++ back/models/funcionalityAcl.json | 24 ++++++++ .../10490-august/00-funcionalityAcl.sql | 7 +++ db/dump/fixtures.sql | 26 ++++---- modules/ticket/back/methods/sale/canEdit.js | 4 +- .../back/methods/sale/specs/canEdit.spec.js | 52 ++++++++++++++-- .../methods/sale/specs/updateConcept.spec.js | 2 +- .../methods/sale/specs/updateQuantity.spec.js | 60 ++++++++++++++++--- .../back/methods/sale/updateQuantity.js | 12 ++-- modules/ticket/back/models/ticket-methods.js | 1 + 11 files changed, 197 insertions(+), 64 deletions(-) delete mode 100644 back/methods/account/aclFunc.js create mode 100644 back/methods/account/funcionalityAcl.js create mode 100644 back/models/funcionalityAcl.json create mode 100644 db/changes/10490-august/00-funcionalityAcl.sql diff --git a/back/methods/account/aclFunc.js b/back/methods/account/aclFunc.js deleted file mode 100644 index 47dcd6f7a8..0000000000 --- a/back/methods/account/aclFunc.js +++ /dev/null @@ -1,33 +0,0 @@ -module.exports = Self => { - Self.remoteMethodCtx('aclFunc', { - description: 'Get the user information and permissions', - accepts: [ - { - arg: 'property', - type: 'String', - description: 'The user name or email', - required: true - } - ], - returns: { - type: 'Object', - root: true - }, - http: { - path: `/aclFunc`, - verb: 'GET' - } - }); - - Self.aclFunc = async function(ctx, property) { - const userId = ctx.req.accessToken.userId; - const models = Self.app.models; - - const [acl] = await Self.rawSql( - `SELECT a.principalId - FROM salix.ACL a - WHERE a.property = ?`, [property]); - - return await models.Account.hasRole(userId, acl.principalId); - }; -}; diff --git a/back/methods/account/funcionalityAcl.js b/back/methods/account/funcionalityAcl.js new file mode 100644 index 0000000000..3a5e09720d --- /dev/null +++ b/back/methods/account/funcionalityAcl.js @@ -0,0 +1,40 @@ +module.exports = Self => { + Self.remoteMethod('funcionalityAcl', { + description: 'Return if user has permissions', + accepts: [ + { + arg: 'model', + type: 'String', + description: 'The model', + required: true + }, + { + arg: 'property', + type: 'String', + description: 'The property', + required: true + } + ], + returns: { + type: 'Object', + root: true + }, + http: { + path: `/funcionalityAcl`, + verb: 'GET' + } + }); + + Self.funcionalityAcl = async function(ctx, model, property) { + const userId = ctx.req.accessToken.userId; + const models = Self.app.models; + + const [acl] = await Self.rawSql( + `SELECT f.role + FROM salix.funcionalityAcl f + WHERE f.model = ? + AND f.property = ?`, [model, property]); + + return await models.Account.hasRole(userId, acl.role); + }; +}; diff --git a/back/models/funcionalityAcl.json b/back/models/funcionalityAcl.json new file mode 100644 index 0000000000..840c4f6f3a --- /dev/null +++ b/back/models/funcionalityAcl.json @@ -0,0 +1,24 @@ +{ + "name": "FuncionalityAcl", + "base": "VnModel", + "options": { + "mysql": { + "table": "salix.funcionalityAcl" + } + }, + "properties": { + "id": { + "type": "number", + "id": true + }, + "model": { + "type": "string" + }, + "property": { + "type": "string" + }, + "role": { + "type": "string" + } + } +} diff --git a/db/changes/10490-august/00-funcionalityAcl.sql b/db/changes/10490-august/00-funcionalityAcl.sql new file mode 100644 index 0000000000..889a92c48d --- /dev/null +++ b/db/changes/10490-august/00-funcionalityAcl.sql @@ -0,0 +1,7 @@ +CREATE TABLE `funcionalityAcl` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `model` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, + `property` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, + `role` varchar(45) COLLATE utf8mb3_unicode_ci DEFAULT NULL, + PRIMARY KEY (`id`) + ) ENGINE=InnoDB AUTO_INCREMENT=65 DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci diff --git a/db/dump/fixtures.sql b/db/dump/fixtures.sql index a09755479d..cde9345efa 100644 --- a/db/dump/fixtures.sql +++ b/db/dump/fixtures.sql @@ -14,10 +14,10 @@ INSERT INTO `salix`.`AccessToken` (`id`, `ttl`, `created`, `userId`) ('DEFAULT_TOKEN', '1209600', util.VN_CURDATE(), 66); INSERT INTO `salix`.`printConfig` (`id`, `itRecipient`, `incidencesEmail`) - VALUES + VALUES (1, 'it@gotamcity.com', 'incidences@gotamcity.com'); -INSERT INTO `vn`.`ticketConfig` (`id`, `scopeDays`) +INSERT INTO `vn`.`ticketConfig` (`id`, `scopeDays`) VALUES ('1', '6'); @@ -1146,10 +1146,11 @@ INSERT INTO `vncontrol`.`accion`(`accion_id`, `accion`) INSERT INTO `vn`.`saleTracking`(`saleFk`, `isChecked`, `created`, `originalQuantity`, `workerFk`, `actionFk`, `id`, `stateFk`) VALUES - (1, 0, util.VN_CURDATE(), 5, 55, 3, 1, 14), - (1, 1, util.VN_CURDATE(), 5, 54, 3, 2, 8), - (2, 1, util.VN_CURDATE(), 10, 40, 4, 3, 8), - (3, 1, util.VN_CURDATE(), 2, 40, 4, 4, 8); + (1, 0, util.VN_CURDATE(), 5, 55, 3, 1, 14), + (1, 1, util.VN_CURDATE(), 5, 54, 3, 2, 8), + (2, 1, util.VN_CURDATE(), 10, 40, 4, 3, 8), + (3, 1, util.VN_CURDATE(), 2, 40, 4, 4, 8), + (31, 1, util.VN_CURDATE(), -5, 40, 4, 5, 8); INSERT INTO `vn`.`itemBarcode`(`id`, `itemFk`, `code`) VALUES @@ -2664,8 +2665,11 @@ INSERT INTO `vn`.`ticketCollection` (`ticketFk`, `collectionFk`, `created`, `lev VALUES (9, 3, util.VN_NOW(), NULL, 0, NULL, NULL, NULL, NULL); -INSERT INTO salix.ACL -(model, property, accessType, permission, principalType, principalId) -VALUES -('Sale', 'editTracked', 'WRITE', 'ALLOW', 'ROLE', 'production'), -('Sale', 'editCloned', 'WRITE', 'ALLOW', 'ROLE', 'production'); +INSERT INTO `salix`.`funcionalityAcl` (`model`, `property`, `role`) + VALUES + ('Sale', 'editTracked', 'production'), + ('Sale', 'editCloned', 'production'); + +INSERT INTO `vn`.`saleCloned` (`saleClonedFk`, `saleOriginalFk`) + VALUES + ('26', '25'); diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index 94c2fc19a9..1b9e471ef8 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -37,8 +37,8 @@ module.exports = Self => { const saleCloned = await models.SaleCloned.find({where: {saleClonedFk: {inq: sales}}}, myOptions); const hasSaleCloned = saleCloned.length; - const canEditTracked = await models.Account.aclFunc(ctx, 'editTracked'); - const canEditCloned = await models.Account.aclFunc(ctx, 'editCloned'); + const canEditTracked = await models.Account.funcionalityAcl(ctx, 'Sale', 'editTracked'); + const canEditCloned = await models.Account.funcionalityAcl(ctx, 'Sale', 'editCloned'); const canEdit = (canEditTracked || !hasSaleTracking) && (canEditCloned || !hasSaleCloned); diff --git a/modules/ticket/back/methods/sale/specs/canEdit.spec.js b/modules/ticket/back/methods/sale/specs/canEdit.spec.js index 667ce98da3..9533d64641 100644 --- a/modules/ticket/back/methods/sale/specs/canEdit.spec.js +++ b/modules/ticket/back/methods/sale/specs/canEdit.spec.js @@ -10,7 +10,7 @@ describe('sale canEdit()', () => { const productionUserID = 49; const ctx = {req: {accessToken: {userId: productionUserID}}}; - const sales = [3]; + const sales = [25]; const result = await models.Sale.canEdit(ctx, sales, options); @@ -51,10 +51,10 @@ describe('sale canEdit()', () => { try { const options = {transaction: tx}; - const salesPersonUserID = 18; - const ctx = {req: {accessToken: {userId: salesPersonUserID}}}; + const buyerId = 35; + const ctx = {req: {accessToken: {userId: buyerId}}}; - const sales = [3]; + const sales = [31]; const result = await models.Sale.canEdit(ctx, sales, options); @@ -66,4 +66,48 @@ describe('sale canEdit()', () => { throw e; } }); + + it('should return false if any of the sales is cloned', async() => { + const tx = await models.Sale.beginTransaction({}); + + try { + const options = {transaction: tx}; + + const buyerId = 35; + const ctx = {req: {accessToken: {userId: buyerId}}}; + + const sales = [26]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(false); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should return true if any of the sales is cloned and has the correct role', async() => { + const tx = await models.Sale.beginTransaction({}); + // modify? + try { + const options = {transaction: tx}; + + const productionId = 49; + const ctx = {req: {accessToken: {userId: productionId}}}; + + const sales = [26]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(true); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); }); diff --git a/modules/ticket/back/methods/sale/specs/updateConcept.spec.js b/modules/ticket/back/methods/sale/specs/updateConcept.spec.js index 01f610d36d..0e7e9bf0fa 100644 --- a/modules/ticket/back/methods/sale/specs/updateConcept.spec.js +++ b/modules/ticket/back/methods/sale/specs/updateConcept.spec.js @@ -2,7 +2,7 @@ const models = require('vn-loopback/server/server').models; describe('sale updateConcept()', () => { const ctx = {req: {accessToken: {userId: 9}}}; - const saleId = 1; + const saleId = 25; it('should throw if ID was undefined', async() => { const tx = await models.Sale.beginTransaction({}); diff --git a/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js b/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js index 4c961faab5..bf139ab115 100644 --- a/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js +++ b/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js @@ -28,13 +28,20 @@ describe('sale updateQuantity()', () => { }); it('should throw an error if the quantity is greater than it should be', async() => { + const ctx = { + req: { + accessToken: {userId: 1}, + headers: {origin: 'localhost:5000'}, + __: () => {} + } + }; const tx = await models.Sale.beginTransaction({}); let error; try { const options = {transaction: tx}; - await models.Sale.updateQuantity(ctx, 1, 99, options); + await models.Sale.updateQuantity(ctx, 17, 99, options); await tx.rollback(); } catch (e) { @@ -45,21 +52,60 @@ describe('sale updateQuantity()', () => { expect(error).toEqual(new Error('The new quantity should be smaller than the old one')); }); - it('should update the quantity of a given sale current line', async() => { + it('should add quantity if the quantity is greater than it should be and is role advanced', async() => { const tx = await models.Sale.beginTransaction({}); + const saleId = 17; + const buyerId = 35; + const ctx = { + req: { + accessToken: {userId: buyerId}, + headers: {origin: 'localhost:5000'}, + __: () => {} + } + }; try { const options = {transaction: tx}; - const originalLine = await models.Sale.findOne({where: {id: 1}, fields: ['quantity']}, options); + const isRoleAdvanced = await models.Ticket.isRoleAdvanced(ctx, options); - expect(originalLine.quantity).toEqual(5); + expect(isRoleAdvanced).toEqual(true); - await models.Sale.updateQuantity(ctx, 1, 4, options); + const originalLine = await models.Sale.findOne({where: {id: saleId}, fields: ['quantity']}, options); - const modifiedLine = await models.Sale.findOne({where: {id: 1}, fields: ['quantity']}, options); + expect(originalLine.quantity).toEqual(30); - expect(modifiedLine.quantity).toEqual(4); + const newQuantity = originalLine.quantity + 1; + await models.Sale.updateQuantity(ctx, saleId, newQuantity, options); + + const modifiedLine = await models.Sale.findOne({where: {id: saleId}, fields: ['quantity']}, options); + + expect(modifiedLine.quantity).toEqual(newQuantity); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should update the quantity of a given sale current line', async() => { + const tx = await models.Sale.beginTransaction({}); + const saleId = 25; + const newQuantity = 4; + + try { + const options = {transaction: tx}; + + const originalLine = await models.Sale.findOne({where: {id: saleId}, fields: ['quantity']}, options); + + expect(originalLine.quantity).toEqual(20); + + await models.Sale.updateQuantity(ctx, saleId, newQuantity, options); + + const modifiedLine = await models.Sale.findOne({where: {id: saleId}, fields: ['quantity']}, options); + + expect(modifiedLine.quantity).toEqual(newQuantity); await tx.rollback(); } catch (e) { diff --git a/modules/ticket/back/methods/sale/updateQuantity.js b/modules/ticket/back/methods/sale/updateQuantity.js index 24e3736f37..f3bc0ca380 100644 --- a/modules/ticket/back/methods/sale/updateQuantity.js +++ b/modules/ticket/back/methods/sale/updateQuantity.js @@ -41,14 +41,13 @@ module.exports = Self => { } try { - const canEditSale = await models.Sale.canEdit(ctx, [id], myOptions); - - if (!canEditSale) - throw new UserError(`Sale(s) blocked, please contact production`); - if (isNaN(newQuantity)) throw new UserError(`The value should be a number`); + const canEditSale = await models.Sale.canEdit(ctx, [id], myOptions); + if (!canEditSale) + throw new UserError(`Sale(s) blocked, please contact production`); + const filter = { include: { relation: 'ticket', @@ -70,7 +69,8 @@ module.exports = Self => { const sale = await models.Sale.findById(id, filter, myOptions); - if (newQuantity > sale.quantity) + const isRoleAdvanced = await models.Ticket.isRoleAdvanced(ctx, myOptions); + if (newQuantity > sale.quantity && !isRoleAdvanced) throw new UserError('The new quantity should be smaller than the old one'); const oldQuantity = sale.quantity; diff --git a/modules/ticket/back/models/ticket-methods.js b/modules/ticket/back/models/ticket-methods.js index 9255e52e62..8ab1845d96 100644 --- a/modules/ticket/back/models/ticket-methods.js +++ b/modules/ticket/back/models/ticket-methods.js @@ -33,4 +33,5 @@ module.exports = function(Self) { require('../methods/ticket/closeByTicket')(Self); require('../methods/ticket/closeByAgency')(Self); require('../methods/ticket/closeByRoute')(Self); + require('../methods/ticket/isRoleAdvanced')(Self); }; From c29625f079c50a56a30308d9d8cdd45157209401 Mon Sep 17 00:00:00 2001 From: alexm Date: Thu, 6 Oct 2022 15:04:27 +0200 Subject: [PATCH 04/14] fix: funcionalityAcl --- back/methods/account/funcionalityAcl.js | 20 +++++++++++++------ back/model-config.json | 3 +++ back/models/account.js | 2 +- .../10490-august/00-funcionalityAcl.sql | 10 ++++++++-- db/dump/fixtures.sql | 5 ----- .../back/methods/sale/specs/canEdit.spec.js | 2 +- 6 files changed, 27 insertions(+), 15 deletions(-) diff --git a/back/methods/account/funcionalityAcl.js b/back/methods/account/funcionalityAcl.js index 3a5e09720d..ae73dee0b8 100644 --- a/back/methods/account/funcionalityAcl.js +++ b/back/methods/account/funcionalityAcl.js @@ -29,12 +29,20 @@ module.exports = Self => { const userId = ctx.req.accessToken.userId; const models = Self.app.models; - const [acl] = await Self.rawSql( - `SELECT f.role - FROM salix.funcionalityAcl f - WHERE f.model = ? - AND f.property = ?`, [model, property]); + const acls = await models.FuncionalityAcl.find({ + where: { + model: model, + property: property + } + }); - return await models.Account.hasRole(userId, acl.role); + const hasPermissions = acls.filter(async acl => { + console.log('FILTER: '); + acl.role && await models.Account.hasRole(userId, acl.role); + }); + console.log(hasPermissions); + if (hasPermissions) + return true; + return false; }; }; diff --git a/back/model-config.json b/back/model-config.json index 830a78fd42..c351969552 100644 --- a/back/model-config.json +++ b/back/model-config.json @@ -53,6 +53,9 @@ "EmailUser": { "dataSource": "vn" }, + "FuncionalityAcl": { + "dataSource": "vn" + }, "Image": { "dataSource": "vn" }, diff --git a/back/models/account.js b/back/models/account.js index 5b101eef76..368f154bce 100644 --- a/back/models/account.js +++ b/back/models/account.js @@ -7,7 +7,7 @@ module.exports = Self => { require('../methods/account/change-password')(Self); require('../methods/account/set-password')(Self); require('../methods/account/validate-token')(Self); - require('../methods/account/aclFunc')(Self); + require('../methods/account/funcionalityAcl')(Self); // Validations diff --git a/db/changes/10490-august/00-funcionalityAcl.sql b/db/changes/10490-august/00-funcionalityAcl.sql index 889a92c48d..b525803279 100644 --- a/db/changes/10490-august/00-funcionalityAcl.sql +++ b/db/changes/10490-august/00-funcionalityAcl.sql @@ -1,7 +1,13 @@ -CREATE TABLE `funcionalityAcl` ( +CREATE TABLE `salix`.`funcionalityAcl` ( `id` int(11) NOT NULL AUTO_INCREMENT, `model` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, `property` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, `role` varchar(45) COLLATE utf8mb3_unicode_ci DEFAULT NULL, PRIMARY KEY (`id`) - ) ENGINE=InnoDB AUTO_INCREMENT=65 DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci +) ENGINE=InnoDB AUTO_INCREMENT=65 DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci; + + +INSERT INTO `salix`.`funcionalityAcl` (`model`, `property`, `role`) + VALUES + ('Sale', 'editTracked', 'production'), + ('Sale', 'editCloned', NULL); diff --git a/db/dump/fixtures.sql b/db/dump/fixtures.sql index cde9345efa..fb31c08a56 100644 --- a/db/dump/fixtures.sql +++ b/db/dump/fixtures.sql @@ -2665,11 +2665,6 @@ INSERT INTO `vn`.`ticketCollection` (`ticketFk`, `collectionFk`, `created`, `lev VALUES (9, 3, util.VN_NOW(), NULL, 0, NULL, NULL, NULL, NULL); -INSERT INTO `salix`.`funcionalityAcl` (`model`, `property`, `role`) - VALUES - ('Sale', 'editTracked', 'production'), - ('Sale', 'editCloned', 'production'); - INSERT INTO `vn`.`saleCloned` (`saleClonedFk`, `saleOriginalFk`) VALUES ('26', '25'); diff --git a/modules/ticket/back/methods/sale/specs/canEdit.spec.js b/modules/ticket/back/methods/sale/specs/canEdit.spec.js index 9533d64641..1fe63f33c8 100644 --- a/modules/ticket/back/methods/sale/specs/canEdit.spec.js +++ b/modules/ticket/back/methods/sale/specs/canEdit.spec.js @@ -91,7 +91,7 @@ describe('sale canEdit()', () => { it('should return true if any of the sales is cloned and has the correct role', async() => { const tx = await models.Sale.beginTransaction({}); - // modify? + try { const options = {transaction: tx}; From 376d46b67ec22278a303894449537ce81cac9182 Mon Sep 17 00:00:00 2001 From: alexm Date: Mon, 10 Oct 2022 13:06:49 +0200 Subject: [PATCH 05/14] feat(canEdit): integrate isWeekly and adapt back tests and e2e --- ...ncionalityAcl.js => hasFuncionalityAcl.js} | 15 ++-- back/models/account.js | 2 +- .../00-funcionalityAcl.sql | 8 +- db/changes/10491-august/delete.keep | 0 db/dump/fixtures.sql | 5 +- db/export-data.sh | 1 + e2e/paths/05-ticket/09_weekly.spec.js | 4 +- loopback/locale/en.json | 3 +- modules/ticket/back/methods/sale/canEdit.js | 14 +++- .../back/methods/sale/specs/canEdit.spec.js | 74 ++++++++++++++++++- .../ticket-weekly/specs/filter.spec.js | 2 +- 11 files changed, 103 insertions(+), 25 deletions(-) rename back/methods/account/{funcionalityAcl.js => hasFuncionalityAcl.js} (71%) rename db/changes/{10490-august => 10491-august}/00-funcionalityAcl.sql (60%) delete mode 100644 db/changes/10491-august/delete.keep diff --git a/back/methods/account/funcionalityAcl.js b/back/methods/account/hasFuncionalityAcl.js similarity index 71% rename from back/methods/account/funcionalityAcl.js rename to back/methods/account/hasFuncionalityAcl.js index ae73dee0b8..d6224fffcb 100644 --- a/back/methods/account/funcionalityAcl.js +++ b/back/methods/account/hasFuncionalityAcl.js @@ -1,5 +1,5 @@ module.exports = Self => { - Self.remoteMethod('funcionalityAcl', { + Self.remoteMethod('hasFuncionalityAcl', { description: 'Return if user has permissions', accepts: [ { @@ -20,12 +20,12 @@ module.exports = Self => { root: true }, http: { - path: `/funcionalityAcl`, + path: `/hasFuncionalityAcl`, verb: 'GET' } }); - Self.funcionalityAcl = async function(ctx, model, property) { + Self.hasFuncionalityAcl = async function(ctx, model, property) { const userId = ctx.req.accessToken.userId; const models = Self.app.models; @@ -36,11 +36,10 @@ module.exports = Self => { } }); - const hasPermissions = acls.filter(async acl => { - console.log('FILTER: '); - acl.role && await models.Account.hasRole(userId, acl.role); - }); - console.log(hasPermissions); + let hasPermissions; + for (let acl of acls) + if (!hasPermissions) hasPermissions = await models.Account.hasRole(userId, acl.role); + if (hasPermissions) return true; return false; diff --git a/back/models/account.js b/back/models/account.js index f824ed8986..7d7fa9fe38 100644 --- a/back/models/account.js +++ b/back/models/account.js @@ -7,7 +7,7 @@ module.exports = Self => { require('../methods/account/change-password')(Self); require('../methods/account/set-password')(Self); require('../methods/account/validate-token')(Self); - require('../methods/account/funcionalityAcl')(Self); + require('../methods/account/hasFuncionalityAcl')(Self); require('../methods/account/privileges')(Self); // Validations diff --git a/db/changes/10490-august/00-funcionalityAcl.sql b/db/changes/10491-august/00-funcionalityAcl.sql similarity index 60% rename from db/changes/10490-august/00-funcionalityAcl.sql rename to db/changes/10491-august/00-funcionalityAcl.sql index b525803279..02f3dbcc43 100644 --- a/db/changes/10490-august/00-funcionalityAcl.sql +++ b/db/changes/10491-august/00-funcionalityAcl.sql @@ -3,11 +3,13 @@ CREATE TABLE `salix`.`funcionalityAcl` ( `model` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, `property` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, `role` varchar(45) COLLATE utf8mb3_unicode_ci DEFAULT NULL, - PRIMARY KEY (`id`) -) ENGINE=InnoDB AUTO_INCREMENT=65 DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci; + PRIMARY KEY (`id`), + CONSTRAINT `role_FK` FOREIGN KEY (`role`) REFERENCES `account`.`role` (`name`) ON UPDATE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci; INSERT INTO `salix`.`funcionalityAcl` (`model`, `property`, `role`) VALUES ('Sale', 'editTracked', 'production'), - ('Sale', 'editCloned', NULL); + ('Sale', 'editCloned', 66); + ('Sale', 'editWeekly', 66); diff --git a/db/changes/10491-august/delete.keep b/db/changes/10491-august/delete.keep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/db/dump/fixtures.sql b/db/dump/fixtures.sql index 4bd4dd96b0..fa76f6f842 100644 --- a/db/dump/fixtures.sql +++ b/db/dump/fixtures.sql @@ -1357,7 +1357,8 @@ INSERT INTO `vn`.`ticketWeekly`(`ticketFk`, `weekDay`) (2, 1), (3, 2), (4, 4), - (5, 6); + (5, 6), + (14, 6); INSERT INTO `vn`.`travel`(`id`,`shipped`, `landed`, `warehouseInFk`, `warehouseOutFk`, `agencyModeFk`, `m3`, `kg`,`ref`, `totalEntries`, `cargoSupplierFk`) VALUES @@ -2667,7 +2668,7 @@ INSERT INTO `vn`.`ticketCollection` (`ticketFk`, `collectionFk`, `created`, `lev INSERT INTO `vn`.`saleCloned` (`saleClonedFk`, `saleOriginalFk`) VALUES - ('26', '25'); + ('27', '25'); UPDATE `account`.`user` SET `hasGrant` = 1 diff --git a/db/export-data.sh b/db/export-data.sh index bbbeb7152b..7252267ba8 100755 --- a/db/export-data.sh +++ b/db/export-data.sh @@ -34,6 +34,7 @@ TABLES=( salix ACL fieldAcl + funcionalityAcl module defaultViewConfig ) diff --git a/e2e/paths/05-ticket/09_weekly.spec.js b/e2e/paths/05-ticket/09_weekly.spec.js index d04138ee5f..0ba57aa9dc 100644 --- a/e2e/paths/05-ticket/09_weekly.spec.js +++ b/e2e/paths/05-ticket/09_weekly.spec.js @@ -19,7 +19,7 @@ describe('Ticket descriptor path', () => { it('should count the amount of tickets in the turns section', async() => { const result = await page.countElement(selectors.ticketsIndex.weeklyTicket); - expect(result).toEqual(5); + expect(result).toEqual(6); }); it('should go back to the ticket index then search and access a ticket summary', async() => { @@ -104,7 +104,7 @@ describe('Ticket descriptor path', () => { await page.doSearch(); const nResults = await page.countElement(selectors.ticketsIndex.searchWeeklyResult); - expect(nResults).toEqual(5); + expect(nResults).toEqual(6); }); it('should update the agency then remove it afterwards', async() => { diff --git a/loopback/locale/en.json b/loopback/locale/en.json index e5a0fae322..6d2faf6044 100644 --- a/loopback/locale/en.json +++ b/loopback/locale/en.json @@ -133,5 +133,6 @@ "Descanso semanal 36h. / 72h.": "Weekly rest 36h. / 72h.", "Password does not meet requirements": "Password does not meet requirements", "You don't have privileges to change the zone": "You don't have privileges to change the zone or for these parameters there are more than one shipping options, talk to agencies", - "Not enough privileges to edit a client": "Not enough privileges to edit a client" + "Not enough privileges to edit a client": "Not enough privileges to edit a client", + "Sale(s) blocked, please contact production": "Sale(s) blocked, please contact production" } \ No newline at end of file diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index 1b9e471ef8..c0cd4b7019 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -37,10 +37,18 @@ module.exports = Self => { const saleCloned = await models.SaleCloned.find({where: {saleClonedFk: {inq: sales}}}, myOptions); const hasSaleCloned = saleCloned.length; - const canEditTracked = await models.Account.funcionalityAcl(ctx, 'Sale', 'editTracked'); - const canEditCloned = await models.Account.funcionalityAcl(ctx, 'Sale', 'editCloned'); + const isTicketWeekly = + await models.TicketWeekly.findOne({where: {ticketFk: firstSale.ticketFk}}, myOptions); - const canEdit = (canEditTracked || !hasSaleTracking) && (canEditCloned || !hasSaleCloned); + const canEditTracked = await models.Account.hasFuncionalityAcl(ctx, 'Sale', 'editTracked'); + const canEditCloned = await models.Account.hasFuncionalityAcl(ctx, 'Sale', 'editCloned'); + const canEditWeekly = await models.Account.hasFuncionalityAcl(ctx, 'Ticket', 'editWeekly'); + + const shouldEditTracked = canEditTracked || !hasSaleTracking; + const shouldEditCloned = canEditCloned || !hasSaleCloned; + const shouldEditWeekly = canEditWeekly || !isTicketWeekly; + + const canEdit = shouldEditTracked && shouldEditCloned && shouldEditWeekly; return canEdit; }; diff --git a/modules/ticket/back/methods/sale/specs/canEdit.spec.js b/modules/ticket/back/methods/sale/specs/canEdit.spec.js index 1fe63f33c8..7d89471f63 100644 --- a/modules/ticket/back/methods/sale/specs/canEdit.spec.js +++ b/modules/ticket/back/methods/sale/specs/canEdit.spec.js @@ -76,7 +76,7 @@ describe('sale canEdit()', () => { const buyerId = 35; const ctx = {req: {accessToken: {userId: buyerId}}}; - const sales = [26]; + const sales = [27]; const result = await models.Sale.canEdit(ctx, sales, options); @@ -91,14 +91,80 @@ describe('sale canEdit()', () => { it('should return true if any of the sales is cloned and has the correct role', async() => { const tx = await models.Sale.beginTransaction({}); + const roleEnabled = await models.FuncionalityAcl.findOne({ + where: { + model: 'Sale', + property: 'editCloned' + } + }); + if (!roleEnabled || !roleEnabled.role) return; try { const options = {transaction: tx}; - const productionId = 49; - const ctx = {req: {accessToken: {userId: productionId}}}; + const roleId = await models.Role.findOne({ + where: { + name: roleEnabled.role + } + }); + const ctx = {req: {accessToken: {userId: roleId}}}; - const sales = [26]; + const sales = [27]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(true); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should return false if any of the sales is of ticket weekly', async() => { + const tx = await models.Sale.beginTransaction({}); + + try { + const options = {transaction: tx}; + + const employeeId = 1; + const ctx = {req: {accessToken: {userId: employeeId}}}; + + const sales = [33]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(false); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should return true if any of the sales is of ticketWeekly and has the correct role', async() => { + const tx = await models.Sale.beginTransaction({}); + const roleEnabled = await models.FuncionalityAcl.findOne({ + where: { + model: 'Sale', + property: 'editWeekly' + } + }); + if (!roleEnabled || !roleEnabled.role) return; + + try { + const options = {transaction: tx}; + + const roleId = await models.Role.findOne({ + where: { + name: roleEnabled.role + } + }); + const ctx = {req: {accessToken: {userId: roleId}}}; + + const sales = [33]; const result = await models.Sale.canEdit(ctx, sales, options); diff --git a/modules/ticket/back/methods/ticket-weekly/specs/filter.spec.js b/modules/ticket/back/methods/ticket-weekly/specs/filter.spec.js index 411bbe014d..2587b66576 100644 --- a/modules/ticket/back/methods/ticket-weekly/specs/filter.spec.js +++ b/modules/ticket/back/methods/ticket-weekly/specs/filter.spec.js @@ -17,7 +17,7 @@ describe('ticket-weekly filter()', () => { const firstRow = result[0]; expect(firstRow.ticketFk).toEqual(1); - expect(result.length).toEqual(5); + expect(result.length).toEqual(6); await tx.rollback(); } catch (e) { From 99271f275f7b25c823b8bacd2cfc5689357b5862 Mon Sep 17 00:00:00 2001 From: alexm Date: Mon, 10 Oct 2022 13:26:38 +0200 Subject: [PATCH 06/14] remove unnecessary isEditable --- modules/ticket/back/methods/sale/deleteSales.js | 10 +++------- modules/ticket/back/methods/sale/recalculatePrice.js | 4 ---- modules/ticket/back/methods/sale/reserve.js | 6 +----- modules/ticket/back/methods/sale/updatePrice.js | 5 ----- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/modules/ticket/back/methods/sale/deleteSales.js b/modules/ticket/back/methods/sale/deleteSales.js index 7036821e9c..c045b91971 100644 --- a/modules/ticket/back/methods/sale/deleteSales.js +++ b/modules/ticket/back/methods/sale/deleteSales.js @@ -42,7 +42,10 @@ module.exports = Self => { try { const saleIds = sales.map(sale => sale.id); + const canEditSales = await models.Sale.canEdit(ctx, saleIds, myOptions); + if (!canEditSales) + throw new UserError(`Sale(s) blocked, please contact production`); const ticket = await models.Ticket.findById(ticketId, { include: { @@ -58,13 +61,6 @@ module.exports = Self => { } }, myOptions); - const isTicketEditable = await models.Ticket.isEditable(ctx, ticketId, myOptions); - if (!isTicketEditable) - throw new UserError(`The sales of this ticket can't be modified`); - - if (!canEditSales) - throw new UserError(`Sale(s) blocked, please contact production`); - const promises = []; let deletions = ''; for (let sale of sales) { diff --git a/modules/ticket/back/methods/sale/recalculatePrice.js b/modules/ticket/back/methods/sale/recalculatePrice.js index cbe59cad2d..38c68d7f62 100644 --- a/modules/ticket/back/methods/sale/recalculatePrice.js +++ b/modules/ticket/back/methods/sale/recalculatePrice.js @@ -37,10 +37,6 @@ module.exports = Self => { try { const salesIds = sales.map(sale => sale.id); - const isEditable = await models.Ticket.isEditable(ctx, sales[0].ticketFk, myOptions); - if (!isEditable) - throw new UserError(`The sales of this ticket can't be modified`); - const canEditSale = await models.Sale.canEdit(ctx, salesIds, myOptions); if (!canEditSale) throw new UserError(`Sale(s) blocked, please contact production`); diff --git a/modules/ticket/back/methods/sale/reserve.js b/modules/ticket/back/methods/sale/reserve.js index c3e9ac30f2..648e6de237 100644 --- a/modules/ticket/back/methods/sale/reserve.js +++ b/modules/ticket/back/methods/sale/reserve.js @@ -49,13 +49,9 @@ module.exports = Self => { } try { - const isTicketEditable = await models.Ticket.isEditable(ctx, ticketId, myOptions); - if (!isTicketEditable) - throw new UserError(`The sales of this ticket can't be modified`); - const salesIds = sales.map(sale => sale.id); - const canEditSale = await models.Sale.canEdit(ctx, salesIds, myOptions); + const canEditSale = await models.Sale.canEdit(ctx, salesIds, myOptions); if (!canEditSale) throw new UserError(`Sale(s) blocked, please contact production`); diff --git a/modules/ticket/back/methods/sale/updatePrice.js b/modules/ticket/back/methods/sale/updatePrice.js index bbd9d154db..cca73fef1e 100644 --- a/modules/ticket/back/methods/sale/updatePrice.js +++ b/modules/ticket/back/methods/sale/updatePrice.js @@ -66,12 +66,7 @@ module.exports = Self => { const sale = await models.Sale.findById(id, filter, myOptions); - const isEditable = await models.Ticket.isEditable(ctx, sale.ticketFk, myOptions); - if (!isEditable) - throw new UserError(`The sales of this ticket can't be modified`); - const canEditSale = await models.Sale.canEdit(ctx, [id], myOptions); - if (!canEditSale) throw new UserError(`Sale(s) blocked, please contact production`); From a5ceee07e94bd012cfdf256f4e3e38cd7c78c570 Mon Sep 17 00:00:00 2001 From: alexm Date: Tue, 18 Oct 2022 11:50:16 +0200 Subject: [PATCH 07/14] use loopback --- back/methods/account/hasFuncionalityAcl.js | 47 ------------------- back/models/account.js | 1 - db/changes/10491-august/00-editTrackedACL.sql | 3 ++ .../10491-august/00-funcionalityAcl.sql | 15 ------ modules/ticket/back/methods/sale/canEdit.js | 28 +++++++++-- .../back/methods/sale/specs/canEdit.spec.js | 12 ++--- .../back/methods/sale/specs/reserve.spec.js | 6 +-- 7 files changed, 36 insertions(+), 76 deletions(-) delete mode 100644 back/methods/account/hasFuncionalityAcl.js create mode 100644 db/changes/10491-august/00-editTrackedACL.sql delete mode 100644 db/changes/10491-august/00-funcionalityAcl.sql diff --git a/back/methods/account/hasFuncionalityAcl.js b/back/methods/account/hasFuncionalityAcl.js deleted file mode 100644 index d6224fffcb..0000000000 --- a/back/methods/account/hasFuncionalityAcl.js +++ /dev/null @@ -1,47 +0,0 @@ -module.exports = Self => { - Self.remoteMethod('hasFuncionalityAcl', { - description: 'Return if user has permissions', - accepts: [ - { - arg: 'model', - type: 'String', - description: 'The model', - required: true - }, - { - arg: 'property', - type: 'String', - description: 'The property', - required: true - } - ], - returns: { - type: 'Object', - root: true - }, - http: { - path: `/hasFuncionalityAcl`, - verb: 'GET' - } - }); - - Self.hasFuncionalityAcl = async function(ctx, model, property) { - const userId = ctx.req.accessToken.userId; - const models = Self.app.models; - - const acls = await models.FuncionalityAcl.find({ - where: { - model: model, - property: property - } - }); - - let hasPermissions; - for (let acl of acls) - if (!hasPermissions) hasPermissions = await models.Account.hasRole(userId, acl.role); - - if (hasPermissions) - return true; - return false; - }; -}; diff --git a/back/models/account.js b/back/models/account.js index 7d7fa9fe38..f74052b5c9 100644 --- a/back/models/account.js +++ b/back/models/account.js @@ -7,7 +7,6 @@ module.exports = Self => { require('../methods/account/change-password')(Self); require('../methods/account/set-password')(Self); require('../methods/account/validate-token')(Self); - require('../methods/account/hasFuncionalityAcl')(Self); require('../methods/account/privileges')(Self); // Validations diff --git a/db/changes/10491-august/00-editTrackedACL.sql b/db/changes/10491-august/00-editTrackedACL.sql new file mode 100644 index 0000000000..37d24ac814 --- /dev/null +++ b/db/changes/10491-august/00-editTrackedACL.sql @@ -0,0 +1,3 @@ +INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) + VALUES + ('Sale', 'editTracked', 'READ', 'ALLOW', 'ROLE', 'production'); diff --git a/db/changes/10491-august/00-funcionalityAcl.sql b/db/changes/10491-august/00-funcionalityAcl.sql deleted file mode 100644 index 02f3dbcc43..0000000000 --- a/db/changes/10491-august/00-funcionalityAcl.sql +++ /dev/null @@ -1,15 +0,0 @@ -CREATE TABLE `salix`.`funcionalityAcl` ( - `id` int(11) NOT NULL AUTO_INCREMENT, - `model` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, - `property` varchar(255) COLLATE utf8mb3_unicode_ci DEFAULT NULL, - `role` varchar(45) COLLATE utf8mb3_unicode_ci DEFAULT NULL, - PRIMARY KEY (`id`), - CONSTRAINT `role_FK` FOREIGN KEY (`role`) REFERENCES `account`.`role` (`name`) ON UPDATE CASCADE -) ENGINE=InnoDB DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_unicode_ci; - - -INSERT INTO `salix`.`funcionalityAcl` (`model`, `property`, `role`) - VALUES - ('Sale', 'editTracked', 'production'), - ('Sale', 'editCloned', 66); - ('Sale', 'editWeekly', 66); diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index c0cd4b7019..b1dab82a5d 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -40,16 +40,36 @@ module.exports = Self => { const isTicketWeekly = await models.TicketWeekly.findOne({where: {ticketFk: firstSale.ticketFk}}, myOptions); - const canEditTracked = await models.Account.hasFuncionalityAcl(ctx, 'Sale', 'editTracked'); - const canEditCloned = await models.Account.hasFuncionalityAcl(ctx, 'Sale', 'editCloned'); - const canEditWeekly = await models.Account.hasFuncionalityAcl(ctx, 'Ticket', 'editWeekly'); + // (principalType, principalId,model, property, accessType,callback); + // let canEditTracked = await models.ACL.checkPermission('ROLE', 'employee', 'Sale', 'updateConcept', '*'); + // let canEditTracked2 = await models.ACL.checkPermission('USER', 'developer', 'Sale', 'editTracked', 'READ'); + const array = ['editTracked']; + let canEditTracked3 = await models.ACL.checkAccessForContext({ + principals: [{ + type: 'ROLE', + id: 'employee' + }], + model: 'Sale', + property: 'editTracked', + methodNames: array, + accessType: 'READ' + }); + console.log(canEditTracked3); + // canEditTracked = await models.ACL.resolvePermission(canEditTracked); + // let canEditCloned = await models.ACL.checkPermission('ROLE', 'employee', 'Sale', 'editCloned', '*'); + // let canEditWeekly = await models.ACL.checkPermission('ROLE', 'employee', 'Ticket', 'editWeekly', '*'); + // console.log(canEditTracked, canEditTracked2); + console.log(canEditTracked3); const shouldEditTracked = canEditTracked || !hasSaleTracking; const shouldEditCloned = canEditCloned || !hasSaleCloned; const shouldEditWeekly = canEditWeekly || !isTicketWeekly; const canEdit = shouldEditTracked && shouldEditCloned && shouldEditWeekly; - return canEdit; + if (canEdit) + return true; + + return false; }; }; diff --git a/modules/ticket/back/methods/sale/specs/canEdit.spec.js b/modules/ticket/back/methods/sale/specs/canEdit.spec.js index 7d89471f63..1522ee7a31 100644 --- a/modules/ticket/back/methods/sale/specs/canEdit.spec.js +++ b/modules/ticket/back/methods/sale/specs/canEdit.spec.js @@ -91,20 +91,20 @@ describe('sale canEdit()', () => { it('should return true if any of the sales is cloned and has the correct role', async() => { const tx = await models.Sale.beginTransaction({}); - const roleEnabled = await models.FuncionalityAcl.findOne({ + const roleEnabled = await models.ACL.findOne({ where: { model: 'Sale', property: 'editCloned' } }); - if (!roleEnabled || !roleEnabled.role) return; + if (!roleEnabled || !roleEnabled.principalId) return; try { const options = {transaction: tx}; const roleId = await models.Role.findOne({ where: { - name: roleEnabled.role + name: roleEnabled.principalId } }); const ctx = {req: {accessToken: {userId: roleId}}}; @@ -146,20 +146,20 @@ describe('sale canEdit()', () => { it('should return true if any of the sales is of ticketWeekly and has the correct role', async() => { const tx = await models.Sale.beginTransaction({}); - const roleEnabled = await models.FuncionalityAcl.findOne({ + const roleEnabled = await models.ACL.findOne({ where: { model: 'Sale', property: 'editWeekly' } }); - if (!roleEnabled || !roleEnabled.role) return; + if (!roleEnabled || !roleEnabled.principalId) return; try { const options = {transaction: tx}; const roleId = await models.Role.findOne({ where: { - name: roleEnabled.role + name: roleEnabled.principalId } }); const ctx = {req: {accessToken: {userId: roleId}}}; diff --git a/modules/ticket/back/methods/sale/specs/reserve.spec.js b/modules/ticket/back/methods/sale/specs/reserve.spec.js index c4b3b4e5db..7c2d437151 100644 --- a/modules/ticket/back/methods/sale/specs/reserve.spec.js +++ b/modules/ticket/back/methods/sale/specs/reserve.spec.js @@ -1,9 +1,9 @@ const models = require('vn-loopback/server/server').models; -describe('sale reserve()', () => { +fdescribe('sale reserve()', () => { const ctx = { req: { - accessToken: {userId: 9}, + accessToken: {userId: 1}, headers: {origin: 'localhost:5000'}, __: () => {} } @@ -31,7 +31,7 @@ describe('sale reserve()', () => { expect(error).toEqual(new Error(`The sales of this ticket can't be modified`)); }); - it('should update the given sales of a ticket to reserved', async() => { + fit('should update the given sales of a ticket to reserved', async() => { const tx = await models.Sale.beginTransaction({}); try { From 508decaf0bc5eaa4694c614b513d695f26bc8d73 Mon Sep 17 00:00:00 2001 From: alexm Date: Wed, 19 Oct 2022 08:22:36 +0200 Subject: [PATCH 08/14] try --- modules/ticket/back/methods/sale/canEdit.js | 32 +++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index b1dab82a5d..4992a34990 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -1,4 +1,5 @@ const UserError = require('vn-loopback/util/user-error'); +const loopBackCtx = require('vn-loopback/server/server'); module.exports = Self => { Self.remoteMethodCtx('canEdit', { @@ -41,26 +42,45 @@ module.exports = Self => { await models.TicketWeekly.findOne({where: {ticketFk: firstSale.ticketFk}}, myOptions); // (principalType, principalId,model, property, accessType,callback); - // let canEditTracked = await models.ACL.checkPermission('ROLE', 'employee', 'Sale', 'updateConcept', '*'); + // let canEditTracked = await models.ACL.checkPermission('ROLE', 'employee', 'Sale', 'editTracked', 'WRITE'); + // let canEditTracked2 = await models.ACL.checkPermission('USER', 'developer', 'Sale', 'editTracked', 'READ'); const array = ['editTracked']; - let canEditTracked3 = await models.ACL.checkAccessForContext({ + const AccessContext = loopBackCtx.AccessContext; + const toFind = { principals: [{ type: 'ROLE', id: 'employee' }], model: 'Sale', property: 'editTracked', - methodNames: array, - accessType: 'READ' + methodNames: ['editTracked'], + accessType: 'WRITE' + }; + const newContext = new AccessContext(toFind); + newContext.methodNames = ['editTracked']; + + let canEditTracked3 = await models.ACL.checkAccessForContext(newContext); + + let canEditTracked4 = await models.ACL.checkAccessForContext({ + principals: [{ + type: 'ROLE', + id: 'developer' + }], + model: 'Sale', + property: 'editTracked', + methodName: 'editTracked', + methodNames: ['editTracked'], + accessType: 'WRITE' }); - console.log(canEditTracked3); + // console.log(canEditTracked); // canEditTracked = await models.ACL.resolvePermission(canEditTracked); // let canEditCloned = await models.ACL.checkPermission('ROLE', 'employee', 'Sale', 'editCloned', '*'); // let canEditWeekly = await models.ACL.checkPermission('ROLE', 'employee', 'Ticket', 'editWeekly', '*'); // console.log(canEditTracked, canEditTracked2); - console.log(canEditTracked3); + console.log('DENY: ', canEditTracked3.permission); + console.log('ALLOW: ', canEditTracked4.permission); const shouldEditTracked = canEditTracked || !hasSaleTracking; const shouldEditCloned = canEditCloned || !hasSaleCloned; const shouldEditWeekly = canEditWeekly || !isTicketWeekly; From 342832c4756ad6393b09d690be3b6793fb8facac Mon Sep 17 00:00:00 2001 From: alexm Date: Tue, 25 Oct 2022 15:02:22 +0200 Subject: [PATCH 09/14] try --- db/changes/10491-august/00-editTrackedACL.sql | 2 +- modules/ticket/back/methods/sale/canEdit.js | 29 ++++++++----------- package.json | 2 +- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/db/changes/10491-august/00-editTrackedACL.sql b/db/changes/10491-august/00-editTrackedACL.sql index 37d24ac814..97394fffe0 100644 --- a/db/changes/10491-august/00-editTrackedACL.sql +++ b/db/changes/10491-august/00-editTrackedACL.sql @@ -1,3 +1,3 @@ INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES - ('Sale', 'editTracked', 'READ', 'ALLOW', 'ROLE', 'production'); + ('Sale', 'editTracked', 'WRITE', 'ALLOW', 'ROLE', 'production'); diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index 4992a34990..7a8523fb7f 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -27,25 +27,20 @@ module.exports = Self => { if (typeof options == 'object') Object.assign(myOptions, options); - const firstSale = await models.Sale.findById(sales[0], null, myOptions); - const isTicketEditable = await models.Ticket.isEditable(ctx, firstSale.ticketFk, myOptions); - if (!isTicketEditable) - throw new UserError(`The sales of this ticket can't be modified`); + console.log(ctx.req.accessToken); + const token = ctx.req.accessToken; + let canEditTracked = await models.ACL.checkAccessForToken(token, models.Sale, null, 'refund'); + // const newCtx = ctx; + // newCtx.property = 'refund'; + // newCtx.accessType = 'WRITE'; + // newCtx.methodNames = ['refund']; + // newCtx.model = await models.Sale; - const saleTracking = await models.SaleTracking.find({where: {saleFk: {inq: sales}}}, myOptions); - const hasSaleTracking = saleTracking.length; - - const saleCloned = await models.SaleCloned.find({where: {saleClonedFk: {inq: sales}}}, myOptions); - const hasSaleCloned = saleCloned.length; - - const isTicketWeekly = - await models.TicketWeekly.findOne({where: {ticketFk: firstSale.ticketFk}}, myOptions); - - // (principalType, principalId,model, property, accessType,callback); - // let canEditTracked = await models.ACL.checkPermission('ROLE', 'employee', 'Sale', 'editTracked', 'WRITE'); + // let canEditTracked = await models.ACL.checkAccessForContext(newCtx); + console.log(canEditTracked); // let canEditTracked2 = await models.ACL.checkPermission('USER', 'developer', 'Sale', 'editTracked', 'READ'); - const array = ['editTracked']; + /* const array = ['editTracked']; const AccessContext = loopBackCtx.AccessContext; const toFind = { principals: [{ @@ -90,6 +85,6 @@ module.exports = Self => { if (canEdit) return true; - return false; + return false;*/ }; }; diff --git a/package.json b/package.json index 26c164832a..abf7c9e4b1 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "jsdom": "^16.7.0", "jszip": "^3.10.0", "ldapjs": "^2.2.0", - "loopback": "^3.26.0", + "loopback": "^3.28.0", "loopback-boot": "3.3.1", "loopback-component-explorer": "^6.5.0", "loopback-component-storage": "3.6.1", From b37c25788544f93806febaef0102dc37ab68a702 Mon Sep 17 00:00:00 2001 From: alexm Date: Mon, 31 Oct 2022 14:13:06 +0100 Subject: [PATCH 10/14] feat(canEdit): use checkAccess --- .../00-editTrackedACL.sql | 4 +- loopback/server/boot/acl.js | 22 ++ modules/ticket/back/methods/sale/canEdit.js | 80 ++-- .../back/methods/sale/specs/canEdit.spec.js | 366 +++++++++++------- .../back/methods/sale/specs/reserve.spec.js | 4 +- .../ticket/specs/componentUpdate.spec.js | 8 +- 6 files changed, 281 insertions(+), 203 deletions(-) rename db/changes/{10491-august => 10500-november}/00-editTrackedACL.sql (52%) create mode 100644 loopback/server/boot/acl.js diff --git a/db/changes/10491-august/00-editTrackedACL.sql b/db/changes/10500-november/00-editTrackedACL.sql similarity index 52% rename from db/changes/10491-august/00-editTrackedACL.sql rename to db/changes/10500-november/00-editTrackedACL.sql index 97394fffe0..c661694242 100644 --- a/db/changes/10491-august/00-editTrackedACL.sql +++ b/db/changes/10500-november/00-editTrackedACL.sql @@ -1,3 +1,5 @@ INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES - ('Sale', 'editTracked', 'WRITE', 'ALLOW', 'ROLE', 'production'); + ('Sale', 'editTracked', 'WRITE', 'ALLOW', 'ROLE', 'production'), + ('Sale', 'editFloramondo', 'WRITE', 'ALLOW', 'ROLE', 'salesAssistant'), + ('Ticket', 'editWeekly', 'WRITE', 'DENY', 'ROLE', '$authenticated'); diff --git a/loopback/server/boot/acl.js b/loopback/server/boot/acl.js new file mode 100644 index 0000000000..a230078336 --- /dev/null +++ b/loopback/server/boot/acl.js @@ -0,0 +1,22 @@ + +module.exports = function(app) { + app.models.ACL.checkAccess = async(ctx, modelId, property, accessType = '*') => { + const models = app.models; + const context = { + accessToken: ctx.req.accessToken, + model: models[modelId], + property: property, + modelId: modelId, + accessType: accessType, + sharedMethod: { + name: property, + aliases: [], + sharedClass: true + } + }; + + const acl = await models.ACL.checkAccessForContext(context); + + return acl.permission == 'ALLOW'; + }; +}; diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index 7a8523fb7f..afbf70638a 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -1,5 +1,4 @@ const UserError = require('vn-loopback/util/user-error'); -const loopBackCtx = require('vn-loopback/server/server'); module.exports = Self => { Self.remoteMethodCtx('canEdit', { @@ -7,7 +6,7 @@ module.exports = Self => { accessType: 'READ', accepts: [{ arg: 'sales', - type: ['object'], + type: ['number'], required: true }], returns: { @@ -16,7 +15,7 @@ module.exports = Self => { }, http: { path: `/canEdit`, - verb: 'get' + verb: 'GET' } }); @@ -27,64 +26,39 @@ module.exports = Self => { if (typeof options == 'object') Object.assign(myOptions, options); - console.log(ctx.req.accessToken); - const token = ctx.req.accessToken; - let canEditTracked = await models.ACL.checkAccessForToken(token, models.Sale, null, 'refund'); - // const newCtx = ctx; - // newCtx.property = 'refund'; - // newCtx.accessType = 'WRITE'; - // newCtx.methodNames = ['refund']; - // newCtx.model = await models.Sale; + const salesData = await models.Sale.find({ + fields: ['id', 'itemFk', 'ticketFk'], + where: {id: {inq: sales}}, + include: + { + relation: 'item', + scope: { + fields: ['id', 'isFloramondo'], + } + } + }, myOptions); - // let canEditTracked = await models.ACL.checkAccessForContext(newCtx); - console.log(canEditTracked); + const ticketId = salesData[0].ticketFk; - // let canEditTracked2 = await models.ACL.checkPermission('USER', 'developer', 'Sale', 'editTracked', 'READ'); - /* const array = ['editTracked']; - const AccessContext = loopBackCtx.AccessContext; - const toFind = { - principals: [{ - type: 'ROLE', - id: 'employee' - }], - model: 'Sale', - property: 'editTracked', - methodNames: ['editTracked'], - accessType: 'WRITE' - }; - const newContext = new AccessContext(toFind); - newContext.methodNames = ['editTracked']; + const isTicketEditable = await models.Ticket.isEditable(ctx, ticketId, myOptions); + if (!isTicketEditable) + throw new UserError(`The sales of this ticket can't be modified`); - let canEditTracked3 = await models.ACL.checkAccessForContext(newContext); + const hasSaleTracking = await models.SaleTracking.findOne({where: {saleFk: {inq: sales}}}, myOptions); + const hasSaleCloned = await models.SaleCloned.findOne({where: {saleClonedFk: {inq: sales}}}, myOptions); + const isTicketWeekly = await models.TicketWeekly.findOne({where: {ticketFk: ticketId}}, myOptions); + const hasSaleFloramondo = salesData.find(sale => sale.item().isFloramondo); - let canEditTracked4 = await models.ACL.checkAccessForContext({ - principals: [{ - type: 'ROLE', - id: 'developer' - }], - model: 'Sale', - property: 'editTracked', - methodName: 'editTracked', - methodNames: ['editTracked'], - accessType: 'WRITE' - }); - // console.log(canEditTracked); - // canEditTracked = await models.ACL.resolvePermission(canEditTracked); - // let canEditCloned = await models.ACL.checkPermission('ROLE', 'employee', 'Sale', 'editCloned', '*'); - // let canEditWeekly = await models.ACL.checkPermission('ROLE', 'employee', 'Ticket', 'editWeekly', '*'); + const canEditTracked = await models.ACL.checkAccess(ctx, 'Sale', 'editTracked'); + const canEditCloned = await models.ACL.checkAccess(ctx, 'Sale', 'editCloned'); + const canEditWeekly = await models.ACL.checkAccess(ctx, 'Ticket', 'editWeekly'); + const canEditFloramondo = await models.ACL.checkAccess(ctx, 'Sale', 'editFloramondo'); - // console.log(canEditTracked, canEditTracked2); - console.log('DENY: ', canEditTracked3.permission); - console.log('ALLOW: ', canEditTracked4.permission); const shouldEditTracked = canEditTracked || !hasSaleTracking; const shouldEditCloned = canEditCloned || !hasSaleCloned; const shouldEditWeekly = canEditWeekly || !isTicketWeekly; + const shouldEditFloramondo = canEditFloramondo || !hasSaleFloramondo; - const canEdit = shouldEditTracked && shouldEditCloned && shouldEditWeekly; - - if (canEdit) - return true; - - return false;*/ + return shouldEditTracked && shouldEditCloned && shouldEditWeekly && shouldEditFloramondo; }; }; diff --git a/modules/ticket/back/methods/sale/specs/canEdit.spec.js b/modules/ticket/back/methods/sale/specs/canEdit.spec.js index 1522ee7a31..4f66dcc874 100644 --- a/modules/ticket/back/methods/sale/specs/canEdit.spec.js +++ b/modules/ticket/back/methods/sale/specs/canEdit.spec.js @@ -1,179 +1,253 @@ const models = require('vn-loopback/server/server').models; describe('sale canEdit()', () => { - it('should return true if the role is production regardless of the saleTrackings', async() => { - const tx = await models.Sale.beginTransaction({}); + const employeeId = 1; - try { - const options = {transaction: tx}; + describe('sale editTracked', () => { + it('should return true if the role is production regardless of the saleTrackings', async() => { + const tx = await models.Sale.beginTransaction({}); - const productionUserID = 49; - const ctx = {req: {accessToken: {userId: productionUserID}}}; + try { + const options = {transaction: tx}; - const sales = [25]; + const productionUserID = 49; + const ctx = {req: {accessToken: {userId: productionUserID}}}; - const result = await models.Sale.canEdit(ctx, sales, options); + const sales = [25]; - expect(result).toEqual(true); + const result = await models.Sale.canEdit(ctx, sales, options); - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); + expect(result).toEqual(true); - it('should return true if the role is not production and none of the sales has saleTracking', async() => { - const tx = await models.Sale.beginTransaction({}); - - try { - const options = {transaction: tx}; - - const salesPersonUserID = 18; - const ctx = {req: {accessToken: {userId: salesPersonUserID}}}; - - const sales = [10]; - - const result = await models.Sale.canEdit(ctx, sales, options); - - expect(result).toEqual(true); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); - - it('should return false if any of the sales has a saleTracking record', async() => { - const tx = await models.Sale.beginTransaction({}); - - try { - const options = {transaction: tx}; - - const buyerId = 35; - const ctx = {req: {accessToken: {userId: buyerId}}}; - - const sales = [31]; - - const result = await models.Sale.canEdit(ctx, sales, options); - - expect(result).toEqual(false); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); - - it('should return false if any of the sales is cloned', async() => { - const tx = await models.Sale.beginTransaction({}); - - try { - const options = {transaction: tx}; - - const buyerId = 35; - const ctx = {req: {accessToken: {userId: buyerId}}}; - - const sales = [27]; - - const result = await models.Sale.canEdit(ctx, sales, options); - - expect(result).toEqual(false); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); - - it('should return true if any of the sales is cloned and has the correct role', async() => { - const tx = await models.Sale.beginTransaction({}); - const roleEnabled = await models.ACL.findOne({ - where: { - model: 'Sale', - property: 'editCloned' + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; } }); - if (!roleEnabled || !roleEnabled.principalId) return; - try { - const options = {transaction: tx}; + it('should return true if the role is not production and none of the sales has saleTracking', async() => { + const tx = await models.Sale.beginTransaction({}); - const roleId = await models.Role.findOne({ - where: { - name: roleEnabled.principalId - } - }); - const ctx = {req: {accessToken: {userId: roleId}}}; + try { + const options = {transaction: tx}; - const sales = [27]; + const salesPersonUserID = 18; + const ctx = {req: {accessToken: {userId: salesPersonUserID}}}; - const result = await models.Sale.canEdit(ctx, sales, options); + const sales = [10]; - expect(result).toEqual(true); + const result = await models.Sale.canEdit(ctx, sales, options); - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); + expect(result).toEqual(true); - it('should return false if any of the sales is of ticket weekly', async() => { - const tx = await models.Sale.beginTransaction({}); - - try { - const options = {transaction: tx}; - - const employeeId = 1; - const ctx = {req: {accessToken: {userId: employeeId}}}; - - const sales = [33]; - - const result = await models.Sale.canEdit(ctx, sales, options); - - expect(result).toEqual(false); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); - - it('should return true if any of the sales is of ticketWeekly and has the correct role', async() => { - const tx = await models.Sale.beginTransaction({}); - const roleEnabled = await models.ACL.findOne({ - where: { - model: 'Sale', - property: 'editWeekly' + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; } }); - if (!roleEnabled || !roleEnabled.principalId) return; - try { - const options = {transaction: tx}; + it('should return false if any of the sales has a saleTracking record', async() => { + const tx = await models.Sale.beginTransaction({}); - const roleId = await models.Role.findOne({ + try { + const options = {transaction: tx}; + + const buyerId = 35; + const ctx = {req: {accessToken: {userId: buyerId}}}; + + const sales = [31]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(false); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + }); + + describe('sale editCloned', () => { + it('should return false if any of the sales is cloned', async() => { + const tx = await models.Sale.beginTransaction({}); + + try { + const options = {transaction: tx}; + + const buyerId = 35; + const ctx = {req: {accessToken: {userId: buyerId}}}; + + const sales = [27]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(false); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should return true if any of the sales is cloned and has the correct role', async() => { + const tx = await models.Sale.beginTransaction({}); + const roleEnabled = await models.ACL.findOne({ where: { - name: roleEnabled.principalId + model: 'Sale', + property: 'editCloned', + permission: 'ALLOW' } }); - const ctx = {req: {accessToken: {userId: roleId}}}; + if (!roleEnabled || !roleEnabled.principalId) return await tx.rollback(); + try { + const options = {transaction: tx}; + + const role = await models.Role.findOne({ + where: { + name: roleEnabled.principalId + } + }); + const ctx = {req: {accessToken: {userId: role.id}}}; + + const sales = [27]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(true); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + }); + + describe('ticket editWeekly', () => { + it('should return false if any of the sales is of ticket weekly', async() => { + const tx = await models.Sale.beginTransaction({}); + + try { + const options = {transaction: tx}; + + const ctx = {req: {accessToken: {userId: employeeId}}}; + + const sales = [33]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(false); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should return true if any of the sales is of ticketWeekly and has the correct role', async() => { + const tx = await models.Sale.beginTransaction({}); + const roleEnabled = await models.ACL.findOne({ + where: { + model: 'Ticket', + property: 'editWeekly', + permission: 'ALLOW' + } + }); + + if (!roleEnabled || !roleEnabled.principalId) return await tx.rollback(); + try { + const options = {transaction: tx}; + + const role = await models.Role.findOne({ + where: { + name: roleEnabled.principalId + } + }); + const ctx = {req: {accessToken: {userId: role.id}}}; + + const sales = [33]; + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(true); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + }); + + describe('sale editFloramondo', () => { + it('should return false if any of the sales isFloramondo', async() => { + const tx = await models.Sale.beginTransaction({}); const sales = [33]; - const result = await models.Sale.canEdit(ctx, sales, options); + try { + const options = {transaction: tx}; - expect(result).toEqual(true); + const ctx = {req: {accessToken: {userId: employeeId}}}; - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } + // For test + const saleToEdit = await models.Sale.findById(sales[0], null, options); + await saleToEdit.updateAttribute('itemFk', 9, options); + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(false); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); + + it('should return true if any of the sales is of isFloramondo and has the correct role', async() => { + const tx = await models.Sale.beginTransaction({}); + const sales = [32]; + + const roleEnabled = await models.ACL.findOne({ + where: { + model: 'Sale', + property: 'editFloramondo', + permission: 'ALLOW' + } + }); + + if (!roleEnabled || !roleEnabled.principalId) return await tx.rollback(); + + try { + const options = {transaction: tx}; + + const role = await models.Role.findOne({ + where: { + name: roleEnabled.principalId + } + }); + const ctx = {req: {accessToken: {userId: role.id}}}; + + // For test + const saleToEdit = await models.Sale.findById(sales[0], null, options); + await saleToEdit.updateAttribute('itemFk', 9, options); + + const result = await models.Sale.canEdit(ctx, sales, options); + + expect(result).toEqual(true); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); }); }); diff --git a/modules/ticket/back/methods/sale/specs/reserve.spec.js b/modules/ticket/back/methods/sale/specs/reserve.spec.js index 7c2d437151..7ab79f9c0a 100644 --- a/modules/ticket/back/methods/sale/specs/reserve.spec.js +++ b/modules/ticket/back/methods/sale/specs/reserve.spec.js @@ -1,6 +1,6 @@ const models = require('vn-loopback/server/server').models; -fdescribe('sale reserve()', () => { +describe('sale reserve()', () => { const ctx = { req: { accessToken: {userId: 1}, @@ -31,7 +31,7 @@ fdescribe('sale reserve()', () => { expect(error).toEqual(new Error(`The sales of this ticket can't be modified`)); }); - fit('should update the given sales of a ticket to reserved', async() => { + it('should update the given sales of a ticket to reserved', async() => { const tx = await models.Sale.beginTransaction({}); try { diff --git a/modules/ticket/back/methods/ticket/specs/componentUpdate.spec.js b/modules/ticket/back/methods/ticket/specs/componentUpdate.spec.js index 2aa2a07c4c..49128ded8e 100644 --- a/modules/ticket/back/methods/ticket/specs/componentUpdate.spec.js +++ b/modules/ticket/back/methods/ticket/specs/componentUpdate.spec.js @@ -1,4 +1,5 @@ const models = require('vn-loopback/server/server').models; +const LoopBackContext = require('loopback-context'); describe('ticket componentUpdate()', () => { const userID = 1101; @@ -175,10 +176,15 @@ describe('ticket componentUpdate()', () => { } } }; + + spyOn(LoopBackContext, 'getCurrentContext').and.returnValue({ + active: ctx.req + }); + const oldTicket = await models.Ticket.findById(ticketID, null, options); + await models.Ticket.componentUpdate(ctx, options); const [newTicketID] = await models.Ticket.rawSql('SELECT MAX(id) as id FROM ticket', null, options); - const oldTicket = await models.Ticket.findById(ticketID, null, options); const newTicket = await models.Ticket.findById(newTicketID.id, null, options); const newTicketSale = await models.Sale.findOne({where: {ticketFk: args.id}}, options); From e7047e982f992b51889fc426e92562d0336012be Mon Sep 17 00:00:00 2001 From: alexm Date: Mon, 31 Oct 2022 15:07:22 +0100 Subject: [PATCH 11/14] fix e2e --- back/model-config.json | 3 --- back/models/funcionalityAcl.json | 24 --------------------- db/export-data.sh | 1 - loopback/server/boot/acl.js | 2 +- modules/ticket/back/methods/sale/canEdit.js | 8 +++---- 5 files changed, 5 insertions(+), 33 deletions(-) delete mode 100644 back/models/funcionalityAcl.json diff --git a/back/model-config.json b/back/model-config.json index dd35302a39..29676e979e 100644 --- a/back/model-config.json +++ b/back/model-config.json @@ -53,9 +53,6 @@ "EmailUser": { "dataSource": "vn" }, - "FuncionalityAcl": { - "dataSource": "vn" - }, "Image": { "dataSource": "vn" }, diff --git a/back/models/funcionalityAcl.json b/back/models/funcionalityAcl.json deleted file mode 100644 index 840c4f6f3a..0000000000 --- a/back/models/funcionalityAcl.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "name": "FuncionalityAcl", - "base": "VnModel", - "options": { - "mysql": { - "table": "salix.funcionalityAcl" - } - }, - "properties": { - "id": { - "type": "number", - "id": true - }, - "model": { - "type": "string" - }, - "property": { - "type": "string" - }, - "role": { - "type": "string" - } - } -} diff --git a/db/export-data.sh b/db/export-data.sh index 7252267ba8..bbbeb7152b 100755 --- a/db/export-data.sh +++ b/db/export-data.sh @@ -34,7 +34,6 @@ TABLES=( salix ACL fieldAcl - funcionalityAcl module defaultViewConfig ) diff --git a/loopback/server/boot/acl.js b/loopback/server/boot/acl.js index a230078336..f9375e0c1d 100644 --- a/loopback/server/boot/acl.js +++ b/loopback/server/boot/acl.js @@ -1,6 +1,6 @@ module.exports = function(app) { - app.models.ACL.checkAccess = async(ctx, modelId, property, accessType = '*') => { + app.models.ACL.checkAccessAcl = async(ctx, modelId, property, accessType = '*') => { const models = app.models; const context = { accessToken: ctx.req.accessToken, diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index afbf70638a..bd6a56200b 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -49,10 +49,10 @@ module.exports = Self => { const isTicketWeekly = await models.TicketWeekly.findOne({where: {ticketFk: ticketId}}, myOptions); const hasSaleFloramondo = salesData.find(sale => sale.item().isFloramondo); - const canEditTracked = await models.ACL.checkAccess(ctx, 'Sale', 'editTracked'); - const canEditCloned = await models.ACL.checkAccess(ctx, 'Sale', 'editCloned'); - const canEditWeekly = await models.ACL.checkAccess(ctx, 'Ticket', 'editWeekly'); - const canEditFloramondo = await models.ACL.checkAccess(ctx, 'Sale', 'editFloramondo'); + const canEditTracked = await models.ACL.checkAccessAcl(ctx, 'Sale', 'editTracked'); + const canEditCloned = await models.ACL.checkAccessAcl(ctx, 'Sale', 'editCloned'); + const canEditWeekly = await models.ACL.checkAccessAcl(ctx, 'Ticket', 'editWeekly'); + const canEditFloramondo = await models.ACL.checkAccessAcl(ctx, 'Sale', 'editFloramondo'); const shouldEditTracked = canEditTracked || !hasSaleTracking; const shouldEditCloned = canEditCloned || !hasSaleCloned; From 03ec8e27e6628101674bde5102b7fb83ddfbfcdf Mon Sep 17 00:00:00 2001 From: alexm Date: Mon, 7 Nov 2022 09:19:20 +0100 Subject: [PATCH 12/14] feat(ticket_isEditable): use ticketWeekly --- .../10500-november/00-editTrackedACL.sql | 3 +- db/dump/fixtures.sql | 8 +-- modules/ticket/back/methods/sale/canEdit.js | 5 +- .../back/methods/sale/specs/canEdit.spec.js | 70 ++----------------- .../ticket/back/methods/ticket/isEditable.js | 4 +- .../methods/ticket/specs/isEditable.spec.js | 19 +++++ 6 files changed, 33 insertions(+), 76 deletions(-) diff --git a/db/changes/10500-november/00-editTrackedACL.sql b/db/changes/10500-november/00-editTrackedACL.sql index c661694242..e768fb7c70 100644 --- a/db/changes/10500-november/00-editTrackedACL.sql +++ b/db/changes/10500-november/00-editTrackedACL.sql @@ -1,5 +1,4 @@ INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES ('Sale', 'editTracked', 'WRITE', 'ALLOW', 'ROLE', 'production'), - ('Sale', 'editFloramondo', 'WRITE', 'ALLOW', 'ROLE', 'salesAssistant'), - ('Ticket', 'editWeekly', 'WRITE', 'DENY', 'ROLE', '$authenticated'); + ('Sale', 'editFloramondo', 'WRITE', 'ALLOW', 'ROLE', 'salesAssistant'); diff --git a/db/dump/fixtures.sql b/db/dump/fixtures.sql index d5500360c6..5db3bba63a 100644 --- a/db/dump/fixtures.sql +++ b/db/dump/fixtures.sql @@ -984,7 +984,7 @@ INSERT INTO `vn`.`sale`(`id`, `itemFk`, `ticketFk`, `concept`, `quantity`, `pric (30, 4, 18, 'Melee weapon heavy shield 1x0.5m', 20, 1.72, 0, 0, 0, util.VN_CURDATE()), (31, 2, 23, 'Melee weapon combat fist 15cm', -5, 7.08, 0, 0, 0, util.VN_CURDATE()), (32, 1, 24, 'Ranged weapon longbow 2m', -1, 8.07, 0, 0, 0, util.VN_CURDATE()), - (33, 5, 14, 'Ranged weapon pistol 9mm', 50, 1.79, 0, 0, 0, util.VN_CURDATE()); + (33, 5, 14, 'Ranged weapon pistol 9mm', 50, 1.79, 0, 0, 0, util.VN_CURDATE()); INSERT INTO `vn`.`saleChecked`(`saleFk`, `isChecked`) VALUES @@ -1358,7 +1358,7 @@ INSERT INTO `vn`.`ticketWeekly`(`ticketFk`, `weekDay`) (3, 2), (4, 4), (5, 6), - (14, 6); + (15, 6); INSERT INTO `vn`.`travel`(`id`,`shipped`, `landed`, `warehouseInFk`, `warehouseOutFk`, `agencyModeFk`, `m3`, `kg`,`ref`, `totalEntries`, `cargoSupplierFk`) VALUES @@ -2712,7 +2712,7 @@ INSERT INTO `vn`.`ticketCollection` (`ticketFk`, `collectionFk`, `created`, `lev INSERT INTO `vn`.`saleCloned` (`saleClonedFk`, `saleOriginalFk`) VALUES - ('27', '25'); + (29, 25); UPDATE `account`.`user` SET `hasGrant` = 1 @@ -2720,4 +2720,4 @@ UPDATE `account`.`user` INSERT INTO `vn`.`osTicketConfig` (`id`, `host`, `user`, `password`, `oldStatus`, `newStatusId`, `day`, `comment`, `hostDb`, `userDb`, `passwordDb`, `portDb`, `responseType`, `fromEmailId`, `replyTo`) VALUES - (0, 'http://localhost:56596/scp', 'ostadmin', 'Admin1', 'open', 3, 60, 'Este CAU se ha cerrado automáticamente. Si el problema persiste responda a este mensaje.', 'localhost', 'osticket', 'osticket', 40003, 'reply', 1, 'all'); \ No newline at end of file + (0, 'http://localhost:56596/scp', 'ostadmin', 'Admin1', 'open', 3, 60, 'Este CAU se ha cerrado automáticamente. Si el problema persiste responda a este mensaje.', 'localhost', 'osticket', 'osticket', 40003, 'reply', 1, 'all'); diff --git a/modules/ticket/back/methods/sale/canEdit.js b/modules/ticket/back/methods/sale/canEdit.js index bd6a56200b..f44bd67438 100644 --- a/modules/ticket/back/methods/sale/canEdit.js +++ b/modules/ticket/back/methods/sale/canEdit.js @@ -46,19 +46,16 @@ module.exports = Self => { const hasSaleTracking = await models.SaleTracking.findOne({where: {saleFk: {inq: sales}}}, myOptions); const hasSaleCloned = await models.SaleCloned.findOne({where: {saleClonedFk: {inq: sales}}}, myOptions); - const isTicketWeekly = await models.TicketWeekly.findOne({where: {ticketFk: ticketId}}, myOptions); const hasSaleFloramondo = salesData.find(sale => sale.item().isFloramondo); const canEditTracked = await models.ACL.checkAccessAcl(ctx, 'Sale', 'editTracked'); const canEditCloned = await models.ACL.checkAccessAcl(ctx, 'Sale', 'editCloned'); - const canEditWeekly = await models.ACL.checkAccessAcl(ctx, 'Ticket', 'editWeekly'); const canEditFloramondo = await models.ACL.checkAccessAcl(ctx, 'Sale', 'editFloramondo'); const shouldEditTracked = canEditTracked || !hasSaleTracking; const shouldEditCloned = canEditCloned || !hasSaleCloned; - const shouldEditWeekly = canEditWeekly || !isTicketWeekly; const shouldEditFloramondo = canEditFloramondo || !hasSaleFloramondo; - return shouldEditTracked && shouldEditCloned && shouldEditWeekly && shouldEditFloramondo; + return shouldEditTracked && shouldEditCloned && shouldEditFloramondo; }; }; diff --git a/modules/ticket/back/methods/sale/specs/canEdit.spec.js b/modules/ticket/back/methods/sale/specs/canEdit.spec.js index 4f66dcc874..2aa873df56 100644 --- a/modules/ticket/back/methods/sale/specs/canEdit.spec.js +++ b/modules/ticket/back/methods/sale/specs/canEdit.spec.js @@ -72,6 +72,7 @@ describe('sale canEdit()', () => { }); describe('sale editCloned', () => { + const saleCloned = [29]; it('should return false if any of the sales is cloned', async() => { const tx = await models.Sale.beginTransaction({}); @@ -81,9 +82,7 @@ describe('sale canEdit()', () => { const buyerId = 35; const ctx = {req: {accessToken: {userId: buyerId}}}; - const sales = [27]; - - const result = await models.Sale.canEdit(ctx, sales, options); + const result = await models.Sale.canEdit(ctx, saleCloned, options); expect(result).toEqual(false); @@ -115,66 +114,7 @@ describe('sale canEdit()', () => { }); const ctx = {req: {accessToken: {userId: role.id}}}; - const sales = [27]; - - const result = await models.Sale.canEdit(ctx, sales, options); - - expect(result).toEqual(true); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); - }); - - describe('ticket editWeekly', () => { - it('should return false if any of the sales is of ticket weekly', async() => { - const tx = await models.Sale.beginTransaction({}); - - try { - const options = {transaction: tx}; - - const ctx = {req: {accessToken: {userId: employeeId}}}; - - const sales = [33]; - - const result = await models.Sale.canEdit(ctx, sales, options); - - expect(result).toEqual(false); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } - }); - - it('should return true if any of the sales is of ticketWeekly and has the correct role', async() => { - const tx = await models.Sale.beginTransaction({}); - const roleEnabled = await models.ACL.findOne({ - where: { - model: 'Ticket', - property: 'editWeekly', - permission: 'ALLOW' - } - }); - - if (!roleEnabled || !roleEnabled.principalId) return await tx.rollback(); - try { - const options = {transaction: tx}; - - const role = await models.Role.findOne({ - where: { - name: roleEnabled.principalId - } - }); - const ctx = {req: {accessToken: {userId: role.id}}}; - - const sales = [33]; - - const result = await models.Sale.canEdit(ctx, sales, options); + const result = await models.Sale.canEdit(ctx, saleCloned, options); expect(result).toEqual(true); @@ -189,7 +129,7 @@ describe('sale canEdit()', () => { describe('sale editFloramondo', () => { it('should return false if any of the sales isFloramondo', async() => { const tx = await models.Sale.beginTransaction({}); - const sales = [33]; + const sales = [26]; try { const options = {transaction: tx}; @@ -213,7 +153,7 @@ describe('sale canEdit()', () => { it('should return true if any of the sales is of isFloramondo and has the correct role', async() => { const tx = await models.Sale.beginTransaction({}); - const sales = [32]; + const sales = [26]; const roleEnabled = await models.ACL.findOne({ where: { diff --git a/modules/ticket/back/methods/ticket/isEditable.js b/modules/ticket/back/methods/ticket/isEditable.js index bdd18d7de4..d8fbb86ce8 100644 --- a/modules/ticket/back/methods/ticket/isEditable.js +++ b/modules/ticket/back/methods/ticket/isEditable.js @@ -44,13 +44,15 @@ module.exports = Self => { } }] }, myOptions); + const isLocked = await models.Ticket.isLocked(id, myOptions); + const isWeekly = await models.TicketWeekly.findOne({where: {ticketFk: id}}, myOptions); const alertLevelGreaterThanZero = (alertLevel && alertLevel > 0); const isNormalClient = ticket && ticket.client().type().code == 'normal'; const isEditable = !(alertLevelGreaterThanZero && isNormalClient); - if (ticket && (isEditable || isRoleAdvanced) && !isLocked) + if (ticket && (isEditable || isRoleAdvanced) && !isLocked && !isWeekly) return true; return false; diff --git a/modules/ticket/back/methods/ticket/specs/isEditable.spec.js b/modules/ticket/back/methods/ticket/specs/isEditable.spec.js index adc2acdee5..7337017d6a 100644 --- a/modules/ticket/back/methods/ticket/specs/isEditable.spec.js +++ b/modules/ticket/back/methods/ticket/specs/isEditable.spec.js @@ -134,4 +134,23 @@ describe('ticket isEditable()', () => { expect(result).toEqual(false); }); + + it('should not be able to edit if is a ticket weekly', async() => { + const tx = await models.Ticket.beginTransaction({}); + + try { + const options = {transaction: tx}; + + const ctx = {req: {accessToken: {userId: 1}}}; + + const result = await models.Ticket.isEditable(ctx, 15, options); + + expect(result).toEqual(false); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } + }); }); From 37d412bf3bbc254c00e03d7ced1f24d12f7134b8 Mon Sep 17 00:00:00 2001 From: alexm Date: Thu, 10 Nov 2022 14:38:37 +0100 Subject: [PATCH 13/14] refactor: delete unnecessary test --- .../00-editTrackedACL.sql | 0 .../05-ticket/02_expeditions_and_log.spec.js | 2 +- .../methods/sale/specs/updateQuantity.spec.js | 18 ------------------ .../ticket/back/methods/sale/updateQuantity.js | 3 --- 4 files changed, 1 insertion(+), 22 deletions(-) rename db/changes/{10500-november => 10502-november}/00-editTrackedACL.sql (100%) diff --git a/db/changes/10500-november/00-editTrackedACL.sql b/db/changes/10502-november/00-editTrackedACL.sql similarity index 100% rename from db/changes/10500-november/00-editTrackedACL.sql rename to db/changes/10502-november/00-editTrackedACL.sql diff --git a/e2e/paths/05-ticket/02_expeditions_and_log.spec.js b/e2e/paths/05-ticket/02_expeditions_and_log.spec.js index f970247e51..66df3e9e65 100644 --- a/e2e/paths/05-ticket/02_expeditions_and_log.spec.js +++ b/e2e/paths/05-ticket/02_expeditions_and_log.spec.js @@ -1,7 +1,7 @@ import selectors from '../../helpers/selectors.js'; import getBrowser from '../../helpers/puppeteer'; -describe('Ticket expeditions and log path', () => { +fdescribe('Ticket expeditions and log path', () => { let browser; let page; diff --git a/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js b/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js index bf139ab115..53a05cd7ee 100644 --- a/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js +++ b/modules/ticket/back/methods/sale/specs/updateQuantity.spec.js @@ -9,24 +9,6 @@ describe('sale updateQuantity()', () => { } }; - it('should throw an error if the quantity is not a number', async() => { - const tx = await models.Sale.beginTransaction({}); - - let error; - try { - const options = {transaction: tx}; - - await models.Sale.updateQuantity(ctx, 1, 'wrong quantity!', options); - - await tx.rollback(); - } catch (e) { - await tx.rollback(); - error = e; - } - - expect(error).toEqual(new Error('The value should be a number')); - }); - it('should throw an error if the quantity is greater than it should be', async() => { const ctx = { req: { diff --git a/modules/ticket/back/methods/sale/updateQuantity.js b/modules/ticket/back/methods/sale/updateQuantity.js index f3bc0ca380..8cf0720ce0 100644 --- a/modules/ticket/back/methods/sale/updateQuantity.js +++ b/modules/ticket/back/methods/sale/updateQuantity.js @@ -41,9 +41,6 @@ module.exports = Self => { } try { - if (isNaN(newQuantity)) - throw new UserError(`The value should be a number`); - const canEditSale = await models.Sale.canEdit(ctx, [id], myOptions); if (!canEditSale) throw new UserError(`Sale(s) blocked, please contact production`); From 72400ebf02a52f1846def667a5a2fad707706f5e Mon Sep 17 00:00:00 2001 From: alexm Date: Tue, 15 Nov 2022 09:37:14 +0100 Subject: [PATCH 14/14] change folder db --- .vscode/settings.json | 22 +++++++------------ .../00-editTrackedACL.sql | 0 db/changes/10503-november/delete.keep | 0 3 files changed, 8 insertions(+), 14 deletions(-) rename db/changes/{10502-november => 10503-november}/00-editTrackedACL.sql (100%) delete mode 100644 db/changes/10503-november/delete.keep diff --git a/.vscode/settings.json b/.vscode/settings.json index c27d01a765..b5da1e8e69 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,14 +1,8 @@ -// Coloque su configuración en este archivo para sobrescribir la configuración predeterminada y de usuario. -{ - // Carácter predeterminado de final de línea. - "files.eol": "\n", - "editor.bracketPairColorization.enabled": true, - "editor.guides.bracketPairs": true, - "editor.formatOnSave": true, - "editor.defaultFormatter": "dbaeumer.vscode-eslint", - "editor.codeActionsOnSave": ["source.fixAll.eslint"], - "eslint.validate": [ - "javascript", - "json" - ] -} \ No newline at end of file +// Coloque su configuración en este archivo para sobrescribir la configuración predeterminada y de usuario. +{ + // Carácter predeterminado de final de línea. + "files.eol": "\n", + "editor.codeActionsOnSave": { + "source.fixAll.eslint": true + } +} diff --git a/db/changes/10502-november/00-editTrackedACL.sql b/db/changes/10503-november/00-editTrackedACL.sql similarity index 100% rename from db/changes/10502-november/00-editTrackedACL.sql rename to db/changes/10503-november/00-editTrackedACL.sql diff --git a/db/changes/10503-november/delete.keep b/db/changes/10503-november/delete.keep deleted file mode 100644 index e69de29bb2..0000000000