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] Ao mapear a Linha de Operação Fiscal o método precisa considerar os casos onde o Partner do objeto não é o Partner para Faturar/Partner to Invoice #2849

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Jan 9, 2024

When mapping Line Fiscal Operation the Partner used can be or not the Partner to Invoice, so to has the correct mapping the method should use the Partner to Invoice.

Ao selecionar uma Operação Fiscal nas linhas o método line_definition chamado pelo _onchange_fiscal_operation_id sempre usa o campo partner_id/res.partner do objeto https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_fiscal/models/document_fiscal_line_mixin_methods.py#L342 porém existem Casos de Uso onde o Partner selecionado no objeto pode não ser o Partner to Invoice/Parceiro para Faturar, um res.partner pode ter um Contato/res.partner definido com o campo Type como Invoice e se estiver com o campo company_type como Person o método address_get retorna esse Contato/res.partner na criação da Fatura/Invoice ou quando o objeto tem um campo especifico para isso o partner_invoice_id. Ao testar e validar esse problema também foi preciso fazer algumas correções referentes, foi preciso alterar mais de um modulo nesse PR para permitir testes mas dependendo posso ver de dividir o PR, segue abaixo os detalhes das alterações em cada modulo e os Casos de Uso mapeados, por enquanto, por isso testes e a verificação se faltou atender algum Caso de Uso são bem vindos:

l10n_br_base e l10n_br_fiscal

  • apenas inclusão de Dados de Demonstração usando dois Parceiros que não estavam sendo usados antes em outros testes um definido com um Contato de Fatura e outro Contato de Entrega

l10n_br_purchase

  • esse caso pode ser remoto( qual a situação/caso de uso um pedido seria criado com o Fornecedor sendo diferente do res.partner de Faturamento? Ou usando o Endereço de Entrega? Seria o caso do dropshipping a Empresa faz o Pedido de Compra porém o Endereço de Entrega e o Cliente?) porém como ao criar uma Fatura o método do modulo purchase chama o address_get para preencher o Partner https://github.com/OCA/OCB/blob/14.0/addons/purchase/models/purchase.py#L556 isso permite que o Partner do objeto possa ser diferente do Partner to Invoice, por isso é preciso considerar o Partner retornado pelo address_get na Operação Fiscal

  • inclui nos Dados de Demonstração quatro Pedidos de Compras:
    (1) Partner principal que tem um contato com Endereço de Faturamento
    Cria a Fatura com o Contato definido com o type invoice, que é diferente do Partner Principal( TODO: Nesse caso deveria considerar esse partner do Pedido o partner_shippping_id da Fatura? Isso ocorre no caso de criar a Fatura a partir de um Picking relacionado a um Pedido de Compra quando o modulo l10n_br_purchase_stock está instalado)
    (2) Contato de um Partner definido como o Partner to Invoice
    A Fatura é criada com o mesmo Partner do Pedido
    (3) Partner que tem um Contato como Endereço de Entrega
    TODO: Deveria criar Fatura com partner_shipping_id com o Partner do Pedido de Compra?
    (4) Contato de um Partner definido como o Partner to Delivery/Parceiro de Entrega
    TODO: Não cria a Fatura com o partner_shipping_id definido, deveria estar com o contato?

l10n_br_purchase_stock

  • adicionado apenas testes, as correções necessárias para esse caso foram feitas no modulo l10n_br_stock_account por usar o address_get para definir o Partner to Invoice

l10n_br_sale

  • esse caso o objeto tem um campo partner_invoice_id e quando o Partner selecionado tem um Contato Invoice o onchange preenche o campo com esse Contato mas o usuário também pode alterar o campo manualmente, por isso a Operação Fiscal precisa considerar o partner_invoice_id e não o partner_id,

  • Correção no método _prepare_invoice https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_sale/models/sale_order.py#L172 porque ao chamar o _prepare_br_fiscal_dict e fazer o update com esse dicionario acabava substituindo o Partner que é mapeado pelos métodos do modulo Sale e a Fatura acabava sendo criada com o partner_id e não o partner_invoice_id,

Apenas o modulo sale instalado

image

image

l10n_br_sale instalado

image

image

  • adicionado nos Dados de Demontração dois Pedido de Vendas e testes para validar se os campos estão sendo preenchidos nas Faturas criadas:
    (1) Partner que possui o Endereço de Cobrança/partner_invoice_id
    (2) Partner que possui o Endereço de Entrega/partner_shipping_id

l10n_br_sale_stock

  • esse caso o Picking quando tem um Pedido de Venda/sale.order relacionado precisa usar o partner_invoice_id definido no Pedido e não o Partner do stock.picking que pode ser o partner_shipping_id do Pedido

  • Correção no metodo _get_partner_to_invoice nesse mesmo sentido de usar o partner_invoice_id do Pedido

  • Sobre os Casos de Uso antes desse PR ao criar uma Fatura onde o partner_id era igual ao partner_shipping_id esse era apagado e assim a Fatura era criada sem o Endereço de Entrega porém vendo que o comportamento padrão do modulo sale é de sempre criar a Fatura com o partner_shipping_id preenchido mesmo quando igual ao partner_id eu estou alterando nesse PR para fazer o mesmo ( TODO: Isso é o certo? Ou o melhor é manter como antes e alterar o l10n_br_sale para que quando o partner_id for igual ao partner_shipping_id deveria apagar o valor do campo e assim criar a Fatura sem o partner_shipping_id )

l10n_br_stock_account

  • esse Caso de Uso precisa considerar o Partner retornado pelo address_get( isso também atende o caso do l10n_br_purchase_stock ), a criação da Fatura já considera esse caso https://github.com/OCA/account-invoicing/blob/14.0/stock_picking_invoicing/models/stock_picking.py#L38 isso também resolve um TODO que existia que é a criação de uma Ordem de Entrega ou Recebimento/stock.picking sem relação com Pedido de Venda ou Compra que precisa ter um Endereço de Entrega/Recebimento diferente do Faturamento,

  • correção feita no método que retorna a Operação Fiscal padrão da empresa é preciso fazer self.env.company e não self.env.user.company_id

  • Incluí nos Dados de Demonstração mais quatro pickings
    (1) Partner principal que tem um Partner com Endereço de Entrega definido
    Caso Remoto? Um erro do usuário? Ou pode ocorrer um caso onde o usuário precisa fazer uma Entrega para um endereço diferente do que está definido com o type delivery
    (2) Partner de Endereço de Entrega
    A Fatura precisa ser criada com Partner principal
    (3) Partner principal que tem um Endereço de Faturamento definido
    A Fatura deve ser criada com o partner do addres_get e não do Picking
    (4) Partner de Endereço de Faturamento
    A Fatura criada deve ter o mesmo Partner do Picking

isso também está sendo feito para testar a criação de Faturas com Agrupamento Partner/Products nesses casos apesar das Faturas criadas acabarem sendo iguais não são agrupadas porque o método _get_picking_key considera o partner do picking https://github.com/OCA/account-invoicing/blob/14.0/stock_picking_invoicing/wizards/stock_invoice_onshipping.py#L316 TODO: é preciso avaliar se isso deve ser alterado para nesses casos Agrupar ou na localização ou mesmo no modulo stock_picking_invoicing

cc @renatonlima @rvalyi @marcelsavegnago @mileo

@OCA-git-bot
Copy link
Contributor

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

@mileo
Copy link
Member

mileo commented Jul 30, 2024

@marcelsavegnago esse PR não tem relação com o #3185

@marcelsavegnago
Copy link
Member

@mileo aparentemente sim.. @mbcosta pode fazer um rebase por favor ?

@mbcosta mbcosta force-pushed the 14.0-FIX-mapping_line_op_fiscal_with_partner_to_invoice branch 2 times, most recently from 96391ba to 99390e0 Compare August 13, 2024 20:40
@marcelsavegnago
Copy link
Member

