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][WIP][NEW] l10n_br_cte #2586

Closed
wants to merge 35 commits into from
Closed

Conversation

ygcarvalh
Copy link
Contributor

@ygcarvalh ygcarvalh commented Jul 5, 2023

Pegando a ideia do que temos com a NFe e baseado na branch deste PR: #2596, o módulo vai realizar as implementações necessárias para a emissão desse modelo de documento fiscal e deixar padronizado com o que temos na localização atualmente.

Esses modelos iniciais são os que consegui pensar que são os básicos para esse tipo de documento e posteriormente devem ser adicionados mais para abranger todos os casos disponíveis.

cc: @mileo @AndreMarcos

@ygcarvalh ygcarvalh force-pushed the 14.0-add-l10n_br_cte branch 4 times, most recently from 8f4d3d3 to d7ff673 Compare July 10, 2023 18:16
@rvalyi
Copy link
Member

rvalyi commented Jul 21, 2023

Ola @ygcarvalh @ODBreno , eu vejo que nos mappings voces estão usando campos como cte40_choiceXY com numero XY. Isso parece ser coisas que vinha da geração pelo generateDS. Porem a geração destes campos choice do generateDS era um ponto fraco dele: se tivesse uma mudança forte de algum xsd, era bem capaz do generateDS mudar toda numeração arbitraria desses campos choice e ia zoar tudo. O xsdata não gera esses campos choices (nos bindings nfelib não tem) eu apenas fiz algo com xpath dentro do xsdata-odoo para prencher o attributo choice quando o campo pertence de uma tag choice: https://github.com/akretion/xsdata-odoo/blob/master/xsdata_odoo/generator.py#L245

Esta atributo choice não esta sendo usado pelo spec_driven_model na v14, mas poderia ser (em especial depois de migrar o sistema de visão automatica).

Enfim onde eu quero chegar: eu acho valido introduzir campos choice como vcs fizeram na PR, porem eu acho melhor botar um nome mais explicito do que cte40_choiceXY. Se achar uma forma de botar um nome sistematico unico sem ser um numero arbitrario melhor ainda. Mas é bom que o nome do campo começa por cte40_choice mesmo, apenas tem que trocar esses numeros.
Inclusive tou com a ideia de fazer um script de migração no l10n_br_nfe para mudar a dezena de canpos nfe40_choiceXY que temos lá e que vem do generateDS.

@ygcarvalh eu tb tinha respondido para vc aqui #2596 (comment)

cc @antoniospneto @marcelsavegnago @renatonlima

@rvalyi
Copy link
Member

rvalyi commented Aug 4, 2023

Ola @ODBreno so para entender, qual é a ideia quando vc defina campos related como ?

    cte40_xNavio = fields.Char(related="xNavio", store=True)

    xNavio = fields.Char(string="Identificação do Navio", store=True)

em https://github.com/kmee/l10n-brazil/blob/14.0-add-l10n_br_cte/l10n_br_cte/models/aquaviario.py#L28C1-L30C70 ?
isso parece ser meio sistemático, ai eu fico me perguntando se não seria melhor automatizar e para que serve? Eh uma estrategia para a CTeOS?

@ODBreno
Copy link
Contributor

ODBreno commented Aug 7, 2023

Então @rvalyi, estou colocando os campos sem cte40 quando eles tem que ser preenchidos manualmente, quando não existe nenhum campo que já possui essa informação no odoo, aí o campo sem cte40 seria o campo que vai pra view e o com cte40 o que vai ser usado pra gerar o xml, nos casos em que essa informação já está em algum lugar eu só uso o related e não faço isso de tirar o cte40, mas não sei se é a maneira mais otimizada e legível de fazer isso, teria ideia de alguma forma melhor de estruturar?

@rvalyi
Copy link
Member

rvalyi commented Aug 7, 2023

Ou seja, aínda bem que eu perguntei... Então a grande justificativa do gerador é exactemente de não ter que re-definir esses campos manualmente... Ou seja quando o campo não existe no Odoo deixa o campo com o nfe40_ para armazenar a informação mesmo, é exactemente feito para isso! duplicar isso tudo para que? Pense DRY.

Outra coisa: se eu fosse vc eu ia começar por botar testes de importação e exportação e eu ia apenas aumentar os mappings verificando que os testes continuam passando. Senão vc pode ficar mapeando 1 mês e depois descobrir que não tá funcionando e vai ser procurar a agulha de procurar o que impede de funcionar...

Sobre as views, eu pretendo migrar e melhorar a parte da visões automáticas do spec_driven_model, em breve, pode deixar essa parte das visões para depois para não fazer trabalho inútil com visões manuais que poderiam ser automáticas ou pelo menos em grande parte.

cc @renatonlima @marcelsavegnago @antoniospneto @mileo @ygcarvalh @mbcosta

@rvalyi
Copy link
Member

rvalyi commented Aug 8, 2023

Olá @ODBreno , vi que vc tirou os related inúteis porém, continuou com overrides por exemplo do campo cte40_navio. Porque esses overrides?? Vi que vc botou store=True la dentro. Isso já é o padrão quando o campo não tem atributo 'compute', ou seja não precisa definir esses store=True. Daí eu pergunto: são necessários mesmos aqueles overrides todos?

@mileo vc tem certeza que vc está colocando as pessoas certas para contribuir coisas na OCA?? Se a Kmee tem iniciantes, nada contra, todos nós já fomos. Agora se vc realmente praticasse de contribuír na OCA, vc viria que em nenhum repo da OCA os módulos estão sendo feitos por iniciantes, muito pelo contrário. Eu falo isso porque infelizmente é um problèma super recorrente com a sua empresa e que já tivemos que reclamar muito disso com vc para evitar erros importantes no projeto... Depois tem vários módulos, como portal, website_sale, gateway de pagamento que depois pessoas tentando usar de vdd fazem PRs e vcs nem revisam apesar dos módulos dizer que tem 3 ou 4 mantenedores da KMEE...

Aí quando eu vejo que vcs não dá conta de manter os módulos que vcs já subiram na OCA, começam agora esses novos módulos com pessoas boiando e aínda pretendem agora mexer com o SPED de novo eu fico com algumas dúvidas se toda essa agitação depois de quase 2 anos sumidos no projeto é bem razoável.

cc @marcelsavegnago @renatonlima @antoniospneto

@mileo
Copy link
Member

mileo commented Aug 8, 2023

Olá @ODBreno , vi que vc tirou os related inúteis porém, continuou com overrides por exemplo do campo cte40_navio. Porque esses overrides?? Vi que vc botou store=True la dentro. Isso já é o padrão quando o campo não tem atributo 'compute', ou seja não precisa definir esses store=True. Daí eu pergunto: são necessários mesmos aqueles overrides todos?

@mileo vc tem certeza que vc está colocando as pessoas certas para contribuir coisas na OCA?? Se a Kmee tem iniciantes, nada contra, todos nós já fomos. Agora se vc realmente praticasse de contribuír na OCA, vc viria que em nenhum repo da OCA os módulos estão sendo feitos por iniciantes, muito pelo contrário. Eu falo isso porque infelizmente é um problèma super recorrente com a sua empresa e que já tivemos que reclamar muito disso com vc para evitar erros importantes no projeto... Depois tem vários módulos, como portal, website_sale, gateway de pagamento que depois pessoas tentando usar de vdd fazem PRs e vcs nem revisam apesar dos módulos dizer que tem 3 ou 4 mantenedores da KMEE...

Aí quando eu vejo que vcs não dá conta de manter os módulos que vcs já subiram na OCA, começam agora esses novos módulos com pessoas boiando e aínda pretendem agora mexer com o SPED de novo eu fico com algumas dúvidas se toda essa agitação depois de quase 2 anos sumidos no projeto é bem razoável.

cc @marcelsavegnago @renatonlima @antoniospneto

Olá @rvalyi,

A base do módulo foi habilmente projetada por @ygcarvalh, e o time liderado por @AndreMarcos está se dedicando ao desenvolvimento desse módulo.

Sendo que tanto @ygcarvalh quanto @AndreMarcos são responsáveis pela revisão inicial.

Uma vez concluída essa etapa, o PR sairá do status de draft, e uma revisão mais ampla da comunidade será solicitada, incluindo a minha participação.

Estou confiante na capacidade e dedicação do time de @AndreMarcos, que tem aprendido e crescido significativamente através das revisões de toda a comunidade.

Embora seja um time relativamente novo, com apenas 6 meses, suas contribuições relevantes nos últimos meses incluem:

  • PIX com BACEN;
  • Consulta de CNPJ;
  • NFS-e GINFES;
  • NFS-e Barueri;
  • Entre outras iniciativas menores que não me recordo no momento.

A visão estratégica é que essa squad se concentre somente em melhorias para a localização, com @ygcarvalh supervisionando a equipe de @AndreMarcos (mudança que fizemos não tem 30 dias). Por isso, novos pull requests significativos estão sendo abertos pelo @ygcarvalh e deverão ser conduzidos pelo time de @AndreMarcos a longo prazo.

Além disso, mantenho conversas diárias com @ygcarvalh para discutir questões de design e arquitetura, sem me perder em detalhes microgerenciais, como campos com store=true, por exemplo.

Em relação às questões dos módulos de portal, website e gateway, listei os seguintes pull requests:

