OCA / l10n-mexico

Mexican Localization.
GNU Affero General Public License v3.0
3 stars 19 forks source link

Base modules for CFDI Issuing #30

Closed azubieta closed 7 months ago

azubieta commented 1 year ago

Add base modules to store the CFDI catalogs required to emit Invoices, Payment and Credit Note documents.

Closes #29 Related to #28

azubieta commented 1 year ago

This is a very early proposal, notice that I'm still getting used to the OCA coding standards so feel free to point out any mistake I have made.

rvalyi commented 1 year ago

Hello @azubieta . Could you point me to the xsd files you should comply to for you electronic invoicing, cause eventually I might show you how to get all these entities entirely generated. In some countries where xsds are like 500 lignes of code a manual approach is better. In some others like Brazil where it's 8000 lignes just for the invoice, generating these bindings is a huge win. I have no idea where Mexico stands in this game, but if you point me to the xsds, I can probably tell you...

azubieta commented 1 year ago

Hello @rvalyi you can find the xsd here http://www.sat.gob.mx/sitio_internet/cfd/4/cfdv40.xsd I think that having the base model generated from the XSD would be a bit win and would save us a lot of time.

rvalyi commented 1 year ago

Hello @rvalyi you can find the xsd here http://www.sat.gob.mx/sitio_internet/cfd/4/cfdv40.xsd I think that having the base model generated from the XSD would be a bit win and would save us a lot of time.

all right, what do you think about these xsdata bindings for generating and parsing the XML (and doing validation, pydantic beeing an option too): https://gist.github.com/rvalyi/d78d1e309694fb54cef38d938bc41917

here you can find the documentation how to use it to parse an XML document or on the contrary to serialize a the Python binding as the XML document (doing both is a cool way to do tests):

https://xsdata.readthedocs.io/en/latest/xml.html