@mileo aparentemente sim.. @mbcosta pode fazer um rebase por favor ?

@mileo tem sim e com certeza é um trabalho bem mais aprofundado. @mbcosta na PR #3185 tem este commit do @rvalyi que talvez queira considerar

@mbcosta
Copy link
Contributor Author

mbcosta commented Aug 13, 2024

valeu @mileo @marcelsavegnago , @rvalyi

Certo Marcel posso de ver de incluir a alteração para o related=partner_invoice_id talvez não afete esse PR porque eu estou usando um método, aqui é preciso avaliar a melhor solução para obter o Partner Fiscal:

  1. Mapeando o campo via related e um metodo
  2. Apenas via metodo

Fiz o rebase porém atualizei o PR, então para quem tinha visto antes a principal diferença e que mudei o nome do metodo para _get_partner_fiscal e além do Mapeamento da Linha de Operação Fiscal também alterei dentro do Mix de Métodos Fiscais de self.partner_id para self._get_partner_fiscal() em

estou considerando ao invés de herdar esse método em vários objetos colocar a maioria dos casos no super l10n_br_fiscal/models/document_fiscal_line_mixin_methods.py :

    def _get_fiscal_partner(self):
        """
        Metodo para ser herdado e permitir mapear o Partner para os casos onde
        o objeto pode ter um Partner to Invoice diferente do Partner,
        nesses casos é preciso usar o Partner to Invoice, isso acontece por
        exemplo no Pedido de Vendas, Compras e Picking.
        """
        for record in self:
            partner = record.partner_id
            if record._name == "l10n_br_fiscal.document.line":
                partner = record.document_id.partner_id
            elif record._name == "account.move.line":
                partner = record.move_id.partner_id
            elif record._name in ("purchase.order.line", "stock.move"):
                partner = record.env["res.partner"].browse(
                    partner.address_get(["invoice"]).get("invoice")
                )
                if record.name == "stock.move":
                    if hasattr(record, "sale_line_id") and record.sale_line_id:
                        partner = record.sale_line_id.order_id.partner_invoice_id
            elif record._name == "sale.order.line":
                partner = record.order_id.partner_invoice_id

            return partner

Com isso ficaria desnecessário incluir nos outros módulos diminuindo código, talvez não seja o ideal mas como o PR também está com Testes depois do merge vai ser possível refatorar até ter um solução melhor, o que vocês acham?

Os Dados de Demonstração e Testes também foram separados nos Commits permitindo cherry-pick para caso alguém também queira ver de testar soluções.

@marcelsavegnago
Copy link
Member

@mbcosta pode fazer um rebase por favor ?

@mbcosta mbcosta force-pushed the 14.0-FIX-mapping_line_op_fiscal_with_partner_to_invoice branch from 99390e0 to a5048d0 Compare August 24, 2024 20:40
@marcelsavegnago
Copy link
Member

@mbcosta podemos considerar como pronto para revisão ?

@mileo
Copy link
Member

mileo commented Aug 27, 2024

@mbcosta nao testei funcionalmente já testo, mas o único conceito que eu acho que pode agregar é:

No caso de um contato + empresa no partner_id, jogar no partner_invoice_id o contato da empresa. E se a empresa tiver um endereço de faturamento pegar ele.

Pq no Brasil o endereço de faturamento é meio falho e diferente do de cobrança como o @DiegoParadeda comentou no PR do Marcel.

Ou ainda adicionar uma configuração pra decidir isso.

Mas eu vou testar assim que possível junto com ele e deixo um feedback aqui.

@mileo
Copy link
Member

mileo commented Aug 27, 2024

Fiz um teste no runbot e para esse caso de uso ainda não esta legal:

http://oca-l10n-brazil-14-0-pr2849-a5048d01f7a2.runboat.odoo-community.org/web#id=70&action=666&model=sale.order&view_type=form&cids=3&menu_id=491

image

Quando um pedido é lançado para um contato, o comercial_partner_id não é respeitado.

image

E ai o documento fiscal fica sem dados, deve ser considerado o comercial_partner_id ou sugiro alterar o método address_get para retornar no tipo invoice o comercial partner id e poderíamos criar um parâmetro para definir isso por empresa.

@mileo
Copy link
Member

mileo commented Aug 27, 2024

Na venda 50 alterei manualmente:

image

E ai os dados do fiscal foram carregados:

image

@mileo
Copy link
Member

mileo commented Aug 27, 2024

Tirando os comentários acima, parece uma implementação robusta.

@DiegoParadeda
Copy link
Contributor

@mbcosta testei aqui e tudo parece estar funcionando conforme o esperado, tá bem legal. Vejo que daria pra melhorar duas coisas só:

Primeiro seria esse ponto que o @mileo já comentou, sobre o valor default do campo partner_invoice_id na SO. No caso de ter um contado tipo "KMEE, Diego" e assumindo que nem Diego e nem KMEE tenham subcontatos do tipo invoice, então ao criar uma venda com o parceiro "KMEE, Diego" acho que seria legal que o contato KMEE viesse como padrão no campo partner_invoice_id. É claro que o usuário pode forçar manualmente o partner_invoice_id depois se quiser, mas imagino que o padrão seja faturar para a empresa.

O segundo ponto é que acho que a inclusão do campo partner_invoice_id no stock.picking ajudaria demais na usabilidade. Concordo 100% com os casos que você criou pra faturar um picking sem relação com Pedido de Vendas ou Compras, mas no caso eu tive que ler o código pra entender isso, acho que o usuário vai ficar bem confuso quando ver que o contato na fatura nem sempre bate com o contato do picking sabe?

De qualquer forma, já está muito legal o PR

@mbcosta
Copy link
Contributor Author

mbcosta commented Aug 30, 2024

Pessoal vendo de responder as Revisões percebi que estava corrigindo o problema nas Linhas mas não no cabeçalho/corpo do objeto que herdam o Mixin do Documento Fiscal, exemplo era corrigido no sale.order.line mas não no sale.order, fiz a mesma solução das Linhas.

Até onde vi o campo era usado apenas no _onchange_partner_id_fiscal então alterei para buscar pelo método document_fiscal_mixin_methods.py

De

    @api.onchange("partner_id")
    def _onchange_partner_id_fiscal(self):
        if self.partner_id:
            self.ind_final = self.partner_id.ind_final
            for line in self._get_amount_lines():
                # reload fiscal data, operation line, cfop, taxes, etc.
                line._onchange_fiscal_operation_id()

Para

    def _get_fiscal_partner(self):
        """
        Método para ser herdado e permitir mapear o Partner para os casos onde
        o objeto pode ter um Partner to Invoice diferente do Partner,
        nesses casos é preciso usar o Partner to Invoice, isso acontece por
        exemplo no Pedido de Vendas, Compras e Picking.
        """
        self.ensure_one()
        # Caso Fatura, o campo informado pelo usuário tem prioridade
        # e não é considerado quando o address_get retorna um Contato
        # definido como Tipo Invoice.

        return self.partner_id

    @api.onchange("partner_id")
    def _onchange_partner_id_fiscal(self):
        partner = self._get_fiscal_partner()
        if partner:
            self.ind_final = partner.ind_final
            for line in self._get_amount_lines():
                # reload fiscal data, operation line, cfop, taxes, etc.
                line._onchange_fiscal_operation_id()

Como tanto na Linha como no Cabeçalho o super por padrão devolve o self.partner_id isso deve garantir compatibilidade com todos os casos que já funcionam hoje.

Os commits estão separados para permitir cherry-picks e extrações, porém acredito que será melhor o merge com a correção tanto da Linha como do Cabeçalho juntos para evitar diferenças entre os objetos.

Os testes e Dados de Demonstração também estão em commits separados para, caso se queira, poderem ser extraídos para testar outras soluções.

