Skip to content

feat: finalInvoice#84

Open
horvbalint wants to merge 3 commits intoewngs:masterfrom
horvbalint:master
Open

feat: finalInvoice#84
horvbalint wants to merge 3 commits intoewngs:masterfrom
horvbalint:master

Conversation

@horvbalint
Copy link
Copy Markdown

@horvbalint horvbalint commented Jan 19, 2025

This PR adds support for creating final invoices.
It introduces two new fields to the Invoice class:

  • finalInvoice: bool - it indicates whether the invoice is a final invoice
  • prepaymentInvoiceNumber: string - not always needed, but contains the invoiceId of the prepayment invoice to which this final invoice belongs

Since prepaymentInvoiceNumber is not required for creating a final invoice (at least according to the documentation), I did not use the pattern what the adjustmentInvoiceNumber field is doing (hiding the adjustmentInvoice field behind it).

Copy link
Copy Markdown
Collaborator

@ert78gb ert78gb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution I have a question and a change request. Please also extend the readme file with the new field. Please add the The invoice number of the down payment invoice can be defined here if the final bill of the down payment invoice can't be identified with the order number. If both invoice number and order number is present, order number takes precedent. note too. It is already in the szamlazz.hu documentation.

if (this._options.prepaymentInvoiceNumber !== null && this._options.prepaymentInvoiceNumber !== undefined) {
assert(typeof this._options.prepaymentInvoiceNumber === 'string', '"prepaymentInvoiceNumber" should be a string')
assert(this._options.prepaymentInvoiceNumber.length > 0, '"prepaymentInvoiceNumber" should be minimum 1 character')
assert(this._options.finalInvoice === true, '"prepaymentInvoiceNumber" should only be set if "finalInvoice" is true')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use this assert in the line 83 or just set the finalInvoice = true automatically when the prepaymentInvoiceNumber field has a value?
I think better to set finalInvoice = true automatically. It is more developer friendly.
Do you know anything that against it?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deliberately did not do this, because in this case finalInvoice is not a 'hidden' field (like adjustmentInvoice is), so I think it would be weird to change it without notice.
But if you think that is the better pattern, I can change ot of course.

Copy link
Copy Markdown
Collaborator

@ert78gb ert78gb Jan 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some API call to identify how it is working.

Case 1:

  • orderNumber is not provided
  • prepaymentInvoiceNumber is not provided
  • finalInvoice: true

Error: A hivatkozott előlegszámla nem beazonosítható. Rendelésszám: nincs megadva, előlegszámla száma: nincs megadva.


Case 2:

  • orderNumber: '1223' Invoice with this order number has not been created before
  • prepaymentInvoiceNumber is not provided
  • finalInvoice: true

Error: A hivatkozott előlegszámla nem beazonosítható. Rendelésszám: 1223, előlegszámla száma: nincs megadva.


Case 3:

  • orderNumber is not provided
  • prepaymentInvoiceNumber: Test-01
  • finalInvoice: true

Error: A hivatkozott előlegszámla nem beazonosítható. Rendelésszám: nincs megadva, előlegszámla száma: Test-01.


Case 4:

  • orderNumber is not provided
  • prepaymentInvoiceNumber: Test-01
  • finalInvoice os not provided

Successfully created a new invoice.


Case 5:

  • orderNumber: C001
  • prepaymentInvoiceNumber: Test-01
  • finalInvoice os not provided

Successfully created a new invoice.


Case 6:

  • orderNumber: C001
  • prepaymentInvoiceNumber: Test-01
  • finalInvoice os not provided

Just tested could I create more invoices with same order number. Szamlazz.hu allows it every data is same, but if I modify anything the response is Már létező rendelésszám: C001. Az ismétlődés engedélyezhető a Beállítások oldalon.


Case 7:

  • orderNumber: C001
  • prepaymentInvoiceNumber is not provided
  • finalInvoice: true

Error: A hivatkozott előlegszámla nem beazonosítható. Rendelésszám: C001, előlegszámla száma: nincs megadva.


Case 8:

  • orderNumber is not provided
  • prepaymentInvoiceNumber: Test-02 Invoice has been created in earlier
  • finalInvoice: true

Error: A hivatkozott előlegszámla nem beazonosítható. Rendelésszám: nincs megadva, előlegszámla száma: Test-02


Case 9:

  • orderNumber: C002
  • prepaymentInvoiceNumber is not provided
  • finalInvoice is not provided
  • prepaymentInvoice: true

Successfully created a new pre-payment invoice.


Case 10:

  • orderNumber: C002
  • prepaymentInvoiceNumber is not provided
  • finalInvoice is not provided
  • prepaymentInvoice: true

Not created new pre-invoice. Same as Case6


Case 11:

  • orderNumber: C002
  • prepaymentInvoiceNumber is not provided
  • finalInvoice is not provided
  • prepaymentInvoice is not provided

Created new a new invoice. It is surprising me.


Case 12:

  • orderNumber: C002
  • prepaymentInvoiceNumber is not provided
  • finalInvoice: true
  • prepaymentInvoice is not provided

Created a new final invoice


Case 13:

  • orderNumber: C002
  • prepaymentInvoiceNumber is not provided
  • finalInvoice: true
  • prepaymentInvoice is not provided

Error: A hivatkozott előlegszámla nem beazonosítható. Rendelésszám: C002, előlegszámla száma: nincs megadva.


Based on these tests

  • szamlazz.hu differensiate 3 invoice type (alap.tipus element in the invoice query xml)

    • SZ => standard invoice
    • ES => pre-invoice
    • VS => final invoice
  • orderNumber is an optional field

  • To create a final invoice have to provide the orderNumber the invoice number of the pre-invoice prepaymentInvoiceNumber. Please add this validation to the PR.

  • Does not matter do we provide the prepaymentInvoiceNumber when we create a standard or pre-invoice. After that I agree with you keep the current validation.

@horvbalint
Copy link
Copy Markdown
Author

@ert78gb By accident I commited a small change here that I didn't mean to. It fixes the order of 2 fields in the buyer xml. Should I revert the commit here and open another PR for it (as I planned), or can it remain here since it is a small change?

Ps.: I did not had time to resolve the review stuff yet, but still plan to do it.

@ert78gb
Copy link
Copy Markdown
Collaborator

ert78gb commented Mar 11, 2025

I am thinking on finish your PR because it is valuable but I am also sort on time.

regarding to theadoszamEU I am creating a new PR about this fix in then next hours. I prefer dedicated PR. It helps in bisecting in case of bug hunting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants