Add basic structs for MX fuel complement #192

cavalle · 2023-09-01T12:31:16Z

A very early draft PR to discuss the structure of the fuel complement in MX.
Just the structs, completely unintegrated, for the moment. Validations, calculations and integration with the main GOBL document are pending.
See the comments in the code for some design considerations.

cavalle · 2023-09-01T12:46:26Z

regimes/mx/complements/fuel_complement.go

@@ -0,0 +1,58 @@
+// Package complements provides GOBL Complements for Mexican invoices
+package complements


Not sure what's the best way of structuring complements. For the moment, I'm putting everything under a complements package within the mx path. Suggestions on how to better do this are welcome!

I'm not sure... if you imagine a user who needs to set up complements for multiple countries, they'd have to deal with the namespace collisions of complements in multiple places. Could be awkward. I'd also think about the final schema URL, I was thinking of: https://gobl.org/draft-0/regimes/mx/fuel-complement

Thanks, Sam! Agreed, there's no need for a complements package. Moved it to the main mx package in e178705. I also generated the schema file in the URL that you advised in 87305b0.

cavalle · 2023-09-01T13:11:19Z

regimes/mx/complements/fuel_complement.go

+// FuelComplement provides detailed information about fuel purchases made with
+// electronic wallets. In Mexico, e-wallet suppliers are required to report this
+// information as part of the invoices they issue to their customers.
+type FuelComplement struct {


When creating the structs of this complement, I considered two general approaches:

Mirror the target CFDI structure: The first option was to replicate the CFDI fields and structure directly (not unlike what we've done with ext fields – straightforward and explicit).

Align with GOBL's idioms: I chose this. It should make the structs more intuitive for GOBL developers who may not be familiar with CFDIs, but will definitely be used to GOBL. It also ensures that the resulting GOBL document looks harmonious and not like a Frankenstein.

What do you think?

Agreed. I think we should at least try to smooth out the edges.

cavalle · 2023-09-01T13:15:35Z

regimes/mx/complements/fuel_complement.go

+	// List of fuel purchases made with the customer's e-wallets
+	Lines []*FuelLine `json:"lines" jsonschema:"title=Lines"`
+	// Summary of all the purchases totals, including taxes (calculated)
+	Totals bill.Totals `json:"totals" jsonschema:"title=Totals" jsonschema_extras:"calculated=true"`


I'm reusing bill.Totals here but I'm not 100% sure about this as we only need the Total and TotalWithTaxes fields from that struct. It may be confusing for developers to have all the other fields available.

A penny for your thoughts!

I'm inclined to agree... this strikes me as a new structure alongside the complements.

I agree! Addressed in 4be0288

cavalle · 2023-09-01T13:22:53Z

regimes/mx/complements/fuel_complement.go

+
+// FuelLine represents a single fuel purchase made with an e-wallet issued by
+// the invoice's supplier to the invoice's customer.
+type FuelLine struct {


I used the "line" word because the whole structure resembles very much to the bill.Line struct. We could even extend the bill.Line struct here with the specific fields we need in this context, which enable to potentially reuse part of the validation/calculation logic. However, I thought that binding the evolution of the bill.Line struct and behaviour to this struct may not be a good idea if those diverge in the future. Also, like I mentioned in other comments, bill.Line has fields that won't be used here, and that could confuse developers.

I think we're okay recreating the structures for this use-case. FuelLine makes sense.

cavalle · 2023-09-01T13:24:18Z

regimes/mx/complements/fuel_complement.go

+	// Identifier of the e-wallet used to make the purchase
+	WalletID cbc.Code `json:"wallet-id" jsonschema:"title=Wallet Identifier"`
+	// Date and time of the purchase
+	PurchaseDateTime civil.DateTime `json:"purchase-date" jsonschema:"title=Purchase Date"`


We don't have cal.DateTime at the moment. Should we add it?

🤔 interesting. It might indeed be a good idea to have a cal.DateTime which enforces ISO formatting. We have a similar "at" package in github.com/invopop/lib which does something similar with database storage. I'll think a bit more about this.

Ok! Leaving civil.DateTime for now

Have you also considered two fields? I'm not convinced, and we'd still need a new type, but "Date" and "Time" types could make sense.

I was right now drafting a cal.DateTime type that would marshal and unmarshall in the ISO format. I personally don't see any major problem on having a DateTime type, it's quite universally and very well standardised. What concerns do you have? Separating Date and Time is definitely an option, but then we will need a new cal.Time type too, wouldn't we?

I just pushed 5a3388e with a barebones cal.DateTime implementation (I'll complete it once we confirm that's the like we'd like to follow). The only concern I have about the type is whether or not it should support time zones. The CFDI expects a date and time without a timezone, but in the future we may need a Date and Time with Timezones for other purposes. Not sure we'll want to add just another type then.

cavalle · 2023-09-01T13:26:54Z

regimes/mx/complements/fuel_complement.go

+}
+
+// FuelItem provides the details of the fuel purchased.
+type FuelItem struct {


As discussed in other parts of this PR, the similarity of this struct with org.Item makes it tempting to use it directly. In this particular case, I decided not to, but the considerations discussed in other comments could be applied here to make a different decision. WDYT?

I'd be tempted to store the FuelItem fields inside the FuelLine... the criteria is dependent on how re-usable the Item needs to be. Will the FuelItem ever need to be persisted independently?

I created a separate FuelItem struct just to mirror the existing pattern in bill.Line and org.Item. It neatly separates item details from purchase info, and it's an idiom that GOBL developers will be familiar with. I also expect this data to be persisted separately in the source systems.

On the other hand, in the CFDI the line and item data are at the same level. So if you're not convinced by this approach, let me know and I'll flatten down here too.

I think I accidentally deleted your last comment here. Sorry for the mess, I'm bringing it back:

@samlown said:

My thoughts are around the path of least resistance for users trying to figure out what to do. I think it might be a good idea in this case to stick as closely as possible to the original source structure, with translated fields...

Finding the path of least resistance for users is the same design principle I'm trying to follow. Question is which path is that! From my perspective, one of the main benefits that GOBL brings to developers is providing them with a single unified language, with a consistent vocabulary and set of idioms and patterns that they can use across regimes. That's why I thought the least resistance path was modelling lines and items resembling the ones in GOBL.

In any case, I'll propose an alternative version sticking as closely as possible to the original source structure with translated fields.

cavalle · 2023-09-01T13:31:36Z

regimes/mx/complements/fuel_complement.go

+	// Reference unit of measure used in the price and the quantity
+	Unit org.Unit `json:"unit" jsonschema:"title=Unit"`
+	// Type of fuel (c_ClaveTipoCombustible codes)
+	Type cbc.Code `json:"type,omitempty" jsonschema:"title=Type"`


There are only a bunch of simple values that can be used here:

c_ClaveTipoCombustible Tipo de combustible

1 Gasolina menor a 92 octanos

2 Gasolina mayor o igual a 92 octanos

3 Diesel

4 Diesel Marino

5 Otros

We can simply use a cbc.Code with explicit codes (and validate them), or go fancy and create a new type (FuelType) that is a cbc.Keyand with valid values are more meaningful (diesel, gasoline…) like we do with the org.Units and in many other places in GOBL.

I opted for the simple solution, but I can see the merits of the meaningful key option. Any thoughts about this one?

Initial impression: I think unless there is a fuel type elsewhere, we can just use the code specific for this document. Given the local nature of this, I'm not as worried... I envisage most use-cases of this will be hard coded in code and very unlikely to use a front-end... my impression anyway.

cavalle · 2023-09-01T13:35:22Z

regimes/mx/complements/fuel_complement.go

+	// Result of quantity multiplied by the item's price (calculated)
+	Sum num.Amount `json:"sum" jsonschema:"title=Sum" jsonschema_extras:"calculated=true"`
+	// Map of taxes applied to the purchase
+	Taxes tax.Set `json:"taxes" jsonschema:"title=Taxes"`


Reusing GOBL's tax.Set. This is the case where reusing makes more sense to me, we need all the fields and can take advantage of the rate calculation using regime's data.

samlown · 2023-09-01T17:40:47Z

regimes/mx/complements/fuel_complement.go

+// information as part of the invoices they issue to their customers.
+type FuelComplement struct {
+	// Customer's e-wallet account number
+	WalletAccountNumber string `json:"wallet-account-number" jsonschema:"title=Wallet Account Number"`


GOBL convention is to use _ (underscore) to separate words. The initial aim was to be database compatible.

My oversight. Fixed on 7b4ed40. Good catch!

samlown · 2023-09-02T20:02:05Z

regimes/mx/complements/fuel_complement.go

+
+// FuelComplement provides detailed information about fuel purchases made with
+// electronic wallets. In Mexico, e-wallet suppliers are required to report this
+// information as part of the invoices they issue to their customers.


I think we need to reference here the exact name of the complement and version that will be generated as a result.

👍 Addressed in 653e8bc

cavalle · 2023-09-12T10:17:07Z

@sam I just pushed more changes, merging main (with complements and datetimes), and flattening down FuelItem into FuelLine as discussed above. Let me know if you have any other feedback on the structure, naming, documentation of the complement when you can, please.

samlown · 2023-09-13T12:44:51Z

regimes/mx/fuel_complement.go

+// providing detailed information about fuel purchases made with electronic
+// wallets. In Mexico, e-wallet suppliers are required to report this
+// complementary information in the invoices they issue to their customers.
+type FuelComplement struct {


So after reading the official name, had you considered any alternative names? Perhaps EWalletFuelComplement? I'm worried about potential colisions, and the fact that "FuelComplement" doesn't cover the wallet part...

Considering this name will also be part of the schema name, I tried to make it as short as possible. I suppose future name collisions are indeed a possibility (although probably small). In the XML, they call the root tag EstadoCuentaCombustibles, so they give more priority to the "estado de cuenta" part of the name than to the "e-wallet" aspect. So here's a list of options with their trade-offs:

Name Trade-offs

FuelComplement 👍 short and to the point
👎 not fully meaningful, collision-able

EWalletFuelComplement 👍 meaningful, no collisions
👎 long schema name

FuelAccountStatement 👍 direct translation of EstadoCuentaCombustibles
👎 no mention of e-wallet, no mention of being a complement

I don't love any of them, but I think all of them would work. Let me know if you have any preference (or any other suggestion) and we'll go with that.

I'd be inclined to go with the direct translation: FuelAccountStatement or FuelAccountBalance which is the translation I got from ChatGPT.

hehe, I got "Account Statement" from Deepl, but I like "Account Balance" better! (AIs FTW!)

samlown · 2023-09-13T12:46:11Z

regimes/mx/fuel_complement.go

+	Total num.Amount `json:"total" jsonschema:"title=Total"`
+	// Grand total after taxes have been applied.
+	TotalWithTax num.Amount `json:"total_with_tax" jsonschema:"title=Total with Tax"`
+}


Is it worth adding any extra validation? Even if just the basics of ensuring that each field is a valid value.

Sure thing! I just wanted us to focus on the data structure of the GOBL complement. But yes, I'm planning to add validations and calculations also. I think we're reaching a point where the structure is generally agreed on. I'll start pushing other changes soon.

cavalle · 2023-09-29T14:26:31Z

Superseded by #210

cavalle added 3 commits September 1, 2023 12:27

Add basic structs for MX fuel complement

9499268

Change and document package name

5fb582a

Make linter happy with the package name

57f722c

cavalle commented Sep 1, 2023

View reviewed changes

cavalle requested a review from samlown September 1, 2023 13:35

samlown reviewed Sep 1, 2023

View reviewed changes

samlown reviewed Sep 2, 2023

View reviewed changes

cavalle added 2 commits September 4, 2023 09:38

Move complements to mx package

e178705

Generate schema for MX complement

87305b0

cavalle force-pushed the fuel-complements branch from d344e88 to 87305b0 Compare September 4, 2023 10:04

cavalle added 4 commits September 4, 2023 10:34

Replace dashes with underscores in JSON keys

7b4ed40

Mention CFDI standard in complement description

653e8bc

Use custom totals struct instead of bill.Totals

4be0288

Add missing description

fccc293

invopop deleted a comment from samlown Sep 4, 2023

cavalle added 5 commits September 4, 2023 15:52

Add cal.DateTime and use it from fuel complement

5a3388e

Add DateTime description comment

a4a18e8

Rename Purchaser to Seller

5112d09

Merge branch 'main' into fuel-complements

ce4472f

Flatten down FuelItem into FuelLine & minor tweaks

86e2823

samlown reviewed Sep 13, 2023

View reviewed changes

cavalle mentioned this pull request Sep 29, 2023

Fuel account balance complement to MX regime #210

Merged

cavalle closed this Sep 29, 2023

cavalle mentioned this pull request Oct 3, 2023

Food vouchers complement in MX #211

Merged

samlown deleted the fuel-complements branch July 24, 2024 16:05

samlown restored the fuel-complements branch July 24, 2024 16:06

samlown deleted the fuel-complements branch July 24, 2024 16:10

		@@ -0,0 +1,58 @@
		// Package complements provides GOBL Complements for Mexican invoices
		package complements

c_ClaveTipoCombustible	Tipo de combustible
1	Gasolina menor a 92 octanos
2	Gasolina mayor o igual a 92 octanos
3	Diesel
4	Diesel Marino
5	Otros

Name	Trade-offs
`FuelComplement`	👍 short and to the point 👎 not fully meaningful, collision-able
`EWalletFuelComplement`	👍 meaningful, no collisions 👎 long schema name
`FuelAccountStatement`	👍 direct translation of `EstadoCuentaCombustibles` 👎 no mention of e-wallet, no mention of being a complement

Add basic structs for MX fuel complement #192

Add basic structs for MX fuel complement #192

Conversation

cavalle commented Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavalle Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavalle Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavalle Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

cavalle Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

cavalle Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavalle Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavalle commented Sep 12, 2023

Choose a reason for hiding this comment

cavalle Sep 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavalle commented Sep 29, 2023

cavalle commented Sep 1, 2023 •

edited

Loading

cavalle Sep 4, 2023 •

edited

Loading

cavalle Sep 1, 2023 •

edited

Loading

cavalle Sep 4, 2023 •

edited

Loading

cavalle Sep 4, 2023 •

edited

Loading

cavalle Sep 1, 2023 •

edited

Loading

cavalle Sep 1, 2023 •

edited

Loading

cavalle Sep 13, 2023 •

edited

Loading