Skip to content

Conversation

@pmenendz
Copy link
Contributor

@pmenendz pmenendz commented May 1, 2025

The percentage from GOBL to UBL was being wrongly mapped.

Before:

  • 10% -> 0.1

Now:

  • 10% -> 10

Here it is explained how to map it: https://docs.peppol.eu/poacc/billing/3.0/syntax/ubl-invoice/cac-AllowanceCharge/cbc-MultiplierFactorNumeric/

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the wrong percentage mapping from GOBL to UBL by switching from using the Base() method to the Amount() method when mapping percentage values, as per the official mapping documentation.

  • Updates percentage conversion logic in lines.go, charges.go, and related discount mappings.
  • Adjusts test assertions in lines_test.go and charges_test.go to reflect the corrected mapping values.

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

File Description
lines_test.go Updated test assertions to expect corrected multiplier values and amounts.
lines.go Modified percentage mapping logic to use Amount() instead of Base().
charges_test.go Adjusted test expectations for multiplier factor in discount charges.
charges.go Updated percentage conversion from Base() to Amount() for both charge and discount flows.
Files not reviewed (1)
  • test/data/convert/invoice-without-buyers-tax-id.json: Language not supported
Comments suppressed due to low confidence (3)

charges_test.go:26

  • Confirm that the expected multiplier factor value in the test is aligned with the updated percentage mapping logic, ensuring full coverage of the new behavior.
assert.Equal(t, "5", *doc.InvoiceLines[0].AllowanceCharge[0].MultiplierFactorNumeric)

lines.go:176

  • The conversion logic has been updated from Base() to Amount(), which should now correctly map percentages according to the documentation. Please verify that this change consistently reflects the intended percentage format across all usages in line charges.
p := ch.Percent.Amount().String()

charges.go:49

  • Ensure that the updated conversion from Base() to Amount() properly handles percentage mapping for charges in line with the new specification.
p := ch.Percent.Amount().String()

Copy link
Collaborator

@alvarolivie alvarolivie left a comment

Choose a reason for hiding this comment

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

LGTM. Take a look at CII for a different approach, I used Percent.StringWithoutSymbol().

Copy link
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

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

Are you sure? What is the source of this change?

<cac:AllowanceCharge>
<cbc:ChargeIndicator>false</cbc:ChargeIndicator>
<cbc:MultiplierFactorNumeric>0.05</cbc:MultiplierFactorNumeric>
<cbc:MultiplierFactorNumeric>5</cbc:MultiplierFactorNumeric>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about this? Multiplier factor I'd assume means a factor to apply from a percentage, so get 5% of something, you multiple by a factor if 0.05.

Copy link
Contributor Author

@pmenendz pmenendz May 1, 2025

Choose a reason for hiding this comment

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

For me it also didn't make sense, but I was sending an invoice with a 50% discount and receiving a 0.5% discount. I checked it and it is stated here that 20% should be 20: https://docs.peppol.eu/poacc/billing/3.0/syntax/ubl-invoice/cac-AllowanceCharge/cbc-MultiplierFactorNumeric/#:~:text=To%20state%2020%25%2C%20use%20value%2020.

However, when checking this example (https://docs.oasis-open.org/ubl/os-UBL-2.1/xml/UBL-Invoice-2.1-Example.xml) it appears to be as you say

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the error isn't there it is in how we convert back from the UBL to GOBL, because the approach is different from GOBL to UBL and from UBL to GOBL

@pmenendz pmenendz marked this pull request as draft May 1, 2025 22:00
@samlown
Copy link
Contributor

samlown commented Jun 19, 2025

So we have a bit of extra context here... this is still an issue. It turns out that standard UBL has a different approach to Peppol UBL. This change shouldn't just be a blanket switch, it needs to be adjusted according to the document's profile.

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.

4 participants