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][FIX] l10n br fiscal: ncm com inicio 09 conflita com tipo servico #3096

Draft
wants to merge 19 commits into
base: 14.0
Choose a base branch
from

Conversation

crsilveira
Copy link
Contributor

@crsilveira crsilveira commented May 24, 2024

Issue: #3095

@OCA-git-bot
Copy link
Contributor

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

@@ -32,5 +32,5 @@ def _onchange_ncm_id(self):
[("code", "=", r.ncm_id.code[0:2])]
)

if r.fiscal_genre_id.code == PRODUCT_FISCAL_TYPE_SERVICE:
if r.fiscal_type == PRODUCT_FISCAL_TYPE_SERVICE:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acho que vc tem que alterar o decorator do metodo tbm, trocar o fiscal_genre_id por fiscal_type

@antoniospneto
Copy link
Contributor

@crsilveira depois precisa arrumar os commits tbm, juntar em um só, use o opção de squash no rebase iterativo
e renomear com o padrão oca, que nesse caso anotar com a tag "[FIX]" veja:
https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#commit-message

@marcelsavegnago
Copy link
Member

marcelsavegnago commented May 24, 2024

Ontem passei estas instruções em uma outra PR... segue como exemplo.

Here are the steps to do that:

  1. Start an interactive rebase:
    In your terminal, navigate to the root directory of your repository and run:

    git rebase -i HEAD~3

    This command opens the last three commits for interactive rebase.

  2. Edit the rebase list:
    Change "pick" to "squash" for the commits you want to combine into the first commit. Your editor will show something like this:

    pick e73a481 Remove field groups_id from res_partner_view.xml
    squash df28461 [MIG] helpdesk_mgmt_partner_sequence: Migration to 16.0
    squash 625e0fa Merge branch '16.0-mig-helpdesk_mgmt_partner_sequence' of https://git...
    
  3. Save and close the editor:
    After changing the lines, save and close your text editor. Git will combine the commits into one.

  4. Edit the commit message:
    Git will prompt you to edit the commit message for the combined commit. Combine the messages as needed:

    [MIG] helpdesk_mgmt_partner_sequence: Migration to 16.0
    
    - Remove field groups_id from res_partner_view.xml
    - Merge branch '16.0-mig-helpdesk_mgmt_partner_sequence'
    
  5. Save and close the editor again:
    Save the new commit message and close the editor.

  6. Force push to update the remote repository:
    Since you've rewritten the commit history, you need to force push to the remote repository:

    git push REMOTE_NAME BRANCH_NAME --force

By following these steps, you will squash the specified commits into a single migration commit and update the remote repository accordingly.

@rvalyi
Copy link
Member

rvalyi commented May 27, 2024

Ola @crsilveira vc consegue fazer o rebase e fazer o squash por favor?

Talvez vc tem a impressão que nao faz diferença, mas para a gente que mantem o projeto te garanto que faz uma baita diferença...

Sempre que tem algum bug cabeludo e que a gente não sabe bem porque uma linha de codigo esta do jeito que esta a gente mergulha num "git blame" para analisar o histórico e cada commit inutil atrapalha...

te do outro examplo, esse é o tipo de malabarismo que temos que fazer na OCA para mandar cada commit de uma branch para a outra, por exemplo entre as branches 14.0 e 16.0 para migrar o módulo l10n_br_account:
#2865 (comment)

Então se puder aprender a fazer o rebase e squash ajudaria... Até para vc é fondamental vc dominar o rebase para sobreviver com Odoo...

@crsilveira crsilveira changed the title 14.0]bug]l10n br fiscal: ncm com inicio 09 conflita com tipo servico [14.0][FIX]l10n br fiscal: ncm com inicio 09 conflita com tipo servico May 27, 2024
@crsilveira crsilveira changed the title [14.0][FIX]l10n br fiscal: ncm com inicio 09 conflita com tipo servico [14.0][FIX] l10n br fiscal: ncm com inicio 09 conflita com tipo servico May 27, 2024
@crsilveira
Copy link
Contributor Author

Fechando aqui, pra tentar fazer corretamente

@crsilveira crsilveira closed this May 27, 2024
@crsilveira
Copy link
Contributor Author

Uma dúvida: o que é o correto pra ficar neste decorator : " ...decorator do metodo tbm, trocar o fiscal_genre_id por fiscal_type " ???

@antoniospneto
Copy link
Contributor

Uma dúvida: o que é o correto pra ficar neste decorator : " ...decorator do metodo tbm, trocar o fiscal_genre_id por fiscal_type " ???

No decorator vc coloca os campos que vão "acionar" essa função do onchange, no caso da forma que tá hoje é sempre que alterar o campo ncm_id ou fiscal_genre_id
mas agora não faz sentido acionar pelo o fiscal_genre_id tem que acionar quando o valor de fiscal_type mudar que é o que você tá verificando no IF agora.

