Caixa de pesquisa: Atualizações do Backend #1698
Conversation
|
@mthmcalixto is attempting to deploy a commit to the TabNews Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
@mthmcalixto, obrigado pelo PR! 💪
Seria bom fornecer uma descrição com mais informações para ajudar quem for contribuir na revisão do seu PR, por exemplo dizendo como testar enquanto você não criar os testes automatizados da funcionalidade.
Por enquanto, eu removeria as sugestões e trends dessa versão, já que a quantidade de pesquisas que ocorrem atualmente por dentro do TabNews é muito pequena para gerar algo estatisticamente relevante. Pode deixar isso para quando for implementar o front-end, pois pode salvar algo localmente para sugerir termos que o próprio usuário já pesquisou.
Veja também os comentários no código 🤝
|
@aprendendofelipe são apenas 20 pesquisas para entrar em alta ao logo de 7 dias;
As sugestões são qualquer termo encontrado na busca; exemplo se procurou por "Javascript" e encontrou tópicos/publicações sobre "Javascript", apartir daí ela vira um sugestão e quem escrever "Javascript" vai ver ela logo abaixo, ela só é adicionada uma unica vez, por isso é unica e ela não é um trends. Também se alguém pesquisou por "Javascript console", ao escrever "Javascript" ele vai ver "Javascript" e "Javascript console" na susgestão a metida que ele escreve ou apaga. |
De onde você tirou esses parâmetros? Parecem totalmente arbitrários. Dessa forma, só vão entrar para as sugestões o que alguém propositalmente pesquisar mais de 20 vezes seguidas, mas nada entrará organicamente com o volume de pesquisas atuais. Pode remover essa parte do PR por enquanto, pois não faz sentido atualmente. |
Isso acontece com o sistema atual do Google no TabNews também, se eu pesquisar 10 vezes um termo, mesmo que eu seja apenas 1 usuário, ele vai contar as 10 vezes. A ideia do trends é que ele seleciona as 10 buscas mais frequentes dos últimos 7 dias, desde que tenham sido feitas pelo menos 20 vezes. (20 vezes pode ser ajustavel, 5,10 etc...) O usuário iria precisar de 1 hora para preencher 20 termos pesquisados, por que o termo só conta no banco de dados se já passou 3 minutos da ultima atualização, se o usuario pesquisar 10-20 vezes em menos de 3 minutos, não vai contar. |
Nesse ponto, e em quase tudo, não tem comparação as pesquisas pelo Google com o que está sendo implementando no PR. O Google até pode contar 10 vezes, mas isso não vai fazer nenhuma diferença dentro do volume de pesquisas deles. Só para seu próprio usuário é que esse termo irá ser sugerido mais. E isso é justamente o que eu estou sugerindo, que as sugestões sejam individuais, pois não haverá relevância estatística para fazer de outra forma. |
A sugestão aqui é retornar palavras juntas com o que o usuário está procurando, isso individualmente não seria possível. Se ele escrever "Javascript", só tem isso salvo pra ele; só vai aparecer "Javascript" e nada mais. Ao usar a sugestão, vão retornar palavras próximas "javascript" de outras pessoas que já pesquisaram esse termo, independentemente se está em alta ou não. O trends aqui iria chegar em 20 de "Pesquisas" e se estiver dentro de 7 dias, ele vai ficar ali em alta. Já as sugestões vão sempre existir para ajudar o usuário buscar um termo específico. Se for o caso aqui, podemos então apenas trabalhar com "text input" sem trends ou sugestões para o usuário, mas acredito que iria ficar meio "seco" o search box. Fora que poderemos usar a sugestão para "promover" uma publicação no futuro. |
|
Sugestões de termos e trends podem ficar para outro momento. Primeiro é preciso fazer a pesquisa funcionar minimamente para colocarmos um beta em produção. Combinado? 🤝 Da forma como está, desconsiderando as correções necessárias para o endpoint começar a funcionar, a busca ocorre apenas nos títulos, e é preciso digitar os termos corretamente, incluindo acentos, então ainda não é uma melhoria sobre o que temos hoje, mas tem potencial, desde que se atente ao que os revisores apontarem. |
Vamos trabalhar em cima disso... sobre os acentos acredito que funcione, estamos definindo o dicionário "portuguese", então estamos dizendo que aquele title tem acentos. |
mthmcalixto
force-pushed
the
new-search-backend
branch
from
3f7f40e
to
34dcee8
Compare
|
@aprendendofelipe fiz algumas modificações; Removi rotas /trends e /suggestions Da forma que está já é possível fazer as buscas, talvez melhorar algo em;
De certa forma não iria precisar da rota /search na API, já que iria ser tudo interno, apenas buscando em /models/search. A escolhar de abrir para API seria de vocês ou quando precisar dos /suggestions e /trends no futuro. |
- remove routers /trends /suggestions - remove secureOutputValues - refactor search.js/validator.js
mthmcalixto
force-pushed
the
new-search-backend
branch
from
34dcee8
to
9a48d76
Compare
Rafatcb
added
back
|
@aprendendofelipe @Rafatcb antes de continuar com essa feat, quero a opinião de vocês sobre a API do google que faz a busca. Eu já testei ela e tem apenas 1 problema, o limite de pesquisa é 100 por key, é 100 consultas por dia. A key é como se fosse 1 projeto, mas se criar 10 projetos, você tem 10 keys com 100 consultas dia, total permitido 1000 consultas dias se fizer um random em cada chave e garantiar que cada uma consulte pelo menos 1 vez. Consulta API: Retorno: {
"kind": "customsearch#search",
"url": {
"type": "application/json",
"template": "https://www.googleapis.com/customsearch/v1?q={searchTerms}&num={count?}&start={startIndex?}&lr={language?}&safe={safe?}&cx={cx?}&sort={sort?}&filter={filter?}&gl={gl?}&cr={cr?}&googlehost={googleHost?}&c2coff={disableCnTwTranslation?}&hq={hq?}&hl={hl?}&siteSearch={siteSearch?}&siteSearchFilter={siteSearchFilter?}&exactTerms={exactTerms?}&excludeTerms={excludeTerms?}&linkSite={linkSite?}&orTerms={orTerms?}&dateRestrict={dateRestrict?}&lowRange={lowRange?}&highRange={highRange?}&searchType={searchType}&fileType={fileType?}&rights={rights?}&imgSize={imgSize?}&imgType={imgType?}&imgColorType={imgColorType?}&imgDominantColor={imgDominantColor?}&alt=json"
},
"queries": {
"request": [
{
"title": "Google Custom Search - pitch",
"totalResults": "1400",
"searchTerms": "pitch",
"count": 1,
"startIndex": 1,
"inputEncoding": "utf8",
"outputEncoding": "utf8",
"safe": "off",
"cx": "6479f189f9fbc4e67"
}
],
"nextPage": [
{
"title": "Google Custom Search - pitch",
"totalResults": "1400",
"searchTerms": "pitch",
"count": 1,
"startIndex": 2,
"inputEncoding": "utf8",
"outputEncoding": "utf8",
"safe": "off",
"cx": "6479f189f9fbc4e67"
}
]
},
"context": {
"title": "test"
},
"searchInformation": {
"searchTime": 0.368556,
"formattedSearchTime": "0.37",
"totalResults": "1400",
"formattedTotalResults": "1,400"
},
"items": [
{
"kind": "customsearch#result",
"title": "Pitch: Yet another TabNews CLI. Este estou fazendo em Go. · cetorres",
"htmlTitle": "\u003cb\u003ePitch\u003c/b\u003e: Yet another TabNews CLI. Este estou fazendo em Go. · cetorres",
"link": "https://www.tabnews.com.br/cetorres/pitch-yet-another-tabnews-cli-este-estou-fazendo-em-go",
"displayLink": "www.tabnews.com.br",
"snippet": "Dec 6, 2022 ... Estou criando o tn-cli, um CLI para TabNews - mais um ;). O TabNews tem uma API e também tem RSS disponível. Assim, resolvi criar um cliente ...",
"htmlSnippet": "Dec 6, 2022 \u003cb\u003e...\u003c/b\u003e Estou criando o tn-cli, um CLI para TabNews - mais um ;). O TabNews tem uma API e também tem RSS disponível. Assim, resolvi criar um cliente ...",
"cacheId": "s_sQgKfhWhIJ",
"formattedUrl": "https://www.tabnews.com.br/.../pitch-yet-another-tabnews-cli-este-estou-faz...",
"htmlFormattedUrl": "https://www.tabnews.com.br/.../\u003cb\u003epitch\u003c/b\u003e-yet-another-tabnews-cli-este-estou-faz...",
"pagemap": {
"cse_thumbnail": [
{
"src": "https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcQjz2rJt3LLNjgdDovdSgm7h-kc_KgqOJYKkAbb0OrawN2wZpIVVIaf2Vhb&s",
"width": "310",
"height": "163"
}
],
"metatags": [
{
"og:image": "https://www.tabnews.com.br/api/v1/contents/cetorres/pitch-yet-another-tabnews-cli-este-estou-fazendo-em-go/thumbnail",
"twitter:card": "summary_large_image",
"twitter:title": "Pitch: Yet another TabNews CLI. Este estou fazendo em Go. · cetorres",
"article:section": "tecnologia",
"og:type": "article",
"article:published_time": "2022-12-06T00:41:15.000Z",
"og:site_name": "TabNews",
"twitter:url": "https://www.tabnews.com.br/cetorres/pitch-yet-another-tabnews-cli-este-estou-fazendo-em-go",
"og:title": "Pitch: Yet another TabNews CLI. Este estou fazendo em Go. · cetorres",
"title": "Pitch: Yet another TabNews CLI. Este estou fazendo em Go. · cetorres",
"og:description": "tn-cli Estou criando o tn-cli, um CLI para TabNews - mais um ;). O TabNews tem uma API e também tem RSS disponível. Assim, resolvi criar um cliente usando Go e uma biblioteca para TUI (te...",
"twitter:image": "https://www.tabnews.com.br/api/v1/contents/cetorres/pitch-yet-another-tabnews-cli-este-estou-fazendo-em-go/thumbnail",
"article:author": "cetorres",
"next-head-count": "26",
"article:modified_time": "2022-12-07T17:52:00.380Z",
"viewport": "width=device-width, initial-scale=1",
"apple-mobile-web-app-capable": "yes",
"twitter:description": "tn-cli Estou criando o tn-cli, um CLI para TabNews - mais um ;). O TabNews tem uma API e também tem RSS disponível. Assim, resolvi criar um cliente usando Go e uma biblioteca para TUI (te...",
"mobile-web-app-capable": "yes",
"og:url": "https://www.tabnews.com.br/cetorres/pitch-yet-another-tabnews-cli-este-estou-fazendo-em-go"
}
],
"cse_image": [
{
"src": "https://www.tabnews.com.br/api/v1/contents/cetorres/pitch-yet-another-tabnews-cli-este-estou-fazendo-em-go/thumbnail"
}
]
}
}
]
} |
|
Não podemos violar os termos de uso de qualquer serviço. Veja os Termos do Google Cloud, especificamente o seguinte:
Aproveitando... Evite marcar usuários específicos nos pedidos de opinião, já que a participação nas issues e revisões dos PRs é aberta a todos que quiserem contribuir 💪 |
Eu imaginei que teria essa regra kkk, podemos tentar buscar algo parecido com o retorno deles, assim deixa o retorno json padrão. |
|
@aprendendofelipe @Rafatcb fiz as alterações. Acredito que precisa mexer mais em;
As vezes não encontra um titulo completo, apenas algumas metades dele, mas já é possível fazer uma busca;
{
"rows": [
{
"id": "7ab1fdd2-8a2d-444b-81bc-614c8cf5a15c",
"owner_id": "c0ad7184-917d-42c1-b4c7-2ebb7528d40b",
"parent_id": null,
"slug": "alias-fugit-exercitationem-non-at-expedita",
"title": "Alias fugit exercitationem non at expedita.",
"status": "published",
"source_url": null,
"created_at": "2024-05-22T16:20:09.171Z",
"updated_at": "2024-05-22T16:20:09.171Z",
"published_at": "2024-05-22T16:20:09.171Z",
"deleted_at": null,
"totalRows": 900,
"path": [],
"owner_username": "admin",
"tabcoins": "0",
"tabcoins_credit": "0",
"tabcoins_debit": "0",
"children_deep_count": "0"
}
],
"pagination": {
"currentPage": 1,
"totalRows": 900,
"perPage": 1,
"firstPage": 1,
"nextPage": 2,
"previousPage": null,
"lastPage": 900
}
} |
There was a problem hiding this comment.
Deixei mais alguns comentários. Pode criar testes antes de pedir para revisar de novo, porque irá ajudar a entender o comportamento esperado da API e testar mudanças como a que propus em um dos meus comentários, de usar somente websearch_to_tsquery.
As vezes não encontra um titulo completo, apenas algumas metades dele, mas já é possível fazer uma busca;
Os testes também ajudariam a tentar alterações para resolver isso. Fica mais simples para um revisor clonar a branch e experimentar algumas ideias.
Não são exatamente alterações, um frase completa as vezes não é encontrata mas ao começar tirar palavras dela é encontrada, talvez seja a forma que o websearch_to_tsquery trabalha. |
|
Quais os próximos passos para encerrar as atualizações do backend, apenas o websearch_to_tsquery e testes? |
- remove routers /trends /suggestions - remove secureOutputValues - refactor search.js/validator.js
mthmcalixto
force-pushed
the
new-search-backend
branch
from
eb2908c
to
5e40353
Compare
…abnews.com.br into new-search-backend
Estou fazendo os testes, em breve mando para revisão. |
|
Adicionei alguns testes e fiz algumas modifcações, retirando ts_query e deixando apenas websearch_to_tsquery, ele sozinho consegue lidar com muitas coisas. também alterei de "ts" a coluna de rank para "search" e agora o rank é retornado apenas se estiver com um rank > 0.6 que é o mais próximo possível pelo o que eu testei; function generateOrderByClause(sortBy, tsQuery) {
switch (sortBy) {
case 'new':
return 'ORDER BY created_at DESC';
case 'relevant':
return `ORDER BY CASE WHEN ts_rank(search, ${tsQuery}) > 0.6 THEN ts_rank(search, ${tsQuery}) ELSE 0 END DESC`;
case 'old':
return 'ORDER BY created_at ASC';
default:
return '';
}
}Quando o "sort" é relevant, ele retorna o "rank" daquela pesquisa; if (values.sort === 'relevant') {
selectClause += `,
ts_rank(search, ${tsQuery}) as rank
`;
}Estou buscando em title e em slug também, assim o acentos vão funcionar melhor; |
mthmcalixto
force-pushed
the
new-search-backend
branch
from
6b67ecb
to
57443fe
Compare
|
Estou tentando adicionar um teste, mas está dando erro ao enviar, localmente os testes estão passando. |
There was a problem hiding this comment.
O que acham de chamar o endpoint de GET /api/v1/contents/search ao invés de GET /api/v1/search?
Sobre o teste quebrando no PR: parece que o serviço de e-mail não subiu. Não investiguei a fundo, mas tem chances de ser um problema na imagem do docker (o issue sj26/mailcatcher#559 foi aberto 5 horas atrás, e faz sentido estar passando localmente e no GitHub não porque localmente já temos a imagem baixada, e teve uma atualização 2 dias atrás).
| expect(uuidVersion(responseBody.request_id)).toEqual(4); | ||
| }); | ||
|
|
||
| test('With 1 "published" entries and sort "new"', async () => { |
There was a problem hiding this comment.
O que acha de fazer um test.each como o que tem em /api/v1/contents/get.test.js? https://github.com/filipedeschamps/tabnews.com.br/blob/main/tests/integration/api/v1/contents/get.test.js#L1247-L1414
O que pensei foi criar alguns conteúdos uma vez só, remover o beforeEach desse arquivo, criar os conteúdos no beforeAll, ter um test.each para o sort="new" e sort="old", e criar alguns testes diferentes para os casos do sort="relevant".
Esses testes do sort="relevant" poderiam mostrar a busca com termos diferentes, e também com termos que deixe clara a diferença de peso entre os campos verificados pela busca. Imagino que seriam necessários pelo menos 3 ou 4 testes com sort="relevant".
Também pode ter um teste mostrando o potencial de remover um termo da busca, por exemplo busca -termo, ou buscando por uma frase exata, por exemplo busca "termo exato". Isso seria uma forma de garantir que esse é um comportamento desejado e evitar que seja removido futuramente. de forma não intencional
Faz sentido?
| }) | ||
| .use(controller.injectRequestMetadata) | ||
| .use(controller.logRequest) | ||
| .get(cacheControl.swrMaxAge(10), getValidationHandler, getHandler); |
There was a problem hiding this comment.
Não sei se esse cache poderia ser maior, ou se um valor maior seria pior, já que dificilmente vão buscar os mesmos termos num curto período.
There was a problem hiding this comment.
Talvez o cache poderia ser até maior aqui, caso alguém faça um pesquisa o próximo vai ser muito mais rápido o retorno pra ele, e como a busca não precisa mudar em real-time, acho que valeria a pena aumentar.
There was a problem hiding this comment.
@Rafatcb o que acha?
There was a problem hiding this comment.
Eu realmente não sei, imagino serem baixas as chances de alguém fazer a mesma busca num curto período. O maior cache que temos hoje é 300 (5 minutos, o valor está em segundos). Acho que podemos experimentar deixar 300 aqui também, ao menos inicialmente.
There was a problem hiding this comment.
Não é interessante um valor muito alto para a versão preliminar. Por mim, mantém 10s
| } | ||
|
|
||
| let selectClause = ` | ||
| SELECT |
There was a problem hiding this comment.
Acredito que faça sentido retornar o body na busca. Hoje, o TabNews já exibe uma parte nos resultados, então não mostrar me parece uma regressão:
E, acredito que melhor do que o body, seria o destaque do ts_headline (documentação). Se for algo complicado, podemos retornar o body e deixar essa implementação como uma melhoria para outro PR. A documentação diz que o ts_headline é lento, então precisa de uma atenção extra na implementação.
There was a problem hiding this comment.
@Rafatcb a ideia não é recriar o google, apenas como eu fiz no inicio, aparecer as publicações igual aparece hoje e eles não tem body: https://www.youtube.com/watch?v=SJY_jdZ2Gww
Acredito que body nao seja necessario aqui.
There was a problem hiding this comment.
Mas se um comentário for encontrado, e não exibir o body, não terá como eu saber vendo a lista se é o que eu estava procurando. E o body também é benéfico no caso de publicações.
There was a problem hiding this comment.
@Rafatcb É pra isso que será usado a relevância, onde tem o peso de title,slug e body. O restante é apenas por title,slug.
E comentários não estão nas buscas, apenas publicações.
There was a problem hiding this comment.
É pra isso que será usado a relevância, onde tem o peso de title,slug e body. O restante é apenas por title,slug.
Certo, mas o que estou dizendo é que o usuário precisa entender o que foi retornado. Às vezes o que ele procura está no corpo, e o título é sobre outra coisa, então lendo apenas o título, ele não saberá se encontrou o que buscava.
E comentários não estão nas buscas, apenas publicações.
Bom, acredito que precisamos mudar isso então 😅. A busca de comentários é tão importante quanto a das publicações, pois "tudo é conteúdo" no TabNews.
There was a problem hiding this comment.
@aprendendofelipe entendo, mas teria que mexer em muita coisa pra retornar o corpo do comentário como o @Rafatcb está querendo fazer, não que seja díficil.
Se eu busco um produto na loja, eu estou buscando o título do produto e não comentários do produto.
Por exemplo o Stack Overflow o google não retorna os comentários das questões, mas apenas as questões.
O que poderia ser feito é, http://localhost:3000/search?q= - publicações, http://localhost:3000/search/comments?q= - comentários.
Onde http://localhost:3000/search?q= é apenas para retornar publicações, e http://localhost:3000/search/comments?q= para comentários, redirecionando para comentário.
A pesquisa ocorre em todo contéudo, quando usado "strategy = relevant"
There was a problem hiding this comment.
Mas, comparando com o Stack Overflow, seria como poder procurar nas respostas, e não apenas nas perguntas.
Os comentários não possuem um valor também na coluna search? E removendo a condição AND contents.parent_id IS NULL, não seriam considerados na busca? E imagino que precisaria adicionar o body na tokenização to_tsvector.
There was a problem hiding this comment.
@Rafatcb fiz novas alterações.
There was a problem hiding this comment.
@mthmcalixto deixando sua UI proposta de lado e levando em consideração tanto o que temos hoje, quanto os comportamentos de outros mecanismos de busca, creio que devemos retornar o body mesmo que a busca envolva apenas publicações.
Talvez retornar apenas os primeiros X caracteres para não transferir tantos dados do banco para o backend, e depois do backend para o cliente. O problema de fazer esse "corte" direto no SQL é que pode deixar alguma tag HTML/Markdown aberta e atrapalhar o tratamento de remoção de Markdown no cliente.
Alguma sugestão sobre isso?
There was a problem hiding this comment.
Isso do "corte" direto no SQL é outro problema pendente, que não parece complexo, mas que merece discussão adequada, então é melhor não tentar resolver aqui nesse PR, pois ele pode seguir o padrão atual.
A busca deve sempre retornar o body, mas no retorno da API pode fazer o mesmo que na lista de comentários:
content.body = removeMarkdown(content.body, { maxLength: 255 });
O que temos hoje em produção (a busca do Google) não retorna apenas os conteúdos, mas também usuários, termos de uso e qualquer coisa pública no TabNews. Por isso acho que
Estranho que essa mesma imagem de 2 dias atrás está subindo normalmente agora, seja local ou nas |
|
Agora pode ser buscado por comentário. Foi trocado o nome "sort" para "strategy" para usar injectPaginationHeaders, e em injectPaginationHeaders foi adicionado "searchQuery" por causa do "?q=". Foi adicionado dois novos params: Por padrão, ao buscar em "search", ela retorna apenas por publicações. Foi adicionado "decodeURIComponent" em generateTsQuery caso o termo da pesquisa venha em URL Encode. Foi trocado a forma como retorna os dados, agora é dentro de "contents": Caso não tenha contents: Coloquei caso não tenha contents, se não for assim, gera um erro. |
- add searchQuery, injectPaginationHeaders models/controller - add decodeURIComponent in generateTsQuery - new only_children & with_children
mthmcalixto
force-pushed
the
new-search-backend
branch
from
118f2c4
to
83a4d26
Compare
|
Vou adicionar mais alguns testes para dar continuidade ao PR. |
| const links = []; | ||
| const baseUrl = `${webserver.host}${endpoint}`; | ||
|
|
||
| const searchParams = new URLSearchParams(); | ||
|
|
||
| if (searchQuery) { |
There was a problem hiding this comment.
Será que não faz mais sentido receber um parâmetro query? Pensei aqui, e tudo o que está na query influencia a paginação:
page, que é tratado por meio do objetopagination.per_page, que é tratado porpagination.per_page.strategy, que tem umifespecífico.with_rootewith_children(noGET /api/v1/contents): que hoje não são tratados na paginação.q: novo parâmetro sugerido agora.
E potencialmente qualquer parâmetro que vá influenciar na filtragem ou ordenação, e precisaríamos nos preocupar em adicionar aqui.
Acredito que essa refatoração deva ocorrer em outro PR (não necessariamente pelo mesmo autor deste PR), para poder adicionar testes e modificar os outros endpoints, mas neste PR pode mudar de searchQuery para query e passar as chaves e valores para o searchParams, já tendo o tratamento correto nesse endpoint de busca.
O que vocês acham?
There was a problem hiding this comment.
Para a refatoração futura, acho que faz sentido esse parâmetro query vir dentro de pagination. 👍
Para esse PR, acho que faz mais sentido seguir o padrão atual, ou seja, fazer o mesmo de strategy:
if (pagination.q) {
searchParams.set('q', pagination.q);
}Daí não precisa passar request.query.q para injectPaginationHeaders, pois q já é passado para o model pagination, então só precisa tratar lá da mesma forma que strategy, ou seja:
if (q) {
pagination.q = q;
}| only_children: 'optional', | ||
| with_children: 'optional', |
There was a problem hiding this comment.
Acredito que podemos seguir o padrão do GET /api/v1/contents, que utiliza with_root e with_children, e também seguirmos os mesmos valores padrão, caso esses parâmetros sejam undefined:
with_root, padrãotrue.with_children, padrãofalse.
Assim mantemos a consistência na API e a flexibilidade que você imaginou.
There was a problem hiding this comment.
Acho melhor não adicionar esses filtros nesse PR, até porque não está clara a função deles. É melhor primeiro tentar fazer funcionar o básico.
Considero que esse PR seja principalmente para verificar a viabilidade de um sistema de busca usando apenas nosso banco de dados principal, então a prioridade deveria ser responder isso.
Enquanto não sabemos se é viável, não vale a pena gastar energia com novas funcionalidades, e isso também pode prejudicar a comparação.
Se descobrirmos que é viável usar nosso banco, então podemos passar para a criação de uma "busca avançada" que aceite filtros e outros parâmetros. E, quando chegar esse momento, acho que o ponto mais importante não deveria ser em filtrar root e/ou children, mas sim por TAG, data, autor...
| @@ -201,6 +201,18 @@ const schemas = { | |||
| }); | |||
| }, | |||
|
|
|||
| rank: function () { | |||
There was a problem hiding this comment.
Tem algum motivo para retornar o rank para o cliente? Me parece que esse valor não é importante para o cliente, e podemos manter apenas no backend.
There was a problem hiding this comment.
rank não deveria ser retornado
| const withChildrenClause = values.only_children | ||
| ? 'AND contents.parent_id IS NOT NULL' | ||
| : values.with_children | ||
| ? '' | ||
| : 'AND contents.parent_id IS NULL'; |
There was a problem hiding this comment.
Com a mudança que sugeri na interface, podemos adaptar aqui para tratar caso !with_children e !with_root, algo parecido com:
| const withChildrenClause = values.only_children | |
| ? 'AND contents.parent_id IS NOT NULL' | |
| : values.with_children | |
| ? '' | |
| : 'AND contents.parent_id IS NULL'; | |
| const whereParentId = []; | |
| if (!values.with_children) { | |
| whereParentId.push('AND contents.parent_id IS NULL'); | |
| } | |
| if (!values.with_root) { | |
| whereParentId.push('AND contents.parent_id IS NOT NULL'); | |
| } | |
| const whereParentIdClause = whereParentId.join(' '); |
(código apenas de exemplo, pode fazer diferente)
| : values.with_children | ||
| ? '' | ||
| : 'AND contents.parent_id IS NULL'; | ||
| const bodyClause = values.only_children || values.with_children ? " || ' ' || contents.body" : ''; |
There was a problem hiding this comment.
Essa é uma condição para levar em consideração ou não o body no ranqueamento?
Se sim, acho que devemos levar sempre em consideração, não importando se estou buscando uma publicação, comentário ou ambos, já que a maior parte do conteúdo está no corpo.
| case 'new': | ||
| return 'ORDER BY published_at DESC'; | ||
| case 'relevant': | ||
| return `ORDER BY CASE WHEN ts_rank(search, ${tsQuery}) > 0.6 THEN ts_rank(search, ${tsQuery}) ELSE 0 END DESC`; |
There was a problem hiding this comment.
Essa condição não deveria estar no WHERE? Para não retornar os resultados que o rank seja menor que 0.6.
Outra pergunta, não dá para referenciar a coluna rank (que está no SELECT quando a estratégia é relevant) ao invés de chamar o ts_rank?
| } | ||
|
|
||
| let selectClause = ` | ||
| SELECT |
There was a problem hiding this comment.
@mthmcalixto deixando sua UI proposta de lado e levando em consideração tanto o que temos hoje, quanto os comportamentos de outros mecanismos de busca, creio que devemos retornar o body mesmo que a busca envolva apenas publicações.
Talvez retornar apenas os primeiros X caracteres para não transferir tantos dados do banco para o backend, e depois do backend para o cliente. O problema de fazer esse "corte" direto no SQL é que pode deixar alguma tag HTML/Markdown aberta e atrapalhar o tratamento de remoção de Markdown no cliente.
Alguma sugestão sobre isso?
| only_children: 'optional', | ||
| with_children: 'optional', |
There was a problem hiding this comment.
Acho melhor não adicionar esses filtros nesse PR, até porque não está clara a função deles. É melhor primeiro tentar fazer funcionar o básico.
Considero que esse PR seja principalmente para verificar a viabilidade de um sistema de busca usando apenas nosso banco de dados principal, então a prioridade deveria ser responder isso.
Enquanto não sabemos se é viável, não vale a pena gastar energia com novas funcionalidades, e isso também pode prejudicar a comparação.
Se descobrirmos que é viável usar nosso banco, então podemos passar para a criação de uma "busca avançada" que aceite filtros e outros parâmetros. E, quando chegar esse momento, acho que o ponto mais importante não deveria ser em filtrar root e/ou children, mas sim por TAG, data, autor...
| const links = []; | ||
| const baseUrl = `${webserver.host}${endpoint}`; | ||
|
|
||
| const searchParams = new URLSearchParams(); | ||
|
|
||
| if (searchQuery) { |
There was a problem hiding this comment.
Para a refatoração futura, acho que faz sentido esse parâmetro query vir dentro de pagination. 👍
Para esse PR, acho que faz mais sentido seguir o padrão atual, ou seja, fazer o mesmo de strategy:
if (pagination.q) {
searchParams.set('q', pagination.q);
}Daí não precisa passar request.query.q para injectPaginationHeaders, pois q já é passado para o model pagination, então só precisa tratar lá da mesma forma que strategy, ou seja:
if (q) {
pagination.q = q;
}| @@ -201,6 +201,18 @@ const schemas = { | |||
| }); | |||
| }, | |||
|
|
|||
| rank: function () { | |||
There was a problem hiding this comment.
rank não deveria ser retornado
| } | ||
|
|
||
| let selectClause = ` | ||
| SELECT |
There was a problem hiding this comment.
Isso do "corte" direto no SQL é outro problema pendente, que não parece complexo, mas que merece discussão adequada, então é melhor não tentar resolver aqui nesse PR, pois ele pode seguir o padrão atual.
A busca deve sempre retornar o body, mas no retorno da API pode fazer o mesmo que na lista de comentários:
content.body = removeMarkdown(content.body, { maxLength: 255 });| }) | ||
| .use(controller.injectRequestMetadata) | ||
| .use(controller.logRequest) | ||
| .get(cacheControl.swrMaxAge(10), getValidationHandler, getHandler); |
There was a problem hiding this comment.
Não é interessante um valor muito alto para a versão preliminar. Por mim, mantém 10s
| contents.parent_id, | ||
| contents.slug, | ||
| contents.title, | ||
| ${values.only_children || values.with_children ? 'contents.body,' : ''} |
There was a problem hiding this comment.
Apenar reforçando o outro comentário sobre o body:
| ${values.only_children || values.with_children ? 'contents.body,' : ''} | |
| contents.body, |
Mudanças realizadas
Adiciona lógicas relacionadas ao backend para o funcionamento do feat:searchbox.
Adiciona nova rota de API para buscar:
/api/v1/search - GETCom params:
Nova caixa de pesquisa utilizando pesquisa de texto completo (Full Text Search) do Postgresql; Utiliza to_tsvector com o dicionário portuguese e websearch_to_tsquery.
Cria uma coluna em "contents" chamada "search" para criar uma relevancia entre "title" / "slug" / "body" das publicações; Com esses dados usa
ORDER BY ts_rankna coluna "search" retornando apenas os ranks acima de 0.6Cria um novo modelo "models" chamado "search" para buscar esses dados no Banco de Dados.
Tipo de mudança
[x] Nova funcionalidade
Checklist: