-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
[16.0][REF+IMP] l10n_br_stock_account, l10n_br_fiscal: Carregando os Dados de Demo via hook e incluído método para associar o Diário a OP Fiscal #3607
base: 16.0
Are you sure you want to change the base?
Conversation
Hi @renatonlima, |
@mbcosta vamos olhar isso com calma. Apenas para te informar, estamos ciente que essa parte de dados de demo e de testes esta um pouco uma bagunça (aquelas contribuiçoẽs de dados de demo que o pessoal de sempre fez sem pensar muito e depois caga e anda né...). Ai eu iniciei esses 2 PRs para começar a melhorar isso na 16.0: |
@rvalyi valeu, obrigado pelo retorno, eu vi os PRs mas peço para avaliarem os seguintes pontos:
É importante dizer que esses casos ir.property são os principais responsáveis por muitos dos erros que são vistos nessa combinação de Dados de Demonstração e Testes, usando um método isso fica dinâmico e podendo ser usado dentro dos Testes sem grandes dificuldades como estou fazendo nesse PR E ao deixar de usar os Dados de Demonstração vai ser possível apenas alterar os XML IDs por valores criados pelos Testes e chamar o método inform_journal_in_fiscal_operation por exemplo
Isso funciona sem erros, mas ao evitar tanto que esse módulo ou outros acabem carregando os Dados de Demonstração duas vezes nós podemos salvar Tempo de Processamento do CI tornando o CI mais rápido e eficiente com uma mudança simples que não gera muito DIFF facilitando a Revisão e assim podendo integrar isso de forma rápida sendo um passo inicial nessa melhoria dos Testes.
Portanto na minha opinião hoje seria melhor considerar incluir esses PRs para que as Migrações e a Separações dos Testes dos Dados de Demonstração possam ser feitas de forma independente o que penso ser melhor nesse momento por não colocar as migrações desses módulos dependendo da separação dos testes, ou em outras palavras vai permitir ver essas duas questões em paralelo de forma independente. Se acreditarem ser melhor para Revisão posso tanto ver de criar um PR único par alteração no l10n_br_fiscal 2e00dea ou mesmo criar um PR com todas as alterações desse PR e do #3608 #3609 |
l10n_br_stock_account load demo data by hook
Alterações do PR:
Ocorre um problema semelhante ao do PR #3606 com o l10n_stock_account
ao instalar duas vezes o modulo, como o CI faz em alguns PRs, por exemplo o PR de migração do #3571 o noupdate=1 dos XML parece ser ignorado e acaba retornando um Warning
https://github.com/OCA/l10n-brazil/actions/runs/13035384911/job/36364402057?pr=3571#step:9:711
Mas nesse caso além do Warning também ocorre um erro
https://github.com/OCA/l10n-brazil/actions/runs/13035384911/job/36364402057?pr=3571#step:9:751
Para resolver o problema o que fiz foi criar um método que Cria ou Informa o ir.property para associar o Diário Contábil a Operação Fiscal algo que antes era feito via XML pelos arquivos https://github.com/OCA/l10n-brazil/blob/16.0/l10n_br_stock_account/demo/fiscal_operation_generic.xml https://github.com/OCA/l10n-brazil/blob/16.0/l10n_br_stock_account/demo/fiscal_operation_simple.xml com esse método foi possível apagar esses arquivos XML e se necessário por algum motivo, como nos testes desse módulo, chamar o método com uma linha e essa associação é feita.
Ao rodar o pre-commit algumas alterações foram incluidas, estão em um commit separado.
Inclui o método para criar os ir.property no l10n_br_fiscal para permitir usa-lo em outros módulos, devido aos erros no PR de migração do l10n_br_delivery vai ser necessário fazer o mesmo com os módulos l10n_br_sale, l10n_br_purchase, l10n_br_sale_stock dessa forma é possível evitar códigos duplicados, vou fazer as alterações em PRs separados para diminuir o DIFF e facilitar a Revisão e deixar esse PR de referencia com o método no l10n_br_fiscal.
cc @OCA/local-brazil-maintainers