From ee79f231851fa7e998dbe8a2e96c223286a72010 Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:14:23 +0200 Subject: [PATCH 1/9] createFromSales refactor + tests --- .../back/methods/claim/createFromSales.js | 26 +++++++---- .../claim/specs/createFromSales.spec.js | 43 +++++++++++-------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/modules/claim/back/methods/claim/createFromSales.js b/modules/claim/back/methods/claim/createFromSales.js index 989b87c957..32d1edd97e 100644 --- a/modules/claim/back/methods/claim/createFromSales.js +++ b/modules/claim/back/methods/claim/createFromSales.js @@ -24,15 +24,22 @@ module.exports = Self => { } }); - Self.createFromSales = async(ctx, ticketId, sales) => { + Self.createFromSales = async(ctx, ticketId, sales, options) => { + let tx; + let myOptions = {}; + + if (typeof options == 'object') + Object.assign(myOptions, options); + + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } const models = Self.app.models; const userId = ctx.req.accessToken.userId; - const tx = await Self.beginTransaction({}); try { - let options = {transaction: tx}; - - const ticket = await models.Ticket.findById(ticketId, null, options); + const ticket = await models.Ticket.findById(ticketId, null, myOptions); if (ticket.isDeleted) throw new UserError(`You can't create a claim for a removed ticket`); @@ -41,7 +48,7 @@ module.exports = Self => { clientFk: ticket.clientFk, ticketCreated: ticket.shipped, workerFk: userId - }, options); + }, myOptions); const promises = []; for (const sale of sales) { @@ -49,17 +56,18 @@ module.exports = Self => { saleFk: sale.id, claimFk: newClaim.id, quantity: sale.quantity - }, options); + }, myOptions); promises.push(newClaimBeginning); } await Promise.all(promises); - await tx.commit(); + if (tx) await tx.commit(); + return newClaim; } catch (e) { - await tx.rollback(); + if (tx) await tx.rollback(); throw e; } }; diff --git a/modules/claim/back/methods/claim/specs/createFromSales.spec.js b/modules/claim/back/methods/claim/specs/createFromSales.spec.js index e211143f94..f089140252 100644 --- a/modules/claim/back/methods/claim/specs/createFromSales.spec.js +++ b/modules/claim/back/methods/claim/specs/createFromSales.spec.js @@ -10,36 +10,45 @@ describe('Claim createFromSales()', () => { const ctx = {req: {accessToken: {userId: 1}}}; it('should create a new claim', async() => { - let claim = await app.models.Claim.createFromSales(ctx, ticketId, newSale); + const tx = await app.models.Claim.beginTransaction({}); - expect(claim.ticketFk).toEqual(ticketId); + try { + const options = {transaction: tx}; - let claimBeginning = await app.models.ClaimBeginning.findOne({where: {claimFk: claim.id}}); + const claim = await app.models.Claim.createFromSales(ctx, ticketId, newSale, options); - expect(claimBeginning.saleFk).toEqual(newSale[0].id); - expect(claimBeginning.quantity).toEqual(newSale[0].quantity); + expect(claim.ticketFk).toEqual(ticketId); - const createdClaimId = claim.id; + let claimBeginning = await app.models.ClaimBeginning.findOne({where: {claimFk: claim.id}}, options); - // restores - await app.models.Claim.destroyById(createdClaimId); + expect(claimBeginning.saleFk).toEqual(newSale[0].id); + expect(claimBeginning.quantity).toEqual(newSale[0].quantity); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); it('should not be able to create a claim if exists that sale', async() => { - let claim = await app.models.Claim.createFromSales(ctx, ticketId, newSale); - const createdClaimId = claim.id; + const tx = await app.models.Claim.beginTransaction({}); let error; - await app.models.Claim.createFromSales(ctx, ticketId, newSale) + try { + const options = {transaction: tx}; - .catch(e => { - error = e; - }); + await app.models.Claim.createFromSales(ctx, ticketId, newSale, options); + + await app.models.Claim.createFromSales(ctx, ticketId, newSale, options); + + await tx.rollback(); + } catch (e) { + error = e; + await tx.rollback(); + } expect(error.toString()).toContain(`A claim with that sale already exists`); - - // restores - await app.models.Claim.destroyById(createdClaimId); }); }); From c4cbc064c99d51da4f2fede060be9abb2424e3d6 Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:15:58 +0200 Subject: [PATCH 2/9] regularice claim refactor + test --- .../back/methods/claim/regularizeClaim.js | 39 +++--- .../claim/specs/regularizeClaim.spec.js | 127 ++++++++++-------- 2 files changed, 93 insertions(+), 73 deletions(-) diff --git a/modules/claim/back/methods/claim/regularizeClaim.js b/modules/claim/back/methods/claim/regularizeClaim.js index e8f9fe3b79..99d9055b22 100644 --- a/modules/claim/back/methods/claim/regularizeClaim.js +++ b/modules/claim/back/methods/claim/regularizeClaim.js @@ -18,32 +18,39 @@ module.exports = Self => { } }); - Self.regularizeClaim = async(ctx, claimFk) => { + Self.regularizeClaim = async(ctx, claimFk, options) => { const models = Self.app.models; const $t = ctx.req.__; // $translate const resolvedState = 3; - let tx = await Self.beginTransaction({}); - try { - let options = {transaction: tx}; + let tx; + let myOptions = {}; + if (typeof options == 'object') + Object.assign(myOptions, options); + + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } + + try { const claimEnds = await models.ClaimEnd.find({ include: { relation: 'claimDestination', fields: ['addressFk'] }, where: {claimFk: claimFk} - }, options); + }, myOptions); - for (let i = 0; i < claimEnds.length; i++) { - const claimEnd = claimEnds[i]; + for (let claimEnd of claimEnds) { const destination = claimEnd.claimDestination(); - const sale = await getSale(claimEnd.saleFk, options); + const sale = await getSale(claimEnd.saleFk, myOptions); const addressId = destination && destination.addressFk; let address; if (addressId) - address = await models.Address.findById(addressId, null, options); + address = await models.Address.findById(addressId, null, myOptions); const salesPerson = sale.ticket().client().salesPersonUser(); if (salesPerson) { @@ -67,7 +74,7 @@ module.exports = Self => { addressFk: addressId, companyFk: sale.ticket().companyFk, warehouseFk: sale.ticket().warehouseFk - }, options); + }, myOptions); if (!ticketFk) { ticketFk = await createTicket(ctx, { @@ -75,7 +82,7 @@ module.exports = Self => { warehouseId: sale.ticket().warehouseFk, companyId: sale.ticket().companyFk, addressId: addressId - }, options); + }, myOptions); } await models.Sale.create({ @@ -85,19 +92,19 @@ module.exports = Self => { quantity: -sale.quantity, price: sale.price, discount: 100 - }, options); + }, myOptions); } - let claim = await Self.findById(claimFk, null, options); + let claim = await Self.findById(claimFk, null, myOptions); claim = await claim.updateAttributes({ claimStateFk: resolvedState - }, options); + }, myOptions); - await tx.commit(); + if (tx) await tx.commit(); return claim; } catch (e) { - await tx.rollback(); + if (tx) await tx.rollback(); throw e; } }; diff --git a/modules/claim/back/methods/claim/specs/regularizeClaim.spec.js b/modules/claim/back/methods/claim/specs/regularizeClaim.spec.js index 0a280f5ac4..5430019226 100644 --- a/modules/claim/back/methods/claim/specs/regularizeClaim.spec.js +++ b/modules/claim/back/methods/claim/specs/regularizeClaim.spec.js @@ -21,81 +21,94 @@ describe('regularizeClaim()', () => { let claimEnds = []; let trashTicket; - afterEach(async done => { + it('should send a chat message with value "Trash" and then change claim state to resolved', async() => { + const tx = await app.models.Claim.beginTransaction({}); + try { - let claim = await app.models.Claim.findById(claimFk); - await claim.updateAttributes({ - claimStateFk: pendentState, - hasToPickUp: false - }); + const options = {transaction: tx}; + + spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); + + claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { + claimFk: claimFk, + ticketFk: 1 + }, options); for (claimEnd of claimEnds) - await claimEnd.destroy(); + await claimEnd.updateAttributes({claimDestinationFk: trashDestination}, options); - if (trashTicket) - await app.models.Ticket.destroyById(trashTicket.id); - } catch (error) { - console.error(error); + let claimBefore = await app.models.Claim.findById(claimFk, null, options); + await app.models.Claim.regularizeClaim(ctx, claimFk, options); + let claimAfter = await app.models.Claim.findById(claimFk, null, options); + + trashTicket = await app.models.Ticket.findOne({where: {addressFk: 12}}, options); + + expect(trashTicket.addressFk).toEqual(trashAddress); + expect(claimBefore.claimStateFk).toEqual(pendentState); + expect(claimAfter.claimStateFk).toEqual(resolvedState); + expect(chatModel.sendCheckingPresence).toHaveBeenCalledWith(ctx, 18, 'Trash'); + expect(chatModel.sendCheckingPresence).toHaveBeenCalledTimes(4); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; } - - done(); - }); - - it('should send a chat message with value "Trash" and then change claim state to resolved', async() => { - spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); - - claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { - claimFk: claimFk, - ticketFk: 1 - }); - - for (claimEnd of claimEnds) - await claimEnd.updateAttributes({claimDestinationFk: trashDestination}); - - let claimBefore = await app.models.Claim.findById(claimFk); - await app.models.Claim.regularizeClaim(ctx, claimFk); - let claimAfter = await app.models.Claim.findById(claimFk); - - trashTicket = await app.models.Ticket.findOne({where: {addressFk: 12}}); - - expect(trashTicket.addressFk).toEqual(trashAddress); - expect(claimBefore.claimStateFk).toEqual(pendentState); - expect(claimAfter.claimStateFk).toEqual(resolvedState); - expect(chatModel.sendCheckingPresence).toHaveBeenCalledWith(ctx, 18, 'Trash'); - expect(chatModel.sendCheckingPresence).toHaveBeenCalledTimes(4); }); it('should send a chat message with value "Bueno" and then change claim state to resolved', async() => { - spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); + const tx = await app.models.Claim.beginTransaction({}); - claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { - claimFk: claimFk, - ticketFk: 1 - }); + try { + const options = {transaction: tx}; - for (claimEnd of claimEnds) - await claimEnd.updateAttributes({claimDestinationFk: okDestination}); + spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); - await app.models.Claim.regularizeClaim(ctx, claimFk); + claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { + claimFk: claimFk, + ticketFk: 1 + }, options); - expect(chatModel.sendCheckingPresence).toHaveBeenCalledWith(ctx, 18, 'Bueno'); - expect(chatModel.sendCheckingPresence).toHaveBeenCalledTimes(4); + for (claimEnd of claimEnds) + await claimEnd.updateAttributes({claimDestinationFk: okDestination}, options); + + await app.models.Claim.regularizeClaim(ctx, claimFk, options); + + expect(chatModel.sendCheckingPresence).toHaveBeenCalledWith(ctx, 18, 'Bueno'); + expect(chatModel.sendCheckingPresence).toHaveBeenCalledTimes(4); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); it('should send a chat message to the salesPerson when claim isPickUp is enabled', async() => { - spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); + const tx = await app.models.Claim.beginTransaction({}); - claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { - claimFk: claimFk, - ticketFk: 1 - }); + try { + const options = {transaction: tx}; - for (claimEnd of claimEnds) - await claimEnd.updateAttributes({claimDestinationFk: okDestination}); + spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); - await app.models.Claim.regularizeClaim(ctx, claimFk); + claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { + claimFk: claimFk, + ticketFk: 1 + }, options); - expect(chatModel.sendCheckingPresence).toHaveBeenCalledWith(ctx, 18, 'Bueno'); - expect(chatModel.sendCheckingPresence).toHaveBeenCalledTimes(4); + for (claimEnd of claimEnds) + await claimEnd.updateAttributes({claimDestinationFk: okDestination}, options); + + await app.models.Claim.regularizeClaim(ctx, claimFk, options); + + expect(chatModel.sendCheckingPresence).toHaveBeenCalledWith(ctx, 18, 'Bueno'); + expect(chatModel.sendCheckingPresence).toHaveBeenCalledTimes(4); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); }); From 3054cbcadb505e87be2d5f4410eed2979ad251c8 Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:17:10 +0200 Subject: [PATCH 3/9] updateClaim refactor + tests --- .../methods/claim/specs/updateClaim.spec.js | 159 ++++++++++-------- .../claim/back/methods/claim/updateClaim.js | 101 ++++++----- 2 files changed, 152 insertions(+), 108 deletions(-) diff --git a/modules/claim/back/methods/claim/specs/updateClaim.spec.js b/modules/claim/back/methods/claim/specs/updateClaim.spec.js index 7ce6a4d3c7..a5de92ccff 100644 --- a/modules/claim/back/methods/claim/specs/updateClaim.spec.js +++ b/modules/claim/back/methods/claim/specs/updateClaim.spec.js @@ -1,7 +1,7 @@ const app = require('vn-loopback/server/server'); describe('Update Claim', () => { - let newDate = new Date(); + const newDate = new Date(); const originalData = { ticketFk: 3, clientFk: 101, @@ -14,89 +14,114 @@ describe('Update Claim', () => { }; it(`should throw an error as the user doesn't have rights`, async() => { - let newClaim = await app.models.Claim.create(originalData); - const forbiddenState = 3; - const salesPersonId = 18; - const ctx = { - req: { - accessToken: { - userId: salesPersonId + const tx = await app.models.Claim.beginTransaction({}); + + let error; + + try { + const options = {transaction: tx}; + + const newClaim = await app.models.Claim.create(originalData, options); + + const forbiddenState = 3; + const salesPersonId = 18; + const ctx = { + req: { + accessToken: { + userId: salesPersonId + } + }, + args: { + claimStateFk: forbiddenState, + observation: 'valid observation' } - }, - args: { - claimStateFk: forbiddenState, - observation: 'valid observation' - } - }; - await app.models.Claim.updateClaim(ctx, newClaim.id) - .catch(e => { - error = e; - }); + }; + await app.models.Claim.updateClaim(ctx, newClaim.id, options); + + await tx.rollback(); + } catch (e) { + error = e; + await tx.rollback(); + } expect(error.message).toEqual(`You don't have enough privileges to change that field`); - - // restores - await app.models.Claim.destroyById(newClaim.id); }); it(`should success to update the claim within privileges `, async() => { - let newClaim = await app.models.Claim.create(originalData); + const tx = await app.models.Claim.beginTransaction({}); - const canceledState = 4; - const claimManagerId = 72; - const ctx = { - req: { - accessToken: { - userId: claimManagerId + try { + const options = {transaction: tx}; + + const newClaim = await app.models.Claim.create(originalData, options); + + const canceledState = 4; + const claimManagerId = 72; + const ctx = { + req: { + accessToken: { + userId: claimManagerId + } + }, + args: { + observation: 'valid observation', + claimStateFk: canceledState, + hasToPickUp: false } - }, - args: { - observation: 'valid observation', - claimStateFk: canceledState, - hasToPickUp: false - } - }; - await app.models.Claim.updateClaim(ctx, newClaim.id); + }; + await app.models.Claim.updateClaim(ctx, newClaim.id, options); - let updatedClaim = await app.models.Claim.findById(newClaim.id); + let updatedClaim = await app.models.Claim.findById(newClaim.id, null, options); - expect(updatedClaim.observation).toEqual(ctx.args.observation); + expect(updatedClaim.observation).toEqual(ctx.args.observation); - // restores - await app.models.Claim.destroyById(newClaim.id); + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); it('should change some sensible fields as claimManager', async() => { - let newClaim = await app.models.Claim.create(originalData); - const chatModel = app.models.Chat; - spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); + const tx = await app.models.Claim.beginTransaction({}); - const claimManagerId = 72; - const ctx = { - req: { - accessToken: {userId: claimManagerId}, - headers: {origin: 'http://localhost'} - }, - args: { - claimStateFk: 3, - workerFk: 5, - observation: 'another valid observation', - hasToPickUp: true - } - }; - ctx.req.__ = (value, params) => { - return params.nickname; - }; - await app.models.Claim.updateClaim(ctx, newClaim.id); + try { + const options = {transaction: tx}; - let updatedClaim = await app.models.Claim.findById(newClaim.id); + const newClaim = await app.models.Claim.create(originalData, options); - expect(updatedClaim.observation).toEqual(ctx.args.observation); - expect(updatedClaim.claimStateFk).toEqual(ctx.args.claimStateFk); - expect(updatedClaim.workerFk).toEqual(ctx.args.workerFk); - expect(chatModel.sendCheckingPresence).toHaveBeenCalled(); + const chatModel = app.models.Chat; + spyOn(chatModel, 'sendCheckingPresence').and.callThrough(); - // restores - await app.models.Claim.destroyById(newClaim.id); + const claimManagerId = 72; + const ctx = { + req: { + accessToken: {userId: claimManagerId}, + headers: {origin: 'http://localhost'} + }, + args: { + claimStateFk: 3, + workerFk: 5, + observation: 'another valid observation', + hasToPickUp: true + } + }; + ctx.req.__ = (value, params) => { + return params.nickname; + }; + await app.models.Claim.updateClaim(ctx, newClaim.id, options); + + let updatedClaim = await app.models.Claim.findById(newClaim.id, null, options); + + expect(updatedClaim.observation).toEqual(ctx.args.observation); + expect(updatedClaim.claimStateFk).toEqual(ctx.args.claimStateFk); + expect(updatedClaim.workerFk).toEqual(ctx.args.workerFk); + expect(chatModel.sendCheckingPresence).toHaveBeenCalled(); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); }); diff --git a/modules/claim/back/methods/claim/updateClaim.js b/modules/claim/back/methods/claim/updateClaim.js index 6c16b57374..02515ed668 100644 --- a/modules/claim/back/methods/claim/updateClaim.js +++ b/modules/claim/back/methods/claim/updateClaim.js @@ -4,25 +4,26 @@ module.exports = Self => { description: 'Update a claim with privileges', accepts: [{ arg: 'ctx', - type: 'Object', + type: 'object', http: {source: 'context'} - }, { + }, + { arg: 'id', - type: 'Number', + type: 'number', description: 'Claim id', http: {source: 'path'} }, { arg: 'workerFk', - type: 'Number' + type: 'number' }, { arg: 'claimStateFk', - type: 'Number' + type: 'number' }, { arg: 'observation', - type: 'String' + type: 'string' }, { arg: 'hasToPickUp', @@ -38,51 +39,69 @@ module.exports = Self => { } }); - Self.updateClaim = async(ctx, id) => { + Self.updateClaim = async(ctx, id, options) => { const models = Self.app.models; const userId = ctx.req.accessToken.userId; const args = ctx.args; const $t = ctx.req.__; // $translate - const claim = await models.Claim.findById(id, { - include: { - relation: 'client', - scope: { - include: { - relation: 'salesPersonUser' + let tx; + let myOptions = {}; + + if (typeof options == 'object') + Object.assign(myOptions, options); + + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } + + try { + const claim = await models.Claim.findById(id, { + include: { + relation: 'client', + scope: { + include: { + relation: 'salesPersonUser' + } } } + }, myOptions); + let changedHasToPickUp = false; + if (args.hasToPickUp) + changedHasToPickUp = true; + + if (args.claimStateFk) { + const canUpdate = await canChangeState(ctx, claim.claimStateFk, myOptions); + const hasRights = await canChangeState(ctx, args.claimStateFk, myOptions); + const isClaimManager = await models.Account.hasRole(userId, 'claimManager', myOptions); + + if (!canUpdate || !hasRights || changedHasToPickUp && !isClaimManager) + throw new UserError(`You don't have enough privileges to change that field`); + } + delete args.ctx; + const updatedClaim = await claim.updateAttributes(args, myOptions); + // Get sales person from claim client + const salesPerson = claim.client().salesPersonUser(); + if (salesPerson && changedHasToPickUp && updatedClaim.hasToPickUp) { + const origin = ctx.req.headers.origin; + const message = $t('Claim will be picked', { + claimId: claim.id, + clientName: claim.client().name, + claimUrl: `${origin}/#!/claim/${claim.id}/summary` + }); + await models.Chat.sendCheckingPresence(ctx, salesPerson.id, message); } - }); - let changedHasToPickUp = false; - if (args.hasToPickUp) - changedHasToPickUp = true; - if (args.claimStateFk) { - const canUpdate = await canChangeState(ctx, claim.claimStateFk); - const hasRights = await canChangeState(ctx, args.claimStateFk); - const isClaimManager = await models.Account.hasRole(userId, 'claimManager'); + if (tx) await tx.commit(); - if (!canUpdate || !hasRights || changedHasToPickUp && !isClaimManager) - throw new UserError(`You don't have enough privileges to change that field`); + return updatedClaim; + } catch (e) { + if (tx) await tx.rollback(); + throw e; } - delete args.ctx; - const updatedClaim = await claim.updateAttributes(args); - // Get sales person from claim client - const salesPerson = claim.client().salesPersonUser(); - if (salesPerson && changedHasToPickUp && updatedClaim.hasToPickUp) { - const origin = ctx.req.headers.origin; - const message = $t('Claim will be picked', { - claimId: claim.id, - clientName: claim.client().name, - claimUrl: `${origin}/#!/claim/${claim.id}/summary` - }); - await models.Chat.sendCheckingPresence(ctx, salesPerson.id, message); - } - - return updatedClaim; }; - async function canChangeState(ctx, id) { + async function canChangeState(ctx, id, options) { let models = Self.app.models; let userId = ctx.req.accessToken.userId; @@ -90,9 +109,9 @@ module.exports = Self => { include: { relation: 'writeRole' } - }); + }, options); let stateRole = state.writeRole().name; - let canUpdate = await models.Account.hasRole(userId, stateRole); + let canUpdate = await models.Account.hasRole(userId, stateRole, options); return canUpdate; } From 8d8bdacd8616cf7b089b82a7d98e6226358b1c41 Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:18:14 +0200 Subject: [PATCH 4/9] updateClaimAction refactor + tests --- .../claim/specs/updateClaimAction.spec.js | 56 +++++++++++-------- .../back/methods/claim/updateClaimAction.js | 32 +++++++++-- 2 files changed, 60 insertions(+), 28 deletions(-) diff --git a/modules/claim/back/methods/claim/specs/updateClaimAction.spec.js b/modules/claim/back/methods/claim/specs/updateClaimAction.spec.js index 4848974fef..8cdcea4dab 100644 --- a/modules/claim/back/methods/claim/specs/updateClaimAction.spec.js +++ b/modules/claim/back/methods/claim/specs/updateClaimAction.spec.js @@ -1,9 +1,8 @@ const app = require('vn-loopback/server/server'); describe('Update Claim', () => { - let newDate = new Date(); - let newInstance; - let original = { + const newDate = new Date(); + const original = { ticketFk: 3, clientFk: 101, ticketCreated: newDate, @@ -14,30 +13,43 @@ describe('Update Claim', () => { observation: 'observation' }; - beforeAll(async done => { - newInstance = await app.models.Claim.create(original); - - done(); - }); - - afterAll(async done => { - await app.models.Claim.destroyById(newInstance.id); - done(); - }); - it('should update the claim isChargedToMana attribute', async() => { - const ctx = {args: {isChargedToMana: false}}; - const result = await app.models.Claim.updateClaimAction(ctx, newInstance.id); + const tx = await app.models.Claim.beginTransaction({}); - expect(result.id).toEqual(newInstance.id); - expect(result.isChargedToMana).toBeFalsy(); + try { + const options = {transaction: tx}; + const ctx = {args: {isChargedToMana: false}}; + + const newInstance = await app.models.Claim.create(original, options); + const result = await app.models.Claim.updateClaimAction(ctx, newInstance.id, options); + + expect(result.id).toEqual(newInstance.id); + expect(result.isChargedToMana).toBeFalsy(); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); it('should update the claim responsibility attribute', async() => { - const ctx = {args: {responsibility: 2}}; - const result = await app.models.Claim.updateClaimAction(ctx, newInstance.id); + const tx = await app.models.Claim.beginTransaction({}); - expect(result.id).toEqual(newInstance.id); - expect(result.responsibility).toEqual(2); + try { + const options = {transaction: tx}; + const ctx = {args: {responsibility: 2}}; + + const newInstance = await app.models.Claim.create(original, options); + const result = await app.models.Claim.updateClaimAction(ctx, newInstance.id, options); + + expect(result.id).toEqual(newInstance.id); + expect(result.responsibility).toEqual(2); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); }); diff --git a/modules/claim/back/methods/claim/updateClaimAction.js b/modules/claim/back/methods/claim/updateClaimAction.js index d11e2ed080..12749fb6ba 100644 --- a/modules/claim/back/methods/claim/updateClaimAction.js +++ b/modules/claim/back/methods/claim/updateClaimAction.js @@ -28,12 +28,32 @@ module.exports = Self => { } }); - Self.updateClaimAction = async(ctx, id) => { - const models = Self.app.models; - const claim = await models.Claim.findById(id); - const args = ctx.args; - delete args.ctx; + Self.updateClaimAction = async(ctx, id, options) => { + let tx; + let myOptions = {}; - return await claim.updateAttributes(args); + if (typeof options == 'object') + Object.assign(myOptions, options); + + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } + + try { + const models = Self.app.models; + const claim = await models.Claim.findById(id, null, myOptions); + const args = ctx.args; + delete args.ctx; + + const updatedClaim = await claim.updateAttributes(args, myOptions); + + if (tx) await tx.commit(); + + return updatedClaim; + } catch (e) { + if (tx) await tx.rollback(); + throw e; + } }; }; From de2a17e84094a759f5a737609f67b111e0b2e3d7 Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:18:49 +0200 Subject: [PATCH 5/9] removeFile and filter refactors --- back/methods/dms/removeFile.js | 40 +++++++++++++----- modules/claim/back/methods/claim/filter.js | 49 +++++++++++++--------- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/back/methods/dms/removeFile.js b/back/methods/dms/removeFile.js index 93fae9728b..1c50137a0f 100644 --- a/back/methods/dms/removeFile.js +++ b/back/methods/dms/removeFile.js @@ -20,17 +20,37 @@ module.exports = Self => { } }); - Self.removeFile = async(ctx, id) => { - const models = Self.app.models; - const dms = await models.Dms.findById(id); - const trashDmsType = await models.DmsType.findOne({ - where: {code: 'trash'} - }); + Self.removeFile = async(ctx, id, options) => { + let tx; + let myOptions = {}; - const hasWriteRole = await models.DmsType.hasWriteRole(ctx, dms.dmsTypeFk); - if (!hasWriteRole) - throw new UserError(`You don't have enough privileges`); + if (typeof options == 'object') + Object.assign(myOptions, options); - return dms.updateAttribute('dmsTypeFk', trashDmsType.id); + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } + + try { + const models = Self.app.models; + const dms = await models.Dms.findById(id, null, myOptions); + const trashDmsType = await models.DmsType.findOne({ + where: {code: 'trash'} + }, myOptions); + + const hasWriteRole = await models.DmsType.hasWriteRole(ctx, dms.dmsTypeFk, myOptions); + if (!hasWriteRole) + throw new UserError(`You don't have enough privileges`); + + await dms.updateAttribute('dmsTypeFk', trashDmsType.id, myOptions); + + if (tx) await tx.commit(); + + return dms; + } catch (e) { + if (tx) await tx.rollback(); + throw e; + } }; }; diff --git a/modules/claim/back/methods/claim/filter.js b/modules/claim/back/methods/claim/filter.js index 7ba89089a8..50d20410bd 100644 --- a/modules/claim/back/methods/claim/filter.js +++ b/modules/claim/back/methods/claim/filter.js @@ -10,58 +10,67 @@ module.exports = Self => { accepts: [ { arg: 'filter', - type: 'Object', + type: 'object', description: 'Filter defining where, order, offset, and limit - must be a JSON-encoded string', http: {source: 'query'} - }, { + }, + { arg: 'tags', - type: ['Object'], + type: ['object'], description: 'List of tags to filter with', http: {source: 'query'} - }, { + }, + { arg: 'search', - type: 'String', + type: 'string', description: `If it's and integer searchs by id, otherwise it searchs by client name`, http: {source: 'query'} - }, { + }, + { arg: 'client', - type: 'String', + type: 'string', description: 'The worker name', http: {source: 'query'} - }, { + }, + { arg: 'id', - type: 'Integer', + type: 'integer', description: 'The claim id', http: {source: 'query'} - }, { + }, + { arg: 'clientFk', - type: 'Integer', + type: 'integer', description: 'The client id', http: {source: 'query'} - }, { + }, + { arg: 'claimStateFk', - type: 'Integer', + type: 'integer', description: 'The claim state id', http: {source: 'query'} - }, { + }, + { arg: 'salesPersonFk', - type: 'Integer', + type: 'integer', description: 'The salesPerson id', http: {source: 'query'} - }, { + }, + { arg: 'attenderFk', - type: 'Integer', + type: 'integer', description: 'The attender worker id', http: {source: 'query'} - }, { + }, + { arg: 'created', - type: 'Date', + type: 'date', description: 'The to date filter', http: {source: 'query'} } ], returns: { - type: ['Object'], + type: ['object'], root: true }, http: { From 740cc79bc0f2d76a6defa0f705f957e1f06733af Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:19:43 +0200 Subject: [PATCH 6/9] uploadFile refactor --- .../claim/back/methods/claim/uploadFile.js | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/modules/claim/back/methods/claim/uploadFile.js b/modules/claim/back/methods/claim/uploadFile.js index c7565701da..23d0c02732 100644 --- a/modules/claim/back/methods/claim/uploadFile.js +++ b/modules/claim/back/methods/claim/uploadFile.js @@ -4,40 +4,46 @@ module.exports = Self => { accessType: 'WRITE', accepts: [{ arg: 'id', - type: 'Number', + type: 'number', description: 'The claim id', http: {source: 'path'} - }, { + }, + { arg: 'warehouseId', - type: 'Number', + type: 'number', description: 'The warehouse id', required: true - }, { + }, + { arg: 'companyId', - type: 'Number', + type: 'number', description: 'The company id', required: true - }, { + }, + { arg: 'dmsTypeId', - type: 'Number', + type: 'number', description: 'The dms type id', required: true - }, { + }, + { arg: 'reference', - type: 'String', + type: 'string', required: true - }, { + }, + { arg: 'description', - type: 'String', + type: 'string', required: true - }, { + }, + { arg: 'hasFile', - type: 'Boolean', + type: 'boolean', description: 'True if has an attached file', required: true }], returns: { - type: 'Object', + type: 'object', root: true }, http: { @@ -46,20 +52,27 @@ module.exports = Self => { } }); - Self.uploadFile = async(ctx, id) => { + Self.uploadFile = async(ctx, id, options) => { + let tx; + let myOptions = {}; + + if (typeof options == 'object') + Object.assign(myOptions, options); + + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } const models = Self.app.models; const promises = []; - const tx = await Self.beginTransaction({}); try { - const options = {transaction: tx}; - - const uploadedFiles = await models.Dms.uploadFile(ctx, options); + const uploadedFiles = await models.Dms.uploadFile(ctx, myOptions); uploadedFiles.forEach(dms => { const newClaimDms = models.ClaimDms.create({ claimFk: id, dmsFk: dms.id - }, options); + }, myOptions); promises.push(newClaimDms); }); From c1f457c2637a8f4a1e8dba4876ca0f7a9c118291 Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:42:14 +0200 Subject: [PATCH 7/9] allowContentTypes and removeFile refactors --- .../methods/claim-dms/allowedContentTypes.js | 2 +- .../back/methods/claim-dms/removeFile.js | 40 ++++++++++++++----- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/modules/claim/back/methods/claim-dms/allowedContentTypes.js b/modules/claim/back/methods/claim-dms/allowedContentTypes.js index 3d4b908767..58b7a9ba34 100644 --- a/modules/claim/back/methods/claim-dms/allowedContentTypes.js +++ b/modules/claim/back/methods/claim-dms/allowedContentTypes.js @@ -3,7 +3,7 @@ module.exports = Self => { description: 'Returns a list of allowed contentTypes', accessType: 'READ', returns: { - type: ['Object'], + type: ['object'], root: true }, http: { diff --git a/modules/claim/back/methods/claim-dms/removeFile.js b/modules/claim/back/methods/claim-dms/removeFile.js index ac546455a4..310d2b941e 100644 --- a/modules/claim/back/methods/claim-dms/removeFile.js +++ b/modules/claim/back/methods/claim-dms/removeFile.js @@ -4,12 +4,12 @@ module.exports = Self => { accessType: 'WRITE', accepts: { arg: 'id', - type: 'Number', + type: 'number', description: 'The document id', http: {source: 'path'} }, returns: { - type: 'Object', + type: 'object', root: true }, http: { @@ -18,16 +18,36 @@ module.exports = Self => { } }); - Self.removeFile = async(ctx, id) => { - const models = Self.app.models; - const targetClaimDms = await models.ClaimDms.findById(id); - const targetDms = await models.Dms.findById(targetClaimDms.dmsFk); - const trashDmsType = await models.DmsType.findOne({where: {code: 'trash'}}); + Self.removeFile = async(ctx, id, options) => { + let tx; + let myOptions = {}; - await models.Dms.removeFile(ctx, targetClaimDms.dmsFk); - await targetClaimDms.destroy(); + if (typeof options == 'object') + Object.assign(myOptions, options); - return targetDms.updateAttribute('dmsTypeFk', trashDmsType.id); + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } + + try { + const models = Self.app.models; + const targetClaimDms = await models.ClaimDms.findById(id, null, myOptions); + const targetDms = await models.Dms.findById(targetClaimDms.dmsFk, null, myOptions); + const trashDmsType = await models.DmsType.findOne({where: {code: 'trash'}}, myOptions); + + await models.Dms.removeFile(ctx, targetClaimDms.dmsFk, myOptions); + await targetClaimDms.destroy(myOptions); + + await targetDms.updateAttribute('dmsTypeFk', trashDmsType.id, myOptions); + + if (tx) await tx.commit(); + + return targetDms; + } catch (e) { + if (tx) await tx.rollback(); + throw e; + } }; }; From 87974445531166c19110ea884d6f3031a6d73c9b Mon Sep 17 00:00:00 2001 From: carlosjr Date: Mon, 29 Mar 2021 15:42:29 +0200 Subject: [PATCH 8/9] importTicketSales refactor + tests --- .../methods/claim-end/importTicketSales.js | 47 ++++++++++++++----- .../claim-end/specs/importTicketSales.spec.js | 37 ++++++++------- 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/modules/claim/back/methods/claim-end/importTicketSales.js b/modules/claim/back/methods/claim-end/importTicketSales.js index c7ca931974..106313f142 100644 --- a/modules/claim/back/methods/claim-end/importTicketSales.js +++ b/modules/claim/back/methods/claim-end/importTicketSales.js @@ -17,24 +17,45 @@ module.exports = Self => { } }); - Self.importTicketSales = async(ctx, params) => { + Self.importTicketSales = async(ctx, params, options) => { let models = Self.app.models; let userId = ctx.req.accessToken.userId; - let worker = await models.Worker.findOne({where: {userFk: userId}}); - let ticketSales = await models.Sale.find({ - where: {ticketFk: params.ticketFk} - }); + let tx; + let myOptions = {}; - let claimEnds = []; - ticketSales.forEach(sale => { - claimEnds.push({ - saleFk: sale.id, - claimFk: params.claimFk, - workerFk: worker.id + if (typeof options == 'object') + Object.assign(myOptions, options); + + if (!myOptions.transaction) { + tx = await Self.beginTransaction({}); + myOptions.transaction = tx; + } + + try { + const worker = await models.Worker.findOne({where: {userFk: userId}}, myOptions); + + let ticketSales = await models.Sale.find({ + where: {ticketFk: params.ticketFk} + }, myOptions); + + let claimEnds = []; + ticketSales.forEach(sale => { + claimEnds.push({ + saleFk: sale.id, + claimFk: params.claimFk, + workerFk: worker.id + }); }); - }); - return await Self.create(claimEnds); + const createdClaimEnds = await Self.create(claimEnds, myOptions); + + if (tx) await tx.commit(); + + return createdClaimEnds; + } catch (e) { + if (tx) await tx.rollback(); + throw e; + } }; }; diff --git a/modules/claim/back/methods/claim-end/specs/importTicketSales.spec.js b/modules/claim/back/methods/claim-end/specs/importTicketSales.spec.js index 3c2a3e0a6c..93f11e21fe 100644 --- a/modules/claim/back/methods/claim-end/specs/importTicketSales.spec.js +++ b/modules/claim/back/methods/claim-end/specs/importTicketSales.spec.js @@ -1,25 +1,26 @@ const app = require('vn-loopback/server/server'); describe('Claim importTicketSales()', () => { - let claimEnds; - - afterAll(async done => { - claimEnds.forEach(async line => { - await line.destroy(); - }); - - done(); - }); - it('should import sales to a claim actions from an specific ticket', async() => { - let ctx = {req: {accessToken: {userId: 5}}}; - claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { - claimFk: 1, - ticketFk: 1 - }); + const ctx = {req: {accessToken: {userId: 5}}}; - expect(claimEnds.length).toEqual(4); - expect(claimEnds[0].saleFk).toEqual(1); - expect(claimEnds[2].saleFk).toEqual(3); + const tx = await app.models.Entry.beginTransaction({}); + try { + const options = {transaction: tx}; + + const claimEnds = await app.models.ClaimEnd.importTicketSales(ctx, { + claimFk: 1, + ticketFk: 1 + }, options); + + expect(claimEnds.length).toEqual(4); + expect(claimEnds[0].saleFk).toEqual(1); + expect(claimEnds[2].saleFk).toEqual(3); + + await tx.rollback(); + } catch (e) { + await tx.rollback(); + throw e; + } }); }); From e025368af1f59908561298a0161fe457ea97d55f Mon Sep 17 00:00:00 2001 From: carlosjr Date: Tue, 30 Mar 2021 10:17:16 +0200 Subject: [PATCH 9/9] updaloadFile transaction commit conditional --- modules/claim/back/methods/claim/uploadFile.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/claim/back/methods/claim/uploadFile.js b/modules/claim/back/methods/claim/uploadFile.js index 23d0c02732..daab9341af 100644 --- a/modules/claim/back/methods/claim/uploadFile.js +++ b/modules/claim/back/methods/claim/uploadFile.js @@ -78,12 +78,12 @@ module.exports = Self => { }); const resolvedPromises = await Promise.all(promises); - await tx.commit(); + if (tx) await tx.commit(); return resolvedPromises; - } catch (err) { - await tx.rollback(); - throw err; + } catch (e) { + if (tx) await tx.rollback(); + throw e; } }; };