From 32f8d133594d27d6930810ff3a349b0133cb1865 Mon Sep 17 00:00:00 2001 From: jorgep Date: Wed, 14 Feb 2024 13:15:31 +0100 Subject: [PATCH 1/2] fix: refs #6760 back modified & revoke update on ticket.clientFk --- .../vn/triggers/ticket_beforeUpdate.sql | 10 --- .../00-revokeUpdateClient.sql | 37 ++++++++++ .../ticket/specs/transferClient.spec.js | 71 ++++++++++-------- .../back/methods/ticket/transferClient.js | 73 ++++++++++++------- 4 files changed, 126 insertions(+), 65 deletions(-) create mode 100644 db/versions/10885-wheatHydrangea/00-revokeUpdateClient.sql diff --git a/db/routines/vn/triggers/ticket_beforeUpdate.sql b/db/routines/vn/triggers/ticket_beforeUpdate.sql index 0836b2486..72831bc3d 100644 --- a/db/routines/vn/triggers/ticket_beforeUpdate.sql +++ b/db/routines/vn/triggers/ticket_beforeUpdate.sql @@ -4,7 +4,6 @@ CREATE OR REPLACE DEFINER=`root`@`localhost` TRIGGER `vn`.`ticket_beforeUpdate` FOR EACH ROW BEGIN DECLARE vNewTime TIME; - DECLARE vHasTicketRefund BOOL; SET NEW.editorFk = account.myUser_getId(); @@ -64,14 +63,5 @@ BEGIN CALL vn.routeUpdateM3(NEW.routeFk); END IF; - - SELECT COUNT(*) INTO vHasTicketRefund - FROM ticketRefund - WHERE originalTicketFk = NEW.id - OR refundTicketFk = NEW.id; - - IF vHasTicketRefund AND NEW.clientFk <> OLD.clientFk THEN - CALL util.throw('The ticket has a refund associated'); - END IF; END$$ DELIMITER ; diff --git a/db/versions/10885-wheatHydrangea/00-revokeUpdateClient.sql b/db/versions/10885-wheatHydrangea/00-revokeUpdateClient.sql new file mode 100644 index 000000000..b22e09615 --- /dev/null +++ b/db/versions/10885-wheatHydrangea/00-revokeUpdateClient.sql @@ -0,0 +1,37 @@ +REVOKE UPDATE ON vn.ticket FROM employee; + +GRANT UPDATE (id, + warehouseFk, + shipped, + nickname, + refFk, + addressFk, + workerFk, + observations, + isSigned, + isLabeled, + isPrinted, + packages, + location, + hour, + created, + isBlocked, + solution, + routeFk, + priority, + hasPriority, + companyFk, + agencyModeFk, + landed, + isBoxed, + isDeleted, + zoneFk, + zonePrice, + zoneBonus, + totalWithVat, + totalWithoutVat, + weight, + clonedFrom, + cmrFk, + editorFk) + ON vn.ticket TO employee; diff --git a/modules/ticket/back/methods/ticket/specs/transferClient.spec.js b/modules/ticket/back/methods/ticket/specs/transferClient.spec.js index 5a9edd17e..5f1c09776 100644 --- a/modules/ticket/back/methods/ticket/specs/transferClient.spec.js +++ b/modules/ticket/back/methods/ticket/specs/transferClient.spec.js @@ -1,49 +1,60 @@ const models = require('vn-loopback/server/server').models; describe('Ticket transferClient()', () => { - const userId = 9; - const activeCtx = { - accessToken: {userId: userId}, - }; - const ctx = {req: activeCtx}; + const originalTicketId = 8; + const refundTicketId = 24; + const clientId = 1; + let ctx; + let options; + let tx; + beforeEach(async() => { + ctx = { + req: { + accessToken: {userId: 9}, + headers: {origin: 'http://localhost'} + }, + args: {} + }; + + options = {transaction: tx}; + tx = await models.Ticket.beginTransaction({}); + options.transaction = tx; + }); + + afterEach(async() => { + await tx.rollback(); + }); it('should throw an error as the ticket is not editable', async() => { - const tx = await models.Ticket.beginTransaction({}); - let error; - try { - const options = {transaction: tx}; const ticketId = 4; const clientId = 1; await models.Ticket.transferClient(ctx, ticketId, clientId, options); - - await tx.rollback(); } catch (e) { - await tx.rollback(); - error = e; + expect(e.message).toEqual(`This ticket is locked`); } - - expect(error.message).toEqual(`This ticket is locked`); }); - it('should be assigned a different clientFk', async() => { - const tx = await models.Ticket.beginTransaction({}); - let updatedTicket; - const ticketId = 10; - const clientId = 1; + it('should be assigned a different clientFk in the original ticket', async() => { + await models.Ticket.transferClient(ctx, 2, clientId, options); + const afterTransfer = await models.Ticket.findById(2, null, options); - try { - const options = {transaction: tx}; + expect(afterTransfer.clientFk).toEqual(clientId); + }); - await models.Ticket.transferClient(ctx, ticketId, clientId, options); - updatedTicket = await models.Ticket.findById(ticketId, {fields: ['clientFk']}, options); + it('should be assigned a different clientFk in the original and refund ticket and claim', async() => { + await models.Ticket.transferClient(ctx, originalTicketId, clientId, options); - await tx.rollback(); - } catch (e) { - await tx.rollback(); - throw e; - } + const [originalTicket, refundTicket] = await models.Ticket.find({ + where: {id: {inq: [originalTicketId, refundTicketId]}} + }, options); - expect(updatedTicket.clientFk).toEqual(clientId); + const claim = await models.Claim.findOne({ + where: {ticketFk: originalTicketId} + }, options); + + expect(originalTicket.clientFk).toEqual(clientId); + expect(refundTicket.clientFk).toEqual(clientId); + expect(claim.clientFk).toEqual(clientId); }); }); diff --git a/modules/ticket/back/methods/ticket/transferClient.js b/modules/ticket/back/methods/ticket/transferClient.js index 60e70d710..1db49b274 100644 --- a/modules/ticket/back/methods/ticket/transferClient.js +++ b/modules/ticket/back/methods/ticket/transferClient.js @@ -2,20 +2,17 @@ module.exports = Self => { Self.remoteMethodCtx('transferClient', { description: 'Transfering ticket to another client', accessType: 'WRITE', - accepts: [ - { - arg: 'id', - type: 'number', - required: true, - description: 'the ticket id', - http: {source: 'path'} - }, - { - arg: 'clientFk', - type: 'number', - required: true, - }, - ], + accepts: [{ + arg: 'id', + type: 'number', + required: true, + description: 'the ticket id', + http: {source: 'path'} + }, { + arg: 'clientFk', + type: 'number', + required: true, + }], http: { path: `/:id/transferClient`, verb: 'PATCH' @@ -25,21 +22,47 @@ module.exports = Self => { Self.transferClient = async(ctx, id, clientFk, options) => { const models = Self.app.models; const myOptions = {}; + const tickets = []; + let tx; + if (typeof options == 'object') Object.assign(myOptions, options); - await Self.isEditableOrThrow(ctx, id, myOptions); + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } - const ticket = await models.Ticket.findById( - id, - {fields: ['id', 'shipped', 'clientFk', 'addressFk']}, - myOptions - ); - const client = await models.Client.findById(clientFk, {fields: ['id', 'defaultAddressFk']}, myOptions); + try { + await Self.isEditableOrThrow(ctx, id, myOptions); - await ticket.updateAttributes({ - clientFk, - addressFk: client.defaultAddressFk, - }); + const ticketRelation = await models.TicketRefund.findOne({ + where: {or: [{originalTicketFk: id}, {refundTicketFk: id}]}, + include: [{relation: 'refundTicket'}, {relation: 'originalTicket'}] + }, myOptions); + + const {defaultAddressFk: addressFk} = await models.Client.findById(clientFk, + {fields: ['id', 'defaultAddressFk']}, myOptions); + + const attributes = {clientFk, addressFk}; + + if (ticketRelation) { + const {refundTicket, originalTicket} = ticketRelation; + tickets.push(refundTicket(), originalTicket()); + + for (const ticket of tickets) + await ticket.updateAttributes(attributes, myOptions); + } else + await Self.updateAll({id}, attributes, myOptions); + + const ticketIds = tickets.length ? tickets.map(ticket => ticket.id) : [id]; + + await models.Claim.updateAll({ticketFk: {inq: ticketIds}}, {clientFk}, myOptions); + + if (tx) await tx.commit(); + } catch (e) { + if (tx) await tx.rollback(); + throw e; + } }; }; From 78583751ec227a10305271afa07f1d4c4f03274f Mon Sep 17 00:00:00 2001 From: jorgep Date: Thu, 15 Feb 2024 16:22:13 +0100 Subject: [PATCH 2/2] refactor: refs #6760 ticketsIds improved --- .../back/methods/ticket/transferClient.js | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/modules/ticket/back/methods/ticket/transferClient.js b/modules/ticket/back/methods/ticket/transferClient.js index 1db49b274..d38c0e8a7 100644 --- a/modules/ticket/back/methods/ticket/transferClient.js +++ b/modules/ticket/back/methods/ticket/transferClient.js @@ -22,7 +22,6 @@ module.exports = Self => { Self.transferClient = async(ctx, id, clientFk, options) => { const models = Self.app.models; const myOptions = {}; - const tickets = []; let tx; if (typeof options == 'object') @@ -36,7 +35,7 @@ module.exports = Self => { try { await Self.isEditableOrThrow(ctx, id, myOptions); - const ticketRelation = await models.TicketRefund.findOne({ + const ticketRefund = await models.TicketRefund.findOne({ where: {or: [{originalTicketFk: id}, {refundTicketFk: id}]}, include: [{relation: 'refundTicket'}, {relation: 'originalTicket'}] }, myOptions); @@ -46,16 +45,21 @@ module.exports = Self => { const attributes = {clientFk, addressFk}; - if (ticketRelation) { - const {refundTicket, originalTicket} = ticketRelation; + const tickets = []; + const ticketIds = []; + + if (ticketRefund) { + const {refundTicket, originalTicket} = ticketRefund; tickets.push(refundTicket(), originalTicket()); - for (const ticket of tickets) + for (const ticket of tickets) { await ticket.updateAttributes(attributes, myOptions); - } else + ticketIds.push(ticket.id); + } + } else { await Self.updateAll({id}, attributes, myOptions); - - const ticketIds = tickets.length ? tickets.map(ticket => ticket.id) : [id]; + ticketIds.push(id); + } await models.Claim.updateAll({ticketFk: {inq: ticketIds}}, {clientFk}, myOptions);