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

[WIP] Criação de invoice ao importar nota fiscal #2763

Closed
wants to merge 5 commits into from

Conversation

hirokibastos
Copy link
Contributor

WIP

@OCA-git-bot
Copy link
Contributor

Hi @mbcosta, @antoniospneto, @felipemotter,
some modules you are maintaining are being modified, check this out!

@mileo
Copy link
Member

mileo commented Oct 31, 2023

@hirokibastos eu vejo que podemos usar uma abordagem similar ao product.template e ao product.product (variant)

O product.product tem um inherits com o product.template:

class ProductProduct(models.Model):
    _name = "product.product"
    _description = "Product"
    _inherits = {'product.template': 'product_tmpl_id'}

    @api.model_create_multi
    def create(self, vals_list):
        products = super(ProductProduct, self.with_context(create_product_product=True)).create(vals_list)
        # `_get_variant_id_for_combination` depends on existing variants
        self.clear_caches()
        return products

https://github.com/odoo/odoo/blob/14.0/addons/product/models/product.py#L336C1-L341C24

class ProductTemplate(models.Model):
    _name = "product.template"

    @api.model_create_multi
    def create(self, vals_list):
        ''' Store the initial standard price in order to be able to retrieve the cost of a product template for a given date'''
        templates = super(ProductTemplate, self).create(vals_list)
        if "create_product_product" not in self._context:
            templates._create_variant_ids()

https://github.com/odoo/odoo/blob/14.0/addons/product/models/product_template.py#L407-L412
https://github.com/odoo/odoo/blob/14.0/addons/product/models/product_template.py#L580

Assim como o account.move e o fiscal.document:

class AccountMove(models.Model):
    _name = "account.move"
    _inherit = [
        _name,
        "l10n_br_fiscal.document.move.mixin",
    ]
    _inherits = {"l10n_br_fiscal.document": "fiscal_document_id"}

https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_account/models/account_move.py#L79

Então a ideia é que se criarmos um fiscal.document quando o l10n_br_account estiver instalado e o contexto determinar que sim, criaremos também um account.move.

Isso garante um pouco mais de integridade na criação de documentos fiscais, mas acaba amarrando um pouco o processo para ser sempre desta forma, não sei dizer nesse momento se seria melhor opção, podemos colocar um contexto pra determinar quando isso precisa ser feito.

Nesse momento só vejo a necessidade no caso da importação do documento fiscal para fins de uso vinculado aos processos como faturamento/compras/estoque. No futuro podemos ver outros casos de uso de não criação.

@rvalyi @antoniospneto @renatonlima @ygcarvalh @luismalta @renatonlima

@rvalyi
Copy link
Member

rvalyi commented Oct 31, 2023

Na boa @mileo , vcs estão perdendo tempo. Eu falei sexta que ia subir que a gente começou aqui e que é muito diferente do que vcs propuseram nesses 3 meses gastando nosso tempo de review e continua diferente de agora onde tá melhorzinho mas continua bem longe do que vou propor daqui pouco; por isso não posso apenas dar uma dica ou outra para acertar, para a gente não compensa. Novamente @mileo isso não é culpa do @hirokibastos nem dos outros iniciantes da KMEE , o problema é de vc insistir em botar iniciantes para fazer coisas críticas na OCA. E não é noia minha não, esses 10 anos vc viu que pessoas iniciantes assim não conseguem emplacar um só módulo nem ou outros repos da OCA na nem KMEE nem na China; viu como como foi com o Messias e outros... Vê de uns 10 PRs no OCA/pos quantos tiveram plebiscito por examplo... (aliás eu revisei lá p PR que eu tinha falado que eu ia revisar). Porque aqui teria que ser diferente e nivelar as coisas por baixo e depois ficar anos com projeto atrasado com designs problemáticos que não conseguem agregar novos contribuidores relevantes nem migrar direito? Novamente se em vez de tentar engolir o mundo, a KMEE tentasse fazer 10x menos coisas teria o dobro de código aceito na OCA.
O outro PR anterior da importação eu pedi para recortar e a parte OK fizemos o merge e olha que tou corrigindo várias coisas que poderiam ter entrado melhor já... Mas esses últimos tá longe demais desculpa.

