feat: Record search queries (#1554)

* Record search queries

* feat: add totalCount to the search response

* feat: add comments to explain why we use setTimeout
This commit is contained in:
Renan Filipe
2020-09-22 03:05:42 -03:00
committed by GitHub
parent 0fa8a6ed2e
commit 98626ebbaf
9 changed files with 330 additions and 54 deletions

View File

@ -11,6 +11,7 @@ import {
Document, Document,
Event, Event,
Revision, Revision,
SearchQuery,
Share, Share,
Star, Star,
User, User,
@ -586,7 +587,7 @@ router.post("documents.search", auth(), pagination(), async (ctx) => {
); );
} }
const results = await Document.searchForUser(user, query, { const { results, totalCount } = await Document.searchForUser(user, query, {
includeArchived: includeArchived === "true", includeArchived: includeArchived === "true",
includeDrafts: includeDrafts === "true", includeDrafts: includeDrafts === "true",
collaboratorIds, collaboratorIds,
@ -604,6 +605,14 @@ router.post("documents.search", auth(), pagination(), async (ctx) => {
}) })
); );
SearchQuery.create({
userId: user.id,
teamId: user.teamId,
source: "app",
query,
results: totalCount,
});
const policies = presentPolicies(user, documents); const policies = presentPolicies(user, documents);
ctx.body = { ctx.body = {

View File

@ -8,6 +8,7 @@ import {
Revision, Revision,
Backlink, Backlink,
CollectionUser, CollectionUser,
SearchQuery,
} from "../models"; } from "../models";
import { import {
buildShare, buildShare,
@ -954,6 +955,25 @@ describe("#documents.search", () => {
expect(res.status).toEqual(401); expect(res.status).toEqual(401);
expect(body).toMatchSnapshot(); expect(body).toMatchSnapshot();
}); });
it("should save search term, hits and source", async (done) => {
const { user } = await seed();
await server.post("/api/documents.search", {
body: { token: user.getJwtToken(), query: "my term" },
});
// setTimeout is needed here because SearchQuery is saved asynchronously
// in order to not slow down the response time.
setTimeout(async () => {
const searchQuery = await SearchQuery.findAll({
where: { query: "my term" },
});
expect(searchQuery.length).toBe(1);
expect(searchQuery[0].results).toBe(0);
expect(searchQuery[0].source).toBe("app");
done();
}, 100);
});
}); });
describe("#documents.archived", () => { describe("#documents.archived", () => {

View File

@ -2,7 +2,14 @@
import Router from "koa-router"; import Router from "koa-router";
import { escapeRegExp } from "lodash"; import { escapeRegExp } from "lodash";
import { AuthenticationError, InvalidRequestError } from "../errors"; import { AuthenticationError, InvalidRequestError } from "../errors";
import { Authentication, Document, User, Team, Collection } from "../models"; import {
Authentication,
Document,
User,
Team,
Collection,
SearchQuery,
} from "../models";
import { presentSlackAttachment } from "../presenters"; import { presentSlackAttachment } from "../presenters";
import * as Slack from "../slack"; import * as Slack from "../slack";
const router = new Router(); const router = new Router();
@ -146,10 +153,18 @@ router.post("hooks.slack", async (ctx) => {
const options = { const options = {
limit: 5, limit: 5,
}; };
const results = user const { results, totalCount } = user
? await Document.searchForUser(user, text, options) ? await Document.searchForUser(user, text, options)
: await Document.searchForTeam(team, text, options); : await Document.searchForTeam(team, text, options);
SearchQuery.create({
userId: user ? user.id : null,
teamId: team.id,
source: "slack",
query: text,
results: totalCount,
});
if (results.length) { if (results.length) {
const attachments = []; const attachments = [];
for (const result of results) { for (const result of results) {

View File

@ -1,7 +1,7 @@
/* eslint-disable flowtype/require-valid-file-annotation */ /* eslint-disable flowtype/require-valid-file-annotation */
import TestServer from "fetch-test-server"; import TestServer from "fetch-test-server";
import app from "../app"; import app from "../app";
import { Authentication } from "../models"; import { Authentication, SearchQuery } from "../models";
import * as Slack from "../slack"; import * as Slack from "../slack";
import { buildDocument } from "../test/factories"; import { buildDocument } from "../test/factories";
import { flushdb, seed } from "../test/support"; import { flushdb, seed } from "../test/support";
@ -132,6 +132,30 @@ describe("#hooks.slack", () => {
); );
}); });
it("should save search term, hits and source", async (done) => {
const { user, team } = await seed();
await server.post("/api/hooks.slack", {
body: {
token: process.env.SLACK_VERIFICATION_TOKEN,
user_id: user.serviceId,
team_id: team.slackId,
text: "contains",
},
});
// setTimeout is needed here because SearchQuery is saved asynchronously
// in order to not slow down the response time.
setTimeout(async () => {
const searchQuery = await SearchQuery.findAll({
where: { query: "contains" },
});
expect(searchQuery.length).toBe(1);
expect(searchQuery[0].results).toBe(0);
expect(searchQuery[0].source).toBe("slack");
done();
}, 100);
});
it("should respond with help content for help keyword", async () => { it("should respond with help content for help keyword", async () => {
const { user, team } = await seed(); const { user, team } = await seed();
const res = await server.post("/api/hooks.slack", { const res = await server.post("/api/hooks.slack", {

View File

@ -0,0 +1,43 @@
"use strict";
module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.createTable("search_queries", {
id: {
allowNull: false,
primaryKey: true,
type: Sequelize.UUID,
},
userId: {
type: Sequelize.UUID,
references: {
model: "users",
},
},
teamId: {
type: Sequelize.UUID,
references: {
model: "teams",
},
},
source: {
type: Sequelize.ENUM("slack", "app"),
allowNull: false,
},
query: {
type: Sequelize.STRING,
allowNull: false,
},
results: {
type: Sequelize.INTEGER,
allowNull: false,
},
createdAt: {
allowNull: false,
type: Sequelize.DATE,
},
});
},
down: async (queryInterface, Sequelize) => {
await queryInterface.dropTable("search_queries");
},
};

View File

@ -251,10 +251,13 @@ Document.findByPk = async function (id, options = {}) {
} }
}; };
type SearchResult = { type SearchResponse = {
ranking: number, results: {
context: string, ranking: number,
document: Document, context: string,
document: Document,
}[],
totalCount: number,
}; };
type SearchOptions = { type SearchOptions = {
@ -277,7 +280,7 @@ Document.searchForTeam = async (
team, team,
query, query,
options: SearchOptions = {} options: SearchOptions = {}
): Promise<SearchResult[]> => { ): Promise<SearchResponse> => {
const limit = options.limit || 15; const limit = options.limit || 15;
const offset = options.offset || 0; const offset = options.offset || 0;
const wildcardQuery = `${escape(query)}:*`; const wildcardQuery = `${escape(query)}:*`;
@ -285,21 +288,25 @@ Document.searchForTeam = async (
// If the team has access no public collections then shortcircuit the rest of this // If the team has access no public collections then shortcircuit the rest of this
if (!collectionIds.length) { if (!collectionIds.length) {
return []; return { results: [], totalCount: 0 };
} }
// Build the SQL query to get documentIds, ranking, and search term context // Build the SQL query to get documentIds, ranking, and search term context
const sql = ` const whereClause = `
"searchVector" @@ to_tsquery('english', :query) AND
"teamId" = :teamId AND
"collectionId" IN(:collectionIds) AND
"deletedAt" IS NULL AND
"publishedAt" IS NOT NULL
`;
const selectSql = `
SELECT SELECT
id, id,
ts_rank(documents."searchVector", to_tsquery('english', :query)) as "searchRanking", ts_rank(documents."searchVector", to_tsquery('english', :query)) as "searchRanking",
ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=20, MaxWords=30') as "searchContext" ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=20, MaxWords=30') as "searchContext"
FROM documents FROM documents
WHERE "searchVector" @@ to_tsquery('english', :query) AND WHERE ${whereClause}
"teamId" = :teamId AND
"collectionId" IN(:collectionIds) AND
"deletedAt" IS NULL AND
"publishedAt" IS NOT NULL
ORDER BY ORDER BY
"searchRanking" DESC, "searchRanking" DESC,
"updatedAt" DESC "updatedAt" DESC
@ -307,17 +314,34 @@ Document.searchForTeam = async (
OFFSET :offset; OFFSET :offset;
`; `;
const results = await sequelize.query(sql, { const countSql = `
SELECT COUNT(id)
FROM documents
WHERE ${whereClause}
`;
const queryReplacements = {
teamId: team.id,
query: wildcardQuery,
collectionIds,
};
const resultsQuery = sequelize.query(selectSql, {
type: sequelize.QueryTypes.SELECT, type: sequelize.QueryTypes.SELECT,
replacements: { replacements: {
teamId: team.id, ...queryReplacements,
query: wildcardQuery,
limit, limit,
offset, offset,
collectionIds,
}, },
}); });
const countQuery = sequelize.query(countSql, {
type: sequelize.QueryTypes.SELECT,
replacements: queryReplacements,
});
const [results, [{ count }]] = await Promise.all([resultsQuery, countQuery]);
// Final query to get associated document data // Final query to get associated document data
const documents = await Document.findAll({ const documents = await Document.findAll({
where: { where: {
@ -330,20 +354,23 @@ Document.searchForTeam = async (
], ],
}); });
return map(results, (result) => ({ return {
ranking: result.searchRanking, results: map(results, (result) => ({
context: removeMarkdown(unescape(result.searchContext), { ranking: result.searchRanking,
stripHTML: false, context: removeMarkdown(unescape(result.searchContext), {
}), stripHTML: false,
document: find(documents, { id: result.id }), }),
})); document: find(documents, { id: result.id }),
})),
totalCount: count,
};
}; };
Document.searchForUser = async ( Document.searchForUser = async (
user, user,
query, query,
options: SearchOptions = {} options: SearchOptions = {}
): Promise<SearchResult[]> => { ): Promise<SearchResponse> => {
const limit = options.limit || 15; const limit = options.limit || 15;
const offset = options.offset || 0; const offset = options.offset || 0;
const wildcardQuery = `${escape(query)}:*`; const wildcardQuery = `${escape(query)}:*`;
@ -360,7 +387,7 @@ Document.searchForUser = async (
// If the user has access to no collections then shortcircuit the rest of this // If the user has access to no collections then shortcircuit the rest of this
if (!collectionIds.length) { if (!collectionIds.length) {
return []; return { results: [], totalCount: 0 };
} }
let dateFilter; let dateFilter;
@ -369,13 +396,8 @@ Document.searchForUser = async (
} }
// Build the SQL query to get documentIds, ranking, and search term context // Build the SQL query to get documentIds, ranking, and search term context
const sql = ` const whereClause = `
SELECT "searchVector" @@ to_tsquery('english', :query) AND
id,
ts_rank(documents."searchVector", to_tsquery('english', :query)) as "searchRanking",
ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=20, MaxWords=30') as "searchContext"
FROM documents
WHERE "searchVector" @@ to_tsquery('english', :query) AND
"teamId" = :teamId AND "teamId" = :teamId AND
"collectionId" IN(:collectionIds) AND "collectionId" IN(:collectionIds) AND
${ ${
@ -393,27 +415,52 @@ Document.searchForUser = async (
? '("publishedAt" IS NOT NULL OR "createdById" = :userId)' ? '("publishedAt" IS NOT NULL OR "createdById" = :userId)'
: '"publishedAt" IS NOT NULL' : '"publishedAt" IS NOT NULL'
} }
`;
const selectSql = `
SELECT
id,
ts_rank(documents."searchVector", to_tsquery('english', :query)) as "searchRanking",
ts_headline('english', "text", to_tsquery('english', :query), 'MaxFragments=1, MinWords=20, MaxWords=30') as "searchContext"
FROM documents
WHERE ${whereClause}
ORDER BY ORDER BY
"searchRanking" DESC, "searchRanking" DESC,
"updatedAt" DESC "updatedAt" DESC
LIMIT :limit LIMIT :limit
OFFSET :offset; OFFSET :offset;
`; `;
const results = await sequelize.query(sql, { const countSql = `
SELECT COUNT(id)
FROM documents
WHERE ${whereClause}
`;
const queryReplacements = {
teamId: user.teamId,
userId: user.id,
collaboratorIds: options.collaboratorIds,
query: wildcardQuery,
collectionIds,
dateFilter,
};
const resultsQuery = sequelize.query(selectSql, {
type: sequelize.QueryTypes.SELECT, type: sequelize.QueryTypes.SELECT,
replacements: { replacements: {
teamId: user.teamId, ...queryReplacements,
userId: user.id,
collaboratorIds: options.collaboratorIds,
query: wildcardQuery,
limit, limit,
offset, offset,
collectionIds,
dateFilter,
}, },
}); });
const countQuery = sequelize.query(countSql, {
type: sequelize.QueryTypes.SELECT,
replacements: queryReplacements,
});
const [results, [{ count }]] = await Promise.all([resultsQuery, countQuery]);
// Final query to get associated document data // Final query to get associated document data
const documents = await Document.scope( const documents = await Document.scope(
{ {
@ -432,13 +479,16 @@ Document.searchForUser = async (
], ],
}); });
return map(results, (result) => ({ return {
ranking: result.searchRanking, results: map(results, (result) => ({
context: removeMarkdown(unescape(result.searchContext), { ranking: result.searchRanking,
stripHTML: false, context: removeMarkdown(unescape(result.searchContext), {
}), stripHTML: false,
document: find(documents, { id: result.id }), }),
})); document: find(documents, { id: result.id }),
})),
totalCount: count,
};
}; };
// Hooks // Hooks

View File

@ -201,7 +201,7 @@ describe("#searchForTeam", () => {
title: "test", title: "test",
}); });
const results = await Document.searchForTeam(team, "test"); const { results } = await Document.searchForTeam(team, "test");
expect(results.length).toBe(1); expect(results.length).toBe(1);
expect(results[0].document.id).toBe(document.id); expect(results[0].document.id).toBe(document.id);
}); });
@ -218,15 +218,85 @@ describe("#searchForTeam", () => {
title: "test", title: "test",
}); });
const results = await Document.searchForTeam(team, "test"); const { results } = await Document.searchForTeam(team, "test");
expect(results.length).toBe(0); expect(results.length).toBe(0);
}); });
test("should handle no collections", async () => { test("should handle no collections", async () => {
const team = await buildTeam(); const team = await buildTeam();
const results = await Document.searchForTeam(team, "test"); const { results } = await Document.searchForTeam(team, "test");
expect(results.length).toBe(0); expect(results.length).toBe(0);
}); });
test("should return the total count of search results", async () => {
const team = await buildTeam();
const collection = await buildCollection({ teamId: team.id });
await buildDocument({
teamId: team.id,
collectionId: collection.id,
title: "test number 1",
});
await buildDocument({
teamId: team.id,
collectionId: collection.id,
title: "test number 2",
});
const { totalCount } = await Document.searchForTeam(team, "test");
expect(totalCount).toBe("2");
});
});
describe("#searchForUser", () => {
test("should return search results from collections", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const collection = await buildCollection({
userId: user.id,
teamId: team.id,
});
const document = await buildDocument({
userId: user.id,
teamId: team.id,
collectionId: collection.id,
title: "test",
});
const { results } = await Document.searchForUser(user, "test");
expect(results.length).toBe(1);
expect(results[0].document.id).toBe(document.id);
});
test("should handle no collections", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const { results } = await Document.searchForUser(user, "test");
expect(results.length).toBe(0);
});
test("should return the total count of search results", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const collection = await buildCollection({
userId: user.id,
teamId: team.id,
});
await buildDocument({
userId: user.id,
teamId: team.id,
collectionId: collection.id,
title: "test number 1",
});
await buildDocument({
userId: user.id,
teamId: team.id,
collectionId: collection.id,
title: "test number 2",
});
const { totalCount } = await Document.searchForUser(user, "test");
expect(totalCount).toBe("2");
});
}); });
describe("#delete", () => { describe("#delete", () => {

View File

@ -0,0 +1,42 @@
// @flow
import { DataTypes, sequelize } from "../sequelize";
const SearchQuery = sequelize.define(
"search_queries",
{
id: {
type: DataTypes.UUID,
defaultValue: DataTypes.UUIDV4,
primaryKey: true,
},
source: {
type: DataTypes.ENUM("slack", "app"),
allowNull: false,
},
query: {
type: DataTypes.STRING,
allowNull: false,
},
results: {
type: DataTypes.NUMBER,
allowNull: false,
},
},
{
timestamps: true,
updatedAt: false,
}
);
SearchQuery.associate = (models) => {
SearchQuery.belongsTo(models.User, {
as: "user",
foreignKey: "userId",
});
SearchQuery.belongsTo(models.Team, {
as: "team",
foreignKey: "teamId",
});
};
export default SearchQuery;

View File

@ -14,6 +14,7 @@ import Integration from "./Integration";
import Notification from "./Notification"; import Notification from "./Notification";
import NotificationSetting from "./NotificationSetting"; import NotificationSetting from "./NotificationSetting";
import Revision from "./Revision"; import Revision from "./Revision";
import SearchQuery from "./SearchQuery";
import Share from "./Share"; import Share from "./Share";
import Star from "./Star"; import Star from "./Star";
import Team from "./Team"; import Team from "./Team";
@ -36,6 +37,7 @@ const models = {
Notification, Notification,
NotificationSetting, NotificationSetting,
Revision, Revision,
SearchQuery,
Share, Share,
Star, Star,
Team, Team,
@ -66,6 +68,7 @@ export {
Notification, Notification,
NotificationSetting, NotificationSetting,
Revision, Revision,
SearchQuery,
Share, Share,
Star, Star,
Team, Team,