-
-
Notifications
You must be signed in to change notification settings - Fork 247
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_sale_stock: Extraction referent the creation of the module sale_stock_picking_invoicing #2955
base: 14.0
Are you sure you want to change the base?
Conversation
Hi @renatonlima, |
boa @mbcosta . Eu não tive tempo para analisar fundo, mas entendo o raciocínio e faz sentido faz tempo que a gente tinha esse assunto. Vamos ver o que é razoável fazer na v14 ou numa versão superior como a v16. Esse tipo de coisa a gente tem que se anticipar muito para ter uma chance de mudar porque depois a galera não querer mudar módulo "estavel". Na localização aqui a gente sempre meio que avisou que esses módulos ainda nao eram muito maduros (beta) e que por isso quem usava tinha que se preparar para seguir os potências refatores, mas no account-invoicing da 14 o pessoal com certeza deve ter a expectativa de algo mais estavel já. Com eu disse eu ainda nao analisei detalhadamente a PR, é apenas uma consideração mais genérica. Parabéns pelo trabalho! |
8b00d4c
to
10e97a6
Compare
valeu @rvalyi , eu até busquei fazer essa extração durante a migração mas não havia um retorno se haveria algum interesse fora do Brasil, mas como apareceu aquele PR incluindo coisas referentes ao Sale no stock_picking_invoicing parece que o melhor vai ser fazer essa extração para não deixar essa questão em aberto lá no repo do account-invoicing. importante também dizer que isso pode ser feito de forma independente sem afetar o stock_picking_invoicing nem o l10n_br_sale_stock, mas recomendo já buscar fazer essa integração porque nos últimos dias eu revi a implementação e busquei resolver os principais problemas que são a divergências de dados entre a Fatura criada pelo Pedido de Venda com a que é criada pela Ordem de Separação, a necessidade de "glue" módulos para todos os N módulos que podem acabar incluindo novos campos nas Faturas e o Botão "Criar Fatura" que por estar invisível bloqueia o caso de "Down Payments", outro ponto é que o script de migração é simples hoje seria apenas a mudança do nome do campo sale_create_invoice_policy para sale_invoicing_policy, abaixo tem mais detalhes sobre essas alterações. Foram feitas alterações importantes no PR do sale_stock_picking_invoicing que devem melhorar a implementação:
account_payment_sale https://github.com/OCA/bank-payment/blob/14.0/account_payment_sale/models/sale_order.py#L41 Segue um exemplo com os dados de demonstração e os modulos account_payment_sale e sale_commission instalados: Com isso a implementação se torna mais dinâmica, robusta e funcional já que qualquer modulo que seja instalado que adiciona novos campos no sale.order e usa esses métodos "prepare" para inclui-los na Fatura também serão incluídos na criação pela Ordem de Separação/stock.picking, um exemplo na Localização é o modulo l10n_sale_commission_stock que na v14 ainda é um PR em migração #2681 e que com essas alterações se torna desnecessário.
Segue imagens de teste com dados de demonstração:
Se for melhor eu posso ver de extrair esses commits ou junta-los. O erro ao rodar os testes aqui no PR são causados porque o modulo depende do sale_stock_picking_invoicing, mas já é possível testar localmente e rodar os testes da mesma forma como o CI executa esses testes. |
trabalho MUITO top hein @mbcosta ! |
value @rvalyi obrigado, preciso ver se é possível fazer algo semelhante no caso do l10n_br_purchase_stock porque no caso do l10n_br_sale_stock acredito que com esse PR teremos algo melhor do que o que existe hoje no repo. |
10e97a6
to
1da2975
Compare
Force Push para:
Segue exemplo de dois Pedidos de Vendas onde a criação da Fatura é feita a partir das Ordens de Separação/stock.picking mas em um caso o wizard é marcado para Não Agrupar( opção Coleta ) e o outro são Agrupados ( opção Parceiro/Produto ) Pedido 1 Pedido 2 Caso Não Agrupar Fatura 1 Fatura 2 Caso da Fatura Agrupada Existe uma questão que não está ligada diretamente com esse PR que é nesses casos de Documentos Fiscais com Linhas de Seção, Notas e Adiantamentos é preciso analisar se isso deve ser bloqueado ou verificar em outro PR o que deve ser feito para deixar isso funcional porque nos testes ao chamar o método action_post() isso acaba gerando erros: l10n_br_nfe/models/document.py", line 807, in _check_product_default_code l10n_br_nfe/models/document_line.py", line 503, in export_fields_nfe_40_icms Deixei isso comentado nos testes do modulo, para ver os erros é só descomentar e rodar os testes( ao instalar o l10n_br_account e em seguida o l10n_br_sale_stock e o l10n_br_nfe já é possível ver os erros ) As linhas são lançadas vazias no documento Fiscal É preciso avaliar os dados fiscais desse produto "Down Payments" |
1da2975
to
1a44201
Compare
Incluído script de migração, o que permite testes em ambiente de homologação*, dados de demonstração com linhas de Seção e Notas, e teste com o caso Adiantamentos/'down Payments', para evitar o erro l10n_br_nfe/models/document.py", line 807, in _check_product_default_code f"The product {line.product_id.display_name} " esse PR depende do #3055 ainda pendente de solução erro File "l10n_br_nfe/models/document_line.py", line 503, in export_fields_nfe_40_icms self.nfe40_choice_icms.replace("nfe40", "") AttributeError: 'bool' object has no attribute 'replace' Mas como esse PR é sobre a extração do l10n_br_sale_stock acredito que esse problema de Linhas Seção, Notas e Adiamentos no modulo l10n_br_nfe deve ser visto em outro PR e aqui apenas comentar o teste com um TODO |
1a44201
to
d4c7586
Compare
Correção para não criar Linhas do tipo Seção, Nota e Adiantamentos no Documento Fiscal no PR #3064 |
d4c7586
to
f91d829
Compare
Parece que devido a alterações recentes ao Confirmar uma Fatura ao chamar o método action_post no caso Lucro Presumido se o "Modo de Pagamento" não estiver preenchido gera erro ERROR odoo odoo.addons.l10n_br_sale_stock.tests.test_sale_stock: ERROR: TestSaleStock.test_lucro_presumido_company Isso ocorre quando o l10n_br_account_nfe esta instalado, exemplo CI do github, por isso o teste verifica se o modulo está instalado e nesse caso preenche um valor para evitar esse erro https://github.com/akretion/l10n-brazil/blob/14.0-REF-extract_for_sale_stock_picking_invoicing/l10n_br_sale_stock/tests/test_sale_stock.py#L535 |
O Rebase desse PR está dependendo do PR #3327 porque com a alteração feita o arquivo l10n_br_sale_stock/models/stock_picking.py, que antes havia sido apagado na extração, deverá ser mantido devido a função def _get_default_fiscal_operation https://github.com/akretion/l10n-brazil/blob/14.0-FIX-stock_default_op_fiscal/l10n_br_sale_stock/models/stock_picking.py#L17 |
a8b843d
to
91ec07e
Compare
91ec07e
to
b7ddc7c
Compare
O último Force-Push foi um rebase para atualizar o PR, alguns pontos:
Por favor caso alguém veja algum problema ou motivo ou algo que impeça ou que deve bloquear esse PR é importante registrar aqui até para um debate, porque essa extração já vem sendo buscada desde a migração da v12 e foram feitas melhorias que devem tornar a implementação melhor do que existe hoje por tornar a Fatura criada pelo Picking mais similar com a que é criada pelo Pedido de Venda além de evitar a necessidade dos "glue modules" como o modulo para incluir as Comissões por exemplo. |
b7ddc7c
to
2249af1
Compare
2249af1
to
beb467b
Compare
Com o merge do PR do modulo sale_stock_picking_invoicing no dia 25/10/2024 foi feito o rebase desse PR e fiz um novo rebase hoje, com isso já é possível testar tanto diretamente no runboat ou em um ambiente local, foram feitas melhorias em relação ao que existe hoje que podem ser lidas nos comentários acima. Por favor caso exista alguma restrição a inclusão desse PR peço que seja registrado aqui, já que sem esse PR a implementação ainda precisa usar os chamados "glue modules" um exemplo na Localização é o PR de migração do modulo l10n_sale_commission_stock na v14 , mas isso se aplica a todos os outros N módulos que podem existir, e que com essas alterações se tornam desnecessários, portanto a migração ou não desse modulo depende do que for decidido aqui. cc @OCA/local-brazil-maintainers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O diff é grande por causa principalmente dos testes que aumentaram (o que éum excelente coisa). Mas na real a maioria do diff é código que migrou pro novo módulo sale_stock_picking_invoicing que agora vai ser mantido por mais pessoas no repo OCA/account-invoicing
Nisso eu acho bem tranquilo fazer o merge deste refator na 14 ainda.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbcosta é engano meu ou esses arquivos poderiam ser simplesmente removidos do modulo l10n_br_sale_stock
:
- res_company.py
- res_config_settings.py
- sale_order_line.py ?
Pois os 2 primeiros definem o antigo campo sale_create_invoice_policy
que agora foi renomado sale_invoicing_policy
(seu script de migração ta OK) e que me parece que ja vem definido e usado no novo modulo sale_stock_picking_invoicing
de forma que não precisaria sobrecarregar nada aqui no l10n_br_sale_stock. Veja no sale_stock_picking_invoicing
:
- https://github.com/OCA/account-invoicing/blob/14.0/sale_stock_picking_invoicing/models/res_company.py
- https://github.com/OCA/account-invoicing/blob/14.0/sale_stock_picking_invoicing/models/res_config_settings.py
- https://github.com/OCA/account-invoicing/blob/14.0/sale_stock_picking_invoicing/models/sale_order_line.py
Extraction referent the creation of the module sale_stock_picking_invoicing.
Extração referente a criação do modulo sale_stock_picking_invoicing OCA/account-invoicing#1025 , durante a migração da v14 #1961 foi feito esse PR e essa questão foi levantada, porém havia a dúvida se fora da Localização Brasileira haveria interesse de usar essa implementação, mas recentemente essa questão foi respondida porque foi criado um PR para adaptar o modulo stock_picking_invoicing para funcionar com o Pedido de Vendas OCA/account-invoicing#1672 , e como idealmente o modulo stock_picking_invoicing é focado apenas em criar Faturas a partir da Ordem de Separação/stock.picking sem relação com Pedido de Vendas ou Compras ( "create invoices directly from picking, without having to use Sale or Purchase Orders." https://github.com/OCA/account-invoicing/blob/14.0/stock_picking_invoicing/readme/DESCRIPTION.rst ) e apesar de estar com uma dependência indireta do sale devido o modulo stock_picking_invoice_link https://github.com/OCA/stock-logistics-workflow/blob/14.0/stock_picking_invoice_link/__manifest__.py#L20 acredito que é melhor manter essa separação para isolar e modularizar as funcionalidades assim como acontece hoje na Localização, com isso uma parte do que existe aqui vai para esse novo modulo onde mantive os mesmo cabeçalhos de Licença, Mantedores, Contribuidores e Créditos.
Enquanto o PR do sale_stock_picking_invoicing não é aprovado esse PR vai estar com erro, portanto ele depende do OCA/account-invoicing#1025 mas pode ser testado localmente, caso seja aprovado vou ver de atualizar o HISTORY.rst sobre essa extração.
Foi preciso fazer uma correção no l10n_br_stock_account no método que cria a Fatura, quando o campo partner_id mapeado pelo métodos originais é diferente do _prepare_br_fiscal_dict() esse partner deve ter prioridade porque está sendo mapeado pelo método _get_partner_to_invoice, esse erro não era visto antes porque no l10n_br_sale_stock o campo era novamente informado https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_sale_stock/wizards/stock_invoice_onshipping.py#L20 se acharem melhor posso ver de extrair esse commit.
cc @renatonlima @rvalyi @mileo @gabrielcardoso21 @marcelsavegnago