feature/supply-priority-expiration
Task: https://trello.com/c/Ihjztzs9/22-backend-ap%C3%B3s-4horas-trocar-a-prioridade-de-precisa-urgente-para-precisa
BLOCKED: Depende de #60.
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?
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
createdAteupdatedAtno 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?
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.
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
gtusando 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.
@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
@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.
@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!!!
Antes de aprovar, vou deixar 3 sugestões
- Aumentar a periodicidade do CRON (10 segundos é muito pouco)
- 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?
- Colocar o intervalo de tempo pra prioridade ser diminuida no .env também
Obrigado @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
Aguardando #60 para corrigir implementação.
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.