Solicitar conteúdos root e child de um usuário separadamente #747
Conversation
|
@33gustavo33 is attempting to deploy a commit to the TabNews Team on Vercel. To accomplish this, @33gustavo33 needs to request access to the Team. Afterwards, an owner of the Team is required to accept their membership request. If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account. |
|
@33gustavo33 contribuição sensacional! Muito obrigado por ter chego inclusive a implementar os testes 😍 eu demorei para participar desse PR, pois estava montando o novo estúdio do canal, de qualquer forma, eu já tinha lido o código nos primeiros dias que enviou e ele e fiquei pensando bastante sobre a interface pública e qual a melhor forma de interagir com esses dados. Não consegui chegar a uma conclusão, mas gostaria de compartilhar as duas principais estratégias que acredito existir:
Opção 1Na primeira opção, me peguei pensando se não seria melhor ao invés de usar Então por exemplo, se já conseguimos acessar os conteúdos .../contents/[username]/[slug]/children Talvez acessar os conteúdos .../contents/[username]/children E daí na interface quando alguém entra na Home do Usuário, ele veria uma lista das últimas publicações Opção 2Nessa opção, ela usa o
Dado a isso, talvez ao invés de criarmos um query parameter com novo nome, poderíamos controlar o que queremos fazer com o
Do resto, tudo que rabisquei aqui ficou muito feio. Fora que isso pode também complicar na hora do validator entender o que é um ConclusãoNão cheguei a conclusão, mas gostaria de registrar aqui meus pensamentos enquanto isso. Mas aproveito para perguntar o que você achou da interface pública da Opção 1? |
|
@filipedeschamps A opção 1 é mais interessante porque ficaria um "padrão" com o resto da API |
|
Isso! Mas mesmo assim, eu vejo um furo nessa minha sugestão, porque seguindo ela à risca deveria ser assim:
Mas isso ao mesmo tempo impacta no endpoint
Fazer o design de uma API pública é difícil 😂 Se mais alguém estiver observando a discussão e quiser dar alguma sugestão, é super bem vindo 🤝 |
@filipedeschamps Acho que vale a pena fazer essa breaking change por 3 motivos:
Nunca concordei tanto com uma frase.... |
|
Pronto, com esse commit que adicionei agora o children e root são possíveis de acessar assim:
|
|
Acho que faz sentido Estou em dúvida se seria útil um endpoint que retornasse só os
Só tentando contribuir, mas....
Também concordo demais!!! hahaha
Nessa opção de duas navegações independentes provavelmente só vai compensar usar páginas estáticas para a página 1/1. E o restante das navegações compensaria mais fazer pelo client chamando a API. Por isso eu não gosto muito da ideia. O que acham de o padrão na home do usuário ser mostrar root e children tudo junto (classificado por tempo ou outra estratégia) e permitir filtrar os resultados pelo client? |
Gosto desta ideia! Podemos mostrar tudo na mesma lista e diferenciar com um ícone quando é uma resposta (quando tem
Eu imagino que em REST há uma alternativa que consiga ainda respeitar o objeto limpo na raiz, possivelmente com as Uma sugestão do que fazer, que é o mesmo princípio adotado no passado quando as coisas ficam complexas que é dar o menor passo possível, que para esse caso eu sugiro o seguinte:
Essa não é a pior breaking change do mundo, porque o objeto retornado é exatamente o mesmo, porém agora com dados que poderão estar Interessae notar que na página do usuário, O que me faz pensar se no novo retorno do |
Sim, gosto da ideia do ícone e também seria legal mostrar mais informações, como as que aparecem quando acessamos uma resposta. Como nesse exemplo:
Talvez também mostrar um pedaço do body. O que nos leva ao seu questionamento:
Podemos começar retornando só o que fizer sentido para montar a página do usuário. E só retornar outros dados se surgir necessidade. E pensando na sugestão acima de mostrar o "Em resposta a...", percebo que, ao contrário do que eu disse antes, temos sim semelhanças com a rota Que vontade de GraphQL! hahaha |
Acho que o único problema com essa ideia é que não teria 30 itens por página. Como a API retorna 30 itens por padrão |
@33gustavo33, você diz o problema do filtro, né? É verdade! Por enquanto é melhor sem esse filtro. Só o fato de diferenciar a apresentação dos conteúdos já é suficiente. Eu gostei da ideia do @filipedeschamps de mostrar um ícone. Daí precisamos pensar se vamos só mostrar o início do |
@aprendendofelipe Pessoalmente eu prefiro mostrar o início do body, ou o body inteiro mesmo.
Porque não ao invés de |
Se for mostrar o body inteiro, então vai ser algo mais no estilo de
Não tenho nada contra... haha... Mas também não tenho nada contra as outras sugestões de caminhos... Todos fazem sentido 🤝 |
@aprendendofelipe Também prefiro mostrar só o começo do body, algo como os primeiros 35 caracteres seria interessante, não tem necessidade de mostrar mais eu imagino.
O problema aqui é que todas as sugestões fazem sentido, mesmo assim acho que vale a pena ir pelo caminho "fácil" e fazer a breaking change como o @filipedeschamps falou aqui |
|
Show! Sugiro também manter o espírito de ContentList sim e por hora retornar tudo em Sobre a raiz do |
|
Navegando pelas páginas de alguns usuários que publicam bastante eu percebi que não ia ficar tão interessante aquela minha sugestão de deixar os comentários desses usuários estivessem ali misturados com as publicações. Então pensei em outra alternativa... Só adicionar na página do usuário um link para algo como: https://www.tabnews.com.br/gugadeschamps/comentarios E esse caminho pode ter sua própria paginação https://www.tabnews.com.br/gugadeschamps/comentarios/pagina/2 Só para pensarmos, pois isso não é para esse PR 🤝 |
|
Adicionei um commit que:
|
|
@33gustavo33 eu vou primeiro fazer o merge do PR #783 do @aprendendofelipe e depois venho nesse aqui e vou carregar ele até a parte do frontend para colocarmos isso em produção 🤝 me veio mais coisas na cabeça sobre as rotas e assim que conseguir compartilho enviando novos comentários e commits 👍 |
There was a problem hiding this comment.
Muito obrigado pela contribuição @33gustavo33 e vou seguir com ela fazendo alguns pequenos ajustes e também implementando a parte do frontend 🤝
| @@ -173,6 +172,10 @@ async function findAll(values = {}, options = {}) { | |||
| return `contents.${columnName} IS NOT DISTINCT FROM $${globalIndex}`; | |||
| } | |||
|
|
|||
| if (columnName === '$not_null') { | |||
| return `contents.${columnValue} IS NOT NULL`; | |||
There was a problem hiding this comment.
To pensando aqui, se com o que está no validator fica possível fazer um injection aqui caso algum dia esse valor seja exposto na interface pública. Vou colocar outro comentário no validator.
|
|
||
| $not_null: function () { | ||
| return Joi.object({ | ||
| $not_null: Joi.string() |
There was a problem hiding this comment.
Esta é a parte que eu vejo ser possível fazer um injection no banco caso algum dia esse parâmetro for exposto na interface pública (o que não está hoje). Mas destaco isso dado que ele aceita uma string com qualquer valor e pelo que entendi, isso é concatenado na linha 176 do content. O que talvez temos que fazer aqui é limitar com um .valid() o que pode ser de fato enviado.
O model content é o que mais me faz querer voltar a usar um ORM, seria tudo muito mais fácil em alguns casos 😂
|
|
||
| function shortifyText(text) { | ||
| const substring = text.substring(0, 35).trim(); | ||
| return text.length < 35 ? substring : substring + '...'; |
There was a problem hiding this comment.
Aqui vou alterar para 256 para ficar igual ao tamanho da propriedade title 🤝
|
To pensando novamente na idéia de que trazer ou não os conteúdos Então minha cabeça ta me jogando as sugestões abaixo, mas eu não estou seguro:
Mas não é semântico, dado que para o |
|
@33gustavo33 eu vou resolver o conflito e em seguida adicionar commits para reduzir o escopo da implementação, mas quero manter os commits originais para pesquisa depois 🤝 tomei a decisão de, enquanto não chegamos numa conclusão sobre a interface pública para filtro dos dados, não vamos implementar ela (os filtros), e vamos trabalhar apenas em fazer o endpoint E para resolver o conflito no arquivo de teste, eu vou recuperar o arquivo original que está na |
|
@filipedeschamps is attempting to deploy a commit to the TabNews Team on Vercel. A member of the Team first needs to authorize it. |
… `/children` endpoints for now This was originally implemented by @33gustavo33 but as discussed in here #747 (comment) we will reimplement this in the future, probably using query filters
Com esse PR é possível solicitar os conteúdos
roote conteúdoschildde um usuário separadamente. (Ou obter todos os conteúdos do usuário)GET /api/v1/contents/[username]/root: retorna os conteúdos root do usuárioGET /api/v1/contents/[username]/: retorna todos os conteúdos do usuárioGET /api/v1/contents/[username]/children: retorna os conteúdos child do usuário