l10n-brazil
l10n-brazil copied to clipboard
[14.0][FIX] l10n_br_base: changing one of the fields resets the value of the other (state_id and inscr_est)
No momento ao alterar o campo inscr_est do cadastro da empresa o valor de state_id é zerado e quando altera o valor de state_id o campo inscrição estadual é zerado.
Não sei o impacto já que o state_id fica sem inverse neste caso.
cc @augustodinizl
Hi @rvalyi, @renatonlima, some modules you are maintaining are being modified, check this out!
Nos testes efetuados funcionais em ambiente local, funcionou
sim.. testando manualmente parece ok mas deu erro do travis que preciso ver ainda.
pessoal, eu ainda nao tive tempo, mas eu queria revisar essa PR tb...
@marcelsavegnago,
O inverse do campo state_id esta no módulo nativo do Odoo https://github.com/odoo/odoo/blob/14.0/odoo/addons/base/models/res_company.py#L78 eu lembro que existia um problema que se uma empresa tiver preenchido o campo de IE e se você alterar o estado vai gerar um erro da validação na inscrição estadual. por isso que ao alterar o estado ele esta limpando a IE (porque se o estado for diferente a inscrição também vai ser diferente).
Hoje pretendo voltar a mexer nisso novamente.. vlwww
@renatonlima talvez não para agora mas acho que podemos alterar estes testes do fiscal para não ter que alterar o estado da empresa e nem do contato. Por ora para efeito de teste estou alterando a inscrição estadual para ver se passa no Travis
Acredito que o teste passava antes já que ao alterar o estado o sistema zerava a inscrição estadual
@marcelsavegnago Eu vou revisar esse PR para entender melhor a alteração, como eu tinha falado na última revisão, ao alterar o estado de um parceiro estava apagando a IE para que ao salvar não tivesse erro de validação.
@marcelsavegnago Eu vou revisar esse PR para entender melhor a alteração, como eu tinha falado na última revisão, ao alterar o estado de um parceiro estava apagando a IE para que ao salvar não tivesse erro de validação.
certo.. o problema que motivou esta PR é que hoje se vc informa o estado no cadastro da empresa ele zera a inscrição e se vc altera a inscrição ele estava zerando o estado.
No caso, tive que fazer um correção no teste porque provavelmente ao trocar a o estado da empresa utilizada para o teste fazia com que a inscrição fosse zerada e portanto nao fazia validação.
@marcelsavegnago Acredito que esse comportamento foi incluído para evitar inconsistências de dados, pelo que lembro dependendo do Estado a Inscrição Estadual tem tamanhos diferentes e uma empresa não pode estar em um Estado com uma Inscrição Estadual de outro, como o @renatonlima escreveu vai dar erro no método de validação do IE, por isso o correto na minha opinião é se alterar o Estado a IE deve ser apagada e o usuário deve informar novamente não temos como saber qual a IE correta em outro Estado e deixar o antigo valor no campo pode gerar inconsistência ( isso é o mesmo caso do campo Cidade quando o Estado é alterado o campo também deve ser apagado e o usuário deve informar a nova cidade, não podemos deixar o antigo valor e não temos como saber qual é o novo valor ), no caso de se apagar a IE o campo do Estado não deveria ser apagado.
@marcelsavegnago Acredito que esse comportamento foi incluído para evitar inconsistências de dados, pelo que lembro dependendo do Estado a Inscrição Estadual tem tamanhos diferentes e uma empresa não pode estar em um Estado com uma Inscrição Estadual de outro, como o @renatonlima escreveu vai dar erro no método de validação do IE, por isso o correto na minha opinião é se alterar o Estado a IE deve ser apagada e o usuário deve informar novamente não temos como saber qual a IE correta em outro Estado e deixar o antigo valor no campo pode gerar inconsistência ( isso é o mesmo caso do campo Cidade quando o Estado é alterado o campo também deve ser apagado e o usuário deve informar a nova cidade, não podemos deixar o antigo valor e não temos como saber qual é o novo valor ), no caso de se apagar a IE o campo do Estado não deveria ser apagado.
Grande Magno.. Vou tentar resolver de forma mais simples. O problema no momento é que se alterar o estado o sistema zera a IE e se alterar a IE (preenchendo a mesma por exemplo) o campo estado é zerado. Este problema acontece no cadastro da Empresa.. não percebi tal problema no cadastro do parceiro.
Pessoal, eu concordo com o @marcelsavegnago .. não me parece correto ficar apagando o estado e a inscrição estadual, se houver erro de validação o erro tem que ser impresso ao usuário e é ele quem tem que decidir qual valor será alterado/corrigido.
outra questão, agora testando a PR percebi que não está sendo permitido cadastrar uma company sem a inscrição estadual, não é melhor desativar a validação quando a inscrição ou o estado forem nulo ?
Ola @netosjb vc tem certeza que vc testou bem a PR? porque com ela eu consigo criar uma empresa sem insrição estadual, mesmo que for dentro do Brasil, veja aqui no Runboat a empresa test-raph2: http://oca-l10n-brazil-14-0-pr2033-a4348528e8a3.runboat.odoo-community.org/web?#action=52&model=res.company&view_type=list&cids=1&menu_id=4
Não consegui reproduzir o erro na Base SEM PR. https://user-images.githubusercontent.com/20867090/191142135-04e156a0-74be-4667-88ce-a88395bbb291.mp4
é que na verdade o problema não é no modelo res_partner e sim no res_company, tem que ir em settings->companys
Ola @netosjb vc tem certeza que vc testou bem a PR? porque com ela eu consigo criar uma empresa sem insrição estadual, mesmo que for dentro do Brasil, veja aqui no Runboat a empresa test-raph2: http://oca-l10n-brazil-14-0-pr2033-a4348528e8a3.runboat.odoo-community.org/web?#action=52&model=res.company&view_type=list&cids=1&menu_id=4
vou testar novamente
@marcelsavegnago e @douglascstd,
O problema de apagar o campo do state_id esta relacionado a esses métodos no res.partner:
Na localização se o campo state_id é alterado o city_id é apagado: https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_base/models/party_mixin.py#L85
Mas se o campo city_id não está definido, é apagado o state_id: https://github.com/odoo/odoo/blob/14.0/addons/base_address_city/models/res_partner.py#L17
@marcelsavegnago confirmo o problema que você descreveu, no res.company ao informar o IE o Estado é apagado e ao informar o Estado o IE é apagado para contornar o problema está sendo necessário alterar no res.partner relacionado onde isso não acontece, desculpe acebei não vendo com a devida atenção o problema quando foi aberto o PR. Pelo o que testei o que foi feito aqui resolve o problema, falta algo a ser feito para esse PR ser Aprovado e feito merge?
Pessoal desculpem não tinha reparado na revisão feita pelo @renatonlima onde foi pedido alteração.
@marcelsavegnago confirmo o problema que você descreveu, no res.company ao informar o IE o Estado é apagado e ao informar o Estado o IE é apagado para contornar o problema está sendo necessário alterar no res.partner relacionado onde isso não acontece, desculpe acebei não vendo com a devida atenção o problema quando foi aberto o PR. Pelo o que testei o que foi feito aqui resolve o problema, falta algo a ser feito para esse PR ser Aprovado e feito merge?
Pessoal desculpem não tinha reparado na revisão feita pelo @renatonlima onde foi pedido alteração.
@mbcosta de fato o @renatonlima havia feito os comentários e acabei não priorizando. Consegue me ajudar com os ajustes necessários ?
@marcelsavegnago aparentemente consegui resolver separando os métodos Inverse um para o Estado e outro para a Inscrição Estadual, e criando um onchange para o Estado dentro do res.company que no caso de alteração do Estado apaga a IE tanto da Empresa quanto do Parceiro relacionado sem isso acontece o problema que o Renato comentou, segue as alterações
Para não ter erro nos testes a única alteração necessária foi
Sem isso ocorre o erro
2023-09-15 20:38:14,045 1790 ERROR test odoo.addons.l10n_br_fiscal.tests.test_icms_regulation: ERROR: TestICMSRegulation.test_icms_sc_sc_ind_final_no_default Traceback (most recent call last): File "/odoo/external-src/l10n-brazil-14_09_2023/l10n_br_fiscal/tests/test_icms_regulation.py", line 37, in test_icms_sc_sc_ind_final_no_default ind_final=FINAL_CUSTOMER_NO, File "/odoo/external-src/l10n-brazil-14_09_2023/l10n_br_fiscal/tests/test_icms_regulation.py", line 64, in find_icms_tax self.company.state_id = out_state_id File "/odoo/src/odoo/fields.py", line 1150, in set records.write({self.name: write_value}) File "/odoo/src/addons/product/models/res_company.py", line 63, in write return super(ResCompany, self).write(values) File "/odoo/external-src/l10n-brazil-14_09_2023/l10n_br_base/models/res_company.py", line 141, in write raise e
File "/odoo/external-src/l10n-brazil-14_09_2023/l10n_br_base/models/res_partner.py", line 149, in check_ie raise ValidationError(("Estadual Inscription Invalid !")) odoo.exceptions.ValidationError: Estadual Inscription Invalid !
Essa questão do IE parece irrelevante nesse teste mas seria bom confirmar( esse arquivo está sem o cabeçalho de licença)
@mbcosta fiz conforme vc indicou.. se puder avaliar eu agradeço.
valeu @marcelsavegnago tem apenas uma questão que eu não havia reparado antes, como a classe herda o l10n_br_base.party.mixin e lá existe um onchange para o state_id https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_base/models/party_mixin.py#L83 talvez seja melhor aqui herdar esse método e chamar o super, apenas para evitar ter onchanges para o mesmo campo
@api.onchange("state_id")
def _onchange_state_id(self):
for record in self:
super()._onchange_state_id()
record.inscr_est = False
record.partner_id.inscr_est = False
record.partner_id.state_id = record.state_id
valeu @marcelsavegnago tem apenas uma questão que eu não havia reparado antes, como a classe herda o l10n_br_base.party.mixin e lá existe um onchange para o state_id https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_base/models/party_mixin.py#L83 talvez seja melhor aqui herdar esse método e chamar o super, apenas para evitar ter onchanges para o mesmo campo
@api.onchange("state_id") def _onchange_state_id(self): for record in self: super()._onchange_state_id() record.inscr_est = False record.partner_id.inscr_est = False record.partner_id.state_id = record.state_id
Grande @mbcosta ... feito
valeu @marcelsavegnago pelos testes tanto o problema inicial quanto o apontado pelo @renatonlima foram resolvidos
Muito obrigado pela ajuda @mbcosta
Não estou conseguindo acessar o runbot para simular, mas vcs pegaram também um erro ao alterar a lista de inscrições estaduais de outros estados? Estourava um erro e eu só consegui editar esse campo pelo cadastro de parceiros.
Não estou conseguindo acessar o runbot para simular, mas vcs pegaram também um erro ao alterar a lista de inscrições estaduais de outros estados? Estourava um erro e eu só consegui editar esse campo pelo cadastro de parceiros.
acho que agora o runboat está no ar.
Não estou conseguindo acessar o runbot para simular, mas vcs pegaram também um erro ao alterar a lista de inscrições estaduais de outros estados? Estourava um erro e eu só consegui editar esse campo pelo cadastro de parceiros.
acho que agora o runboat está no ar.
confirmei o problema no runbot e criei um issue. Entretanto o problema não tem relação com o que vcs estão corrigindo aqui: https://github.com/OCA/l10n-brazil/issues/2714
ping @renatonlima @rvalyi @mileo
Bora mesclar ?
/ocabot merge patch
On my way to merge this fine PR! Prepared branch 14.0-ocabot-merge-pr-2033-by-rvalyi-bump-patch, awaiting test results.
Congratulations, your PR was merged at e92de739b23007d529008af02467d1b2126a59d3. Thanks a lot for contributing to OCA. ❤️