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][IMP] l10n_br_sale_stock: get analytic account and analytic tags from sale order #2797

Conversation

marcelsavegnago
Copy link
Member

@marcelsavegnago marcelsavegnago commented Nov 23, 2023

No description provided.

@OCA-git-bot
Copy link
Contributor

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

@marcelsavegnago marcelsavegnago changed the title [14.0][IMP] l10n_br_sale_stock: get analytic account and analytic tags from… [14.0][IMP] l10n_br_sale_stock: get analytic account and analytic tags from sale order Nov 23, 2023
@marcelsavegnago marcelsavegnago force-pushed the 14.0-l10n_br_sale_stock-copy-analytic_data-to-invoice branch from 7c2d77d to b794cd3 Compare November 23, 2023 18:45
Copy link
Contributor

@antoniospneto antoniospneto left a comment

Choose a reason for hiding this comment

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

Ver o comentário: #2796 (review)

Será que não vale a pena por essa implementação fora da localização ?

@marcelsavegnago
Copy link
Member Author

marcelsavegnago commented Nov 24, 2023

Ver o comentário: #2796 (review)

Será que não vale a pena por essa implementação fora da localização ?

@antoniospneto Não consegui visualizar isso fora da localização porque entendo que a questão da politica de faturamento de pedido de venda/compra feito no cadastro da empresa é algo próprio da localização.

@marcelsavegnago
Copy link
Member Author

marcelsavegnago commented Nov 24, 2023

@antoniospneto sinceramente estive pensando inclusive em não implantar o sistema com a politica de faturamento via picking porque comecei a perceber certos problemas. Porém, nossos clientes normalmente estão optando pelo faturamento de pedido de venda e compra pelo picking.

@rvalyi
Copy link
Member

rvalyi commented Nov 25, 2023

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-2797-by-rvalyi-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f3e02b0 into OCA:14.0 Nov 25, 2023
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@mbcosta
Copy link
Contributor

mbcosta commented Nov 25, 2023

@antoniospneto alguns pontos sobre o stock_picking_invoicing, como o Marcel disse o objetivo lá é apenas criar Fatura/Invoice a partir do Picking, mas devido a dependência do modulo stock_picking_invoice_link https://github.com/OCA/account-invoicing/blob/14.0/stock_picking_invoicing/__manifest__.py#L10 o sale_stock está sendo instalado https://github.com/OCA/stock-logistics-workflow/blob/14.0/stock_picking_invoice_link/__manifest__.py#L20 acredito que quando foi feita a inclusão do modulo isso acabou passando despercebido e talvez o melhor lá vai ser remover essa dependência, talvez na v16. Em 2021 houve uma tentativa de extração do l10n_br_sale_stock para criar um modulo sale_stock_picking_invoicing no repo account-invoicing OCA/account-invoicing#1025 a ideia seria fazer o mesmo com o l10n_br_purchase_stock, mas o PR ficou em aberto por volta de um ano sem retorno, eu mencionei essa questão na migração do l10n_br_sale_stock "Havia a ideia de extrair boa parte do modulo para o repo account-invoicing mas é preciso confirmar se existe o interesse dos mantedores lá nesse modulo e hoje acredito que perdemos a "janela" para fazer essa extração e será melhor ver isso na futura v16" #1961

@marcelsavegnago sobre "sinceramente estive pensando inclusive em não implantar o sistema com a politica de faturamento via picking porque comecei a perceber certos problemas." eu estou revendo os módulos l10n_br_sale_stock e l10n_br_purchase_stock para manter a compatibilidade com os casos internacionais, mas se existirem outros problemas eu posso ver de resolver, se você achar melhor pode abrir um Issue e me marcar ou mesmo adicionar um Dado de Demonstração com o caso de uso que você encontrou problemas, no caso de Vendas para tentar atender o caso de Comissão e evitar a necessidade de criar um novo modulo intermediário eu chequei a fazer um PR na v12 que buscava de forma dinamica trazer os campos que existissem na criação a partir do Pedido e que faltavam a partir do Picking #2002 talvez isso ajude, e como estou vendo a migração do stock_picking_invoicing para a v16 também podemos ver se tem algo que pode ou precisa ser alterado lá para ter aqui uma melhor compatibilidade.

"analytic_account_id"
] = moves.sale_line_id.order_id.analytic_account_id.id
values["analytic_tag_ids"] = [
(6, 0, moves.sale_line_id.analytic_tag_ids.ids)
Copy link
Contributor

Choose a reason for hiding this comment

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

Uma questão, esses campos sempre vem preenchidos? Pelos testes parece que sim, porque se não pode acabar retornando erro de Bool não tem ID

Copy link
Member Author

Choose a reason for hiding this comment

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

De fato não viria preenchido sempre. vamos testar

@marcelsavegnago marcelsavegnago deleted the 14.0-l10n_br_sale_stock-copy-analytic_data-to-invoice branch November 25, 2023 20:40
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