backend
backend copied to clipboard
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
createdAt
eupdatedAt
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?
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
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
.
@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.