feature/supply-priority-expiration

[gustavocs]

Task: https://trello.com/c/Ihjztzs9/22-backend-ap%C3%B3s-4horas-trocar-a-prioridade-de-precisa-urgente-para-precisa

BLOCKED: Depende de #60.

[AlbuquerqueRafael]

Olá, @gustavocs. Obrigado pelo pull request.

[filipepacheco]

cumpre o requisito, apenas deve fazer a change sugerida em #20 (comment)

[gustavocs]

cumpre o requisito, apenas deve fazer a change sugerida em #20 (comment)

@filipepacheco @AndersonCRocha concordo, dava pra fazer com um updateMany. mas vou ter que dividir em duas queries de qualquer forma porque por alguma razão os campos createdAt e updatedAt no BD são varchar e não consigo usar os operadores de menor ou igual (lte). então faria essa lógica no próprio método. alguma objeção?

[AndersonCRocha]

cumpre o requisito, apenas deve fazer a change sugerida em #20 (comment)

@filipepacheco @AndersonCRocha concordo, dava pra fazer com um updateMany. mas vou ter que dividir em duas queries de qualquer forma porque por alguma razão os campos createdAt e updatedAt no BD são varchar e não consigo usar os operadores de menor ou igual (lte). então faria essa lógica no próprio método. alguma objeção?

Rapazz, eu acho q o momento de ajustar o campo no banco pro tipo correto seria agora q tá no início heim, não faz sentido ser varchar, @filipefraga sabe a motivação?

[filipepacheco]

ok, vou abrir uma issue pra migrar os campos de string pra timestamp.

@gustavocs
o prisma não tem suporte a operadores de cast?
o operador gt usando Date() não funcionaria com essa string de timestamp?

se não, vamos manter assim porque de fato vai seguir sendo 2 queries.

[gustavocs]

ok, vou abrir uma issue pra migrar os campos de string pra timestamp.

@gustavocs o prisma não tem suporte a operadores de cast? o operador gt usando Date() não funcionaria com essa string de timestamp?

se não, vamos manter assim porque de fato vai seguir sendo 2 queries.

@filipepacheco Não consigo converter a Date() pra fazer a query porque os parâmetros da query do prisma são tipados. Tentei sem fazer o cast pra Date e usar lte e não executa conforme o esperado. Acho que foi justamente por isso que a primeira lógica tinha feito assim e não lembrava bem.

Sobre a migração: pelo que vi, não é só nesta tabela. Várias outras estão usando varchar pra createdAt e updatedAt.

[AndersonCRocha]

@filipepacheco Realmente o prisma n oferece o suporte no query builder pro cast, tá deixando a desejar demais, TB n oferece suporte pra inserir uma RAW Clause no meio do query builder, forçando converter toda a query para nativa com $queryRaw, ou então fazer alguma gambiarra. Que foi o caso do unnaccent do outro PR

[filipepacheco]

@AndersonCRocha sim, estamos descobrindo vários pontos negativos do Prisma...

Mas sem problema. Podemos subir assim caso essa entrega seja requisitada para deploy e depois da migração em todas as tabelas passamos por aqui pra arrumar.

Vou aprovar.

[AndersonCRocha]

@gustavocs é importante ressaltar que na sua implementação não está sendo 2 queries não, tá sendo 1 query + N updates em que N é a quantidade de registros q deverão ser alterados, se for continuar com essa abordagem ao invés de corrigir o tipo no banco de dados, sugiro vc fazer um filter() na lista pra pegar os expirados, depois um map() para obter os ida e por fim um único update com where IN

Edit 1: PutzZz, acabei de ver que ShelterSupply é chave composta, deu ruim, desisto!!!

[filipepacheco]

Antes de aprovar, vou deixar 3 sugestões

1. Aumentar a periodicidade do CRON (10 segundos é muito pouco)
2. Colocar a periodicidade no .env pra que seja de fácil ajuste sem a necessidade de deploy
   2.1. Essa aqui tá sendo usado as constantes do Nest. Será que tem como usar o .env de maneira amigável nesse caso?
3. Colocar o intervalo de tempo pra prioridade ser diminuida no .env também

Obrigado @gustavocs

[gustavocs]

@gustavocs é importante ressaltar que na sua implementação não está sendo 2 queries não, tá sendo 1 query + N updates em que N é a quantidade de registros q deverão ser alterados, se for continuar com essa abordagem ao invés de corrigir o tipo no banco de dados, sugiro vc fazer um filter() na lista pra pegar os expirados, depois um map() para obter os ida e por fim um único update com where IN

Edit 1: PutzZz, acabei de ver que ShelterSupply é chave composta, deu ruim, desisto!!!

Sim, eu tava implementando um filter e percebi que não teria como fazer com whereIn. Tá difícil. Vou fazer as mudanças sugeridas pelo @filipepacheco amanhã cedo. Valeu pelo review <3

[gustavocs]

Aguardando #60 para corrigir implementação.

[filipepacheco]

por gentileza corrige o erro de lint

[gustavocs]

por gentileza corrige o erro de lint

Corrigido. O erro de lte vai vai seguir até os campos serem migrados pra timestampz. Já fiz o teste local e funciona.
Esse PR tá dependendo do #60, vou adicionar na descrição.