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
[12.0][IMP] l10n_br_fiscal: tax.py - remove discount from tax base value #1519
[12.0][IMP] l10n_br_fiscal: tax.py - remove discount from tax base value #1519
Conversation
socorro, soltaram o Github co-pilot no projeto! ;-p |
hahahahahah rascunho ainda |
@marcelsavegnago Eu fiz dois PRs #1530 e #1531 que vai ajudar a testar esse PR. |
@marcelsavegnago sempre tem muita discussão em cima do desconto na base de cálculo, qualquer google sobre isso vai ter várias referências para ambos os lados: @bmessiaz alguma opinião sobre? Desconto deve reduzir a base de cálculo de todos os impostos ou só alguns? |
|
02bb31e
to
8d972ff
Compare
a984881
to
6fb9490
Compare
@renatonlima Não lembro se já tinha feito um rebase para considerar estas PRs, massss... fiz um rebase agora. @felipemotter e @netosjb se puderem revisar e testar eu agradeço. |
Assim que conseguir vou dar uma olhada. |
@@ -401,7 +372,8 @@ def _compute_icms(self, tax, taxes_dict, **kwargs): | |||
company.state_id != partner.state_id | |||
and operation_line.fiscal_operation_type == FISCAL_OUT | |||
and partner.ind_ie_dest == NFE_IND_IE_DEST_9 | |||
and taxes_dict[tax.tax_domain].get("tax_value") | |||
and not partner.is_company |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@renatonlima este not partner.is_company está certo ?
6fb9490
to
3e7c36d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
teste funcional + code review 👍🏻
@marcelsavegnago |
3e7c36d
to
68d9f7f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tem que dar uma olhada no Travis, mas ta OK.
@renatonlima OK pro merge desse? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Para fazer o merge desse PR é necessário apenas fazer uma correção:
Adicionar um campo boolean no objeto l10n_br_fiscal.tax.group para indicar se a base de calculo do imposto é adicionado os valores de fretes, seguro e outras despesas;
Alterar o arquivo de dados que carregam os l10n_br_fiscal.tax.group para definir o ICMS, ICMS ST para adicionar na base de calculo os valores de fretes, seguro e outras despesas;
@marcelsavegnago Pode fazer um rebase? |
@felipemotter posso cancelar esta PR ? |
@marcelsavegnago pode sim, os commits foram todos incluídos no merge do #1963 , valeu! |
cc @renatonlima @bmessiaz