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

[RFC][14.0-mig-l10n_br_account] refactor l10n_br_account_due_list #187

Conversation

marcelsavegnago
Copy link

No description provided.

@marcelsavegnago
Copy link
Author

marcelsavegnago commented Mar 11, 2022

@mbcosta eu vi que na 12 você fez este commit OCA@d06c0b9#diff-5793c5d303a7483ae6c4aeae5e70f05de041369b1e7710a76e0afbcf0dde8139R26

O que fiz neste commit abaixo resolveria ?
cd63c17#diff-afc1742e9e7bd84da77883c48f17a0d8cc7745daa6697b4b4e169e88d44eccbfR31

move.financial_move_line_ids = lines.sorted(key="date_maturity", reverse=False)

@mbcosta
Copy link
Member

mbcosta commented Mar 11, 2022

ola @marcelsavegnago a criação das linhas tem que seguir a sequencia da data de vencimento, isso tanto no campo name das account.move.line quanto no Número das Duplicatas na Nota Fiscal como nas imagens do PR OCA#1828 , isso também influencia a criação do Número Sequencial dos Boletos Nosso Número/own_number, se não fica confuso para os usuários ter um lançamento com ex.: 001 em 11/04/2022 e o 002 em 12/03/2022, a diferença pelo o que vejo é que no seu commit você está ordenando apenas o financial_move_line_ids, teria que testar para ver se apenas isso já corrigi o problema da criação fora da sequencia, talvez não porque esse campo é apenas um compute e a criação acontece antes onde isso já precisa estar certo, é preciso testar com um account.payment.terms que gere por exemplo 3 linhas como um 30/60/90 e ver o resultado.

Eu penso em fazer o cherry-pick dos commits desse PR na 14.0 mas o l10n_br_account_payment_order precisa estar integrado na branch para isso, já tem um PR na 13.0 desse modulo que está apenas dependendo de rodar os testes que não estão indo devido o problema com biblioteca erpbrasil.assinatura https://app.travis-ci.com/github/OCA/l10n-brazil/jobs/558877802#L1681 seria importante destravar essa questão da biblioteca para os testes poderem rodar, como os modulos l10n_br_account_payment_order e l10n_br_account_payment_brcobranca não dependem diretamente do l10n_br_account a migração deles já poderia estar andando em paralelo.

@antoniospneto
Copy link

@marcelsavegnago ainda não avaliei a pr, mas só uma observação, o due_list é um modulo a parte ai acho que as pr podem ser feitas fora dessa branch.

já tem um pr aberta do @renatonlima sobre esse módulo tbm: OCA#1789

@marcelsavegnago
Copy link
Author

marcelsavegnago commented Mar 11, 2022

@marcelsavegnago ainda não avaliei a pr, mas só uma observação, o due_list é um modulo a parte ai acho que as pr podem ser feitas fora dessa branch.

já tem um pr aberta do @renatonlima sobre esse módulo tbm: OCA#1789

Eu ia fazer em uma PR separada mas nesta branch de migração o @renatonlima fez modificações no modulo e eu precisava mexer parcialmente no que ele havia modificado.. Mas vou olhar esta PR do Renato.

@marcelsavegnago
Copy link
Author

ola @marcelsavegnago a criação das linhas tem que seguir a sequencia da data de vencimento, isso tanto no campo name das account.move.line quanto no Número das Duplicatas na Nota Fiscal como nas imagens do PR OCA#1828 , isso também influencia a criação do Número Sequencial dos Boletos Nosso Número/own_number, se não fica confuso para os usuários ter um lançamento com ex.: 001 em 11/04/2022 e o 002 em 12/03/2022, a diferença pelo o que vejo é que no seu commit você está ordenando apenas o financial_move_line_ids, teria que testar para ver se apenas isso já corrigi o problema da criação fora da sequencia, talvez não porque esse campo é apenas um compute e a criação acontece antes onde isso já precisa estar certo, é preciso testar com um account.payment.terms que gere por exemplo 3 linhas como um 30/60/90 e ver o resultado.

De fato só vai para financial_move_line_ids ordenado. Então neste caso fica valendo a tua correção que já resolve o problema na raiz neh :D

Eu penso em fazer o cherry-pick dos commits desse PR na 14.0 mas o l10n_br_account_payment_order precisa estar integrado na branch para isso, já tem um PR na 13.0 desse modulo que está apenas dependendo de rodar os testes que não estão indo devido o problema com biblioteca erpbrasil.assinatura https://app.travis-ci.com/github/OCA/l10n-brazil/jobs/558877802#L1681 seria importante destravar essa questão da biblioteca para os testes poderem rodar, como os modulos l10n_br_account_payment_order e l10n_br_account_payment_brcobranca não dependem diretamente do l10n_br_account a migração deles já poderia estar andando em paralelo.

Sobre fazer cherry-pick .. show de bola..
Sobre esse entrave do erpbrasil.assinatura está atrapalhando bastante.
Sobre a migração l10n_br_account_payment_order e l10n_br_account_payment_brcobranca, como não dependendo do account poderia mesmo andar em paralelo.

@mbcosta
Copy link
Member

mbcosta commented Mar 11, 2022

@marcelsavegnago "Sobre esse entrave do erpbrasil.assinatura está atrapalhando bastante." o @renatonlima fez o merge pela manhã tem um outro PR simples lá erpbrasil/erpbrasil.assinatura#29 (review) assim que aprovado eu vou ver de reiniciar os testes do PR do l10n_br_account_payment_order para ver se corrige o problema

@marcelsavegnago
Copy link
Author

@marcelsavegnago "Sobre esse entrave do erpbrasil.assinatura está atrapalhando bastante." o @renatonlima fez o merge pela manhã tem um outro PR simples lá erpbrasil/erpbrasil.assinatura#29 (review) assim que aprovado eu vou ver de reiniciar os testes do PR do l10n_br_account_payment_order para ver se corrige o problema

@mbcosta PRs mescladas

@renatonlima renatonlima force-pushed the 14.0-mig-l10n_br_account branch from 8df727c to 2636c31 Compare March 17, 2022 21:58
@marcelsavegnago marcelsavegnago force-pushed the 14.0-mig-l10n_br_account-refactor-l10n_br_account_due_list branch from e9e3c45 to 5217c2b Compare March 17, 2022 23:42
@mbcosta
Copy link
Member

mbcosta commented Mar 21, 2022

@marcelsavegnago ainda ocorre erro https://app.travis-ci.com/github/OCA/l10n-brazil/jobs/558877802#L1617 acredito que falta a correção proposta no PR "Geração do Certificado Fake, removido dependência do PyOpenSSL." OCA#1777, existe outro PR com a mesma alteração porém para a v14 e é preciso considerar a revisão que foi feita na v14 na v13

@marcelsavegnago marcelsavegnago marked this pull request as draft March 30, 2022 16:32
@renatonlima renatonlima force-pushed the 14.0-mig-l10n_br_account branch from 70081a1 to 5ff2cfc Compare April 7, 2022 16:23
@renatonlima renatonlima force-pushed the 14.0-mig-l10n_br_account branch 2 times, most recently from c0b6b79 to d519839 Compare May 9, 2022 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.