How did I generated it? I downloaded the main schema and the included schemas in a nfelib/schemas/cfd directory (that is matching what we use here in Brazil). I also edited the schema inclusion statements to make it a local file import (it wouldn't work otherwise) and then I did:

xsdata generate nfelib/schemas/cfd --package nfelib.bindings.cfd.v4_0

I will now post an other comment about how to have Odoo abstract mixins with all this data structure ready to be mapped into the existing Odoo objects or new ones and how these Odoo object can be created from XML files or on the contrary generate XML files with minimal mapping code...

rvalyi commented 1 year ago

so this is now the serie of Odoo abstract models corresponding to this schema (it's a bit like the files you wrote manually in this PR right?) https://gist.github.com/rvalyi/6f158be7e8865cd24833a4b2791851c6 (xsdata has many generation option, we can also split the classes in several files)

This was generating using xsdata generate nfelib/schemas/cfd --package nfelib.bindings.cfd.v4_0 --output=odoo

After installing my xsdata-odoo plugin for xsdata: https://github.com/akretion/xsdata-odoo

EDIT: some Float and Monetary fields are not properly inferred here. But it would be easy to tweak xsdata-odoo to get the job done as I did for Brazil where it works 100%.

rvalyi commented 1 year ago

So you can find an equivalent on how we inject these xsd mixins into the Odoo objects for our Brazilian electronic invoicing in these files: https://github.com/akretion/l10n-brazil/blob/14.0-xsdata-l10n_br_nfe-originalCase/l10n_br_nfe/models/document.py (this is a PR branch because we are exactly switching the generator from generateDS to xsdata) here is an example how we can import XML invoice files for instance: https://github.com/akretion/l10n-brazil/blob/14.0-xsdata-l10n_br_nfe-originalCase/l10n_br_nfe/hooks.py#L41

basically this is the module where the magic happen that plug Odoo models directly with the Python bindings: https://github.com/akretion/l10n-brazil/tree/14.0-xsdata-l10n_br_nfe-originalCase/spec_driven_model

I presented it in the OCA 2 years ago. It has been in production for 3 years in the 12 and 14 branch of the OCA/l10n-brazil with hundreds of thousands of electronic invoices: https://www.youtube.com/watch?v=6gFOe7Wh8uA

In Brazil, considering all the fiscal documents we should take care (not just the invoices), this represent more than 200k lines of code that would otherwise have to write manually.

Note: xsdata is also able to do the SOAP transmission.

I'm not telling you should use it for Mexico, but I' want to make sure you know about this option.

Actually a first set would be to use the pure Python bindings I provided in the 1st post to write and parse the XML. But you can do much more than that...

rvalyi commented 1 year ago

@azubieta too bad, your schema is in fact creating a bug (an infinite loop it seems) with xsdata. In my previous posts, the catCFDI.xsd file was not properly included and this is why some fields where not properly casted and fields.Selection were also kept as Char (it should be like this, in Brazil for instance: https://github.com/akretion/l10n-brazil/blob/14.0-xsdata-l10n_br_nfe-originalCase/l10n_br_nfe_spec/models/v4_0/leiaute_nfe_v4_00.py

I just reported the bug here https://github.com/tefra/xsdata/issues/776

really xsdata has an extraordinary large test suite, bugs are very rare (not a single one for 200k lines of generated Python and dozens of xsd here in Brazil), this is bad luck or may be an issue in the xsd itself. May be we will know soon because the xsdata author is really active. If we get a fix I promiss I try again.

azubieta commented 1 year ago

@rvalyi having our models generated from the spec would be great as it could save us a lot of effort. I have been playing with xsdata and was able to make some progress.

If enumeration values are removed using cat catCFDI.xsd | grep 'xs:enumeration value=' -v > catCFDI_min.xsd the generation is done properly. Those values will have to be linked later because we need to show not only the codes but the names.

rvalyi commented 1 year ago

great. As I explained it in the OCA video, I think there are several stages: only use the bindings or use the full blown models. for us in Brazil the question is we really would need to progressively write 200k lines of data structures and mapping code if we would like to support all the Brazilian fiscal documents. The situation is probably not so extreme in Mexico. The advantages of this technique are also more obvious if you consider the electronic order importation and not only the generation. this is done in the spec_driven_module in OCA/10n-brazil but I would like to move it to OCA/server-tools some day, may be sooner if you use it too.

also there is something I need to improve: use a company_currency_id field like in account.move instead of the current brl_currency_id. Again this is this way currently for backward compatibility while we are switching from generateDS to xsdata.

You are the best to know what is the best for your localization, but if you need assistance or small changes in these tools I'm ready to help out.

rvalyi commented 1 year ago

if you would like to move this conversation elsewhere please tell me. But also, it's worth mentioning:

  1. we prefix the fields from the schema name (XSDATA_SCHEMA en var) to avoid field name collision as there are many fields, seems a bit like how you did in your manual version.
  2. we also put a few digits for the schema version in this prefix (using XSDATA_VERSION). so a minor schema evolution will use the same tables and fields and assume --update will do the job. But for a major version, that would change the tables and the fields and allow you to work with several schema versions...
  3. you can export XSDATA_LANG=spanish before the generation, it may help to extract the field strings attributes from xsd annotations.
  4. if you want to use the full blown models I encourage you to study the spec_driven_model tests with the classical PurchaseOrder.xsd from Microsoft, it will be a simpler start.
  5. if you need further assistance please tell me.
azubieta commented 1 year ago

@rvalyi I have created this script to generate the mixin but I have some dubs about how to proceed with the integration with the existent models. Provably I need to study better the materials you shared. In case you have a chance, please take look at the code I just uploaded.

azubieta commented 1 year ago

@rvalyi I was able to turn the generated Mixins into concrete models with the permissions set. To achieve that, I had to patch the spec_driven_mdoel to work on Odoo 15.0. It would be great if you could take a look at my changes at https://github.com/azubieta/l10n-brazil

Now will proceed to test the xml import/export features. If this gets done successfully we will be able to use the full spec driven approach on our localization and therefore we would have to move the module outside l1n-brazil.

rvalyi commented 1 year ago

@rvalyi I was able to turn the generated Mixins into concrete models with the permissions set. To achieve that, I had to patch the spec_driven_mdoel to work on Odoo 15.0. It would be great if you could take a look at my changes at https://github.com/azubieta/l10n-brazil

Now will proceed to test the xml import/export features. If this gets done successfully we will be able to use the full spec driven approach on our localization and therefore we would have to move the module outside l1n-brazil.

Hello @azubieta this is great news! Sorry I had no time, but here are some comments:

  1. did you see xsdata author just fixed the issue with xsdata for your schemas? I told you the guy is really serious, I reported the issue the 13 and he fixed it the 17: https://github.com/tefra/xsdata/issues/776 did you use it (you should install the master branch from Github then) for your test?
  2. as for understanding well how to integrate the abstract mixins with concrete models, you better look at the canonical example in spec_driven_model: https://github.com/akretion/l10n-brazil/tree/14.0-xsdata-l10n_br_nfe-originalCase/spec_driven_model/tests

Again you better also look my video https://www.youtube.com/watch?v=6gFOe7Wh8uA and our NFe (Brazilian Electronic Invoice) example in the same branch:

VERY IMPORTANT NOTE about field names

if you use the default xsdata generation you will notice the fields get snake_cased. It means it can change from the original XML tags and possibly makes it harder to use the lib. In Brazil, we decided (also for backward compat with generateDS) to use the "originalCase" option for fields that will preserve the names as they are in the xsd (or in the xsdata-odoo generation). Of course some fields still need to be changed to avoid reserved names/existing Class names. xsdata has some support to detect Python keywords but would typically fail to deal with fields named like Class names. So in Brazil we use this .xsdata.xml configuration for the generation: https://github.com/akretion/nfelib/blob/master-xsdata/.xsdata.xml it may help you too.

I'm sorry, as I told you it has been rocking in production for 3 years and dozens of thousands of electronic invoices, but we are now switching the framework from generateDS to xsdata and it takes some time to make a smooth transition. For instance you can see we package the pure xsdata binding in e Python package https://github.com/akretion/nfelib and as you can see I'm still organizing a bit the switch from generateDS. As for the xsdata-odoo abstract model, we package them in an "_spec" module we depend on: https://github.com/akretion/l10n-brazil/tree/14.0-xsdata-l10n_br_nfe-originalCase/l10n_br_nfe_spec so other modules could also use it.

One last point: as for access rights: the way spec_driven_model works today is: either you inject abstract mixins into existing concrete Odoo models, either, non injected asbtract models can auto-magically get turned concrete with this hook: https://github.com/akretion/l10n-brazil/blob/14.0-xsdata-l10n_br_nfe-originalCase/spec_driven_model/hooks.py#L135 And finally in some module injecting abstract models into Odoo models, you can also auto-magically generate default access rights for these class by calling this post_init_hook: https://github.com/akretion/l10n-brazil/blob/14.0-xsdata-l10n_br_nfe-originalCase/spec_driven_model/hooks.py#L15 in l10n_br_nfe for instance we call these hooks here https://github.com/akretion/l10n-brazil/blob/14.0-xsdata-l10n_br_nfe-originalCase/l10n_br_nfe/hooks.py#L20 Important: I designed these hooks in a way you could extend the mapping module. Like for instance, the Brazilian NFe schema defines default classes for fleets. Our l10n_br_nfe module doesn't depend on the fleet Odoo module. But we could make a new l10n_br_fleet module that would extend both the l10n_br_nfe and fleet modules so that instead of using the default fleet objects coming from the NFe xsd, our electronic invoice would suddenly use the Odoo fleet models with the provided mapping how it maps to the fleet xsd classes... These register_hook are called after all modules are loaded and this is what makes it possible.

I'll make my best to assist you and make the required changes if you are willing to trill this path for the full blown XML to Odoo model mapping for your localization. I'm also willing to move spec_driven_model to OCA-server/tools or OCA/edi ? But may be we better adjust it first together before it becomes too bureaucratic to evolve.

I also promise I will take a look to what you did.

Best regards.

cc @renatonlima @antoniospneto @marcelsavegnago @max3903

azubieta commented 1 year ago

@rvalyi thanks a lot for the hints. I was able to make some progress but I found that some generated models have the same name which causes conflicts in Odoo.

Right now I'm trying to figure out a way of generating a different names from xsdata-odoo. Hints are welcome.

rvalyi commented 1 year ago

Hello @azubieta what names are the same for instance? Something I can think about:

1) may be two inner xsds tags have the same names despite they belong to a different xml hierarchy. In this case the pure Python xsdata bindings will be properly nested by default so names won't collide. However, in Odoo we declare abstracts models in a flat sequence and indeed in this case names could collide. A solution might be to change (or override) the definition of the registry_name in xsdata-odoo/filters.py in order to also print the the parent(s) class name(s). I advice you try on your local xsdata-odoo fork. we could then think how to activate such features or not (always active would be very verbose)

2) eventually you have the issue right with xsdata pure Python bindings already. then you could try to generate the bindings of some xsd files separately with the -ss single-package option.

did any of this fix your problem?

azubieta commented 1 year ago

Hi @rvalyi sorry for giving you so few details. I ended exhausted yesterday.

may be two inner xsds tags have the same names despite they belong to a different xml hierarchy

That's exactly what is happening. The following names are duplicated

"l10n_mx_cfdi.4_0.retencion"
"l10n_mx_cfdi.4_0.informacionaduanera"
"l10n_mx_cfdi.4_0.traslado"
"l10n_mx_cfdi.4_0.retenciones"
"l10n_mx_cfdi.4_0.impuestos"
"l10n_mx_cfdi.4_0.traslados"
  1. A solution might be to change (or override) the definition of the registry_name in xsdata-odoo/filters.py in order to also print the the parent(s) class name(s)

I thought the same, but if I just add the whole parent list the names will be to long for Odoo and it would not work. We would have to define a naming method that creates minimal names. I'll thinker about it a bit today and get back to you.

rvalyi commented 1 year ago

hello @azubieta , here is an idea: nested XML elements don't hold any explicit many2one relationship to their parent. But in Odoo we need it: for instance the invoice_id m2o in the invoice.line.

So in xsdata-odoo I deal with the relationship, to create the appropriate fields.Many2one fields. look in filters.py, it is called implicit_many2ones and it's available in each object that is being templatized as an Odoo abstract model.

And you can look in generator.py, we first collect all these implicit_many2ones.

My idea would be: while these implicit_many2ones are being collected we would also kind of maintain a new dict of unique Odoo model names. And if at some point some name is already found: we be would prepend the name of the parent object both in the existing name and in the new one. And we keep stacking the parents until the name is unique.

then in filters.py, in the registry_name filter, we take this unique name from this dict we passed in the filters initialization (just like the implicit_many2ones dict for instance).

is that clear? what do you think? I would happily merge such a feature.

azubieta commented 1 year ago

Hello @rvalyi, I tried to fix the duplicated names issues on xsdata-odoo with very little success. You will find my changes at: https://github.com/akretion/xsdata-odoo/pull/7

So far I have spend a week and a half working around xsdata-odoo. It could be really helpful and save time. But IMHO it's not ready to be used in other projects. Therefore I'll manually fix the generated code with the hope of some day getting this tool complete for my scenario. We could continue collaborating on this topic on the xsdata-odoo PR.

I would like to thank you very much for the time you've spend.

rvalyi commented 1 year ago

Hello @rvalyi, I tried to fix the duplicated names issues on xsdata-odoo with very little success. You will find my changes at: akretion/xsdata-odoo#7

So far I have spend a week and a half working around xsdata-odoo. It could be really helpful and save time. But IMHO it's not ready to be used in other projects. Therefore I'll manually fix the generated code with the hope of some day getting this tool complete for my scenario. We could continue collaborating on this topic on the xsdata-odoo PR.

I would like to thank you very much for the time you've spend.

Thank you for the PR @azubieta I'll take a look. As for this duplication issue with your schema (doesn't affect us in Brazil) it seems you nearly nailed it. So more work for using your PR, would mean, adapting the relational fields to point to these new unique model names right? I'll give it a try, if it's all what is missing (something that is worth checking with your schema is the numerical field detection too), then there are chance I fix that over the week end. I'll let you know in the xsdata-odoo PR. Meanwhile if you could separate in isolated commits what you needed to fix manually for your use case, it may help me to bridge the gap in the generation.

