#6147 - Refactor Invoice make PDF and notify #1933

Merged
jsegarra merged 13 commits from 6174_refactor_invoicePdfNotify into dev 2024-01-29 14:10:33 +00:00
13 changed files with 200 additions and 16 deletions

View File

@ -0,0 +1,3 @@
INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES
('Ticket', 'makePdfList', '*', 'ALLOW', 'ROLE', 'employee'),
('Ticket', 'invoiceTicketsAndPdf', '*', 'ALLOW', 'ROLE', 'employee');

View File

@ -0,0 +1,31 @@
module.exports = Self => {
Self.remoteMethodCtx('makePdfList', {
description: 'Handle a list of invoices to generate PDF and send it to client',
accessType: 'WRITE',
accepts: [
{
arg: 'ids',
type: ['number'],
description: 'The invoice id',
required: true,
http: {source: 'path'}
}, {
arg: 'printerFk',
type: 'number',
description: 'The printer to print'
}
],
http: {
path: '/:id/makePdfList',
verb: 'POST'
}
});
Self.makePdfList = async function(ctx, ids, printerFk, options) {
const pdfs = ids.map(id =>
jsegarra marked this conversation as resolved Outdated
Outdated
Review

Jo crec que forEach no funciona en async await

Jo crec que forEach no funciona en async await

He buscado lo que comentas y si que hay alguna limitación comos e refleja aquí(https://gist.github.com/joeytwiddle/37d2085425c049629b80956d3c618971). Sin embargo, a nosotros no nos hace falta controlar si ha fallado la generación del PDF, o si?

Es verdad que de normal hacemos for...of, pero no he visto que crasheaba.

He buscado lo que comentas y si que hay alguna limitación comos e refleja aquí(https://gist.github.com/joeytwiddle/37d2085425c049629b80956d3c618971). Sin embargo, a nosotros no nos hace falta controlar si ha fallado la generación del PDF, o si? Es verdad que de normal hacemos for...of, pero no he visto que crasheaba.
Outdated
Review

Esq dejarlo con foreach con async await, si no funciona asi no lo veo.
Aparte al no manejarlo bien si fallase mas tarde podria romper otra parte y no es facil de debuggear despues

Esq dejarlo con foreach con async await, si no funciona asi no lo veo. Aparte al no manejarlo bien si fallase mas tarde podria romper otra parte y no es facil de debuggear despues
Self.makePdfAndNotify(ctx, id, printerFk, options)
);
await Promise.all(pdfs);
};
};

View File

@ -17,6 +17,7 @@ describe('InvoiceOut transferInvoice()', () => {
spyOn(LoopBackContext, 'getCurrentContext').and.returnValue({ spyOn(LoopBackContext, 'getCurrentContext').and.returnValue({
active: activeCtx active: activeCtx
}); });
spyOn(models.InvoiceOut, 'makePdfAndNotify');
}); });
it('should return the id of the created issued invoice', async() => { it('should return the id of the created issued invoice', async() => {

View File

@ -86,17 +86,17 @@ module.exports = Self => {
clonedTicketIds.push(clonedTicket.id); clonedTicketIds.push(clonedTicket.id);
} }
const invoiceCorrection = const invoiceCorrection = {
{correctedFk: id, cplusRectificationTypeFk, siiTypeInvoiceOutFk, invoiceCorrectionTypeFk}; correctedFk: id,
cplusRectificationTypeFk,
siiTypeInvoiceOutFk,
invoiceCorrectionTypeFk
};
const refundTicketIds = refundTickets.map(ticket => ticket.id); const refundTicketIds = refundTickets.map(ticket => ticket.id);
await models.Ticket.invoiceTickets(ctx, refundTicketIds, invoiceCorrection, myOptions); await models.Ticket.invoiceTickets(ctx, refundTicketIds, invoiceCorrection, myOptions);
const [invoiceId] = await models.Ticket.invoiceTickets(ctx, clonedTicketIds, null, myOptions);
if (tx) { const [invoiceId] = await models.Ticket.invoiceTicketsAndPdf(ctx, clonedTicketIds, null, myOptions);
await tx.commit();
await models.InvoiceOut.makePdfAndNotify(ctx, invoiceId, null);
}
return invoiceId; return invoiceId;
} catch (e) { } catch (e) {

View File

@ -13,6 +13,7 @@ module.exports = Self => {
require('../methods/invoiceOut/createManualInvoice')(Self); require('../methods/invoiceOut/createManualInvoice')(Self);
require('../methods/invoiceOut/clientsToInvoice')(Self); require('../methods/invoiceOut/clientsToInvoice')(Self);
require('../methods/invoiceOut/invoiceClient')(Self); require('../methods/invoiceOut/invoiceClient')(Self);
require('../methods/invoiceOut/makePdfList')(Self);
require('../methods/invoiceOut/makePdfAndNotify')(Self); require('../methods/invoiceOut/makePdfAndNotify')(Self);
require('../methods/invoiceOut/refund')(Self); require('../methods/invoiceOut/refund')(Self);
require('../methods/invoiceOut/invoiceEmail')(Self); require('../methods/invoiceOut/invoiceEmail')(Self);

View File

@ -76,15 +76,11 @@ module.exports = function(Self) {
for (const ticketIds of ticketsByAddress) for (const ticketIds of ticketsByAddress)
invoicesIds.push(await createInvoice(ctx, companyId, ticketIds, invoiceCorrection, myOptions)); invoicesIds.push(await createInvoice(ctx, companyId, ticketIds, invoiceCorrection, myOptions));
if (tx) await tx.commit(); tx && await tx.commit();
} catch (e) { } catch (e) {
if (tx) await tx.rollback(); if (tx) await tx.rollback();
throw e; throw e;
} }
if (tx) {
for (const invoiceId of invoicesIds)
await models.InvoiceOut.makePdfAndNotify(ctx, invoiceId, null);
}
return invoicesIds; return invoicesIds;
}; };

View File

@ -0,0 +1,36 @@
module.exports = function(Self) {
Self.remoteMethodCtx('invoiceTicketsAndPdf', {
description: 'Make out an invoice from one or more tickets',
accessType: 'WRITE',
accepts: [
{
arg: 'ticketsIds',
description: 'The tickets id',
type: ['number'],
required: true
},
{
arg: 'invoiceCorrection',
description: 'The invoice correction',
type: 'object',
}
],
returns: {
type: ['object'],
root: true
},
http: {
path: `/invoiceTicketsAndPdf`,
verb: 'POST'
}
});
Self.invoiceTicketsAndPdf = async(ctx, ticketsIds, invoiceCorrection, options) => {
const invoiceIds = await Self.invoiceTickets(ctx, ticketsIds, invoiceCorrection, options);
await Self.app.models.InvoiceOut.makePdfList(ctx, invoiceIds, null, options);
return invoiceIds;
};
};
jsegarra marked this conversation as resolved Outdated
Outdated
Review

Jo diria que no cal el if, pq si estas cridant aci es pq si o si vols el pdf. Si no cridaries al altre

Jo diria que no cal el if, pq si estas cridant aci es pq si o si vols el pdf. Si no cridaries al altre

View File

@ -102,7 +102,7 @@ describe('ticket invoiceTickets()', () => {
const options = {transaction: tx}; const options = {transaction: tx};
const ticketsIds = [11]; const ticketsIds = [11];
const invoicesIds = await models.Ticket.invoiceTickets(ctx, ticketsIds, null, options); const invoicesIds = await models.Ticket.invoiceTicketsAndPdf(ctx, ticketsIds, null, options);
jsegarra marked this conversation as resolved
Review

invoiceTickets.spec.js solo deberia hacer tests de invoiceTickets

invoiceTickets.spec.js solo deberia hacer tests de invoiceTickets
Review

Entonces deberíamos hacer un test para invoiceTicketsAndPdf.
En realidad he modificado el test porque así comprobamos 2 remoteMethod, ya que invoiceTicketsAndPdf llama a InvoiceTickets

Entonces deberíamos hacer un test para invoiceTicketsAndPdf. En realidad he modificado el test porque así comprobamos 2 remoteMethod, ya que invoiceTicketsAndPdf llama a InvoiceTickets
Review

Deberia haber uno para invoiceTickets especifico y si se puede testear invoiceTicketsAndPdf tambien

Deberia haber uno para invoiceTickets especifico y si se puede testear invoiceTicketsAndPdf tambien
expect(invoicesIds.length).toBeGreaterThan(0); expect(invoicesIds.length).toBeGreaterThan(0);

View File

@ -0,0 +1,115 @@
const models = require('vn-loopback/server/server').models;
const LoopBackContext = require('loopback-context');
describe('ticket invoiceTicketsAndPdf()', () => {
const userId = 19;
const clientId = 1102;
const activeCtx = {
getLocale: () => {
return 'en';
},
accessToken: {userId: userId},
headers: {origin: 'http://localhost:5000'},
};
const ctx = {req: activeCtx};
beforeAll(async() => {
spyOn(LoopBackContext, 'getCurrentContext').and.returnValue({
active: activeCtx
});
});
it('should throw an error when invoicing tickets from multiple clients', async() => {
const invoiceOutModel = models.InvoiceOut;
spyOn(invoiceOutModel, 'makePdfAndNotify');
const tx = await models.Ticket.beginTransaction({});
let error;
try {
const options = {transaction: tx};
const ticketsIds = [11, 16];
await models.Ticket.invoiceTicketsAndPdf(ctx, ticketsIds, null, options);
await tx.rollback();
} catch (e) {
error = e;
await tx.rollback();
}
expect(error.message).toEqual(`You can't invoice tickets from multiple clients`);
});
it(`should throw an error when invoicing a client without tax data checked`, async() => {
const invoiceOutModel = models.InvoiceOut;
spyOn(invoiceOutModel, 'makePdfAndNotify');
const tx = await models.Ticket.beginTransaction({});
let error;
try {
const options = {transaction: tx};
const client = await models.Client.findById(clientId, null, options);
await client.updateAttribute('isTaxDataChecked', false, options);
const ticketsIds = [11];
await models.Ticket.invoiceTicketsAndPdf(ctx, ticketsIds, null, options);
await tx.rollback();
} catch (e) {
error = e;
await tx.rollback();
}
expect(error.message).toEqual(`This client can't be invoiced`);
});
it('should invoice a ticket, then try again to fail', async() => {
const invoiceOutModel = models.InvoiceOut;
spyOn(invoiceOutModel, 'makePdfAndNotify');
const tx = await models.Ticket.beginTransaction({});
let error;
try {
const options = {transaction: tx};
const ticketsIds = [11];
await models.Ticket.invoiceTicketsAndPdf(ctx, ticketsIds, null, options);
await models.Ticket.invoiceTicketsAndPdf(ctx, ticketsIds, null, options);
await tx.rollback();
} catch (e) {
error = e;
await tx.rollback();
}
expect(error.message).toEqual(`This ticket is already invoiced`);
});
it('should success to invoice a ticket', async() => {
const invoiceOutModel = models.InvoiceOut;
spyOn(invoiceOutModel, 'makePdfAndNotify');
const tx = await models.Ticket.beginTransaction({});
try {
const options = {transaction: tx};
const ticketsIds = [11];
const invoicesIds = await models.Ticket.invoiceTicketsAndPdf(ctx, ticketsIds, null, options);
expect(invoicesIds.length).toBeGreaterThan(0);
await tx.rollback();
} catch (e) {
await tx.rollback();
throw e;
}
});
});

View File

@ -42,5 +42,6 @@ module.exports = function(Self) {
require('../methods/ticket/expeditionPalletLabel')(Self); require('../methods/ticket/expeditionPalletLabel')(Self);
require('../methods/ticket/saveSign')(Self); require('../methods/ticket/saveSign')(Self);
require('../methods/ticket/invoiceTickets')(Self); require('../methods/ticket/invoiceTickets')(Self);
require('../methods/ticket/invoiceTicketsAndPdf')(Self);
require('../methods/ticket/docuwareDownload')(Self); require('../methods/ticket/docuwareDownload')(Self);
}; };

View File

@ -265,7 +265,7 @@ class Controller extends Section {
}); });
} }
return this.$http.post(`Tickets/invoiceTickets`, {ticketsIds: [this.id]}) return this.$http.post(`Tickets/invoiceTicketsAndPdf`, {ticketsIds: [this.id]})
.then(() => this.reload()) .then(() => this.reload())
.then(() => this.vnApp.showSuccess(this.$t('Ticket invoiced'))); .then(() => this.vnApp.showSuccess(this.$t('Ticket invoiced')));
} }

