Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0][REF] l10n_br_fiscal: Unificar classes em document #3237

Closed
wants to merge 3 commits into from

Conversation

antoniospneto
Copy link
Contributor

@antoniospneto antoniospneto commented Jul 26, 2024

O objetivo desta PR é simplificar a estrutura do módulo fiscal ao remover as classes abstratas document_workflow e document_eletronic, integrando seus campos e métodos diretamente na classe concreta document. Essa alteração visa reduzir a complexidade do código, já que apenas a classe document utiliza essas funcionalidades.

@OCA-git-bot
Copy link
Contributor

Hi @renatonlima,
some modules you are maintaining are being modified, check this out!

@antoniospneto
Copy link
Contributor Author

se for aprovado faço o port nas outras versões :)

@antoniospneto antoniospneto marked this pull request as draft July 26, 2024 18:21
@rvalyi
Copy link
Member

rvalyi commented Jul 26, 2024

Ola @antoniospneto tou sugerindo de ver isso com muito cuidado...

Com certeza o codigo atual deixa muito a desejar. Mas deixa lembrar alguns motivos historicos e fazer consideraçoes:

A)
Ate o Odoo versao 9, existia um "finite state engine"/BPM do pobre dentro do Odoo e os documentos como invoice, sale ou purchase tinham um tal workflow configuravel pelo power user. Em tese era flexivel, mas na pratica era um inferno para debugar e migrar, foi bom isso sair. Para referença https://tvtmarine.com/docs/8.0/d/workflows-and-user-processes-494

Mas enfim eh para refletir isso do BPM da v8 que a Kmee fez metodos tipo BEFORE_EXEC... AFTER_EXEC... no document_workflow.py na v10... Hoje francamente me parece bem questionavel a gente ter que ter "hooks" assim. Sera se nao basta ter metodos "normais" para quais a gente chama super().. antes ou depois do codigo no override para decidir se acontece antes ou depois? Enfim isso nunca tinha sido limpado direito.

Vale a pena mencionar que a Acsone fez uma implementaçao mais simples e mais limpa desse BPM aqui: https://github.com/acsone/scobidoo

B)
os primeiros passos da NFe tinham sido feito pela Akretion inicialmente com txt no modulo l10n_br_account (ou l10n_br_account_product na vdd) nas versoes 7 e 8. E na epoca teve contribuicoes essenciais da Kmee num outro repo onde tinha mais coisas aproximativas: https://github.com/odoo-brazil/odoo-brazil-eletronic-documents No inicio ate a versao 8 nao existia a OCA e foi na v8 que passamos o repo da Akretion onde era feito para a OCA que tinhamos acabado de criar com CampToCamp, Savoir Faire Linux, Vauxoo e Therp. Enfim por isso que era repos separados antes e com modulos separados. Na v12 fizemos um certo merge desses modulos extra no repo da OCA enfim por isso que as classes ainda eram separadas.

C)
Contudo o modulo l10n_br_fiscal ja ta MUITO acima do peso "normal" dos modulos da OCA. Tem 10k linhas de Python e 10k linhas de XML sem contar os dados... E existe outras necessidades de extender a gestao dos eventos de comunicaçao electronica...
Nisso eu tava pensando em extrair um modulo l0n_br_fiscal_edi aqui #3012
Eh bastante facil terminar, eu ate faria esses dias...

Ai vem a minha pergunta aberta; sera se nao eh melhor deixar um override separado no intuido de extrair esse codigo de workflow/eventos num modulo l10n_br_fiscal_edi? Ai por examplo os modulos que precisariam apenas do "motor fiscal" nao precisaria desse l10n_br_fiscal_edi. Hoje apenas os modulos l10n_br_nfe, l10n_br_nfse ou l10n_br_cte iriam depender desse l10n_br_fiscal_edi

O que vc acha disso? Realmente @renatonlima e eu estamos abertos pro debate a partir do momento que a proposta nao eh enfiar mais codigo que poderia ir em outros modulos no l10n_br_fiscal (o que nao eh o caso aqui).

@antoniospneto
Copy link
Contributor Author

@rvalyi valeu pelas explicações, eu me esqueci que já tem o a pr do l10n_br_fiscal_edi em aberto, concerteza é melhor organizar lá, valeu!

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.

3 participants