[14.0][FIX][l10n_br_account] reordering vals_list to correctly associate with with fiscal_document_line #3077
Conversation
|
Hi @renatonlima, |
rvalyi
changed the title
rvalyi
force-pushed
the
14.0-l10n-br-account-fix-fiscal-document-line
branch
3 times, most recently
from
418020b
to
4766e81
Compare
|
@rvalyi Vou testar aqui e ja te falo |
marcelsavegnago
requested review from
antoniospneto,
mileo,
felipemotter,
kaynnan and
mbcosta
| # For example, if vals_list[0] in amls does not match vals_list[0] in the | ||
| # fiscal document (which is a manipulated vals_list), it results in erroneous | ||
| # associations. | ||
| if len(vals_list) > 1 and self.env.company.country_id.code == "BR": |
There was a problem hiding this comment.
Não olhei profundamente mas essa condição de forma genérica self.env.company.country_id.code == "BR" não parece ser a melhor opção para identificar os casos que não tem Documentos Fiscais, o melhor parece ser verificar se o account.move tem um fiscal_document_id ou o fiscal_operation_id preenchidos, para atender os casos onde a empresa é do Brasil mas emite tanto Documentos Fiscais quanto Faturas Sem Doc Fiscal/Invoices(account.move original) ou o caso de múltiplas localizações instaladas no mesmo banco de dados
There was a problem hiding this comment.
verdade e pode correr o risco de estar manipulando a fatura de um outra empresa mas sob o contexto da empresa x..
There was a problem hiding this comment.
eu coloquei verificação do pais porque havia dado erro em um teste do account (test_out_invoice_reverse_move_tags) ... talvez com a reordenação do result para o a sequencial original nao seja necessário verificar o pais para resequenciar antes da criação
There was a problem hiding this comment.
valeu @mbcosta. Vai ser mais robusto sim, vou amendar ja.
There was a problem hiding this comment.
Amendei. Optei por não adicionar mais teste para ver se o caso é do Brasil, já que a re-ordenaçao é pela presencia do fiscal_operation_line_id, se for empresa de fora não vai alterar a ordenação original...
Eu tb removi o teste if len(vals_list) > 1 pois eu acho que não vale o aumento de complexidade no codigo, já que o que a gente faz para re-ordenar é simples e não é um overhead que precisa procurar otimizar.
There was a problem hiding this comment.
@rvalyi nesse mesmo método tem outra linha usando essa condição linha 184
if (
move_id.is_invoice(include_receipts=True)
and move_id.company_id.country_id.code == "BR"
and any(
values.get(field)
for field in [*ACCOUNTING_FIELDS, *BUSINESS_FIELDS]
)
e tem um tipo de validação na linha 163
move_id = self.env["account.move"].browse(values["move_id"])
fiscal_doc_id = move_id.fiscal_document_id.id
if not fiscal_doc_id:
continue
Talvez algo nesse sentido pode ser feito no inicio do método para quando não tem um Doc Fiscal simplesmente fazer um "return super()...
There was a problem hiding this comment.
ok, mas então isso é melhor melhorar num outro PR. Pois aqui o foco é apenas resolver a regressão de #3064.
Se pilhar para mandar PR...
There was a problem hiding this comment.
pessoal, eu tirei então os testes para ver se a empresa era do Brasil e deu treta nos testes internacionais aqui:
https://github.com/OCA/l10n-brazil/actions/runs/9069191266/job/24918186228?pr=3077#step:8:2523
Vale a pena a gente analisar, pode ser que algum problema importante foi detetado...
There was a problem hiding this comment.
@marcelsavegnago @mbcosta @gabrielcardoso21 então deixando o codigo rodar sem procurar testar se a empresa é do Brasil (porque não é codigo pesado, nem vale nem a complexidade de 2 if), pelo menos descobri algo errado com o sort initial do @marcelsavegnago key=lambda x: x[1].get("fiscal_operation_line_id") is False pois podia ter dict que vinha com "fiscal_operation_line_id": False e nesse caso uma linha dessa ia na frente da lista. Por isso que um teste international falhava, mas podia acontecer num caso do Brasil tb. Então na nova ordenação eu usei:
key=lambda x: not x[1].get("fiscal_operation_line_id") que funciona sempre.
Eu botei isso naquele fixup aqui 49c69d3
vcs conseguem revisar de novo?
Novamente devido a regressão introduzida pelo PR #3064, esse fix é bem urgente...
rvalyi
force-pushed
the
14.0-l10n-br-account-fix-fiscal-document-line
branch
2 times, most recently
from
412a29b
to
18b5d42
Compare
create should return the records in the initial order of the vals_list to avoid side effects.
rvalyi
force-pushed
the
14.0-l10n-br-account-fix-fiscal-document-line
branch
from
18b5d42
to
49c69d3
Compare
|
pessoal, o @marcelsavegnago achou um problema com esse PR onde a re-ordenacão não ta sendo correta, eu ainda nao pude olhar, vou vou tentar hoje. |
rvalyi
force-pushed
the
14.0-l10n-br-account-fix-fiscal-document-line
branch
from
18ca0ed
to
f1ba3ce
Compare
|
Bora... testado. A sequencia final está igual a sequencia original do vals_list antes da reordenação que é passada para o super do account.move.line e consequentemente par ao fiscal.document.line (necessário para permitir o vinculo correto da linha do documento fiscal com o AML).
|
|
/ocabot merge patch |
|
On my way to merge this fine PR! |
Pessoal, o problema que usava o index em vez de usar index "invertido", foi resolvido naquele fixup f1ba3ce. O @marcelsavegnago pude confirmar que tava OK para ele tb. |
|
Congratulations, your PR was merged at c634884. Thanks a lot for contributing to OCA. ❤️ |
solução de re-ordenação alternativa para #3076
cc @marcelsavegnago