fix: Allow searching of previous document titles (#2326)

* Add migrations

* Handle previousTitles when titles is updated

* Add necessary test cases

* Use previous title while searching

* Rewrite logic to update previousTitles in beforeSave hook

* Update weights

* Update test to match new rank order

* Add tooltip to inform user on document

* Add code comment

* Remove previous title tooltip

* fix: Remove unused string, add model tests

Co-authored-by: Tom Moor <tom.moor@gmail.com>

server/api/documents.test.js

@@ -900,6 +900,7 @@ describe("#documents.search", () => {
       userId: user.id,
       teamId: user.teamId,
     });
+
     const secondResult = await buildDocument({
       title: "random text",
       text: "search term",
@@ -907,15 +908,26 @@ describe("#documents.search", () => {
       teamId: user.teamId,
     });
 
+    const thirdResult = await buildDocument({
+      title: "search term",
+      text: "random text",
+      userId: user.id,
+      teamId: user.teamId,
+    });
+
+    thirdResult.title = "change";
+    await thirdResult.save();
+
     const res = await server.post("/api/documents.search", {
       body: { token: user.getJwtToken(), query: "search term" },
     });
     const body = await res.json();
 
     expect(res.status).toEqual(200);
-    expect(body.data.length).toEqual(2);
+    expect(body.data.length).toEqual(3);
     expect(body.data[0].document.id).toEqual(firstResult.id);
     expect(body.data[1].document.id).toEqual(secondResult.id);
+    expect(body.data[2].document.id).toEqual(thirdResult.id);
   });
 
   it("should return partial results in ranked order", async () => {
@@ -933,15 +945,26 @@ describe("#documents.search", () => {
       teamId: user.teamId,
     });
 
+    const thirdResult = await buildDocument({
+      title: "search term",
+      text: "random text",
+      userId: user.id,
+      teamId: user.teamId,
+    });
+
+    thirdResult.title = "change";
+    await thirdResult.save();
+
     const res = await server.post("/api/documents.search", {
       body: { token: user.getJwtToken(), query: "sear &" },
     });
     const body = await res.json();
 
     expect(res.status).toEqual(200);
-    expect(body.data.length).toEqual(2);
+    expect(body.data.length).toEqual(3);
     expect(body.data[0].document.id).toEqual(firstResult.id);
     expect(body.data[1].document.id).toEqual(secondResult.id);
+    expect(body.data[2].document.id).toEqual(thirdResult.id);
   });
 
   it("should strip junk from search term", async () => {

server/migrations/20210716064654-introduce-previousTitles.js

@@ -0,0 +1,13 @@
+'use strict';
+
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+   await queryInterface.addColumn("documents","previousTitles",{
+     type: Sequelize.ARRAY(Sequelize.STRING)
+   })
+  },
+
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.removeColumn("documents", "previousTitles");
+  }
+};

server/migrations/20210716071454-search-index-previousTitles.js

@@ -0,0 +1,33 @@
+'use strict';
+
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    const searchDocument = `
+    CREATE OR REPLACE FUNCTION documents_search_trigger() RETURNS trigger AS $$
+    begin
+      new."searchVector" :=
+        setweight(to_tsvector('english', coalesce(new.title, '')),'A') ||
+        setweight(to_tsvector('english', coalesce(array_to_string(new."previousTitles", ' , '),'')),'C') ||
+        setweight(to_tsvector('english', coalesce(new.text, '')), 'B');
+      return new;
+    end
+    $$ LANGUAGE plpgsql;
+        `;
+    await queryInterface.sequelize.query(searchDocument);
+  },
+
+  down: async (queryInterface, Sequelize) => {
+    const searchDocument = `
+    CREATE OR REPLACE FUNCTION documents_search_trigger() RETURNS trigger AS $$
+    begin
+      new."searchVector" :=
+        setweight(to_tsvector('english', coalesce(new.title, '')),'A') ||
+        setweight(to_tsvector('english', coalesce(new.text, '')), 'C');
+      return new;
+    end
+    $$ LANGUAGE plpgsql;
+        `;
+    await queryInterface.sequelize.query(searchDocument);
+
+  }
+};

server/models/Document.js

