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][FIX] l10n_br_stock_account, l10n_br_purchase_stock, l10n_br_sale_stock: Método para obter a Operação Fiscal Padrão #3327

Merged
merged 5 commits into from
Oct 4, 2024

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Aug 29, 2024

Fix stock default op fiscal.

Método para obter a Operação Fiscal Padrão na Ordem de Seleção/ stock.picking, depende do #3325 .

Apesar de envolver mais de um modulo o PR é simples, ao invés de usar o método default para obter a Operação Fiscal Padrão isso passou para um Onchange do campo invoice_state isso foi necessário porque não encontrei outra forma de resolver um problema do caso Sem Operação Fiscal ou Internacional

image

image

Eu tinha buscado resolver esse problema com os hasattrs mas como foi visto no PR #3288 isso gerava outro erro, a Ordem de Seleção não trazia a o Operação Fiscal Padrão, o caso de Compras até foi resolvido mas o de Vendas como estava sem o teste acabou passando, esse erro ocorre porque no momento da criação do stock.picking o metodo default era chamado mas o self.sale_id sempre era vazio.

Com esse PR ao alterar o invoice_state a Operação Fiscal deverá ser preenchida com a Padrão e buscando evitar o possível problema de ter no Pedido uma Operação Fiscal e a Ordem de Seleção outra sempre que tiver um Pedido de Vendas ou Compras associado a Operação Fiscal padrão será a mesma do Pedido, o usuário pode alterar o campo mas dessa forma forçamos a decisão de não usar a mesma do Pedido.

Separei os commits com Testes para caso alguém queira ver de buscar outra solução, se necessário também posso ver de dividir esse PR para cada modulo especifico se acharem melhor.

cc @rvalyi @renatonlima @marcelsavegnago @mileo @antoniospneto @DiegoParadeda

rvalyi
rvalyi previously requested changes Aug 29, 2024
Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

alguns detalhes de inglês para melhorar...

fiscal_operation = super()._get_default_fiscal_operation()
if self.purchase_id:
if self.purchase_id.fiscal_operation_id:
# Evita a incosistencia de ter o Pedido de Compras com uma
Copy link
Member

Choose a reason for hiding this comment

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

s/incosistencia/inconsistencia

