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][MIG] l10n_br_sale_stock #1961

Merged
merged 196 commits into from
Sep 16, 2022
Merged

Conversation

renatonlima
Copy link
Member

Migração do módulo l10n_br_sale_stock depende do PR #1937 (assim que esse PR estiver verde eu já coloco os commits nesse PR para continuar a migração)

@marcelsavegnago marcelsavegnago mentioned this pull request Jun 8, 2022
47 tasks
@marcelsavegnago
Copy link
Member

marcelsavegnago commented Jul 15, 2022

@renatonlima tem alguns commits interessantes que o @mbcosta fez na PR de migração para 13.0 ##1786. Pode verificar e se possivel fazer o cherry-pick para a branch desta PR fazendo favor?

0b0c045
112d1be

@mbcosta mbcosta force-pushed the 14.0-mig-l10n_br_sale_stock branch 2 times, most recently from fcf65f0 to 7de8d31 Compare September 14, 2022 19:58
@mbcosta
Copy link
Contributor

mbcosta commented Sep 14, 2022

ola @marcelsavegnago eu fiz os cherry-pick que você mencionou, o PR está verde e acredito que já é possível fazer a revisão, alguns pontos:

  • O campo do Endereço de Entrega/partner_shipping_id já está vindo preenchido pelo modulo stock_picking_invoicing mesmo quando o campo tem o mesmo valor do partner_id, mas esse caso na v12 o campo hoje vai vazio, existe um teste tanto na v12 https://github.com/OCA/l10n-brazil/blob/12.0/l10n_br_sale_stock/tests/test_sale_stock.py#L469 quanto na v14 https://github.com/akretion/l10n-brazil/blob/14.0-mig-l10n_br_sale_stock/l10n_br_sale_stock/tests/test_sale_stock.py#L486 que confirmam isso, afim de evitar que na NFe criada esteja com o campo de Endereço de Entrega preenchido quando é o mesmo do Endereço do Destinatario eu estou apagando essa chave no commit 7de8d31 coloquei um TODO para confirmação, mas pelo o que entendo é melhor manter o mesmo funcionamento da v12 e ter a NFe sem isso por ser desnecessário nesse caso, mas é um assunto que pode ser debatido

  • Foi incluído no l10n_br_stock_account o Diário para Devoluções de Vendas, devido a questão de apenas os Diários do Tipo sale e purchase poderem gerar as Faturas e nos testes do l10n_br_sale_stock inclui a empresa do Lucro Presumido porque nos testes do travis o caso do main_company falha um Browse nos impostos( mesmo problema comentado no l10n_br_stock_account ), erro que pelo o que parece falha devido a forma de instalação dos módulos mas que não acontece no caso Lucro Presumido, portanto parece ser algo especifico do travis

  • Havia a ideia de extrair boa parte do modulo para o repo account-invoicing [14.0][ADD] Module sale_stock_picking_invoicing account-invoicing#1025 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

cc @rvalyi @renatonlima @mileo

Copy link

@vanderleiromera vanderleiromera left a comment

Choose a reason for hiding this comment

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

Review Funcional

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

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 15, 2022

/ocabot merge nobump

@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-1961-by-rvalyi-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 15, 2022
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-1961-by-rvalyi-bump-nobump.

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.

@rvalyi
Copy link
Member

rvalyi commented Sep 15, 2022

/ocabot merge nobump

@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-1961-by-rvalyi-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 15, 2022
Signed-off-by rvalyi
renatonlima and others added 19 commits September 15, 2022 23:08
…ing depending of Product Stock Valuation defined.
…move and account.move.line and the name of method to Create Invoice in sale.order was changed.
…cessary to get the main Fiscal Operation from Sale Order to used in creation of Stock Picking.
…eation of Invoice when it's different from Partner.
@rvalyi
Copy link
Member

rvalyi commented Sep 16, 2022

/ocabot merge nobump

@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-1961-by-rvalyi-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit cb02a69 into OCA:14.0 Sep 16, 2022
@OCA-git-bot
Copy link
Contributor

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