OCA / l10n-brazil

Localização brasileira oficial do Odoo.
https://odoo-community.org/psc-teams/brazil-66
GNU Affero General Public License v3.0
235 stars 244 forks source link

[14.0][NEW] Edoc Send Email #3183

Open mileo opened 1 month ago

mileo commented 1 month ago

Uppport: https://github.com/OCA/l10n-brazil/pull/1406

OCA-git-bot commented 1 month ago

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

antoniospneto commented 1 month ago

Sei que a PR ainda está em rascunho, mas gostaria de deixar minha opinião sobre o gatilho do método de envio de e-mail. Atualmente, esse método é acionado ao mudar o estado do documento, o que faz com que a lógica de envio seja processada na mesma requisição de comunicação com os webservices. Isso inclui a criação dos PDFs necessários para o anexo do e-mail, tornando a preparação do e-mail bastante pesada, mesmo que o envio em si seja programado para ocorrer em seguida.

Acredito que seria interessante remover esse gatilho para que o envio do e-mail não seja acionado na mesma requisição. Uma solução seria marcar uma flag, como "envio de e-mail pendente", e utilizar um cron para verificar a cada 1 ou 5 minutos os e-mails que precisam ser enviados, acionando então o método send_email.

O que acham?

@mileo @marcelsavegnago @rvalyi @renatonlima @felipemotter

mileo commented 1 month ago

Sei que a PR ainda está em rascunho, mas gostaria de deixar minha opinião sobre o gatilho do método de envio de e-mail. Atualmente, esse método é acionado ao mudar o estado do documento, o que faz com que a lógica de envio seja processada na mesma requisição de comunicação com os webservices. Isso inclui a criação dos PDFs necessários para o anexo do e-mail, tornando a preparação do e-mail bastante pesada, mesmo que o envio em si seja programado para ocorrer em seguida.

Acredito que seria interessante remover esse gatilho para que o envio do e-mail não seja acionado na mesma requisição. Uma solução seria marcar uma flag, como "envio de e-mail pendente", e utilizar um cron para verificar a cada 1 ou 5 minutos os e-mails que precisam ser enviados, acionando então o método send_email.

O que acham?

@mileo @marcelsavegnago @rvalyi @renatonlima @felipemotter

Vou considerar isso na implementação.

rvalyi commented 1 month ago

alem disso, essas coisas de notificação por email é mais um examplo de código que poderia ser estraido num módulo pequeno. Se o l10n_br_fiscal pesasse apenas 5000 linhas, seria de boa ter isso dentro. Mas com mais de 20k linhas definitivamente faz parte das coisas que poderiam ir num novo modulo...

O foco do l10n_br_fiscal realmente tem que ser o conceito basico de documento fiscal e o calculo dos impostos que até empresas do simples podem ter ou que são comuns numa NFe ou NFSe, o restante da para botar em outros módulos geralmente. Apenas com esse escopo que eu mentionei, já é difícil fazer menos de 10k linhas... Mas hoje 80% ou mais dos módulos que dependem do l10_b_fiscal não dependem dessas coisas de email, logo não ha motivo algum de ficar junto no mesmo modulo...

rvalyi commented 1 month ago

sendo que se for um outro módulo e vc nao gostar do desempenho, ai vc poderia nao usa esse módulo ou usa um modulo alternativo.

Tb hoje por exemplo o l10_br_fiscal é relativamente facil de migrar, mas por exemplo para migrar ele para a v16, eu tive que gastar tempo migrando os templates de e-mail de mako para qweb. Algo que não era tão necessário para destravar a migração de outros módulos; enfim apenas um exemplo de porque essa falta de modularidade do módulo l10n_br_fiscal é ruim.

mileo commented 1 month ago

alem disso, essas coisas de notificação por email é mais um examplo de código que poderia ser estraido num módulo pequeno. Se o l10n_br_fiscal pessasse apenas 5000 linhas, seria de boa ter isso dentro. Mas com mais de 20k linhas definitivamente faz parte das coisas que poderiam ir num novo modulo...

O foco do l10n_br_fiscal realmente tem que ser o conceito basico de documento fiscal e o calculo dos impostos que até empresas do simples podem ter ou que são comuns numa NFe ou NFSe, o restante da para botar em outros módulos geralmente. Apenas com esse escopo que eu mentionei, já é difícil fazer menos de 10k linhas...

Isso eu concordo que pode ser extraído em outro módulo sem problemas, pois tem as mesma sistemática do fiscal_close e não impacta vendas/compras/estoque.

Ou seja não faz parte do motor de impostos.

antoniospneto commented 1 month ago

acho interessante por em outro módulo tbm, até pq realmente podem ser usado estrategias diferentes para o envio dos e-mails. nós por exemplo estamos usando muito o módulo nativo de ações automatizadas para esse fim.

rvalyi commented 1 month ago

entao sobre extrair, quando mais cedo melhor... Alem disso isso sim eh algo que o cara nao precisa ser pica das galaxias para conseguir fazer.

mileo commented 1 month ago

Posso fazer isso nesse PR, primeiro passo é fazer isso pegar direito, ontem eu só fiz o cherry-pick dos commits e por incrível que pareça os teses passaram. Vamos revisar funcionalmente se continuar ok e planejar a extração.