verdnatura/salix
verdnatura
/
salix
3
4
Fork 0
Code Issues Pull Requests 57 Releases Wiki Activity

#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
Conversation 1 Commits 13 Files Changed 13 +200 -16
jsegarra commented 2024-01-15 09:46:29 +00:00
Member
Copy Link
No description provided.
jsegarra changed title from WIP: Refactor Invoice make PDF and notify to WIP: #6174 - Refactor Invoice make PDF and notify 2024-01-15 09:46:41 +00:00
jsegarra changed title from WIP: #6174 - Refactor Invoice make PDF and notify to WIP: #6147 - Refactor Invoice make PDF and notify 2024-01-17 09:39:36 +00:00
jsegarra added 2 commits 2024-01-17 09:39:49 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
21ce57d217 refs #6174 feat: update back to separate invoice and pdf
jsegarra changed title from WIP: #6147 - Refactor Invoice make PDF and notify to #6147 - Refactor Invoice make PDF and notify 2024-01-17 09:41:43 +00:00
jsegarra requested review from alexm 2024-01-17 09:43:12 +00:00
jsegarra added 1 commit 2024-01-17 10:12:38 +00:00
alexm requested changes 2024-01-17 12:47:41 +00:00
modules/invoiceOut/back/methods/invoiceOut/makePdfList.js Outdated
@ -0,0 +23,4 @@
});
Self.makePdfList = async function(ctx, ids, printerFk, options) {
ids.forEach(async id => {
alexm commented 2024-01-17 12:42:51 +00:00
Member
Copy Link

Jo crec que forEach no funciona en async await

Jo crec que forEach no funciona en async await
jsegarra commented 2024-01-17 12:59:11 +00:00
Author
Member
Copy Link

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.
alexm commented 2024-01-23 09:20:06 +00:00
Member
Copy Link

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
jsegarra marked this conversation as resolved
modules/ticket/back/methods/ticket/specs/invoiceTickets.spec.js
@ -103,3 +103,3 @@
const ticketsIds = [11];
const invoicesIds = await models.Ticket.invoiceTickets(ctx, ticketsIds, null, options);
const invoicesIds = await models.Ticket.invoiceTicketsAndPdf(ctx, ticketsIds, null, options);
alexm commented 2024-01-17 12:47:32 +00:00
Member
Copy Link

invoiceTickets.spec.js solo deberia hacer tests de invoiceTickets

invoiceTickets.spec.js solo deberia hacer tests de invoiceTickets
jsegarra commented 2024-01-17 12:51:23 +00:00
Author
Member
Copy Link

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
alexm commented 2024-01-23 09:08:06 +00:00
Member
Copy Link

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
jsegarra marked this conversation as resolved
jsegarra added 1 commit 2024-01-22 09:01:01 +00:00
gitea/salix/pipeline/head This commit looks good Details
ba2815becb Merge branch 'dev' into 6174_refactor_invoicePdfNotify
jsegarra requested review from alexm 2024-01-22 09:01:01 +00:00
jsegarra added 1 commit 2024-01-23 07:43:46 +00:00
gitea/salix/pipeline/head This commit looks good Details
dcb2907d49 Merge branch 'dev' into 6174_refactor_invoicePdfNotify
alexm requested changes 2024-01-23 09:20:22 +00:00
alexm left a comment
Member
Copy Link

Comentarios en PR

Comentarios en PR
jsegarra requested review from alexm 2024-01-23 22:53:33 +00:00
jsegarra added 1 commit 2024-01-23 22:54:10 +00:00
gitea/salix/pipeline/head This commit looks good Details
bc6f9bc074 refs #6174 test: new test for new method
alexm requested changes 2024-01-26 07:08:03 +00:00
modules/ticket/back/methods/ticket/invoiceTicketsAndPdf.js Outdated
@ -0,0 +32,4 @@
invoiceIds = await Self.invoiceTickets(ctx, ticketsIds, invoiceCorrection, options);
if (!invoiceIds) return [];
if (!options.transaction)
alexm commented 2024-01-26 07:07:46 +00:00
Member
Copy Link

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
jsegarra marked this conversation as resolved
jsegarra added 2 commits 2024-01-26 08:55:48 +00:00
gitea/salix/pipeline/head This commit looks good Details
0afc9ab109 refs #6174 test: fix test bug
jsegarra requested review from alexm 2024-01-26 08:55:51 +00:00
alexm requested changes 2024-01-29 10:08:55 +00:00
alexm left a comment
Member
Copy Link

Falta redirigir en el front donde se llamaba a invoiceTickets para que llame a lo nuevo

Falta redirigir en el front donde se llamaba a invoiceTickets para que llame a lo nuevo
jsegarra added 3 commits 2024-01-29 13:12:10 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
5396b402a5 refs #6174 feat: acl
jsegarra requested review from alexm 2024-01-29 13:13:11 +00:00
jsegarra added 1 commit 2024-01-29 13:13:11 +00:00
gitea/salix/pipeline/pr-dev Build started... Details
gitea/salix/pipeline/head There was a failure building this commit Details
7880f41d1c Merge branch 'dev' of https://gitea.verdnatura.es/verdnatura/salix into 6174_refactor_invoicePdfNotify
jsegarra commented 2024-01-29 13:13:38 +00:00
Author
Member
Copy Link

@alexm He añadido las ACL a employee de los nuevos métodos

@alexm He añadido las ACL a employee de los nuevos métodos
jsegarra added 1 commit 2024-01-29 13:54:43 +00:00
gitea/salix/pipeline/head This commit looks good Details
2e0da0881b refs #6174 test: fix method
alexm approved these changes 2024-01-29 14:10:10 +00:00
jsegarra merged commit 89a5f3dff8 into dev 2024-01-29 14:10:33 +00:00
jsegarra deleted branch 6174_refactor_invoicePdfNotify 2024-01-29 14:10:34 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
alexm
Labels
Clear labels   
CR / Tests passed

Unit tests and e2e passed

  
e2e

Implement e2e

  
Missing tests

Missing tests

  
Needs refactor

   
Simple

Simple check / No CR needed

No Label
CR / Tests passed
e2e
Missing tests
Needs refactor
Simple
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: verdnatura/salix#1933
No description provided.
Delete Branch "6174_refactor_invoicePdfNotify"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?