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

Feature/payment in fiscal #820

Merged
merged 36 commits into from
Dec 28, 2020
Merged

Conversation

mileo
Copy link
Member

@mileo mileo commented Apr 28, 2020

No description provided.

Copy link
Sponsor Member

@marcelsavegnago marcelsavegnago left a comment

Choose a reason for hiding this comment

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

Como sugestão.

  • Remover o field payment_term _id conforme comentário na view do documento fiscal.
  • Não sei se cabe no momento mas talvez seja importante sinalizar quando o montante configurado para pagamento/recebimento for maior que do documento fiscal.

l10n_br_fiscal/views/document_view.xml Outdated Show resolved Hide resolved
l10n_br_fiscal/views/payment_view.xml Show resolved Hide resolved
l10n_br_fiscal/views/document_view.xml Show resolved Hide resolved
@mileo mileo requested a review from rvalyi April 30, 2020 01:48
@mileo
Copy link
Member Author

mileo commented May 5, 2020

@luismalta

Encontrei um bug:

image

@mileo mileo force-pushed the feature/payment_in_fiscal branch from 1f8213c to 9c27fb0 Compare May 5, 2020 01:32
@mileo mileo force-pushed the feature/payment_in_fiscal branch 3 times, most recently from edd4c47 to 4250b45 Compare May 5, 2020 22:19
@marcelsavegnago
Copy link
Sponsor Member

marcelsavegnago commented May 6, 2020

Erro após inclusão da condição de pagamento e tentar salvar o documento.

image

Além disso, recebi esta mensagem quando tento clicar na condição de pagamento que foi adiciona na grid.
image

Error:
Uncaught TypeError: Cannot read property 'type' of undefined

http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/600-901c79c/web.assets_backend.js:1955
Traceback:
TypeError: Cannot read property 'type' of undefined
at Class.renderBodyCell (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/600-901c79c/web.assets_backend.js:1955:268)
at http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/600-901c79c/web.assets_backend.js:1976:132
at Function.
.map._.collect (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/594-79e246c/web.assets_common.js:13:270)
at Class._renderRow [as _super] (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/600-901c79c/web.assets_backend.js:1976:82)
at Class.renderRow (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/600-901c79c/web.assets_backend.js:1917:71)
at Class.renderRow (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/594-79e246c/web.assets_common.js:3541:371)
at Function.
.map.
.collect (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/594-79e246c/web.assets_common.js:13:270)
at Class._renderRows (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/600-901c79c/web.assets_backend.js:1977:85)
at Class._renderRows (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/600-901c79c/web.assets_backend.js:1918:67)
at Class._renderRows (http://3420972-820-fce8ff.runbot2-2.odoo-community.org/web/content/594-79e246c/web.assets_common.js:3541:371)

@marcelsavegnago
Copy link
Sponsor Member

marcelsavegnago commented May 6, 2020

Não entendi o propósito destes dois campos neste form.

image

@mileo
Copy link
Member Author

mileo commented May 6, 2020

@marcelsavegnago ainda estamos trabalhando, pra resolver os últimos detalhes. Assim que ficar pronto eu te mando uma msg.

@marcelsavegnago
Copy link
Sponsor Member

@marcelsavegnago ainda estamos trabalhando, pra resolver os últimos detalhes. Assim que ficar pronto eu te mando uma msg.

Blzz.. dá um toque e testamos aqui. Vlw

@rvalyi
Copy link
Member

rvalyi commented May 6, 2020

Pessoal, só para deixar claro, o @renatonlima e eu estamos muito com pé atras sobre esse PR.

Basicamente a gente pensa que muita coisa poderia ser feito ao nível do l10n_br_account (ou módulos que dependeriam dele ainda) porque quando começa a se considerar a questão financeira a nossa ideia e usar o modulo account, mesmo que tendo sim um espaço para provisão puramente financeira através desse modulo da OCA popular e de qualidade mis_builder_cash_flow https://github.com/OCA/account-financial-reporting/tree/12.0/mis_builder_cash_flow (mas repara que ele depende do modulo account. Sugeri pro @mileo se ele queria tentar extrair algo para não depender do account qdo foi migrado para a v12 mas não foi feito e tb na Akretion não achamos isso um problema assumir a dependência do account para isso).

A gente pode ate tolerar outra forma de fazer o financeiro no repo OCA/l10n-brazil, mas certamente não embutida nesses módulos super centrais que devem ser mais agnósticos com isso. Enfim nada de definitivo, falta a gente olhar no detalhe, mas eh só um toque adiantado para evitar trabalho inútil, tendo já tanta coisa útil para finalizar...

@marcelsavegnago
Copy link
Sponsor Member

marcelsavegnago commented May 6, 2020

Pessoal, só para deixar claro, o @renatonlima e eu estamos muito com pé atras sobre esse PR.

Basicamente a gente pensa que muita coisa poderia ser feito ao nível do l10n_br_account (ou módulos que dependeriam dele ainda) porque quando começa a se considerar a questão financeira a nossa ideia e usar o modulo account, mesmo que tendo sim um espaço para provisão puramente financeira através desse modulo da OCA popular e de qualidade mis_builder_cash_flow https://github.com/OCA/account-financial-reporting/tree/12.0/mis_builder_cash_flow (mas repara que ele depende do modulo account. Sugeri pro @mileo se ele queria tentar extrair algo para não depender do account qdo foi migrado para a v12 mas não foi feito e tb na Akretion não achamos isso um problema assumir a dependência do account para isso).

A gente pode ate tolerar outra forma de fazer o financeiro no repo OCA/l10n-brazil, mas certamente não embutida nesses módulos super centrais que devem ser mais agnósticos com isso. Enfim nada de definitivo, falta a gente olhar no detalhe, mas eh só um toque adiantado para evitar trabalho inútil, tendo já tanta coisa útil para finalizar...

Sinceramente eu prefiro que tenha o maior alinhamento possível com o account. O MIS Builder me parece bem poderoso. Uns dois dias atrás fizemos alguns testes no mis_builder_cash_flow e adicionamos uma linha para considerar as notas fiscais de fatura que ainda estão no modo provisório e pareceu uma forma interessante de ter uma previsão de entradas e saídas bem dinâmicas sem a necessidade de usar as linhas manuais.

O lançamento do diário sendo a fonte primária de dados de faturamento/financeiro/contábil passa uma segurança maior e entendo que fica praticamente impossível ter informações incorretas ou divergentes. Entendo que é um pouco diferente das ferramentas que alguns clientes estão acostumados mas a amarração que isso gera entendo que é um ponto positivo até na hora da venda de um projeto Odoo.

@marcelsavegnago
Copy link
Sponsor Member

marcelsavegnago commented May 6, 2020

@rvalyi Se seguir esta linha de um outro módulo financeiro isto não vai impactar relatórios como Fluxo de Caixa por Regime de Caixa e demais relatórios contábeis?

@rvalyi
Copy link
Member

rvalyi commented May 6, 2020

@rvalyi Se seguir esta linha de um outro módulo financeiro isto não vai impactar relatórios como Fluxo de Caixa por Regime de Caixa e demais relatórios contábeis?

@marcelsavegnago o lance eh que a KMEE tinha feito um trabalho bem extenso no financeiro fora da OCA e pouco alinhado com o modulo account (e muito menos mis_builder_cash_flow) na v10 focando mais em implementar um determinado backlog do que se integrar ao ecosistema OCA ou nativo do Odoo (fraco no financeiro conveniamos). Foi depois submetido para a OCA mas como vc pode ver ninguem deu bola OCA/account-financial-tools#805

Do lado da Akretion a gente ja sabe como fazer tudo que a gente precisa em cima do account e mis_builder_cash_flow com pouco codigo adicional. Eu nao posso garantir que a gente vai chegar num denominador comum aqui. Porem, o que parece razoavel, eh que os modulos l10n_br_fiscal e l10n_br_account que sao bem centrais e "estruturadores" da localização sejam o mais agnoticos possivel com isso para não impor algo que foge do que já funciona legal na OCA (e que praticamos bastante na Franca onde a Akretion tem uma filial muito forte). Fiscal eh super especifico do Brasil mas contabilidade e financeiro são bem mais padronizados e queremos evitar de reinventar a roda sempre que for possível.

Ai ou a gente chega num denominador comum na parte financeira ou entao existira 1 modulo financeiro da KMEE e outro da AKRETION no repo. Eu nao vejo isso como grande problema a partir do momento que desenhamos os modulos l10n_br_fiscal e l10n_br_account cuidadosamente para permitir isso sem impor divida tecnológica para ninguem. Eu tenho certeza que conseguimos esse resultados. Sobre ter 1 ou 2 modulos financeiros eu nao posso dizer ainda, ira depender das conversas que teremos com o Luis la na frente. Porem hoje focamos em finalizar a migracao dos modulos da 10 para 12. NFe e NFSe...

@marcelsavegnago
Copy link
Sponsor Member

marcelsavegnago commented May 6, 2020

@rvalyi Se seguir esta linha de um outro módulo financeiro isto não vai impactar relatórios como Fluxo de Caixa por Regime de Caixa e demais relatórios contábeis?

@marcelsavegnago o lance eh que a KMEE tinha feito um trabalho bem extenso no financeiro fora da OCA e pouco alinhado com o modulo account (e muito menos mis_builder_cash_flow) na v10 focando mais em implementar um determinado backlog do que se integrar ao ecosistema OCA ou nativo do Odoo (fraco no financeiro conveniamos). Foi depois submetido para a OCA mas como vc pode ver ninguem deu bola OCA/account-financial-tools#805

Quando vi o MIS_Builder achei bem interessante mas não peguei para mexer nele. Na segunda-feira que o João Carassato (parceiro meu) fez algumas adaptações no cashflow e ficou apaixonado hehehehe.

Sinceramente estou gostando bastante do ecossistema OCA e nativo do Odoo então vou sempre prezar pelo máximo possível de alinhamento.

Do lado da Akretion a gente ja sabe como fazer tudo que a gente precisa em cima do account e mis_builder_cash_flow com pouco codigo adicional. Eu nao posso garantir que a gente vai chegar num denominador comum aqui. Porem, o que parece razoavel, eh que os modulos l10n_br_fiscal e l10n_br_account que sao bem centrais e "estruturadores" da localização sejam o mais agnoticos possivel com isso para não impor algo que foge do que já funciona legal na OCA (e que praticamos bastante na Franca onde a Akretion tem uma filial muito forte). Fiscal eh super especifico do Brasil mas contabilidade e financeiro são bem mais padronizados e queremos evitar de reinventar a roda sempre que for possível.

Sim. Contabilidade e Financeiro pode até ter algumas particularidades mas entendo que é algo mais padrão e não acho válido reinventar a roda em momento algum. Como experiência pessoal eu agradeci por não ser um desenvolvedor python quando comecei a explorar o Odoo já que conta disso me forço a entender mais ainda o funcionamento do Odoo e o ecossistema (cada dia aprendendo algo novo).

Ai ou a gente chega num denominador comum na parte financeira ou entao existira 1 modulo financeiro da KMEE e outro da AKRETION no repo. Eu nao vejo isso como grande problema a partir do momento que desenhamos os modulos l10n_br_fiscal e l10n_br_account cuidadosamente para permitir isso sem impor divida tecnológica para ninguem. Eu tenho certeza que conseguimos esse resultados. Sobre ter 1 ou 2 modulos financeiros eu nao posso dizer ainda, ira depender das conversas que teremos com o Luis la na frente. Porem hoje focamos em finalizar a migracao dos modulos da 10 para 12. NFe e NFSe...

Como um dos interessados no projeto eu entendo que não seja vantagem para ninguém ter trabalho redundante. Se na ideia tanto da KMEE como da Akretion o objetivo é usar como fonte de dados do financeiro o account, então me parece mais simples ter um mis_builder _cash_flow conseguindo gerar um fluxo de caixa por regime de competência e um por regime de caixa (algo que apresente alguma coisa próxima do relatório cashflow do Enterprise), manter o time focado nos demais módulos NFe, NFSe, boleto, fiscal e etc e após esta etapa se necessário alinhar próximos passos para o financeiro.

Como vc disse precisa ter um alinhamento bem definido pelo menos por hora. Mas acredito que isso não vai ser problema.

@mileo mileo force-pushed the feature/payment_in_fiscal branch 7 times, most recently from 3bd1c57 to 9face94 Compare May 7, 2020 19:14
@mileo mileo added the 12.0 label May 8, 2020
Signed-off-by: Luis Felipe Mileo <mileo@kmee.com.br>
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@mileo
Copy link
Member Author

mileo commented Dec 28, 2020

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-820-by-mileo-bump-minor, awaiting test results.

@renatonlima
Copy link
Member

@mileo Você deveria responder os comentários que fiz no PR e esperar eu aprovar porque existe alguns problemas neste PR que precisam ser resolvidos

@OCA-git-bot OCA-git-bot merged commit 2e78372 into OCA:12.0 Dec 28, 2020
@OCA-git-bot
Copy link
Contributor

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

@rvalyi
Copy link
Member

rvalyi commented Dec 28, 2020

@mileo como vc me faz um merge que adiciona mais de 1300 linhas de código com so 6-7 linhas de teste num modulo que ja é enorme, - o maior da OCA toda - feito uns 90% pelo @renatonlima e eu e vc não espera uma aprovação dele??? Ainda com uma copia de 400 linhas de código da Odoo SA (XML incluso) mas atribuindo os copyrights a vc sem pena.
Me diz em que outro repo da OCA vc enfiaria qualquer código sem aprovação dos autores assim?

Ja tive a ocasião de comentar Luis: esse PR é muito borderline: vc quer enfiar coisas de financeiro no fiscal. A maior parte disso deve ser feito pelos modulos account e l10n_ br_account. Ai vc fala que quer harmonizar o financeiro dos documentos fiscais sem ter que instalar o modulo account. A intençao é muito borderline ja que hoje em dia o modulo account da Odoo SA so pretende gerenciar faturas, mas poderia ate ser. Se vc precisasse de 200 linhas para fazer isso a gente poderia ver. Mas 1300 linhas velho... Sendo que em 90% dos casos de uso, 50% dessas linhas se tornam inuteis porque o código do modulo account passa a fazer...

Nao digo que é um não categórico. Mas no minimo acho um super abuso vc não ter a validação do @renatonlima para fazer isso. A ultima vez, que conversamos sobre, te falei que a gente tava ainda ocupado em concertar as merdas que vc tinha feito a ultima vez que fez assim um merge no modulo l10n_br_fiscal sem ter aprovação nossa que foi nesse PR: #928
e que o @renatonlima ainda ta ocupado de limpar aqui: #983

Complicado bicho...

@renatonlima
Copy link
Member

Eu fiz o revert deste merge e abri um novo PR #1058 para ser corrigido e revisado... Eu atualmente estou trabalhando no refatoramento do objetos e wizards dos documentos fiscais #983

@rvalyi rvalyi mentioned this pull request Jan 6, 2021
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.

8 participants