l10n_br_sale_stock
l10n_br_purchase_stock
"""
company = self.env.company
Copy link
Member

Choose a reason for hiding this comment

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

melhor em inglês, algo como; "Meant to be overriden in modules such as l10n_br_sale, l10n_br_purchase_stock..."


def test_compatible_with_international_case(self):
"""Test of compatible with international case or without Fiscal Operation,
create Invoice but not for Brazil."""
Copy link
Member

Choose a reason for hiding this comment

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

English: "Test compatibility with international cases or without Fiscal Operation,
create an Invoice but not for Brazil."

@mbcosta mbcosta force-pushed the 14.0-FIX-stock_default_op_fiscal branch from 2bb0da6 to 01d574a Compare September 2, 2024 15:50
@mbcosta
Copy link
Contributor Author

mbcosta commented Sep 3, 2024

valeu @rvalyi , correções feitas

@marcelsavegnago
Copy link
Member

@douglascstd @WesleyOliveira98 se puderem testar, inclusive aquelas situações que tem problema para carregar operacao fiscal, eu agradeço.

Copy link
Contributor

@WesleyOliveira98 WesleyOliveira98 left a comment

Choose a reason for hiding this comment

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

[FUNCTIONAL AND CODE REVIEW]

A PR está funcionando OK da forma que foi desenvolvida

Teste 1 - Pedido com operação fiscal definida
image

O picking puxa a operação do pedido
image

Teste 2 - Pedido sem operação fiscal
image

Operação fiscal padrão definida no picking type
image

Após alterar manualmente o invoice_state do picking, o mesmo puxa a operação padrão do picking type
image

Teste 3 - Pedido sem operação fiscal
image

Picking type sem operação fiscal padrão definida
image

Operação fiscal padrão da empresa definida
image

Após alterar manualmente o invoice_state do picking, o mesmo puxa a operação padrão da empresa
image

A PR funcionou legal, porém em questão de usabilidade, ter que alterar manualmente o invoice_state do picking não ficou muito legal, principalmente porque o picking já é criado com invoice_state correto, sendo necessário mudar o status e depois voltar para o que já estava selecionado. Isso deveria vir por padrão na criação do picking sem uma ação manual do usuário.

Comment on lines 130 to 162
@api.onchange("invoice_state")
def _onchange_invoice_state(self):
for record in self:
# TODO: Na v16 chamar o super() o update_invoice_state já é
# chamado https://github.com/OCA/account-invoicing/blob/16.0/
# stock_picking_invoicing/models/stock_picking.py#L47
record._update_invoice_state(record.invoice_state)
record.mapped("move_lines")._update_invoice_state(record.invoice_state)

if record.partner_id and record.env.company.country_id == record.env.ref(
"base.br"
):

fiscal_operation = record._get_default_fiscal_operation()
if fiscal_operation:
record.fiscal_operation_id = fiscal_operation
Copy link
Contributor

Choose a reason for hiding this comment

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

Talvez essa parte aqui poderia ser removida e o campo fiscal_operation_id ter como default o método _get_default_fiscal_operation. Ter que alterar o invoice_state manualmente para puxar a operação fiscal padrão não ficou muito bom em questão de usabilidade.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ao usar o método default ocorre erro com o caso Pedido de Vendas Sem Operação Fiscal ou Internacional ao criar o Picking a partir do Pedido de Vendas o método é chamado mas não é possível identificar se o sale.order tem o fiscal_operation_id preenchido ou não e com isso a Operação Fiscal Padrão acaba sendo preenchida o que gera o erro, isso pode ser visto no teste que está sendo incluído no modulo l10n_br_sale_stock, uma possibilidade seria de sobre escrever o método create, porém isso de forma geral acaba sendo desencorajado, seria melhor? Talvez exista alguma outra forma mas não encontrei, é possível fazer um cherry-pick do commit desse testes e ver de buscar outra solução, no inicio do PR eu tentei explicar essa questão.

@mbcosta mbcosta force-pushed the 14.0-FIX-stock_default_op_fiscal branch from 01d574a to da1ff15 Compare September 27, 2024 11:16
@mbcosta mbcosta force-pushed the 14.0-FIX-stock_default_op_fiscal branch from da1ff15 to 0a4cfe0 Compare September 27, 2024 15:46
@mbcosta
Copy link
Contributor Author

mbcosta commented Sep 27, 2024

Pessoal havia um problema a Operação Fiscal era carregada apenas ao selecionar o campo porém ao clicar no botão não era preenchido, também foi feito rebase

image

valeu @WesleyOliveira98 pela revisão sobre:

"porém em questão de usabilidade, ter que alterar manualmente o invoice_state do picking não ficou muito legal, principalmente porque o picking já é criado com invoice_state correto, sendo necessário mudar o status e depois voltar para o que já estava selecionado. Isso deveria vir por padrão na criação do picking sem uma ação manual do usuário."

Você pode dar mais detalhes desse caso de uso? Porque todo Picking por padrão é criado com o invoice_state "Não Faturar" o Usuário precisa ou selecionar o campo ou clicar no botão ( talvez seja o problema corrigido, trazer a OP Fiscal ao clicar no botão), apenas nos casos onde a criação é feita a partir do Pedido de Vendas e Compras e a "Politica de Criação de Fatura" é pelo Picking e nesse caso já parece tudo certo

@WesleyOliveira98
Copy link
Contributor

Você pode dar mais detalhes desse caso de uso? Porque todo Picking por padrão é criado com o invoice_state "Não Faturar"

@mbcosta testei aqui novamente a PR e o status que já vem por padrão no picking gerado por um pedido de venda sem operação fiscal é o "To Be Invoiced"
image

E quando a pessoa valida o picking já aparece o botão "Create Invoice"
image

O que gera uma fatura sem Tipo de Documento e Operação Fiscal
image

A sua solução de puxar a operação quando apertar no botão "Set to be Invoiced" é legal e funciona também, mas ainda assim é um contorno para esse onchange do status, o botão não funcionava para puxar a operação pois o status não estava mudando, ele já era "To Be Invoiced", ainda acho que para o usuário final seria bacana já puxar na criação do picking. Isso se for possível é claro, caso não consiga, acho que seria bacana pelo menos um alerta antes de criar uma invoice através de um picking sem operação fiscal, pois uma vez criado a invoice, não é possível incluir essa informação depois
image

@mbcosta
Copy link
Contributor Author

mbcosta commented Sep 27, 2024

Desculpe @WesleyOliveira98 não sei se entendi bem o caso, vou comentar passo a passo:

"testei aqui novamente a PR e o status que já vem por padrão no picking gerado por um pedido de venda sem operação fiscal é o "To Be Invoiced""

  • Sim se a "Politica de Criação da Fatura" está como Picking vai ser criado com esse invoice_state

"O que gera uma fatura sem Tipo de Documento e Operação Fiscal"

  • Isso também é o certo se não tem Operação Fiscal não é um caso Brasil

"A sua solução de puxar a operação quando apertar no botão "Set to be Invoiced" é legal e funciona também, mas ainda assim é um contorno para esse onchange do status, o botão não funcionava para puxar a operação pois o status não estava mudando, ele já era "To Be Invoiced", ainda acho que para o usuário final seria bacana já puxar na criação do picking."

  • Mas se o Usuário definiu que o Pedido de Vendas não tem Operação Fiscal e mesmo no Picking o campo não é informado o programa "entende" que é o caso Sem Operação Fiscal

"pois uma vez criado a invoice, não é possível incluir essa informação depois"

  • Isso hoje já acontece no caso da criação da Fatura a partir do Pedido de Vendas, modulo l10n_br_sale, e essa questão chegou a ser reportada como um possível erro aqui [14.0][BUG][l10n_br_account] Impossibilidade de inserir impostos em uma fatura gerada por uma SO que não tem operação declarada.  #2877 a questão então seria definir se existe esse Caso de Uso ou não, porque o recomendado é que a Operação Fiscal seja informada já no Pedido de Vendas ou no Picking porque como todos os objetos herdam o Mix Fiscal, incluindo a Fatura/account.move, quando o Usuário informa a OP Fiscal o "motor fiscal" carrega todos os dados necessários em todos os objetos, portando a parametrização ideal é a Empresa é o responsável, ou responsáveis, pelo Fiscal configurarem as Operações usadas pela Empresa porque assim quando um Vendedor cadastra um Pedido de Vendas ele simplesmente informa a OP Fiscal e o mapeamento já é feito mesmo que seja parcial e precise se corrigido na Fatura, já que tendo um "motor fiscal" no programa é improdutivo e repetitivo ( porque existe muita chance de casos fiscais iguais ) ficar preenchendo todos os Dados Fiscais, Impostos e etc na "mão" todas as vezes ao invés de usar o "moto fiscal", bom talvez exista algum caso de uso que justifique esse comportamento mas hoje não sei dizer qual, teria?

@mbcosta
Copy link
Contributor Author

mbcosta commented Oct 1, 2024

Pessoal um ponto importante sobre o caso de uso onde Pedido de Vendas ou Picking Sem Operação Fiscal mas que na Fatura/account.move será preciso incluir a Operação Fiscal, o que foi comentado aqui pelo @WesleyOliveira98 e no issue #2877, basicamente essa mensagem

image

Acredito que isso é algo a ser resolvido no modulo l10n_br_account e não no l10n_br_sale ou l10n_br_stock_account que dizer esse PR não tem relação com esse caso de uso.

Copy link
Member

@marcelsavegnago marcelsavegnago left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@WesleyOliveira98 WesleyOliveira98 left a comment

Choose a reason for hiding this comment

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

Sim, entendo que o certo seria definir a Operação diretamente no pedido, mas ainda acho que seria bacana ela vir por padrão na criação do picking e não depender de alguma ação manual do usuário, de qualquer forma a PR atinge bem o seu propósito.

LGTM

Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

LGTM

@rvalyi
Copy link
Member

rvalyi commented Oct 3, 2024

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-3327-by-rvalyi-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 3baa7ae into OCA:14.0 Oct 4, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at f4be086. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants