From 4561b6eeef6b180d32c7e8ec438f4288ad2a89a7 Mon Sep 17 00:00:00 2001 From: Bernat Exposito Domenech Date: Thu, 27 Feb 2020 12:31:38 +0100 Subject: [PATCH 1/3] refactor ticket.isEditable --- .../client/specs/isValidClient.spec.js | 2 +- .../ticket-request/specs/confirm.spec.js | 2 +- .../ticket/back/methods/ticket/isEditable.js | 10 +++--- .../ticket/back/methods/ticket/isLocked.js | 35 +++++++++++++++++++ .../methods/ticket/specs/isEditable.spec.js | 34 +++++++++--------- .../methods/ticket/specs/isLocked.spec.js | 34 ++++++++++++++++++ .../back/methods/ticket/updateDiscount.js | 15 ++++++-- modules/ticket/back/models/ticket.js | 1 + modules/ticket/front/sale/index.html | 4 +-- modules/ticket/front/sale/index.js | 9 ++++- modules/ticket/front/sale/specs/index.spec.js | 13 +++++++ 11 files changed, 131 insertions(+), 28 deletions(-) create mode 100644 modules/ticket/back/methods/ticket/isLocked.js create mode 100644 modules/ticket/back/methods/ticket/specs/isLocked.spec.js diff --git a/modules/client/back/methods/client/specs/isValidClient.spec.js b/modules/client/back/methods/client/specs/isValidClient.spec.js index 446392374..71d7473f1 100644 --- a/modules/client/back/methods/client/specs/isValidClient.spec.js +++ b/modules/client/back/methods/client/specs/isValidClient.spec.js @@ -16,7 +16,7 @@ describe('Client isValidClient', () => { }); it('should call the isValidClient() method with an unexistant id and receive false', async() => { - let id = 999999; + let id = 999; let result = await app.models.Client.isValidClient(id); expect(result).toBeFalsy(); diff --git a/modules/ticket/back/methods/ticket-request/specs/confirm.spec.js b/modules/ticket/back/methods/ticket-request/specs/confirm.spec.js index 6cce70b9c..134ccca40 100644 --- a/modules/ticket/back/methods/ticket-request/specs/confirm.spec.js +++ b/modules/ticket/back/methods/ticket-request/specs/confirm.spec.js @@ -34,7 +34,7 @@ describe('ticket-request confirm()', () => { expect(error.message).toEqual(`That item doesn't exists`); }); - it(`should throw an error if the item is not available`, async() => { + it('should throw an error if the item is not available', async() => { const requestId = 5; const itemId = 4; const quantity = 99999; diff --git a/modules/ticket/back/methods/ticket/isEditable.js b/modules/ticket/back/methods/ticket/isEditable.js index 3ebf15bf0..317cfac96 100644 --- a/modules/ticket/back/methods/ticket/isEditable.js +++ b/modules/ticket/back/methods/ticket/isEditable.js @@ -32,7 +32,7 @@ module.exports = Self => { let alertLevel = state ? state.alertLevel : null; let ticket = await Self.app.models.Ticket.findById(id, { - fields: ['isDeleted', 'clientFk', 'refFk'], + fields: ['clientFk'], include: [{ relation: 'client', scope: { @@ -42,13 +42,13 @@ module.exports = Self => { } }] }); + const isLocked = await Self.app.models.Ticket.isLocked(id); - const isDeleted = ticket && ticket.isDeleted; - const isOnDelivery = (alertLevel && alertLevel > 0); + const alertLevelGreaterThanZero = (alertLevel && alertLevel > 0); const isNormalClient = ticket && ticket.client().type().code == 'normal'; - const isInvoiced = ticket && ticket.refFk; + const validAlertAndRoleNormalClient = (alertLevelGreaterThanZero && isNormalClient && !isValidRole); - if (!ticket || isInvoiced || isDeleted || (isOnDelivery && isNormalClient && !isValidRole)) + if (!ticket || validAlertAndRoleNormalClient || isLocked) return false; return true; diff --git a/modules/ticket/back/methods/ticket/isLocked.js b/modules/ticket/back/methods/ticket/isLocked.js new file mode 100644 index 000000000..7cf7b807e --- /dev/null +++ b/modules/ticket/back/methods/ticket/isLocked.js @@ -0,0 +1,35 @@ +module.exports = Self => { + Self.remoteMethod('isLocked', { + description: 'Check if a ticket is invoiced or deleted', + accessType: 'READ', + accepts: [{ + arg: 'id', + type: 'number', + required: true, + description: 'the ticket id', + http: {source: 'path'} + }], + returns: { + type: 'boolean', + root: true + }, + http: { + path: `/:id/isLocked`, + verb: 'get' + } + }); + + Self.isLocked = async id => { + const ticket = await Self.app.models.Ticket.findById(id, { + fields: ['isDeleted', 'refFk'] + }); + + const isDeleted = ticket && ticket.isDeleted; + const isInvoiced = ticket && ticket.refFk; + + if (!ticket || isInvoiced || isDeleted) + 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 a40128954..419e5c3b1 100644 --- a/modules/ticket/back/methods/ticket/specs/isEditable.spec.js +++ b/modules/ticket/back/methods/ticket/specs/isEditable.spec.js @@ -1,13 +1,6 @@ const app = require('vn-loopback/server/server'); describe('ticket isEditable()', () => { - it('should return false if the given ticket is not editable', async() => { - let ctx = {req: {accessToken: {userId: 9}}}; - let result = await app.models.Ticket.isEditable(ctx, 2); - - expect(result).toEqual(false); - }); - it('should return false if the given ticket does not exist', async() => { let ctx = {req: {accessToken: {userId: 9}}}; let result = await app.models.Ticket.isEditable(ctx, 99999); @@ -15,37 +8,46 @@ describe('ticket isEditable()', () => { expect(result).toEqual(false); }); - it('should return false if the given ticket isDeleted', async() => { + it(`should return false if the given ticket isn't invoiced but isDeleted`, async() => { let ctx = {req: {accessToken: {userId: 9}}}; - let result = await app.models.Ticket.isEditable(ctx, 19); + let deletedTicket = await app.models.Ticket.findOne({ + where: { + invoiceOut: null, + isDeleted: true + }, + fields: ['id'] + }); + + let result = await app.models.Ticket.isEditable(ctx, deletedTicket.id); expect(result).toEqual(false); }); it('should return true if the given ticket is editable', async() => { let ctx = {req: {accessToken: {userId: 9}}}; + let result = await app.models.Ticket.isEditable(ctx, 16); expect(result).toEqual(true); }); - it('should be able to edit a deleted or invoiced ticket if the role is salesAssistant', async() => { + it('should not be able to edit a deleted or invoiced ticket if the role is salesAssistantº', async() => { let ctx = {req: {accessToken: {userId: 21}}}; - let result = await app.models.Ticket.isEditable(ctx, 8); + let result = await app.models.Ticket.isEditable(ctx, 19); - expect(result).toEqual(true); + expect(result).toEqual(false); }); - it('should be able to edit a deleted or invoiced ticket if the role is productionBoss', async() => { + it('should not be able to edit a deleted or invoiced ticket if the role is productionBoss', async() => { let ctx = {req: {accessToken: {userId: 50}}}; - let result = await app.models.Ticket.isEditable(ctx, 8); + let result = await app.models.Ticket.isEditable(ctx, 19); - expect(result).toEqual(true); + expect(result).toEqual(false); }); it('should not be able to edit a deleted or invoiced ticket if the role is salesPerson', async() => { let ctx = {req: {accessToken: {userId: 18}}}; - let result = await app.models.Ticket.isEditable(ctx, 8); + let result = await app.models.Ticket.isEditable(ctx, 19); expect(result).toEqual(false); }); diff --git a/modules/ticket/back/methods/ticket/specs/isLocked.spec.js b/modules/ticket/back/methods/ticket/specs/isLocked.spec.js new file mode 100644 index 000000000..192c80f10 --- /dev/null +++ b/modules/ticket/back/methods/ticket/specs/isLocked.spec.js @@ -0,0 +1,34 @@ +const app = require('vn-loopback/server/server'); + +describe('ticket isLocked()', () => { + it('should return true if the given ticket does not exist', async() => { + let result = await app.models.Ticket.isLocked(99999); + + expect(result).toEqual(true); + }); + + it('should return true if the given ticket is invoiced', async() => { + let invoicedTicket = await app.models.Ticket.findOne({ + where: {invoiceOut: {neq: null}}, + fields: ['id'] + }); + + let result = await app.models.Ticket.isLocked(invoicedTicket.id); + + expect(result).toEqual(true); + }); + + it(`should return true if the given ticket isn't invoiced but deleted`, async() => { + let deletedTicket = await app.models.Ticket.findOne({ + where: { + invoiceOut: null, + isDeleted: true + }, + fields: ['id'] + }); + + let result = await app.models.Ticket.isLocked(deletedTicket.id); + + expect(result).toEqual(true); + }); +}); diff --git a/modules/ticket/back/methods/ticket/updateDiscount.js b/modules/ticket/back/methods/ticket/updateDiscount.js index 8777a60fc..ddcb787c2 100644 --- a/modules/ticket/back/methods/ticket/updateDiscount.js +++ b/modules/ticket/back/methods/ticket/updateDiscount.js @@ -35,6 +35,7 @@ module.exports = Self => { }); Self.updateDiscount = async(ctx, id, salesIds, newDiscount) => { + const userId = ctx.req.accessToken.userId; const models = Self.app.models; const tx = await Self.beginTransaction({}); @@ -68,8 +69,18 @@ module.exports = Self => { if (!allFromSameTicket) throw new UserError('All sales must belong to the same ticket'); - const isEditable = await models.Ticket.isEditable(ctx, id); - if (!isEditable) + // const isEditable = await models.Ticket.isEditable(ctx, id); + // if (!isEditable) + // throw new UserError(`The sales of this ticket can't be modified`); + + const isLocked = await models.Ticket.isLocked(id); + const isSalesPerson = await models.Account.hasRole(userId, 'salesPerson'); + const state = await Self.app.models.TicketState.findOne({ + where: {ticketFk: id} + }); + const alertLevel = state ? state.alertLevel : null; + + if (isLocked || (!isSalesPerson && alertLevel > 0 )) throw new UserError(`The sales of this ticket can't be modified`); const ticket = await models.Ticket.findById(id, { diff --git a/modules/ticket/back/models/ticket.js b/modules/ticket/back/models/ticket.js index 45284d60d..a2891430a 100644 --- a/modules/ticket/back/models/ticket.js +++ b/modules/ticket/back/models/ticket.js @@ -29,6 +29,7 @@ module.exports = Self => { require('../methods/ticket/recalculateComponents')(Self); require('../methods/ticket/deleteStowaway')(Self); require('../methods/ticket/sendSms')(Self); + require('../methods/ticket/isLocked')(Self); Self.observe('before save', async function(ctx) { if (ctx.isNewInstance) return; diff --git a/modules/ticket/front/sale/index.html b/modules/ticket/front/sale/index.html index 224392deb..41ef74c38 100644 --- a/modules/ticket/front/sale/index.html +++ b/modules/ticket/front/sale/index.html @@ -165,8 +165,8 @@ - {{(sale.discount / 100) | percentage}} diff --git a/modules/ticket/front/sale/index.js b/modules/ticket/front/sale/index.js index 1ecb6fe41..7fc75d13d 100644 --- a/modules/ticket/front/sale/index.js +++ b/modules/ticket/front/sale/index.js @@ -46,6 +46,7 @@ class Controller { set ticket(value) { this._ticket = value; this.isTicketEditable(); + this.isTicketLocked(); } get sales() { @@ -354,7 +355,7 @@ class Controller { } showEditDiscountPopover(event, sale) { - if (!this.isEditable) return; + if (this.isLocked) return; this.sale = sale; this.edit = [{ @@ -540,6 +541,12 @@ class Controller { }); } + isTicketLocked() { + this.$http.get(`Tickets/${this.$state.params.id}/isLocked`).then(res => { + this.isLocked = res.data; + }); + } + hasOneSaleSelected() { if (this.totalCheckedLines() === 1) return true; diff --git a/modules/ticket/front/sale/specs/index.spec.js b/modules/ticket/front/sale/specs/index.spec.js index a23c6f204..7edaff9a3 100644 --- a/modules/ticket/front/sale/specs/index.spec.js +++ b/modules/ticket/front/sale/specs/index.spec.js @@ -69,6 +69,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); $httpBackend.when('POST', `Claims/createFromSales`, {claim: claim, sales: sales}).respond(claim); $httpBackend.expect('POST', `Claims/createFromSales`).respond(claim); controller.createClaim(); @@ -98,6 +99,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); let result = controller.checkedLines(); $httpBackend.flush(); @@ -116,6 +118,7 @@ describe('Ticket', () => { $httpBackend.expectGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.expectGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.onStateOkClick(); $httpBackend.flush(); @@ -129,6 +132,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.onStateChange(3); $httpBackend.flush(); }); @@ -142,6 +146,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.onRemoveLinesClick('accept'); $httpBackend.flush(); @@ -183,6 +188,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.unmarkAsReserved(false); $httpBackend.flush(); }); @@ -213,6 +219,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.updateQuantity(sale); $httpBackend.flush(); @@ -232,6 +239,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.updateConcept(sale); $httpBackend.flush(); @@ -262,6 +270,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.addSale(newSale); $httpBackend.flush(); @@ -287,6 +296,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.transferSales(13); $httpBackend.flush(); @@ -305,6 +315,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.setTransferParams(); $httpBackend.flush(); @@ -330,6 +341,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.newOrderFromTicket(); $httpBackend.flush(); @@ -353,6 +365,7 @@ describe('Ticket', () => { $httpBackend.whenGET(`Tickets/1/subtotal`).respond(200, 227.5); $httpBackend.whenGET(`Tickets/1/getVAT`).respond(200, 10.5); $httpBackend.whenGET(`Tickets/1/isEditable`).respond(); + $httpBackend.whenGET(`Tickets/1/isLocked`).respond(); controller.calculateSalePrice(); $httpBackend.flush(); From 63cd436d3cf5fac992c63a90c61477bd25a96090 Mon Sep 17 00:00:00 2001 From: Bernat Exposito Domenech Date: Mon, 2 Mar 2020 08:30:33 +0100 Subject: [PATCH 2/3] fix request changes --- modules/ticket/back/methods/ticket/specs/isEditable.spec.js | 2 +- modules/ticket/back/methods/ticket/updateDiscount.js | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/modules/ticket/back/methods/ticket/specs/isEditable.spec.js b/modules/ticket/back/methods/ticket/specs/isEditable.spec.js index 419e5c3b1..e423af9f0 100644 --- a/modules/ticket/back/methods/ticket/specs/isEditable.spec.js +++ b/modules/ticket/back/methods/ticket/specs/isEditable.spec.js @@ -31,7 +31,7 @@ describe('ticket isEditable()', () => { expect(result).toEqual(true); }); - it('should not be able to edit a deleted or invoiced ticket if the role is salesAssistantº', async() => { + it('should not be able to edit a deleted or invoiced ticket if the role is salesAssistant', async() => { let ctx = {req: {accessToken: {userId: 21}}}; let result = await app.models.Ticket.isEditable(ctx, 19); diff --git a/modules/ticket/back/methods/ticket/updateDiscount.js b/modules/ticket/back/methods/ticket/updateDiscount.js index ddcb787c2..5ec887836 100644 --- a/modules/ticket/back/methods/ticket/updateDiscount.js +++ b/modules/ticket/back/methods/ticket/updateDiscount.js @@ -69,10 +69,6 @@ module.exports = Self => { if (!allFromSameTicket) throw new UserError('All sales must belong to the same ticket'); - // const isEditable = await models.Ticket.isEditable(ctx, id); - // if (!isEditable) - // throw new UserError(`The sales of this ticket can't be modified`); - const isLocked = await models.Ticket.isLocked(id); const isSalesPerson = await models.Account.hasRole(userId, 'salesPerson'); const state = await Self.app.models.TicketState.findOne({ From 42b151c8493d5dbc9195835f9fcf85693a56c7e4 Mon Sep 17 00:00:00 2001 From: Bernat Exposito Domenech Date: Mon, 2 Mar 2020 09:43:22 +0100 Subject: [PATCH 3/3] fix isEditable spec --- modules/ticket/back/methods/ticket/specs/isEditable.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/ticket/back/methods/ticket/specs/isEditable.spec.js b/modules/ticket/back/methods/ticket/specs/isEditable.spec.js index e423af9f0..276aeacf1 100644 --- a/modules/ticket/back/methods/ticket/specs/isEditable.spec.js +++ b/modules/ticket/back/methods/ticket/specs/isEditable.spec.js @@ -31,21 +31,21 @@ describe('ticket isEditable()', () => { expect(result).toEqual(true); }); - it('should not be able to edit a deleted or invoiced ticket if the role is salesAssistant', async() => { + it('should not be able to edit a deleted or invoiced ticket even for salesAssistant', async() => { let ctx = {req: {accessToken: {userId: 21}}}; let result = await app.models.Ticket.isEditable(ctx, 19); expect(result).toEqual(false); }); - it('should not be able to edit a deleted or invoiced ticket if the role is productionBoss', async() => { + it('should not be able to edit a deleted or invoiced ticket even for productionBoss', async() => { let ctx = {req: {accessToken: {userId: 50}}}; let result = await app.models.Ticket.isEditable(ctx, 19); expect(result).toEqual(false); }); - it('should not be able to edit a deleted or invoiced ticket if the role is salesPerson', async() => { + it('should not be able to edit a deleted or invoiced ticket even for salesPerson', async() => { let ctx = {req: {accessToken: {userId: 18}}}; let result = await app.models.Ticket.isEditable(ctx, 19);