View File

@ -191,7 +191,7 @@ describe('Ticket Component vnTicketDescriptorMenu', () => {
jest.spyOn(controller.vnApp, 'showSuccess'); jest.spyOn(controller.vnApp, 'showSuccess');
const expectedParams = {ticketsIds: [ticket.id]}; const expectedParams = {ticketsIds: [ticket.id]};
$httpBackend.expectPOST(`Tickets/invoiceTickets`, expectedParams).respond(); $httpBackend.expectPOST(`Tickets/invoiceTicketsAndPdf`, expectedParams).respond();
controller.makeInvoice(); controller.makeInvoice();
$httpBackend.flush(); $httpBackend.flush();

View File

@ -163,7 +163,7 @@ export default class Controller extends Section {
makeInvoice() { makeInvoice() {
const ticketsIds = this.checked.map(ticket => ticket.id); const ticketsIds = this.checked.map(ticket => ticket.id);
return this.$http.post(`Tickets/invoiceTickets`, {ticketsIds}) return this.$http.post(`Tickets/invoiceTicketsAndPdf`, {ticketsIds})
.then(() => this.$.model.refresh()) .then(() => this.$.model.refresh())
.then(() => this.vnApp.showSuccess(this.$t('Ticket invoiced'))); .then(() => this.vnApp.showSuccess(this.$t('Ticket invoiced')));
} }