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][RFC] l10n_br_account: refactor method to override create in account.move.line and fiscal.document.line #3064

Conversation

marcelsavegnago
Copy link
Member

@marcelsavegnago marcelsavegnago commented May 6, 2024

Este unlink está afetando os valores de alguns campos do account.move como por exemplo o ind_final, ind_pres e outros e entendo que se faz melhor não deixar criar a linha do documento fiscal ao invés de criar para depois excluir.

image

@OCA-git-bot
Copy link
Contributor

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

@marcelsavegnago marcelsavegnago marked this pull request as ready for review May 6, 2024 16:55
@rvalyi
Copy link
Member

rvalyi commented May 6, 2024

@marcelsavegnago gosto da ideia do PR. Porem vc tirou o decorator @api.create_multi, isso foi voluntário??
Eh algo importante tb, para nao impedir a criação dos account.move.line por batch até nas camadas superiores dos módulos...

@marcelsavegnago
Copy link
Member Author

marcelsavegnago commented May 6, 2024

@marcelsavegnago gosto da ideia do PR. Porem vc tirou o decorator @api.create_multi, isso foi voluntário?? Eh algo importante tb, para nao impedir a criação dos account.move.line por batch até nas camadas superiores dos módulos...

nao foi proposital nao.. errei aqui no copia e cola mesmo :D

@marcelsavegnago marcelsavegnago force-pushed the 14.0-l10n_br_account_refacotr_override_create_method branch from 48e3617 to 529c87f Compare May 6, 2024 17:13
@marcelsavegnago
Copy link
Member Author

@marcelsavegnago gosto da ideia do PR. Porem vc tirou o decorator @api.create_multi, isso foi voluntário?? Eh algo importante tb, para nao impedir a criação dos account.move.line por batch até nas camadas superiores dos módulos...

resolvido

@rvalyi
Copy link
Member

rvalyi commented May 6, 2024

@marcelsavegnago vc chegou a ver o erro com o modulo sale_commission?
https://github.com/OCA/l10n-brazil/actions/runs/8972989735/job/24642187210?pr=3064#step:8:1675

Estranho, mas se for so isso parece ser algo possivel de contornar (primeiro teria que entender o que ele ta fazendo diferente depois do seu PR).

@marcelsavegnago
Copy link
Member Author

@marcelsavegnago vc chegou a ver o erro com o modulo sale_commission? https://github.com/OCA/l10n-brazil/actions/runs/8972989735/job/24642187210?pr=3064#step:8:1675

Estranho, mas se for so isso parece ser algo possivel de contornar (primeiro teria que entender o que ele ta fazendo diferente depois do seu PR).

Organizando agora para testar local.

@marcelsavegnago marcelsavegnago changed the title [14.0][RFC] l10n_br_account refactor method to override create in account.move.line and fiscal.document.line [14.0][RFC] l10n_br_account: refactor method to override create in account.move.line and fiscal.document.line May 7, 2024
@marcelsavegnago marcelsavegnago force-pushed the 14.0-l10n_br_account_refacotr_override_create_method branch from 6551266 to dd28ebd Compare May 7, 2024 17:49
@rvalyi
Copy link
Member

rvalyi commented May 7, 2024

oba passou. Vou revisar com calma já já...

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.

parabéns pelo fix @marcelsavegnago .
Pessoal, é sempre melhor não criar algo do que ter que detonar depois...
Eh provavel que esse unlink que o Marcel removeu era de antes do refator onde eu consegui tirar os documentos fiscais dummy e impedir a criação de linhas de documentos fiscais com o override do create. Agora já parece mais óbvio que tem que ser desse jeito. Vai até facilitar para migrar para a v16...

EDIT: o unlink tava ai por motivo, pois não foi tão simples assim como proposto neste PR. Este PR criou uma regressão com notas de varias linhas e foi solucionado aqui depois: #3077

@marcelsavegnago
Copy link
Member Author

parabéns pelo fix @marcelsavegnago . Pessoal, é sempre melhor não criar algo do que ter que detonar depois... Eh provavel que esse unlink que o Marcel removeu era de antes do refator onde eu consegui tirar os documentos fiscais dummy e impedir a criação de linhas de documentos fiscais com o override do create. Agora já parece mais óbvio que tem que ser desse jeito. Vai até facilitar para migrar para a v16...

opaa.. valeeeeuuu.

@rvalyi
Copy link
Member

rvalyi commented May 8, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-3064-by-rvalyi-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 772048a into OCA:14.0 May 8, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@rvalyi
Copy link
Member

rvalyi commented May 13, 2024

IMPORTANTE: este PR criou uma regressão importante que foi analisada aqui #3037 (review)

Um fix foi proposto aqui: #3077

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