Olhando melhor vi que já existe um onchange exclusivo para o fiscal_type acho que o mais correto é você mover essa atribuição do ncm de serviço diretamente ali no onchange do fiscal_type. depois apagar tbm o fiscal_genre_id do decorator do onchange do ncm.

image

@crsilveira
Copy link
Contributor Author

..... vi que já existe um onchange exclusivo para o fiscal_type ..........

sim, por isso minha dúvida. Obrigado pela resposta vou ver.

@rvalyi
Copy link
Member

rvalyi commented May 27, 2024

outro detalhe: vc pode fazer um git push com a opção -f (force) depois que faz rebase. Se fizer push com -f na mesma branch, vc não precisa criar outro PR, pode ser no mesmo PR. Alias vou re-abrir caso vc quiser fazer isso que ai é melhor as explicações do porque do commit ficam todas no mesmo PR centralizado...

@rvalyi rvalyi reopened this May 27, 2024
@OCA-git-bot
Copy link
Contributor

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

@rvalyi rvalyi marked this pull request as draft May 27, 2024 14:46
WesleyOliveira98 and others added 10 commits May 27, 2024 14:47
Currently translated at 90.4% (1257 of 1389 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_fiscal
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_fiscal/pt_BR/
Currently translated at 0.9% (11 of 1134 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_nfe_spec
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_nfe_spec/pt_BR/
Currently translated at 78.2% (18 of 23 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_crm
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_crm/pt_BR/
Currently translated at 92.3% (277 of 300 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_purchase
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_purchase/pt_BR/
Currently translated at 92.0% (300 of 326 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_sale
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_sale/pt_BR/
Currently translated at 100.0% (274 of 274 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_contract
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_contract/pt_BR/
Currently translated at 12.3% (73 of 591 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_nfe
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_nfe/pt_BR/
Currently translated at 98.6% (288 of 292 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_stock_account
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_stock_account/pt_BR/
marcelsavegnago and others added 4 commits May 27, 2024 14:47
Currently translated at 100.0% (298 of 298 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_repair
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_repair/pt_BR/
Currently translated at 70.5% (342 of 485 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_pos
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_pos/pt_BR/
Currently translated at 100.0% (277 of 277 strings)

Translation: l10n-brazil-14.0/l10n-brazil-14.0-l10n_br_sale_blanket_order
Translate-URL: https://translation.odoo-community.org/projects/l10n-brazil-14-0/l10n-brazil-14-0-l10n_br_sale_blanket_order/pt_BR/
@marcelsavegnago
Copy link
Member

Tem algo errado.. faz um rebase com a branch 14 do remote original

Copy link
Member

@renatonlima renatonlima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crsilveira

Eu vi que o método _onchange_ncm_id tem o decorador @api.onchange("ncm_id", "fiscal_genre_id") que poderia ser alterado removendo o campo fiscal_genre_id, porque não faz sentido agora, então o método ficaria assim:

@api.onchange("ncm_id")
def _onchange_ncm_id(self):
    for r in self:
        if r.ncm_id:
            r.fiscal_genre_id = self.env["l10n_br_fiscal.product.genre"].search(
                [("code", "=", r.ncm_id.code[0:2])]
            )

A ideia original desse método era que quando alterado o fiscal_genre_id para o "00" Serviço, fosse alterado o campo ncm_id para o NCM de serviço 000.00.00, mas como reportado pelo issue, não estava correto.

Eu diria que poderia ser complementado nesse PR era que se for alterado o campo fiscal_genre_id e caso o NCM esteja preenchido com o NCM de outra família, poderia ser limpado o campo para que o usuário preencha o NCM correto

@api.onchange("fiscal_genre_id")
def _onchange_ fiscal_genre_id(self):
    for r in self:
        if r. fiscal_genre_id and r.ncm_id:
            if r.fiscal_genre_id.code != r.ncm_id.code[0:2]:
                r.ncm_id = False

Além esse método também poderia ser adicionado uma constraint para fazer essa validação (garantir que ao salvar o produto o ncm_id seja da mesma família do fiscal_genre_id, mas acho que isso poderia ficar para depois e ser pensado melhor, porque acredito que esse tipo de validação tem o potencial de causar alguns problemas em uma base de dados multi-company com uma empresa que não é do Brasil.

@marcelsavegnago
Copy link
Member

@crsilveira se puder fazer um rebase e depois dos ajustes puder fazer um squash dos commits.. seria bom. Thanks.

@crsilveira crsilveira force-pushed the 14.0]BUG]l10n_br_fiscal-ncm-inicio-09-conflito-servico branch from 933fa71 to ff8cd4a Compare August 2, 2024 18:28
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.

7 participants