@rvalyi
Copy link
Member

rvalyi commented Oct 31, 2023

assim, as vezes pode ser que um iniciante acerta alguma coisa na OCA felizmente. Mas vcs tem que estar preparado para que isso seja a exceção e não a regra. E do nosso lado dos reviews isso é problemático tb porque gasta muito nosso tempo (gratuitamente). Veja que quando a gente revisa um PR da Escodoo ou da Engenere o "signal to noise" é ordem de grandeza melhor do que com um pessoal recem formado ou que nao tem anos de de experiência de ERP ou com Odoo...

Não tou falando maluquice aqui, se tiver a mesma atitude em qualquer outro repo da OCA levaria o mesmo tipo de comentário. Veja que temos iniciantes na Akretion tb, mas a gente não bota para tentar fazer coisas difíceis na OCA...

@mileo
Copy link
Member

mileo commented Oct 31, 2023

@rvalyi seus feedbacks sempre são um exemplo de liderança, eles são extremamente construtivos, sempre elogiando e propondo evoluções na arquitetura, ou ainda comentando explicitamente como melhorar, ou fazendo uma revisão profunda dos pull requests. Isso seria um sonho, mas não é verdade infelizmente. A grande maioria é:

  • Vou fazer do meu jeito;
  • Vou revisar depois e nunca mais apareço
  • Vou revisar depois mas já não concordo com o que esta ai;
  • Vou fazer do meu jeito e o PR nunca chega.
  • etc.

Você pode tocar a Akretion da maneira que você bem entender, mas aqui você esta lidando com contribuidores e sinceramente o seu tom foge do aceitável.

Se seu comentário não for para agregar, pule o pull request por gentileza. Você não vai ver ninguém do nosso time sendo escroto com ninguém da comunidade e também não fazemos isso no dia dia internamente, pois não somos donos da verdade.

Voltando ao assunto anterior você tenta infelizmente abraçar muita coisa sozinho, ao invés de delegar. Querendo criar a arquitetura da NASA, enquanto tem clientes reclamando do seu serviço.

Tem que existir um meio termo entre a arquitetura perfeita e o cliente feliz, sim refatorar da trabalho, mas faz parte dos processos de software.

Nos e outros contribuidores, estamos aqui para ajudar e ajudar uns aos outros, como estamos fazendo com outras empresas que contribuem aqui.

Outro ponto final, muito dos PRs que abrimos nos últimos anos ajudaram a trazer a tona problemas que vocês nem tinham no radar. Se a discussão do PR fosse mais produtiva em como resolver da melhor forma o problema de arquitetura seria muito melhor e não estaríamos tendo esse tipo de discussão.

@mileo
Copy link
Member

mileo commented Oct 31, 2023

Referente a este PR ainda temos o problema dos vencimentos / duplicatas, como fazer para eles gerarem os lançamentos com o vencimento correto. Esse é um ponto que estamos analisando e aceitamos sugestões.

@mileo
Copy link
Member

mileo commented Nov 2, 2023

@hirokibastos ficou bem enxuto o código, ta no caminho, vamos combinar de vc me mostar.

Agora é adapatar com o a integração do purchase e do estoque.

Tem que verificar se não da pra deixar parte do código no l10n_br_account para que não seja necessário criar um módulo de cola como no #2735

Caso o módulo l10n_br_nfe não esteja instalado a ideia é que os módulos de estoque e purchase apenas exibam um erro que não foi possível importar esse documento fiscal.

Então podemos ter a possibilidade de importação de XML de CT-E / SaT e etc na mesma lógica.

@mileo
Copy link
Member

mileo commented Nov 2, 2023

@hirokibastos outra coisa, vc pode ver oq houve com os testes?

@rvalyi
Copy link
Member

rvalyi commented Nov 9, 2023

@hirokibastos @mileo eu tou enrolado numa migração mas para evitar de enfiar codigo no improviso no projeto eu tomei o tempo de fazer o PR da nossa alternativa aqui: #2781 Como eu avisei não tem nenhum codigo em comum nem com esse PR nem com #2735 por isso depois de uns 4 meses da KMEE mexer nisso com 2 ou 3 iniciantes boiando sem eu ver isso progredir na direção certa eu acabei desacreditando que seria possivel apenas guiar o @hirokibastos com uma dica ou outra como eu tenho feito nos PRs de dfe, cte e mdfe que ja estavam mais proximo de algo razoavel.

@mileo vc reclama que eu não comentei logo o que não tava legal nos PRs de vcs. Mas francamente, é totalemente obvio que qualquer modulo que pretende relacionar entrada de estoque com NFe precisa como primeiro passo importar o account.move como espelho da NFe em vez de gerir a nota de novo. O @antoniospneto sacou isso na hora ao ver o primeiro PR de vcs (sendo que antes de eu pedir de dividir ele em 2 tinha varios outros problemas a mais que eu tinha comentado) . @mileo se vc não suja mais a mão com codigo (estranho todos contribuidores fortes da OCA continuam sujando a mão com codigo né), se seu foco é no funcional, para que serve se vc não se toca sobre erros conceituais desses? Quando vi vc começar a improvisar sobre os inherits no inicio desse PR, francamente perdi qualquer esperança de vc conseguir guiar o @hirokibastos para a gente ter um design robusto aqui... Lamento pelo "tom" como vc disse mas tentei te dar o toque de forma mais suave uns meses atras com #2586 (comment) e como eu vi o mesmo tipo de extrapolo aqui acabei insistindo tb, não na intenção de magoar, mas sim de vc se tocar para não ter a frustração de trabalhar para nada...

Super acho que o @hirokibastos é alguem que tem potencial (e alias parabenizei qdo ele contribuiu o wizard de depare de importação 2 anos atras alias #1985 (review)), mas francamente, é bom começar com coisas mais simples... Essa nossa nota fiscal electronica aqui é a mais complexa do mundo infezlimente. Da para se inspirar do @marcelsavegnago @antoniospneto ou @felipemotter que começaram a contribuir não tanto mais tempo antes do @hirokibastos e já tem muitos PRs aqui e na OCA em geral. São pessoas mais experientes mas principalmente tiveram a humildade de começar por coisas mais simples e ir aos poucos hoje contribuindo muitas coisas super bacanas esses meses que vcs não estavam muito por dentro do projeto... Ajudar para ser ajudado, não chegar apenas para disputar design de novas funcionalidades e exigir reviews e merges, enfim aquelas coisas de qualquer projeto open source... Ter mais experiencia com projetos reais tb é muito importante....

Vc pode pensar que vale enfiar qualquer prototipo no codebase que implementa qualquer coisa de forma superficial. Mas não vale não, não é a toa que nehnum repo da OCA aceita mais improviso nos modulos criticos. Pois depois se alguem quiser desenvolver a importação de documentos fiscais na v14 e um outro vai precisar na v16 ai fudeu, cada um vai querer fazer de um jeito e vai divergir, vai fazer um fork dos modulos no proprio projeto... Por isso tem que saber exactamente o que faz quando toca nos modulos centrais para a gente ter eles com design estavel quanto antes.. Ja estamos com 3 versões atrasadas (e vc com seu fork cagado e 2017 que forçou em focar em quantidade em vez de qualidade tem boa culpa no cartorio), não da tempo de ficar improvisando e atrasando para sempre, isso é mo tiro no pé para todo mundo. Da para aprender tocando os projetos, fazendo módulos menos criticos (tipo perfeito os modulos de consulta de CNPJ ou de NFSe Focus) ou ainda deixando os modulos "gambiarra" que vcs precisam em repos fora da OCA assim como a maiora do pessoal faz ou ainda se virando com os PRs e o git-aggregate.... Enfim é o que eu acho.

@rvalyi
Copy link
Member

rvalyi commented Apr 10, 2024

vou fechar essa então agora que ficou claro que #2781 é o caminho a seguir.

@rvalyi rvalyi closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants