feat: Allow sorting collections in sidebar (#1870)

closes #1759

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

server/api/__snapshots__/collections.test.js.snap

@@ -97,6 +97,15 @@ Object {
 }
 `;
 
+exports[`#collections.move should require authentication 1`] = `
+Object {
+  "error": "authentication_required",
+  "message": "Authentication required",
+  "ok": false,
+  "status": 401,
+}
+`;
+
 exports[`#collections.remove_group should require group in team 1`] = `
 Object {
   "error": "authorization_error",

server/api/collections.js

@@ -1,5 +1,6 @@
 // @flow
 import fs from "fs";
+import fractionalIndex from "fractional-index";
 import Router from "koa-router";
 import { ValidationError } from "../errors";
 import { exportCollections } from "../exporter";
@@ -23,7 +24,10 @@ import {
   presentGroup,
   presentCollectionGroupMembership,
 } from "../presenters";
-import { Op } from "../sequelize";
+import { Op, sequelize } from "../sequelize";
+
+import collectionIndexing from "../utils/collectionIndexing";
+import removeIndexCollision from "../utils/removeIndexCollision";
 import { archiveCollection, archiveCollections } from "../utils/zip";
 import pagination from "./middlewares/pagination";
 
@@ -39,6 +43,8 @@ router.post("collections.create", auth(), async (ctx) => {
     icon,
     sort = Collection.DEFAULT_SORT,
   } = ctx.body;
+
+  let { index } = ctx.body;
   const isPrivate = ctx.body.private;
   ctx.assertPresent(name, "name is required");
 
@@ -49,6 +55,30 @@ router.post("collections.create", auth(), async (ctx) => {
   const user = ctx.state.user;
   authorize(user, "create", Collection);
 
+  const collections = await Collection.findAll({
+    where: { teamId: user.teamId, deletedAt: null },
+    attributes: ["id", "index", "updatedAt"],
+    limit: 1,
+    order: [
+      // using LC_COLLATE:"C" because we need byte order to drive the sorting
+      sequelize.literal('"collection"."index" collate "C"'),
+      ["updatedAt", "DESC"],
+    ],
+  });
+
+  if (index) {
+    const allowedASCII = new RegExp(/^[\x21-\x7E]+$/);
+    if (!allowedASCII.test(index)) {
+      throw new ValidationError(
+        "Index characters must be between x21 to x7E ASCII"
+      );
+    }
+  } else {
+    index = fractionalIndex(null, collections[0].index);
+  }
+
+  index = await removeIndexCollision(user.teamId, index);
+
   let collection = await Collection.create({
     name,
     description,
@@ -59,6 +89,7 @@ router.post("collections.create", auth(), async (ctx) => {
     private: isPrivate,
     sharing,
     sort,
+    index,
   });
 
   await Event.create({
@@ -571,6 +602,17 @@ router.post("collections.list", auth(), pagination(), async (ctx) => {
     limit: ctx.state.pagination.limit,
   });
 
+  const nullIndexCollection = collections.findIndex(
+    (collection) => collection.index === null
+  );
+
+  if (nullIndexCollection !== -1) {
+    const indexedCollections = await collectionIndexing(ctx.state.user.teamId);
+    collections.forEach((collection) => {
+      collection.index = indexedCollections[collection.id];
+    });
+  }
+
   ctx.body = {
     pagination: ctx.state.pagination,
     data: collections.map(presentCollection),
@@ -608,4 +650,34 @@ router.post("collections.delete", auth(), async (ctx) => {
   };
 });
 
+router.post("collections.move", auth(), async (ctx) => {
+  const id = ctx.body.id;
+  let index = ctx.body.index;
+
+  ctx.assertPresent(index, "index is required");
+  ctx.assertUuid(id, "id must be a uuid");
+
+  const user = ctx.state.user;
+  const collection = await Collection.findByPk(id);
+
+  authorize(user, "move", collection);
+
+  index = await removeIndexCollision(user.teamId, index);
+
+  await collection.update({ index });
+
+  await Event.create({
+    name: "collections.move",
+    collectionId: collection.id,
+    teamId: collection.teamId,
+    actorId: user.id,
+    data: { index },
+    ip: ctx.request.ip,
+  });
+
+  ctx.body = {
+    success: true,
+    data: { index },
+  };
+});
 export default router;

server/api/collections.test.js

@@ -130,6 +130,127 @@ describe("#collections.import", () => {
   });
 });
 
+describe("#collections.move", () => {
+  it("should require authentication", async () => {
+    const res = await server.post("/api/collections.move");
+    const body = await res.json();
+
+    expect(res.status).toEqual(401);
+    expect(body).toMatchSnapshot();
+  });
+
+  it("should require authorization", async () => {
+    const user = await buildUser();
+    const { collection } = await seed();
+
+    const res = await server.post("/api/collections.move", {
+      body: { token: user.getJwtToken(), id: collection.id, index: "P" },
+    });
+
+    expect(res.status).toEqual(403);
+  });
+
+  it("should return success", async () => {
+    const { admin, collection } = await seed();
+    const res = await server.post("/api/collections.move", {
+      body: { token: admin.getJwtToken(), id: collection.id, index: "P" },
+    });
+
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.success).toBe(true);
+  });
+
+  it("if index collision occurs, should updated index of other collection", async () => {
+    const { user, admin, collection } = await seed();
+    const createdCollectionResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "Test",
+          sharing: false,
+          index: "Q",
+        },
+      }
+    );
+
+    await createdCollectionResponse.json();
+    const movedCollectionRes = await server.post("/api/collections.move", {
+      body: { token: admin.getJwtToken(), id: collection.id, index: "Q" },
+    });
+
+    const movedCollection = await movedCollectionRes.json();
+
+    expect(movedCollectionRes.status).toEqual(200);
+    expect(movedCollection.success).toBe(true);
+    expect(movedCollection.data.index).toEqual("h");
+    expect(movedCollection.data.index > "Q").toBeTruthy();
+  });
+
+  it("if index collision with an extra collection, should updated index of other collection", async () => {
+    const { user, admin } = await seed();
+    const createdCollectionAResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "A",
+          sharing: false,
+          index: "a",
+        },
+      }
+    );
+
+    const createdCollectionBResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "B",
+          sharing: false,
+          index: "b",
+        },
+      }
+    );
+
+    const createdCollectionCResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "C",
+          sharing: false,
+          index: "c",
+        },
+      }
+    );
+
+    await createdCollectionAResponse.json();
+    await createdCollectionBResponse.json();
+    const createdCollectionC = await createdCollectionCResponse.json();
+
+    const movedCollectionCResponse = await server.post(
+      "/api/collections.move",
+      {
+        body: {
+          token: admin.getJwtToken(),
+          id: createdCollectionC.data.id,
+          index: "a",
+        },
+      }
+    );
+
+    const movedCollectionC = await movedCollectionCResponse.json();
+
+    expect(movedCollectionCResponse.status).toEqual(200);
+    expect(movedCollectionC.success).toBe(true);
+    expect(movedCollectionC.data.index).toEqual("aP");
+    expect(movedCollectionC.data.index > "a").toBeTruthy();
+    expect(movedCollectionC.data.index < "b").toBeTruthy();
+  });
+});
+
 describe("#collections.export", () => {
   it("should now allow export of private collection not a member", async () => {
     const { user } = await seed();
@@ -922,6 +1043,89 @@ describe("#collections.create", () => {
     expect(body.policies[0].abilities.read).toBeTruthy();
     expect(body.policies[0].abilities.export).toBeTruthy();
   });
+
+  it("if index collision, should updated index of other collection", async () => {
+    const { user } = await seed();
+    const createdCollectionAResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "A",
+          sharing: false,
+          index: "a",
+        },
+      }
+    );
+    await createdCollectionAResponse.json();
+
+    const createCollectionResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "C",
+          sharing: false,
+          index: "a",
+        },
+      }
+    );
+
+    const createdCollection = await createCollectionResponse.json();
+
+    expect(createCollectionResponse.status).toEqual(200);
+    expect(createdCollection.data.index).toEqual("p");
+    expect(createdCollection.data.index > "a").toBeTruthy();
+  });
+
+  it("if index collision with an extra collection, should updated index of other collection", async () => {
+    const { user } = await seed();
+    const createdCollectionAResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "A",
+          sharing: false,
+          index: "a",
+        },
+      }
+    );
+
+    const createdCollectionBResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "B",
+          sharing: false,
+          index: "b",
+        },
+      }
+    );
+
+    await createdCollectionAResponse.json();
+    await createdCollectionBResponse.json();
+
+    const createCollectionResponse = await server.post(
+      "/api/collections.create",
+      {
+        body: {
+          token: user.getJwtToken(),
+          name: "C",
+          sharing: false,
+          index: "a",
+        },
+      }
+    );
+
+    const createdCollection = await createCollectionResponse.json();
+
+    expect(createCollectionResponse.status).toEqual(200);
+    expect(createdCollection.data.index).toEqual("aP");
+    expect(createdCollection.data.index > "a").toBeTruthy();
+    expect(createdCollection.data.index < "b").toBeTruthy();
+  });
 });
 
 describe("#collections.update", () => {

server/events.js

@@ -118,6 +118,7 @@ export type CollectionEvent =
       collectionId: string,
       teamId: string,
       actorId: string,
+      data: { name: string },
       ip: string,
     }
   | {
@@ -135,6 +136,14 @@ export type CollectionEvent =
       actorId: string,
       data: { name: string, groupId: string },
       ip: string,
+    }
+  | {
+      name: "collections.move",
+      collectionId: string,
+      teamId: string,
+      actorId: string,
+      data: { index: string },
+      ip: string,
     };
 
 export type GroupEvent =

server/migrations/20210218111237-add-collection-index.js

@@ -0,0 +1,14 @@
+"use strict";
+
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    await queryInterface.addColumn("collections", "index", {
+      type: Sequelize.TEXT,
+      defaultValue: null,
+    });
+  },
+
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.removeColumn("collections", "index");
+  },
+};

server/models/Collection.js

@@ -21,6 +21,10 @@ const Collection = sequelize.define(
     description: DataTypes.STRING,
     icon: DataTypes.STRING,
     color: DataTypes.STRING,
+    index: {
+      type: DataTypes.STRING,
+      defaultValue: null,
+    },
     private: DataTypes.BOOLEAN,
     maintainerApprovalRequired: DataTypes.BOOLEAN,
     documentStructure: DataTypes.JSONB,

server/models/Event.js

@@ -57,6 +57,7 @@ Event.add = (event) => {
 Event.ACTIVITY_EVENTS = [
   "collections.create",
   "collections.delete",
+  "collections.move",
   "documents.publish",
   "documents.archive",
   "documents.unarchive",
@@ -73,6 +74,7 @@ Event.AUDIT_EVENTS = [
   "api_keys.delete",
   "collections.create",
   "collections.update",
+  "collections.move",
   "collections.add_user",
   "collections.remove_user",
   "collections.add_group",

server/policies/collection.js

@@ -14,6 +14,13 @@ allow(User, "import", Collection, (actor) => {
   throw new AdminRequiredError();
 });
 
+allow(User, "move", Collection, (user, collection) => {
+  if (!collection || user.teamId !== collection.teamId) return false;
+  if (collection.deletedAt) return false;
+  if (user.isAdmin) return true;
+  throw new AdminRequiredError();
+});
+
 allow(User, ["read", "export"], Collection, (user, collection) => {
   if (!collection || user.teamId !== collection.teamId) return false;
 

server/presenters/collection.js

@@ -28,6 +28,7 @@ export default function present(collection: Collection) {
     description: collection.description,
     sort: collection.sort,
     icon: collection.icon,
+    index: collection.index,
     color: collection.color || "#4E5C6E",
     private: collection.private,
     sharing: collection.sharing,

server/services/websockets.js

@@ -170,6 +170,7 @@ export default class Websockets {
               },
             ],
           });
+
         return socketio
           .to(
             collection.private
@@ -197,6 +198,16 @@ export default class Websockets {
           ],
         });
       }
+
+      case "collections.move": {
+        return socketio
+          .to(`collection-${event.collectionId}`)
+          .emit("collections.update_index", {
+            collectionId: event.collectionId,
+            index: event.data.index,
+          });
+      }
+
       case "collections.add_user": {
         // the user being added isn't yet in the websocket channel for the collection
         // so they need to be notified separately

server/utils/collectionIndexing.js

@@ -0,0 +1,39 @@
+// @flow
+import fractionalIndex from "fractional-index";
+import naturalSort from "../../shared/utils/naturalSort";
+import { Collection } from "../models";
+
+export default async function collectionIndexing(teamId: string) {
+  const collections = await Collection.findAll({
+    where: { teamId, deletedAt: null }, //no point in maintaining index of deleted collections.
+    attributes: ["id", "index", "name"],
+  });
+
+  let sortableCollections = collections.map((collection) => {
+    return [collection, collection.index];
+  });
+
+  sortableCollections = naturalSort(
+    sortableCollections,
+    (collection) => collection[0].name
+  );
+
+  //for each collection with null index, use previous collection index to create new index
+  let previousCollectionIndex = null;
+
+  for (const collection of sortableCollections) {
+    if (collection[1] === null) {
+      const index = fractionalIndex(previousCollectionIndex, collection[1]);
+      collection[0].index = index;
+      await collection[0].save();
+    }
+    previousCollectionIndex = collection[0].index;
+  }
+
+  const indexedCollections = {};
+  sortableCollections.forEach((collection) => {
+    indexedCollections[collection[0].id] = collection[0].index;
+  });
+
+  return indexedCollections;
+}

server/utils/removeIndexCollision.js

@@ -0,0 +1,45 @@
+// @flow
+import fractionalIndex from "fractional-index";
+import { Collection } from "../models";
+import { sequelize, Op } from "../sequelize";
+
+/**
+ *
+ * @param teamId The team id whose collections has to be fetched
+ * @param index the index for which collision has to be checked
+ * @returns An index, if there is collision returns a new index otherwise the same index
+ */
+export default async function removeIndexCollision(
+  teamId: string,
+  index: string
+) {
+  const collection = await Collection.findOne({
+    where: { teamId, deletedAt: null, index },
+  });
+
+  if (!collection) {
+    return index;
+  }
+
+  const nextCollection = await Collection.findAll({
+    where: {
+      teamId,
+      deletedAt: null,
+      index: {
+        [Op.gt]: index,
+      },
+    },
+    attributes: ["id", "index"],
+    limit: 1,
+    order: [
+      sequelize.literal('"collection"."index" collate "C"'),
+      ["updatedAt", "DESC"],
+    ],
+  });
+
+  const nextCollectionIndex = nextCollection.length
+    ? nextCollection[0].index
+    : null;
+
+  return fractionalIndex(index, nextCollectionIndex);
+}