@@ -39,6 +39,11 @@ const beforeSave = async (doc) => {
   // ensure documents have a title
   doc.title = doc.title || "";
 
+  if (doc.previous("title") && doc.previous("title") !== doc.title) {
+    if (!doc.previousTitles) doc.previousTitles = [];
+    doc.previousTitles = uniq(doc.previousTitles.concat(doc.previous("title")));
+  }
+
   // add the current user as a collaborator on this doc
   if (!doc.collaboratorIds) doc.collaboratorIds = [];
   doc.collaboratorIds = uniq(doc.collaboratorIds.concat(doc.lastModifiedById));
@@ -70,6 +75,7 @@ const Document = sequelize.define(
         },
       },
     },
+    previousTitles: DataTypes.ARRAY(DataTypes.STRING),
     version: DataTypes.SMALLINT,
     template: DataTypes.BOOLEAN,
     editorVersion: DataTypes.STRING,

server/models/Document.test.js

@@ -221,6 +221,41 @@ describe("#searchForTeam", () => {
     const { totalCount } = await Document.searchForTeam(team, "test");
     expect(totalCount).toBe("2");
   });
+
+  test("should return the document when searched with their previous titles", async () => {
+    const team = await buildTeam();
+    const collection = await buildCollection({ teamId: team.id });
+    const document = await buildDocument({
+      teamId: team.id,
+      collectionId: collection.id,
+      title: "test number 1",
+    });
+
+    document.title = "change";
+    await document.save();
+
+    const { totalCount } = await Document.searchForTeam(team, "test number");
+    expect(totalCount).toBe("1");
+  });
+
+  test("should not return the document when searched with neither the titles nor the previous titles", async () => {
+    const team = await buildTeam();
+    const collection = await buildCollection({ teamId: team.id });
+    const document = await buildDocument({
+      teamId: team.id,
+      collectionId: collection.id,
+      title: "test number 1",
+    });
+
+    document.title = "change";
+    await document.save();
+
+    const { totalCount } = await Document.searchForTeam(
+      team,
+      "title doesn't exist"
+    );
+    expect(totalCount).toBe("0");
+  });
 });
 
 describe("#searchForUser", () => {
@@ -273,6 +308,51 @@ describe("#searchForUser", () => {
     const { totalCount } = await Document.searchForUser(user, "test");
     expect(totalCount).toBe("2");
   });
+
+  test("should return the document when searched with their previous titles", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+    const collection = await buildCollection({
+      teamId: team.id,
+      userId: user.id,
+    });
+    const document = await buildDocument({
+      teamId: team.id,
+      userId: user.id,
+      collectionId: collection.id,
+      title: "test number 1",
+    });
+
+    document.title = "change";
+    await document.save();
+
+    const { totalCount } = await Document.searchForUser(user, "test number");
+    expect(totalCount).toBe("1");
+  });
+
+  test("should not return the document when searched with neither the titles nor the previous titles", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+    const collection = await buildCollection({
+      teamId: team.id,
+      userId: user.id,
+    });
+    const document = await buildDocument({
+      teamId: team.id,
+      userId: user.id,
+      collectionId: collection.id,
+      title: "test number 1",
+    });
+
+    document.title = "change";
+    await document.save();
+
+    const { totalCount } = await Document.searchForUser(
+      user,
+      "title doesn't exist"
+    );
+    expect(totalCount).toBe("0");
+  });
 });
 
 describe("#delete", () => {
@@ -309,6 +389,37 @@ describe("#delete", () => {
   });
 });
 
+describe("#save", () => {
+  test("should have empty previousTitles by default", async () => {
+    const document = await buildDocument();
+    expect(document.previousTitles).toBe(null);
+  });
+
+  test("should include previousTitles on save", async () => {
+    const document = await buildDocument();
+
+    document.title = "test";
+    await document.save();
+
+    expect(document.previousTitles.length).toBe(1);
+  });
+
+  test("should not duplicate previousTitles", async () => {
+    const document = await buildDocument();
+
+    document.title = "test";
+    await document.save();
+
+    document.title = "example";
+    await document.save();
+
+    document.title = "test";
+    await document.save();
+
+    expect(document.previousTitles.length).toBe(3);
+  });
+});
+
 describe("#findByPk", () => {
   test("should return document when urlId is correct", async () => {
     const { document } = await seed();