-
Notifications
You must be signed in to change notification settings - Fork 388
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
Tratamento de publicações patrocinadas no backend (API) #1719
Conversation
…receiving a comment
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deixei o PR em draft porque ainda há algumas pendências que gostaria da sugestão de como implementar. Além disso, estou deixando dois comentários aqui no código sobre pontos que não consegui resolver da forma ideal.
Como o PR ficou bem grande, já que é praticamente toda a funcionalidade na parte do backend, a revisão deve ser difícil e demorada. Apesar disso, cada commit representa certas funcionalidades, então talvez seja mais fácil revisar commit a commit. Se terem alguma sugestão para facilitar a revisão, ou dúvidas sobre o código, podem falar que eu respondo.
sponsored_content: function () { | ||
return Joi.object() | ||
.required() | ||
.concat(schemas.id()) | ||
.concat(schemas.owner_id()) | ||
.concat(schemas.slug()) | ||
.concat(schemas.title()) | ||
.concat(schemas.body()) | ||
.concat(schemas.source_url()) | ||
.concat(schemas.created_at()) | ||
.concat(schemas.updated_at()) | ||
.concat(schemas.published_at()) | ||
.concat(schemas.deactivate_at()) | ||
.concat(schemas.owner_username()) | ||
.concat(schemas.tabcoins()); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tentei algumas formas diferentes, mas não consegui fazer o required
funcionar reaproveitando o schema
com concat
.
if (feature === 'read:sponsored_content:list') { | ||
filteredOutputValues = output.map((content) => { | ||
return validator(content, { | ||
sponsored_content_complete: 'required', | ||
}); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Não consegui pensar numa boa forma para tratar esse retorno de modo que apenas o próprio usuário e “moderadores” possam ver essas informações completas, deixando o endpoint "desabilitado" para os outros usuários, ou pelo menos não retornando informações como tabcash
.
O endpoint que utiliza este filtro é o GET /api/v1/sponsored_contents/[username]
.
Massa @Rafatcb! 🤩🤩🤩 Mesmo antes de ver o código, já dá para comentar sobre o texto.
😱 Todos fazem sentido serem modificados? Não é melhor criar novos endpoints e deixar a funcionalidade mais desacoplada?
Faz sentido limitar a quantidade de publicações exibidas simultaneamente em uma página, mas não a quantidade de publicações que podem estar ativas no sistema. A quantidade ativa deve ser grande para que a exibição seja otimizada, para dar mais visibilidade para quem contribuiu mais (quem tem mais TabCash). Essa otimização tem que considerar todo mundo que quiser publicar, não só os 5 primeiros que conseguirem.
Publicidades devem ser classificadas entre as publicidades, e conteúdos entre conteúdos. O fato das publicidades aparecerem na lista de conteúdos não implica na existência de uma conversão entre os rankings.
Não precisa considerar o tempo. Podemos considerar apenas as exibições e interações, sendo que deve existir um peso bem maior nas interações do que nas exibições, o que nos permite começar considerando apenas as interações. Limite de tempo pode ser apenas o que a pessoa que publicou definiu.
Com apenas 5 publicações ativas por vez, ficaria difícil tirar conclusões válidas, pois o acaso pesaria muito.
Por IP pode manter a mesma regra atual.
O ranqueamento atual não conta todos os comentários, mas apenas 1 de cada diferente usuário.
Não tem nada que dê para isolar em PRs separados? Mesmo que seja algo que não vá ser utilizado antes da implementação completa, mas se tiver partes que não precisam de muito debate, ou que não dependam de muitas decisões sobre outras partes, pode facilitar a revisão. |
Modifiquei para que as funcionalidades de hoje continuem funcionando com as publicações patrocinadas. Por exemplo, um aplicativo hoje que utilize algumas dessas rotas para buscar a lista de publicações trará a patrocinada; ao entrar na publicação, permitirá buscar a lista de comentários; ao entrar no comentário, permitirá buscar o pai; ao editar, continuará funcionando para editar os valores em comum (
Interpretei errado, então.
Cada commit poderia ser um PR, com pouca ou nenhuma adaptação. Mas algumas coisas seriam melhor avaliadas enxergando o todo, como no caso da migração criando as tabelas e funções (c497fa0), para para entender a necessidade de cada campo, tabela, índice e função. Por exemplo, só percebi a necessidade de Separar complicaria um pouco. Se cada PR puder ter migrações junto, visto que é uma funcionalidade não liberada, complicaria menos do que criar em pares PR de Migração + PR de funcionalidade. |
Vou fechar este PR. Depois abrirei outros PRs conforme comentei, para colocarmos em produção a funcionalidade em pequenas partes. |
Mudanças realizadas
Implementação do backend para o issue #1491.
Endpoints
Novos endpoints:
POST /api/v1/sponsored_contents
GET /api/v1/sponsored_contents/[username]
PATCH /api/v1/sponsored_contents/[username]/[slug]
Endpoints com comportamento modificado para atender tanto publicações patrocinadas quanto conteúdos normais:
GET /api/v1/contents
GET /api/v1/contents/[username]/[slug]
PATCH /api/v1/contents/[username]/[slug]
POST /api/v1/contents/[username]/[slug]/tabcoins
GET /api/v1/contents/[username]/[slug]/thumbnail
GET /api/v1/contents/[username]/[slug]/children
GET /api/v1/contents/[username]/[slug]/parent
DELETE /api/v1/users/[username]
Características dessa implementação
create:sponsored_content
update:sponsored_content
update:sponsored_content:others
tabcash ^ 0.6 / 1.6
, que gera uma curva como ilustrada abaixo, onde alguns pontos para referência são:Os limites de publicações patrocinadas simultaneamente que defini podem ser ampliados futuramente, mas acho importante algo mais restrito para termos um controle maior nessa fase inicial e entendermos o que funciona e o que não funciona.
Pendências/pontos de atenção
deactivate_at && deactivate_at < new Date()
: essa condição é utilizada em alguns lugares diferentes do código, mas não encontrei uma boa forma para encapsular isso melhor. Acredito que o ideal seria já tratar na própria query, mas fiquei em dúvida sobre como implementar sem complicar ainda mais as consultas nomodels/content
.POST /api/v1/contents/[username]/[slug]/tabcoins
: fiquei em dúvida se deveria retornartabcoins_credit
etabcoins_debit
com o valor0
como é feito em alguns outros endpoints.sponsored_content_tabcash_operations
) conforme o tempo passa, e acredito que quando isso for implementado, essa parte do cálculo poderá sair da consulta emrankingQueries
.deactivate_at
: experimentei criar um índice para osponsored_contents.deactivate_at
compgm.createIndex('sponsored_contents', ['deactivate_at'])
, mas nos testes que fiz ele não era utilizado. As consultas envolvendo essa coluna são sempreWHERE deactivate_at IS NULL OR deactivate_at > NOW()
, ou apenasWHERE deactivate_at IS NULL
.Tipo de mudança
Checklist: