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

Separada visão de faturas com a visão de documentos fiscais #363

Merged
merged 37 commits into from Nov 23, 2016

Conversation

renatonlima
Copy link
Member

No description provided.

@renatonlima renatonlima mentioned this pull request Aug 11, 2016
2 tasks
@renatonlima renatonlima force-pushed the 8.0-split-invoice-fiscal-document branch from 44a4e20 to 592e046 Compare August 25, 2016 05:15
@renatonlima renatonlima reopened this Aug 25, 2016
@renatonlima renatonlima force-pushed the 8.0-split-invoice-fiscal-document branch 3 times, most recently from 9b61f7d to c0ded07 Compare August 25, 2016 06:56
@renatonlima renatonlima changed the title [WIP] Separada visão de faturas com a visão de documentos fiscais Separada visão de faturas com a visão de documentos fiscais Aug 25, 2016
@renatonlima
Copy link
Member Author

@mileo @danimaribeiro @mbcosta @rvalyi Este PR esta concluído, podem fazer o review, eu vi que teve uma baixa no coverage do código, mas eu acredito que isso seja causado pela remoção dos métodos fieds_view_get do account.invoice e account.invoice.line, hoje pela manhã é bem provável que eu faça algumas melhorias das visões tree e search da NFe para melhorar a ergonomia.

@rvalyi
Copy link
Member

rvalyi commented Aug 25, 2016

uma nota importante: apesar de ser grande esse PR nao altera a estrutura do banco de dados, um simples --update=all deve dar conta. A unica coisa e aue o @renatonlima redefiniu o campo purchase_ok do core para que ele nao faltasse caso o modulo purchase nao fosse installado, porque isso e um bug do core que foi reportado aqui odoo/odoo#13271
Porem o patch do Renato so sera aplicado na master entao enquanto isso a re-defeinicao do purchase_ok serve.

@mbcosta
Copy link
Contributor

mbcosta commented Aug 30, 2016

@renatonlima acredito que está faltando ou um filtro para se poder separar as Notas dos Fornecedores ou Clientes ou separar as visões de NFe de Entradas e de Saídas. Dependendo da quantidade de notas e de como a empresa funciona mistura-las acaba sendo ruim para visualização do usuário final.

@rvalyi
Copy link
Member

rvalyi commented Sep 8, 2016

👎 @renatonlima so you moved the account_invoice_workflow.xml extension into the l10n_br_account module. This is great but currently this doesn't seem correct the way it is. Indeed, in account_invoice_workflow.xml you call the check_nfe() method which is defined only in the l10n_br_account_product module. Same for the sefaz_export invoice state: it is defined only in l10n_br_account_product. I'm not sure what is better: either define these things in the l10n_br_account module or either override the workflow again where it makes sense again in the l10n_br_account_product module. Don't get me wrong the rest of the work is absolutely awesome, just found this issue so far...

@rvalyi rvalyi mentioned this pull request Sep 13, 2016
@rvalyi rvalyi mentioned this pull request Oct 7, 2016
17 tasks
@mileo
Copy link
Member

mileo commented Oct 20, 2016

@renatonlima

  1. Seria o caso de quando criado um pedido de venda e o tipo da fatura ser nf-e o usuário seja encaminhado para a visão correta?
  2. Quando criada uma fatura somente ela sempre esta indo para a visão de NF-E. Qual a forma de configuração necessária para que ela não seja restrita ao domínio?
  3. Não seria o caso de nas faturas de clientes aparecerem todas as faturas ( nf-e e não nf-e)?

@mileo
Copy link
Member

mileo commented Oct 20, 2016

@renatonlima Revisando com mais calma, ao meu ver o fluxo ainda esta incompleto:

  • Quando o usuário cria um pedido ou compra e fatura a mesma ele deveria ser levado para o documento de destino que ele esta criando, seja uma invoice/nfe/nfse.
  • Outro ponto interessante é que se ele for levado para a tela original de faturas, deveria haver um atalho para a nf-e, mas ainda acredito que não ficaria bom pois seria adicionado mais um passo a processo.
  • Ao criar uma invoice, ela é automaticamente criada como nf-e e some da listagem das invoices.
  • Todas as nfs-e/nf-e criadas não aparecem na listagem de faturas, seria interessante que houvesse um campo tipo do documento fiscal na fatura, de forma a listar todas as faturas seja invoice ou nf-e/nfs-e.
  • Outro ponto importante seria melhor substituir o campo invoice_type por algo mais convencional:
    Entrada / Saida e fazer par com a finalidade da emissão?

👎 A user experiencie muito ruim. Não consegui detectar uma lógica de uso na alterações feitas. Temos que pensar em uma solução. se quiser podemos fazer um hangout. []s

@mileo
Copy link
Member

mileo commented Oct 20, 2016

@renatonlima
Copy link
Member Author

@mileo

Neste PR ainda falta finalizar o fluxo das actions que abrem a fatura, neste caso, se você tiver em venda ou compras que gerou uma NFe, deveria cair na visão de NFe.

Também tem um problema no workflow da invoice.

Daqui a pouco eu devo subir alguns commits para resolver isso, eu também vou fazer um vídeo para explicar um pouco o conceito, eu passei o PR para WIP

@mileo
Copy link
Member

mileo commented Oct 20, 2016

Outra coisa bacana, seria deixar a visão parecida com o DANFE. Mas tem que ver o quanto isto é possível.

@renatonlima renatonlima force-pushed the 8.0-split-invoice-fiscal-document branch 2 times, most recently from 7bc11fa to fe6e18d Compare October 21, 2016 17:29
Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

@renatonlima I restarted the job after the DNS issue was gone but there are still some issues

@mileo: about the change being large and as it was just discussed here: OCA/pylint-odoo#59 (comment) Well in the OCA projects we do make some important changes within a "stable" Odoo release and unlike the Odoo SA release policy, may be because we can develop only once Odoo version stabilize and it's really iterative as being aggressively open source there is little to no up-front investment in the OCA work. So for instance some changes requires to run with --update as they change the schema (not the case here). Today the OCA is NOT an end user distribution. May be it may become one as things stabilize but it's not the case today and it would be populism to present it as such. So system integrators building upon the OCA have the responsibility to upgrade or not (we really don't always blindly upgrade our OCA dependencies in a project).

Now @mileo you got a point, @renatonlima when doing this we should increment the 'x' of the module version number 8.0.x.y.z as specified by the OCA guideline: https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#version-numbers

Today this is just an information but eventually if the OCA package management make significant progress in the future, that may ease automatic-upgrades so this is a discipline we should enforce.

@@ -94,6 +94,7 @@ def _default_fiscal_category(self):

@api.model
def _default_fiscal_document(self):
import pudb; pudb.set_trace()
Copy link
Member

Choose a reason for hiding this comment

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

gah, you committed with pudb!

@renatonlima renatonlima force-pushed the 8.0-split-invoice-fiscal-document branch 3 times, most recently from c23df73 to 4fd577f Compare November 18, 2016 13:01
@rvalyi
Copy link
Member

rvalyi commented Nov 23, 2016

👍

@rvalyi
Copy link
Member

rvalyi commented Nov 23, 2016

ok, enough time on this PR, we got approvals from @mileo and me and work from @renatonlima who wrote 70%+ of the current repo codebase.

Constructive and detailed criticisms had enough time to be opposed. We cannot just accept something like "it needs to be completely different", without detailed argumentation and specially not like 4 months after this PR started and not for a such radical change in v8. I hope that everybody understands that we now need to move forward to address the 9 and 10 series too where we will be more open to progressive design changes coming along with proper migration scripts within their series.

@mileo
Copy link
Member

mileo commented Nov 23, 2016

@rvalyi O @renatonlima concluiu aquela alteração, dos "botões"?

@renatonlima
Copy link
Member Author

Olá pessoal,

Eu fiz as mudanças para retirar o domain das actions das faturas e adicionar um botão na list view form view para ir ao documento eletrônico, havia um problema no domain de um campo que concertei agora pouco no ultimo commit, acredito que agora esta ok.

@rvalyi
Copy link
Member

rvalyi commented Nov 23, 2016

👍 ok next fixes in new PR's if required, seems all right and far better than before already.

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.

None yet

5 participants