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][Crédito de impostos]: Crédito de impostos pt1 - adiciona stock price e impostos creditáveis por Tax/CST #3158

Open
wants to merge 7 commits into
base: 14.0
Choose a base branch
from

Conversation

DiegoParadeda
Copy link
Contributor

Parte 1 de #2635

@OCA-git-bot
Copy link
Contributor

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

@mileo
Copy link
Member

mileo commented Jul 3, 2024

Creio que a lógica pode ficar no fiscal, até por que o crédito precisa ser revisado por vários atores:

  1. Pelo contador (fatura e documento);
  2. Pelo comprador (purchase);
  3. No estoque é mais mais raro, mas é a forma que o Odoo atualiza o preço de custo, mas é mais difícil o estoquista se envolver com precificação.

@DiegoParadeda DiegoParadeda force-pushed the feature/credito_de_impostos_1 branch 2 times, most recently from 08e4cd1 to 0a1121e Compare July 4, 2024 15:04
@DiegoParadeda DiegoParadeda changed the title [14.0][Crédito de impostos]: Crédito de impostos pt1 - adiciona stock price e impostos creditáveis por CST [14.0][Crédito de impostos]: Crédito de impostos pt1 - adiciona stock price e impostos creditáveis por Tax/CST Jul 4, 2024
@DiegoParadeda DiegoParadeda force-pushed the feature/credito_de_impostos_1 branch from 0a1121e to e3de308 Compare July 4, 2024 15:50
@DiegoParadeda DiegoParadeda force-pushed the feature/credito_de_impostos_1 branch from e3de308 to c6272b3 Compare July 4, 2024 18:56
@DiegoParadeda
Copy link
Contributor Author

Creio que a lógica pode ficar no fiscal, até por que o crédito precisa ser revisado por vários atores:

  1. Pelo contador (fatura e documento);
  2. Pelo comprador (purchase);
  3. No estoque é mais mais raro, mas é a forma que o Odoo atualiza o preço de custo, mas é mais difícil o estoquista se envolver com precificação.

Movido para o l10n_br_fiscal!

@DiegoParadeda DiegoParadeda marked this pull request as draft July 8, 2024 13:12
@DiegoParadeda DiegoParadeda force-pushed the feature/credito_de_impostos_1 branch from c6272b3 to 61a74a9 Compare July 8, 2024 15:14
@rvalyi
Copy link
Member

rvalyi commented Jul 8, 2024

a gente ta mais eh tentando fatiar o modulo l10n_br_fiscal que ja ta muito acima do peso dos padroes que fazem o sucesso da OCA. Nao seria melhor extrair um novo modulo para isso?
Tipo a gente conseguiu ter clientes faturando dezenas de milhoes por 10 anos sem isso, talvez pode ser extraido como modulo extra.
Eh apenas uma pergunta aberta nesta altura.
cc @renatonlima

@DiegoParadeda DiegoParadeda marked this pull request as ready for review July 8, 2024 19:17
@mileo
Copy link
Member

mileo commented Jul 9, 2024

a gente ta mais eh tentando fatiar o modulo l10n_br_fiscal que ja ta muito acima do peso dos padroes que fazem o sucesso da OCA. Nao seria melhor extrair um novo modulo para isso?

Tipo a gente conseguiu ter clientes faturando dezenas de milhoes por 10 anos sem isso, talvez pode ser extraido como modulo extra.

Eh apenas uma pergunta aberta nesta altura.

cc @renatonlima

Para emitir o SPED fiscal e importar o XML dos documentos vamos precisar dessa estrutura.

Então pode ser melhor deixar esse código no fiscal mesmo...

Se não vamos ter muitos módulos de cola pra levar isso até o estoque e o compras.

Fazer um esforço pra fechar essa parte e a importação de nota, pois libera o SPED.

E aí entendo melhor a arquitetura final da pra remover oq não precisa do fiscal sem problemas, ou até dividir ele em peqienos módulos.

@DiegoParadeda
Copy link
Contributor Author

a gente ta mais eh tentando fatiar o modulo l10n_br_fiscal que ja ta muito acima do peso dos padroes que fazem o sucesso da OCA. Nao seria melhor extrair um novo modulo para isso? Tipo a gente conseguiu ter clientes faturando dezenas de milhoes por 10 anos sem isso, talvez pode ser extraido como modulo extra. Eh apenas uma pergunta aberta nesta altura. cc @renatonlima

@rvalyi acredito que o stock price seja na verdade uma correção do valor de estoque que já está sendo calculado no fiscal e não está correto por ter diferenças com os lançamentos contábeis.

Por outro lado entendo que a modificação do valor do custo do produto pode ser uma alteração drástica no funcionamento do sistema. Busquei suavizar essa adaptação inserindo uma booleano capaz de manter o funcionamento atual do sistema na configuração da empresa, veja na linha:

<field name="stock_valuation_via_stock_price" />

@mileo
Copy link
Member

mileo commented Jul 11, 2024

@DiegoParadeda pode ser melhor vc colocar esses campos na visão do mixin, pois ai vc remove a necessidade de alterar as views dos módulos purchase, stock e account: l10n_br_fiscal/views/document_fiscal_line_mixin_view.xml

@rvalyi
Copy link
Member

rvalyi commented Jul 11, 2024

a gente ta mais eh tentando fatiar o modulo l10n_br_fiscal que ja ta muito acima do peso dos padroes que fazem o sucesso da OCA. Nao seria melhor extrair um novo modulo para isso? Tipo a gente conseguiu ter clientes faturando dezenas de milhoes por 10 anos sem isso, talvez pode ser extraido como modulo extra. Eh apenas uma pergunta aberta nesta altura. cc @renatonlima

@rvalyi acredito que o stock price seja na verdade uma correção do valor de estoque que já está sendo calculado no fiscal e não está correto por ter diferenças com os lançamentos contábeis.

I don't think this is a valid argument to bloat the l10n_br_fiscal module further. With such reasoning one could say: "by law it's mandatory to post stock valuation moves in the accounting, so the stock_account module and even the stock module should be merged into the account module."

Again, just like the vast majority of Odoo account module users use the account module just for invoicing, the vast majority of the l10n_br_fiscal module users will also use it just for their invoicing and not full blow accounting. Full blown accouting is nice, but certainly not a valid reason to bloat to death the code that 90% of the module users will not use.

I think we should double check with @renatonlima but you are also forcing me to insist that this is a very serious issue with have with the Kmee company contributions in the repo... You quite never do the clean up work, even you are almost not maintaining the modules you add in the repo, often staying months without giving feedback about modules you added, sometimes staying like 2 years without contributing much. Yet, from time to time @mileo comes here and just like a spoilt child insists that feature X or Y should be included in a timely manner to module he didn't do nor maintain to match his own very agenda all the other contributors should promptly adapt too. Sorry but it sucks and this has been 10 years like this already. I don't know about anybody behaving like that across all the OCA repos.

Sometimes features like the DFe were added by @mileo to the l10n_br_fiscal modules in a hurry and stayed like 4 years adding 1000 lines of dead code inside the l10n_br_fiscal module which already weights more than 20k lines of code.

So no sorry, the time has come to be smarter about how to make things more modular.

Look how hard it is later to extract just like 1200 lines from the l10n_br_fiscal module #3012
With 2 such PRs you are already ruining my efforts to keep the module scope under control...
This is the kind of clean up work Kmee never did spontaneously these 10 years in the repo. Also I know that we accepted @mileo to the PSC back in the early days of the project, but frankly, in 10 years Kmee, outside from this repo, Kmee could barely add just this little module to the 3000 modules of the OCA: https://github.com/OCA/pos/tree/14.0/pos_show_clock
I'm sorry, but in 10 years working with Odoo this rather shows you guys still have much to learn from the OCA best practices instead of behaving like this.

Again, if Kmee were doing the cleanup work, if the signal to noise rate of the contributions were as good as the ones from Escodoo or Engenere for instance, that would be a quite different story to take time to add feature X or Y to some module. But given this is a very recurring issue from even days before you @DiegoParadeda were contributing in the repo, I have to tell it stright again, and yes in English because eventually this is an issue to escalate with the OCA.

@DiegoParadeda
Copy link
Contributor Author

DiegoParadeda commented Jul 11, 2024

a gente ta mais eh tentando fatiar o modulo l10n_br_fiscal que ja ta muito acima do peso dos padroes que fazem o sucesso da OCA. Nao seria melhor extrair um novo modulo para isso? Tipo a gente conseguiu ter clientes faturando dezenas de milhoes por 10 anos sem isso, talvez pode ser extraido como modulo extra. Eh apenas uma pergunta aberta nesta altura. cc @renatonlima

@rvalyi acredito que o stock price seja na verdade uma correção do valor de estoque que já está sendo calculado no fiscal e não está correto por ter diferenças com os lançamentos contábeis.

I don't think this is a valid argument to bloat the l10n_br_fiscal module further. With such reasoning one could say: "by law it's mandatory to post stock valuation moves in the accounting, so the stock_account module and even the stock module should be merged into the account module."

I think we should double check with @renatonlima but you are also forcing me to insist that this is a very serious issue with have with the Kmee company contributions in the repo... You quite never do the clean up work, even you are almost not maintaining the modules you add in the repo, often staying months without giving feedback about modules you added, sometimes staying like 2 years without contributing much. Yet, from time to time @mileo comes here and just like a spoilt child insists that feature X or Y should be included in a timely manner to module he didn't do nor maintain to match his own very agenda all the other contributors should promptly adapt too. Sorry but it sucks and this has been 10 years like this already. I don't know about anybody behaving like that across all the OCA repos.

Sometimes features like the DFe were added by @mileo to the l10n_br_fiscal modules in a hurry and stayed like 4 years adding 1000 lines of dead code inside the l10n_br_fiscal module which already weights more than 20k lines of code.

So no sorry, the time has come to be smarter about how to make things more modular.

Look how hard it is later to extract just like 1200 lines from the l10n_br_fiscal module #3012
With 2 such PRs you are already ruining my efforts to keep the module scope under control...
This is the kind of clean up work Kmee never did spontaneously these 10 years in the repo. Also I know that we accepted @mileo to the PSC back in the early days of the project, but frankly, in 10 years Kmee, outside from this repo, Kmee could barely add just this little module to the 3000 modules of the OCA: https://github.com/OCA/pos/tree/14.0/pos_show_clock
I'm sorry, but in 10 years working with Odoo this rather shows you guys still have much to learn from the OCA best practices instead of behaving like this.

Again, if Kmee were doing the cleanup work, if the signal to noise rate of the contributions were as good as the ones from Escodoo or Engenere for instance, that would be a quite different story to take time to add feature X or Y to some module. But given this is a very recurring issue from even days before you @DiegoParadeda were contributing in the repo, I have to tell it stright again, and yes in English because eventually this is an issue to escalate with the OCA.

@rvalyi why do you think my argument isn't valid? Please, let's stick to the point

@rvalyi
Copy link
Member

rvalyi commented Jul 11, 2024

sorry, this pressure to bloat already extremely bloated modules (that uou guys largely bloated) without never doing the cleanup part is a very recurring issue always coming from the same company Kmee.

I don't know if you guys have no clue why I complain about this, but in either cases you are forcing me to repeat it straight.

@mileo it looks you think you were born with an unlimited credit for pressuring for bad contributions in the repo. I don't think that can run like that for ever so I prefer making it clear again sorry.
Sorry but we would expect from PSC people thet would be a barrier against poor designs, not an incentive to let them enter...

Please guys when contributing to some open source project ask yourself always: what am I bringing to the project, and what energy will I be sucking from the project maintainers im exchange. That balance cannot run negative forever.

@DiegoParadeda
Copy link
Contributor Author

sorry, this pressure to bloat already extremely bloated modules (that uou guys largely bloated) without never doing the cleanup part is a very recurring issue always coming from the same company Kmee.

I don't know if you guys have no clue why I complain about this, but in either cases you are forcing me to repeat it straight.

@mileo it looks you think you were born with an unlimited credit for pressuring for bad contributions in the repo. I don't think that can run like that for ever so I prefer making it clear again sorry. Sorry but we would expect from PSC people thet would be a barrier against poor designs, not an incentive to let them enter...

Please guys when contributing to some open source project ask yourself always: what am I bringing to the project, and what energy will I be sucking from the project maintainers im exchange. That balance cannot run negative forever.

I understand your point, and I'm not looking to add any pressure. However, I wanted to bring to your attention an issue regarding the compute methods for product.product.standard_price in the module. It seems these methods do not take taxes into consideration.

In practice, this oversight results in the standard_price field being populated incorrectly when creating a purchase and stock receipt for a product with the AVCO option turned on.

I believe the Pull Request I've submitted addresses this issue, or aims to do so. However, I could use your input on the next steps. Would you prefer we relocate these functions to another module like l10n_br_fiscal_extended? Alternatively, should we delay this feature until after a complete refactor of the module for improved cleanliness?

I'm seeking clarification because I'm unsure about the best course of action. Your repeated reference to @mileo has also contributed to my confusion.

@rvalyi
Copy link
Member

rvalyi commented Jul 12, 2024

generic names like l10n_br_fiscal_extended shoukd be avoided and if you look carefully in the OCA, you won't find such module names. Again contributing advanced stuff to the OCA is all about experience and there is a very famous quote:

There are only two hard things in Computer Science: cache invalidation and naming things.

-- Phil Karlton
https://martinfowler.com/bliki/TwoHardThings.html

So instead try to be feature specific. Good modules size in in the OCA are from 100 lines to some 4000. Anything larger than that should prevent you from sleeping untill you find a solution to improve it. Here l10n_br_fiscal is more than 20k lines, period.

Another very basic and important thing to bare in mind: fixing a design after you created it poorly, deployed it in production in many companies, let other modules depend on it in several branches will at the bare minimum cost an order of magnitude to fix it after compared to fixing it BEFORE. So in doubt always abstain from adding features, use things like git-aggretate to get the job done without introducing technical debt for the others.

So no sorry, making the situation worse and promise you'll fix it later doesn't work for me. Sorry but Kmee has already burnt any credibility here about this during these 10 years. The case with l10n_br_dfe was just an example but if there is any doubt I can easily come with a list with a dozen of such things, including payment gateway module logging credit card numbers for more than one year, easy peasy.

May be you Diego aren't so aware about this, but that doesn't change facts and at the end of the day if your employer will never pay you to fix things later, well it's reasonable we expect things won't before different this time. Remember reputation takes years and years to build and can be destroyed in a split second.

sorry I didn't think too much about how to do it properly. Recently I already had to save the XML NFe importation or the SPED from being screwed the same way. You guys cannot expect we will save the project every week for free to accommodate your very own needs.

What I can tell so far is that this should not be a problem to have some SPED module depend on some extra tax credit module though I gave example to simply test if a module is present or not when serializating the SPED registers so no hard dependency might even be needed.

As for importing XML NFe's, that should require a phD either to come with a small hook that would trigger an override in your tax credit module.

Again, you guys should learn more from the OCA ecosystem before tryjng to contribute complex parts of the project.

Regards.

@rvalyi
Copy link
Member

rvalyi commented Jul 12, 2024

@DiegoParadeda BTW I just checked and the payment gateway that has been logging credit card numbers for more than one year in the repo while you guys just walked way presenting yourselves as key people for the projects was authored...
... by yourself! If you know about similar security issues in the OCA please let me know...

So you should probably be able to understand when I say I cannot trust people from Kmee when they ask "can we do this little extra shit and we promiss we will fix it later?"

#1562
#943

@DiegoParadeda
Copy link
Contributor Author

@DiegoParadeda BTW I just checked and the payment gateway that has been logging credit card numbers for more than one year in the repo while you guys just walked way presenting yourselves as key people for the projects was authored... ... by yourself! If you know about similar security issues in the OCA please let me know...

So you should probably be able to understand when I say I cannot trust people from Kmee when they ask "can we do this little extra shit and we promiss we will fix it later?"

#1562 #943

@rvalyi
Your comments are out of point and disrespectful. This PR is about stock price, if you want to discuss past matters please find an appropriate channel.

@DiegoParadeda
Copy link
Contributor Author

@rvalyi
Copy link
Member

rvalyi commented Jul 12, 2024

I have a different interpretation: forcing bad stuff into the project is the real disrespect of the maintainer time in 1st place, not complaining about it for the tenth time. Really if you think that's me being wrong about this I think you should come first with a few dozens of modules endorsed by other repos of the OCA, 10 years is far enough time to prove you are the good guys you claim to be. There is no way you pile poor code into the project, including major security issues and behave like everyday is a new chance and we should always forgive it all and keep loosing our time with your PRs over and over.

@DiegoParadeda
Copy link
Contributor Author

I have a different interpretation: forcing bad stuff into the project is the real disrespect of the maintainer time in 1st place, not complaining about it for the tenth time. Really if you think that's me being wrong about this I think you should come first with a few dozens of modules endorsed by other repos of the OCA, 10 years is far enough time to prove you are the good guys you claim to be. There is no way you pile poor code into the project, including major security issues and behave like everyday is a new chance and we should always forgive it all and keep loosing our time with your PRs over and over.

This is a work in progress, I'm not forcing anything. Feel free to add your code review and I will surely consider it. Bonus points if you can do it respectfully

@rvalyi
Copy link
Member

rvalyi commented Jul 12, 2024

I have a different interpretation: forcing bad stuff into the project is the real disrespect of the maintainer time in 1st place, not complaining about it for the tenth time. Really if you think that's me being wrong about this I think you should come first with a few dozens of modules endorsed by other repos of the OCA, 10 years is far enough time to prove you are the good guys you claim to be. There is no way you pile poor code into the project, including major security issues and behave like everyday is a new chance and we should always forgive it all and keep loosing our time with your PRs over and over.

This is a work in progress, I'm not forcing anything. Feel free to add your code review and I will surely consider it. Bonus points if you can do it respectfully

I have my doubts about this.

  1. if it's WIP you are welcome to put the PR as draft
  2. that PR for instance got merged by your boss @mileo in just 5 hours this week while we already complained about such behavior a few months ago: [14.0][REF] l10n_br_fiscal: split methods to facilitate inheritance #3165
  3. you may tell: it was a small PR... Yes, now what about that DFe one that got merged by @mileo again during the night without @renatonlima getting a chance to review it Feature/dfe mdfe #928 and led to 1200 lines of dead and broken code in l10n_br_fiscal for... ... 4 years! Even now that is has been extracted in l10n_br_dfe, we still have our doubts if it really does what it claims it does.
  4. What about that other idiot thing Feature/payment in fiscal #820 merged again by @mileo without @renatonlima and that was so utterly broken that we had to hard revert it on the OCA 12.0 branch which got the translation bot broken then for 6 months.
  5. what about your broken XML importation proposal 6 month ago I had to propose a counter PR here [14.0][l10n_br_fiscal][l10n_br_nfe][l10n_br_account][l10n_br_account_nfe] importação dos account.move com as NFe's - contra-proposta #2781 to avoid Kmee ruining our work again.
  6. what about that other idiot attempt to re-invent the IAS2 accounting in some inconsistent way 2 years ago from a person from Kmee again [12.0] Contabilidade de custo avançada: CMV/COGS, contabilização das remessas e entradas com o mode anglo-saxon (IFRS/IAS2) #1561 Even you know now that this ways broken as you finally did the fix I suggested (enabling anglo-saxon mode) in your COA modules. Now read carefully to see was disrespectful while being plain wrong in these threads... Oh you ended up breaking with the guy... Very well but who was the author of the PRs he was defending??
  7. What about the other threat to ruin the SPED feature into beginner manual mapping code last year, that would have been just as impossible to maintain like the NFe mapping code from the v8 from pysped you guys were pressuring us to depend entirely on (you even hired its author to build your abandoned fork).

So no sorry, it's way too much to silent it. If you look carefully you will see that in my entire OCA contributions, I never had to talk to people the way I had to do we a few persons from Kmee. Never. Is it because of the project? Look if I had to talk like this with people from Escodoo or Engenere. Answer: never.

THE PROBLEM IS WITH YOU.

@rvalyi
Copy link
Member

rvalyi commented Jul 13, 2024

I was in a rush a few hours earlier, so may be my message was a bit confused: so basically, my problem is not so much with you @DiegoParadeda but it's much more with the track records from the company you are working for, that is Kmee and @mileo. Though that doesn't exonerate you entirely because the payment gateway logging credit card numbers for a full year was your work and you also left your website_* modules with broken tests annoying all other real contributors for months 2 or 3 years ago.

So this should be no secret that with part of the bad track record from Kmee in the project I mentioned earlier, me and Renato are pissed off to say the least. Now I was also stressed such feature would be merged in a hurry by @mileo just like he merged #3165 in just 5 hours earlier this week which wasn't a bad PR, but he also has track of having merged in a hurry extremely bad contributions such as #928 or #820 just to mention a few ones.

So basically I started complaining more explicitly about bloating l10n_br_fiscal further without Kmee never tackling any clean up job (and even just inside the modules or libs you guys are leading there is plenty of room to express all your talent for making clean things) because I was concerned this PR marked as ready for review would be yet another bloat @mileo would merge and walk away leaving us with the extra burden in the module which is now more than 20k lines of code already largely because of this lack of programming skills when introducing new features.

Again, just like for the IAS2 thing, nobody here is complaining you are attempting to implement legal feature X or Y. It's just really about the way guys are trying to implement these things. It does matter a lot. the OCA modules are done as a volunteer work with little resources, OCA/l10n-brazil is already one of the very most active project out of the 250 OCA repo, l10n_br_fiscal is already the largest of the 3000 OCA modules, so there isn't much room left for bloating things in suboptimal ways as this is already sucking way too much energy to migrate and maintain the way it is.

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.

4 participants