Nesse PR está sendo considerado o cadastro de Endereços o Padrão do Odoo na v14, basicamente com os Endereços com o campo is_company=False, a questão de alterar o o método padrão address_get pode ser debatido mas acho melhor em outro PR ou RFC, e talvez apenas na v16 ou superior, até para confirmar se houve alguma mudança, os testes devem ajudar nisso.

No sale.order também estava faltando alterar o related dos campos cnpj_cpf, legal_name e ie de related=partner_id.cnpj_cpf para related=partner_invoice_id.cnpj_cpf , em Compras/purchase.order isso é um TODO porque hoje não existe um campo para fazer esse related (Deveríamos criar um fiscal_partner_id no Mixin do Documento Fiscal? Ou teríamos alguma forma de solucionar?).

Alguns pontos a estar atento na revisão:

Lembrando, e para reforçar, que como o super do método _get_fiscal_partner devolve por padrão o sef.partner_id hoje não deve haver incompatibilidades.

Obrigado a todos pelas Revisões vou buscar responder logo em seguida, precisava ver de explicar os últimos commits.

@mbcosta
Copy link
Contributor Author

mbcosta commented Aug 30, 2024

valeu @marcelsavegnago , acredito que agora sim, porque o problema passou a ser tratado tanto na Linha quanto no Cabeçalho/Corpo do objeto

@mileo
Copy link
Member

mileo commented Sep 4, 2024

Pessoal vendo de responder as Revisões percebi que estava corrigindo o problema nas Linhas mas não no cabeçalho/corpo do objeto que herdam o Mixin do Documento Fiscal, exemplo era corrigido no sale.order.line mas não no sale.order, fiz a mesma solução das Linhas.

Até onde vi o campo era usado apenas no _onchange_partner_id_fiscal então alterei para buscar pelo método document_fiscal_mixin_methods.py

De

    @api.onchange("partner_id")
    def _onchange_partner_id_fiscal(self):
        if self.partner_id:
            self.ind_final = self.partner_id.ind_final
            for line in self._get_amount_lines():
                # reload fiscal data, operation line, cfop, taxes, etc.
                line._onchange_fiscal_operation_id()

Para

    def _get_fiscal_partner(self):
        """
        Método para ser herdado e permitir mapear o Partner para os casos onde
        o objeto pode ter um Partner to Invoice diferente do Partner,
        nesses casos é preciso usar o Partner to Invoice, isso acontece por
        exemplo no Pedido de Vendas, Compras e Picking.
        """
        self.ensure_one()
        # Caso Fatura, o campo informado pelo usuário tem prioridade
        # e não é considerado quando o address_get retorna um Contato
        # definido como Tipo Invoice.

        return self.partner_id

    @api.onchange("partner_id")
    def _onchange_partner_id_fiscal(self):
        partner = self._get_fiscal_partner()
        if partner:
            self.ind_final = partner.ind_final
            for line in self._get_amount_lines():
                # reload fiscal data, operation line, cfop, taxes, etc.
                line._onchange_fiscal_operation_id()

Como tanto na Linha como no Cabeçalho o super por padrão devolve o self.partner_id isso deve garantir compatibilidade com todos os casos que já funcionam hoje.

Os commits estão separados para permitir cherry-picks e extrações, porém acredito que será melhor o merge com a correção tanto da Linha como do Cabeçalho juntos para evitar diferenças entre os objetos.

Os testes e Dados de Demonstração também estão em commits separados para, caso se queira, poderem ser extraídos para testar outras soluções.

Nesse PR está sendo considerado o cadastro de Endereços o Padrão do Odoo na v14, basicamente com os Endereços com o campo is_company=False, a questão de alterar o o método padrão address_get pode ser debatido mas acho melhor em outro PR ou RFC, e talvez apenas na v16 ou superior, até para confirmar se houve alguma mudança, os testes devem ajudar nisso.