#2030
#2032
#2309

Caso haja algum outro pull request precisando de revisão, sinta-se à vontade para me marcar, assim como @ygcarvalh, e prontamente daremos a devida atenção.

@rvalyi
Copy link
Member

rvalyi commented Sep 22, 2023

@mileo @ygcarvalh eu vejo que vcs andaram bastante neste PR. Por cima eu não vi coisas absurdas a ultima vez que eu olhei uma semana atras. Talvez daqui pouco então entraremos em fase de review, qual a posição de vcs?

  1. Afinal, como vcs trataram a questão da semelhança CTe e CTeOS? (@ygcarvalh e eu falamos sobre isso aqui [14.0][l10n_br_cte_spec] leiautes abstratos para Conhecimento de Transporte electrónico (CT-e) versão 4.00 #2596 )
  2. Não seria ideal também á proceder com o merge do modulo l10n_br_cte_spec aqui [14.0][l10n_br_cte_spec] leiautes abstratos para Conhecimento de Transporte electrónico (CT-e) versão 4.00 #2596 se vcs acham que ele ficou certinho gerido dessa maneira?

Eu estou disposto a debater o gerador se tem que melhorar alguns aspectos mas até onde eu testei me parece que dava conta das MDF-e e CT-e sem problema. Tem sim que melhorar a definição das classes 'stacked" para funcionar com vários esquemas com StackedModel e talvez melhorar algumas coisas para lidar com a semelhança CTe/ CTeOS, por isso eu perguntei como vcs abordaram isso. Isso eu consigo ajudar logo (desde fim de julho eu estava enrolado para dar um start correto nos módulos de SPED ja que se trata de muito codigo não seria aceitavel botar qualquer coisa na OCA e depois criar muita entropia impossível de limpar depois).

E ai avançamos com #2596 para facilitar os reviews aqui depois?

cc @renatonlima @mbcosta @marcelsavegnago @antoniospneto @felipemotter @crsilveira

@rvalyi rvalyi mentioned this pull request Sep 22, 2023
3 tasks
@mileo
Copy link
Member

mileo commented Sep 22, 2023

@mileo @ygcarvalh eu vejo que vcs andaram bastante neste PR. Por cima eu não vi coisas absurdas a ultima vez que eu olhei uma semana atras. Talvez daqui pouco então entraremos em fase de review, qual a posição de vcs?

  1. Afinal, como vcs trataram a questão da semelhança CTe e CTeOS? (@ygcarvalh e eu falamos sobre isso aqui [14.0][l10n_br_cte_spec] leiautes abstratos para Conhecimento de Transporte electrónico (CT-e) versão 4.00 #2596 )
  2. Não seria ideal também á proceder com o merge do modulo l10n_br_cte_spec aqui [14.0][l10n_br_cte_spec] leiautes abstratos para Conhecimento de Transporte electrónico (CT-e) versão 4.00 #2596 se vcs acham que ele ficou certinho gerido dessa maneira?

Eu estou disposto a debater o gerador se tem que melhorar alguns aspectos mas até onde eu testei me parece que dava conta das MDF-e e CT-e sem problema. Tem sim que melhorar a definição das classes 'stacked" para funcionar com vários esquemas com StackedModel e talvez melhorar algumas coisas para lidar com a semelhança CTe/ CTeOS, por isso eu perguntei como vcs abordaram isso. Isso eu consigo ajudar logo (desde fim de julho eu estava enrolado para dar um start correto nos módulos de SPED ja que se trata de muito codigo não seria aceitavel botar qualquer coisa na OCA e depois criar muita entropia impossível de limpar depois).

E ai avançamos com #2596 para facilitar os reviews aqui depois?

cc @renatonlima @mbcosta @marcelsavegnago @antoniospneto @felipemotter @crsilveira

O CT-e esta um pouco atrasado com relação ao MDFE, o MDFE já esta sendo transmitido e consideramos o layout valido, portanto podemos prosseguir com o merge do #2485

Com relação ao CTe e o CTeOS @lfdivino vc já consegue comentar sobre?

O @ygcarvalh ta de folga hj, creio q só segunda. E o @felipezago tb, que esta com o MDFE.

@rvalyi
Copy link
Member

rvalyi commented Sep 22, 2023

@mileo @ygcarvalh eu vejo que vcs andaram bastante neste PR. Por cima eu não vi coisas absurdas a ultima vez que eu olhei uma semana atras. Talvez daqui pouco então entraremos em fase de review, qual a posição de vcs?

  1. Afinal, como vcs trataram a questão da semelhança CTe e CTeOS? (@ygcarvalh e eu falamos sobre isso aqui [14.0][l10n_br_cte_spec] leiautes abstratos para Conhecimento de Transporte electrónico (CT-e) versão 4.00 #2596 )
  2. Não seria ideal também á proceder com o merge do modulo l10n_br_cte_spec aqui [14.0][l10n_br_cte_spec] leiautes abstratos para Conhecimento de Transporte electrónico (CT-e) versão 4.00 #2596 se vcs acham que ele ficou certinho gerido dessa maneira?

Eu estou disposto a debater o gerador se tem que melhorar alguns aspectos mas até onde eu testei me parece que dava conta das MDF-e e CT-e sem problema. Tem sim que melhorar a definição das classes 'stacked" para funcionar com vários esquemas com StackedModel e talvez melhorar algumas coisas para lidar com a semelhança CTe/ CTeOS, por isso eu perguntei como vcs abordaram isso. Isso eu consigo ajudar logo (desde fim de julho eu estava enrolado para dar um start correto nos módulos de SPED ja que se trata de muito codigo não seria aceitavel botar qualquer coisa na OCA e depois criar muita entropia impossível de limpar depois).
E ai avançamos com #2596 para facilitar os reviews aqui depois?
cc @renatonlima @mbcosta @marcelsavegnago @antoniospneto @felipemotter @crsilveira

O CT-e esta um pouco atrasado com relação ao MDFE, o MDFE já esta sendo transmitido e consideramos o layout valido, portanto podemos prosseguir com o merge do #2485

Com relação ao CTe e o CTeOS @lfdivino vc já consegue comentar sobre?

O @ygcarvalh ta de folga hj, creio q só segunda. E o @felipezago tb, que esta com o MDFE.

Sem pressa, se trata de anos de trabalho de qualquer jeito então uns dias a mais ou a menos não é o que vai fazer diferença. Eu avisei logo que a questão da MDFe era mais fácil, então se rolar o merge do #2485 já ajuda a fatiar o boi. Eu espero que fica claro agora as vantagens de ter fatiado o código assim onde isolamos ao máximo o código manual com decisões de humanos da grande massa de código cuja geração e atualização podem ser automatizadas; a meu ver era a unica forma de lidar com tantas especificações de forma open source (basta ver o diff entre CTe 3 e CTe4 por exemplo para ver que sem gerador não era viável ou pelo menos não poderia se chamar de código "OCA").

@ygcarvalh
Copy link
Contributor Author

@rvalyi

Quando eu estava pensando no roadmap de desenvolvimento e utilizando o seu PR do spec como base, eu acabei pedindo para que fosse quebrado em dois processos o desenvolvimento: primeiro fazemos a emissão da CT-e e, só então com a validação, passamos para a CT-e OS e, posteriormente, a GTV. Fiz isso pensando no sentido de que, como os campos são muito parecidos (e eu validei o documento várias vezes), é muito mais trabalhoso tentar criar algo extremamente mirabolante para emitir as duas de uma única vez do que quebrar em metas menores. Claro, todo esse processo pensando para que essas melhorias ficassem mais fáceis de serem implementadas depois.

Em relação a parte do spec, eu não vejo porque não realizar o merge nesse momento. O único problema, até então, estava na definição da classe, e que você corrigiu até onde me lembro. Acredito que, com o processo de emissão da CT-e, ficaria mais fácil de pegar alguns problemas que passaram despercebidos, mas nada que alguns PR's com fix não resolvam. Isso também já diminui a quantidade de PR's que precisam ser mantidos no momento.

cc: @mileo @marcelsavegnago @lfdivino @ODBreno

@mileo
Copy link
Member

mileo commented Sep 25, 2023

Em relação a parte do spec, eu não vejo porque não realizar o merge nesse momento. O único problema, até então, estava na definição da classe, e que você corrigiu até onde me lembro. Acredito que, com o processo de emissão da CT-e, ficaria mais fácil de pegar alguns problemas que passaram despercebidos, mas nada que alguns PR's com fix não resolvam. Isso também já diminui a quantidade de PR's que precisam ser mantidos no momento.

Bora fazer o merge então dos specs!

@mileo
Copy link
Member

mileo commented Sep 27, 2023

Pode fazer um rebase?

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 12, 2024
@marcelsavegnago
Copy link
Member

marcelsavegnago commented May 14, 2024

@ygcarvalh @mileo @ODBreno @lfdivino podem fazer um rebase por favor ?

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 19, 2024
@marcelsavegnago
Copy link
Member

@ygcarvalh @felipezago @mileo podem fazer um rebase por favor ?

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Jun 14, 2024

@mileo pode fazer um rebase e fazer um squash dos commits ?

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Oct 13, 2024
@marcelsavegnago
Copy link
Member

Substituída por: #3492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants