l10n-brazil icon indicating copy to clipboard operation
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)

Open marcelsavegnago opened this issue 2 years ago • 14 comments

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

marcelsavegnago avatar Jul 22 '22 20:07 marcelsavegnago

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

OCA-git-bot avatar Jul 22 '22 20:07 OCA-git-bot

Nos testes efetuados funcionais em ambiente local, funcionou

sim.. testando manualmente parece ok mas deu erro do travis que preciso ver ainda.

marcelsavegnago avatar Jul 25 '22 18:07 marcelsavegnago

pessoal, eu ainda nao tive tempo, mas eu queria revisar essa PR tb...

rvalyi avatar Jul 26 '22 19:07 rvalyi

@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

marcelsavegnago avatar Jul 26 '22 20:07 marcelsavegnago

@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

marcelsavegnago avatar Jul 28 '22 20:07 marcelsavegnago

Acredito que o teste passava antes já que ao alterar o estado o sistema zerava a inscrição estadual

marcelsavegnago avatar Jul 28 '22 21:07 marcelsavegnago

@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.

renatonlima avatar Aug 04 '22 15:08 renatonlima

@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 avatar Aug 04 '22 15:08 marcelsavegnago

@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.

mbcosta avatar Aug 08 '22 16:08 mbcosta

@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.

marcelsavegnago avatar Aug 09 '22 11:08 marcelsavegnago

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 ?

antoniospneto avatar Sep 18 '22 23:09 antoniospneto

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

rvalyi avatar Sep 19 '22 21:09 rvalyi

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

antoniospneto avatar Sep 20 '22 01:09 antoniospneto

@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

renatonlima avatar Sep 20 '22 15:09 renatonlima

@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 avatar Sep 14 '23 20:09 mbcosta

@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 avatar Sep 15 '23 14:09 marcelsavegnago

@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

image

image

Para não ter erro nos testes a única alteração necessária foi

image

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 avatar Sep 15 '23 20:09 mbcosta

@mbcosta fiz conforme vc indicou.. se puder avaliar eu agradeço.

marcelsavegnago avatar Sep 19 '23 21:09 marcelsavegnago

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

mbcosta avatar Sep 22 '23 14:09 mbcosta

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

marcelsavegnago avatar Sep 25 '23 16:09 marcelsavegnago

valeu @marcelsavegnago pelos testes tanto o problema inicial quanto o apontado pelo @renatonlima foram resolvidos

Muito obrigado pela ajuda @mbcosta

marcelsavegnago avatar Sep 26 '23 14:09 marcelsavegnago

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.

mileo avatar Sep 26 '23 14:09 mileo

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.

marcelsavegnago avatar Sep 26 '23 15:09 marcelsavegnago

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

mileo avatar Sep 26 '23 15:09 mileo

ping @renatonlima @rvalyi @mileo

Bora mesclar ?

marcelsavegnago avatar Oct 02 '23 21:10 marcelsavegnago

/ocabot merge patch

rvalyi avatar Oct 02 '23 21:10 rvalyi

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

OCA-git-bot avatar Oct 02 '23 21:10 OCA-git-bot

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

OCA-git-bot avatar Oct 02 '23 21:10 OCA-git-bot