rvalyi commented 1 year ago

@azubieta to be complete, here is another use case where we use the xsdata-odoo model generator but with huge csv inputs (that we actually extract from 2000 pages pdf tables using camelot previously) instead of the standard xsd input. But it also shows you how you could spin your own customized Generator and Filters overrides and do pretty much anything you want: https://github.com/akretion/sped-extractor/blob/master/spedextractor/gen_odoo.py#L248

But I mean, you could also very much but some nasty if/else that would do exactly what you would need for your own schema without having to think too much about how generic things should be or not.

Again in Brazil we have so much of these SOAP documents that it's worth the time spent in these tools (like just for the service electronic invoices I got xsdata to generate these bindings https://github.com/akretion/nfselib to support 550 major Brazilian cities. As as you could check with cloc, without xsdata, just for these pure Python bindings that would mean 250k lines of code to maintain manually (not event talking about xsdata-odoo we won't use for these peculiar city specific SOAP schemas).

azubieta commented 1 year ago

Progress update, I took a step back from using the spec_driven_models approach yet we keep using xsdata. 'xsdata-odoo' mixins will be used were required.

The current approach moves the xsdata generated lib to a pip package named simple-cfdi which provides xml serialization/de-serialization features. In the future this module will also include other features such as signature check. We will use the xml as the main source of truth, only relevant fields will be made available on the main class. To create a CFDI we will use the Comprobante class of the simple-cfid library this provides a simple way of populating or consulting the document information.

Next steps:

azubieta commented 7 months ago

I'm releasing our private implementation check the other PR.