No sale.order também estava faltando alterar o related dos campos cnpj_cpf, legal_name e ie de related=partner_id.cnpj_cpf para related=partner_invoice_id.cnpj_cpf , em Compras/purchase.order isso é um TODO porque hoje não existe um campo para fazer esse related (Deveríamos criar um fiscal_partner_id no Mixin do Documento Fiscal? Ou teríamos alguma forma de solucionar?).

Alguns pontos a estar atento na revisão:

Lembrando, e para reforçar, que como o super do método _get_fiscal_partner devolve por padrão o sef.partner_id hoje não deve haver incompatibilidades.

Obrigado a todos pelas Revisões vou buscar responder logo em seguida, precisava ver de explicar os últimos commits.

Legal, dei uma testada hj e amanhã vou pedir para um cliente validar em dev.

Fiz esse módulo rapidinho aqui para alterar o comportamento do onchange:

https://github.com/kmee/kmee-odoo-addons/blob/14.0/sale_enforce_commercial_partner/models/sale_order.py

posso jogar no sale-workflow em breve.

@rvalyi
Copy link
Member

rvalyi commented Sep 4, 2024

@mileo olhando rapidamente seu módulo, eu acho que faz sentido propor no sale-workflow. (Porem me parece que na versões 16 e 17 pelo menos o proprio modulo sale ja faz isso), apenas teria que chamar o super de forma simples super() e add_list não fica uma abreviação legal, eu acho melhor botar address_list mesmo, pois add apenas sugere "adição".

@rvalyi rvalyi force-pushed the 14.0-FIX-mapping_line_op_fiscal_with_partner_to_invoice branch 4 times, most recently from d29e3e6 to ce50c88 Compare September 26, 2024 16:20
Invoice should be create with partner_invoice_id not with partner_id.

[FIX] l10n_br_sale: Document get Fiscal Partner

[FIX] l10n_br_sale: Related Fiscal Partner

[FIX] l10n_br_sale: partner_invoice_id fix

When mapping the Line Fiscal Operation and Taxes the Partner of the object can be or not the Partner to Invoice, in case of Sale should it use the field partner_invoice_id.
for the case where the Partner of the object can be or not the Partner to Invoice.
When Picking has a Sale Order related the Partner used to create the Invoice should be the partner_invoice_id of Sale, because the Partner of Picking can be the partner_shipping_id of Sale Order.

[FIX] l10n_br_sale_stock: Get Fiscal Partner
When mapping the Line Fiscal Operation and Taxes the Partner of the object can be or not the Partner to Invoice, in case of Picking with related a related SO, it should use the partner_invoice_id field in Sale because the Partner of Picking can be the partner_shipping_id of the SO.
@rvalyi rvalyi force-pushed the 14.0-FIX-mapping_line_op_fiscal_with_partner_to_invoice branch from ce50c88 to 5052fea Compare September 26, 2024 16:22
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 Sep 26, 2024

valeu pelo trabalho @mbcosta !

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-2849-by-rvalyi-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 26, 2024
Signed-off-by rvalyi
@OCA-git-bot
Copy link
Contributor

@rvalyi your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-2849-by-rvalyi-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

When mapping the Line Fiscal Operation and Taxes the Partner of the object can be or not the Partner to Invoice, in the Picking case the code use address_get to create Invoice.

[FIX] l10n_br_stock_account: Get Fiscal Partner
for the case where the Partner of the object can be or not the Partner to Invoice.
when creating an Invoice use the Partner mapping by the methods of module instead of Fiscal Dict and inform partner_shipping_id with Partner of Picking instead in the case of it's different of Partner to Invoice.
@rvalyi rvalyi force-pushed the 14.0-FIX-mapping_line_op_fiscal_with_partner_to_invoice branch from 5052fea to 3efb527 Compare September 26, 2024 17:04
@rvalyi
Copy link
Member

rvalyi commented Sep 26, 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-2849-by-rvalyi-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 18c06c8 into OCA:14.0 Sep 26, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 2ab05b9. 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.

9 participants