feat: Add reordering to starred documents (#2953)

* draft

* reordering

* JIT Index stars on first load

* test

* Remove unused code on client

* small unrefactor
This commit is contained in:
Tom Moor
2022-01-21 18:11:50 -08:00
committed by GitHub
parent 49533d7a3f
commit 79e2cad5b9
32 changed files with 931 additions and 132 deletions

View File

@@ -0,0 +1,58 @@
import { Star, Event } from "@server/models";
import { buildDocument, buildUser } from "@server/test/factories";
import { flushdb } from "@server/test/support";
import starCreator from "./starCreator";
beforeEach(() => flushdb());
describe("starCreator", () => {
const ip = "127.0.0.1";
it("should create star", async () => {
const user = await buildUser();
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const star = await starCreator({
documentId: document.id,
user,
ip,
});
const event = await Event.findOne();
expect(star.documentId).toEqual(document.id);
expect(star.userId).toEqual(user.id);
expect(star.index).toEqual("P");
expect(event!.name).toEqual("stars.create");
expect(event!.modelId).toEqual(star.id);
});
it("should not record event if star is existing", async () => {
const user = await buildUser();
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
await Star.create({
teamId: document.teamId,
documentId: document.id,
userId: user.id,
createdById: user.id,
index: "P",
});
const star = await starCreator({
documentId: document.id,
user,
ip,
});
const events = await Event.count();
expect(star.documentId).toEqual(document.id);
expect(star.userId).toEqual(user.id);
expect(star.index).toEqual("P");
expect(events).toEqual(0);
});
});

View File

@@ -0,0 +1,89 @@
import fractionalIndex from "fractional-index";
import { Sequelize, WhereOptions } from "sequelize";
import { sequelize } from "@server/database/sequelize";
import { Star, User, Event } from "@server/models";
type Props = {
/** The user creating the star */
user: User;
/** The document to star */
documentId: string;
/** The sorted index for the star in the sidebar If no index is provided then it will be at the end */
index?: string;
/** The IP address of the user creating the star */
ip: string;
};
/**
* This command creates a "starred" document via the star relation. Stars are
* only visible to the user that created them.
*
* @param Props The properties of the star to create
* @returns Star The star that was created
*/
export default async function starCreator({
user,
documentId,
ip,
...rest
}: Props): Promise<Star> {
let { index } = rest;
const where: WhereOptions<Star> = {
userId: user.id,
};
if (!index) {
const stars = await Star.findAll({
where,
attributes: ["id", "index", "updatedAt"],
limit: 1,
order: [
// using LC_COLLATE:"C" because we need byte order to drive the sorting
// find only the first star so we can create an index before it
Sequelize.literal('"star"."index" collate "C"'),
["updatedAt", "DESC"],
],
});
// create a star at the beginning of the list
index = fractionalIndex(null, stars.length ? stars[0].index : null);
}
const transaction = await sequelize.transaction();
let star;
try {
const response = await Star.findOrCreate({
where: {
userId: user.id,
documentId,
},
defaults: {
index,
},
transaction,
});
star = response[0];
if (response[1]) {
await Event.create(
{
name: "stars.create",
modelId: star.id,
userId: user.id,
actorId: user.id,
documentId,
ip,
},
{ transaction }
);
}
await transaction.commit();
} catch (err) {
await transaction.rollback();
throw err;
}
return star;
}

View File

@@ -0,0 +1,39 @@
import { Star, Event } from "@server/models";
import { buildDocument, buildUser } from "@server/test/factories";
import { flushdb } from "@server/test/support";
import starDestroyer from "./starDestroyer";
beforeEach(() => flushdb());
describe("starDestroyer", () => {
const ip = "127.0.0.1";
it("should destroy existing star", async () => {
const user = await buildUser();
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const star = await Star.create({
teamId: document.teamId,
documentId: document.id,
userId: user.id,
createdById: user.id,
index: "P",
});
await starDestroyer({
star,
user,
ip,
});
const count = await Star.count();
expect(count).toEqual(0);
const event = await Event.findOne();
expect(event!.name).toEqual("stars.delete");
expect(event!.modelId).toEqual(star.id);
});
});

View File

@@ -0,0 +1,54 @@
import { Transaction } from "sequelize";
import { sequelize } from "@server/database/sequelize";
import { Event, Star, User } from "@server/models";
type Props = {
/** The user destroying the star */
user: User;
/** The star to destroy */
star: Star;
/** The IP address of the user creating the star */
ip: string;
/** Optional existing transaction */
transaction?: Transaction;
};
/**
* This command destroys a document star. This just removes the star itself and
* does not touch the document
*
* @param Props The properties of the star to destroy
* @returns void
*/
export default async function starDestroyer({
user,
star,
ip,
transaction: t,
}: Props): Promise<Star> {
const transaction = t || (await sequelize.transaction());
try {
await star.destroy({ transaction });
await Event.create(
{
name: "stars.delete",
modelId: star.id,
teamId: user.teamId,
actorId: user.id,
userId: star.userId,
documentId: star.documentId,
ip,
},
{ transaction }
);
await transaction.commit();
} catch (err) {
await transaction.rollback();
throw err;
}
return star;
}

View File

@@ -0,0 +1,40 @@
import { Star, Event } from "@server/models";
import { buildDocument, buildUser } from "@server/test/factories";
import { flushdb } from "@server/test/support";
import starUpdater from "./starUpdater";
beforeEach(() => flushdb());
describe("starUpdater", () => {
const ip = "127.0.0.1";
it("should update (move) existing star", async () => {
const user = await buildUser();
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
let star = await Star.create({
teamId: document.teamId,
documentId: document.id,
userId: user.id,
createdById: user.id,
index: "P",
});
star = await starUpdater({
star,
index: "h",
user,
ip,
});
const event = await Event.findOne();
expect(star.documentId).toEqual(document.id);
expect(star.userId).toEqual(user.id);
expect(star.index).toEqual("h");
expect(event!.name).toEqual("stars.update");
expect(event!.modelId).toEqual(star.id);
});
});

View File

@@ -0,0 +1,53 @@
import { sequelize } from "@server/database/sequelize";
import { Event, Star, User } from "@server/models";
type Props = {
/** The user updating the star */
user: User;
/** The existing star */
star: Star;
/** The index to star the document at */
index: string;
/** The IP address of the user creating the star */
ip: string;
};
/**
* This command updates a "starred" document. A star can only be moved to a new
* index (reordered) once created.
*
* @param Props The properties of the star to update
* @returns Star The updated star
*/
export default async function starUpdater({
user,
star,
index,
ip,
}: Props): Promise<Star> {
const transaction = await sequelize.transaction();
try {
star.index = index;
await star.save({ transaction });
await Event.create(
{
name: "stars.update",
modelId: star.id,
userId: star.userId,
teamId: user.teamId,
actorId: user.id,
documentId: star.documentId,
ip,
},
{ transaction }
);
await transaction.commit();
} catch (err) {
await transaction.rollback();
throw err;
}
return star;
}

View File

@@ -0,0 +1,13 @@
"use strict";
module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.addColumn("stars", "index", {
type: Sequelize.STRING,
allowNull: true,
});
},
down: async (queryInterface) => {
await queryInterface.removeColumn("stars", "index");
},
};

View File

@@ -13,6 +13,11 @@ import Fix from "./decorators/Fix";
@Table({ tableName: "stars", modelName: "star" })
@Fix
class Star extends BaseModel {
@Column
index: string | null;
// associations
@BelongsTo(() => User, "userId")
user: User;

View File

@@ -17,6 +17,7 @@ import "./notificationSetting";
import "./pins";
import "./searchQuery";
import "./share";
import "./star";
import "./user";
import "./team";
import "./group";

9
server/policies/star.ts Normal file
View File

@@ -0,0 +1,9 @@
import { User, Star } from "@server/models";
import { allow } from "./cancan";
allow(
User,
["update", "delete"],
Star,
(user, star) => user.id === star?.userId
);

View File

@@ -16,6 +16,7 @@ import presentRevision from "./revision";
import presentSearchQuery from "./searchQuery";
import presentShare from "./share";
import presentSlackAttachment from "./slackAttachment";
import presentStar from "./star";
import presentTeam from "./team";
import presentUser from "./user";
import presentView from "./view";
@@ -32,6 +33,7 @@ export {
presentCollection,
presentShare,
presentSearchQuery,
presentStar,
presentTeam,
presentGroup,
presentIntegration,

11
server/presenters/star.ts Normal file
View File

@@ -0,0 +1,11 @@
import { Star } from "@server/models";
export default function present(star: Star) {
return {
id: star.id,
documentId: star.documentId,
index: star.index,
createdAt: star.createdAt,
updatedAt: star.updatedAt,
};
}

View File

@@ -7,8 +7,9 @@ import {
CollectionGroup,
GroupUser,
Pin,
Star,
} from "@server/models";
import { presentPin } from "@server/presenters";
import { presentPin, presentStar } from "@server/presenters";
import { Event } from "../../types";
export default class WebsocketsProcessor {
@@ -386,6 +387,23 @@ export default class WebsocketsProcessor {
});
}
case "stars.create":
case "stars.update": {
const star = await Star.findByPk(event.modelId);
if (!star) {
return;
}
return socketio
.to(`user-${event.userId}`)
.emit(event.name, presentStar(star));
}
case "stars.delete": {
return socketio.to(`user-${event.userId}`).emit(event.name, {
modelId: event.modelId,
});
}
case "groups.create":
case "groups.update": {
const group = await Group.findByPk(event.modelId, {

View File

@@ -25,7 +25,7 @@ import {
presentCollectionGroupMembership,
presentFileOperation,
} from "@server/presenters";
import collectionIndexing from "@server/utils/collectionIndexing";
import { collectionIndexing } from "@server/utils/indexing";
import removeIndexCollision from "@server/utils/removeIndexCollision";
import {
assertUuid,
@@ -623,12 +623,12 @@ router.post("collections.list", auth(), pagination(), async (ctx) => {
offset: ctx.state.pagination.offset,
limit: ctx.state.pagination.limit,
});
const nullIndexCollection = collections.findIndex(
const nullIndex = collections.findIndex(
(collection) => collection.index === null
);
if (nullIndexCollection !== -1) {
const indexedCollections = await collectionIndexing(ctx.state.user.teamId);
if (nullIndex !== -1) {
const indexedCollections = await collectionIndexing(user.teamId);
collections.forEach((collection) => {
collection.index = indexedCollections[collection.id];
});

View File

@@ -312,6 +312,7 @@ router.post("documents.viewed", auth(), pagination(), async (ctx) => {
};
});
// Deprecated use stars.list instead
router.post("documents.starred", auth(), pagination(), async (ctx) => {
let { direction } = ctx.body;
const { sort = "updatedAt" } = ctx.body;
@@ -864,6 +865,7 @@ router.post("documents.search", auth(), pagination(), async (ctx) => {
};
});
// Deprecated use stars.create instead
router.post("documents.star", auth(), async (ctx) => {
const { id } = ctx.body;
assertPresent(id, "id is required");
@@ -898,6 +900,7 @@ router.post("documents.star", auth(), async (ctx) => {
};
});
// Deprecated use stars.delete instead
router.post("documents.unstar", auth(), async (ctx) => {
const { id } = ctx.body;
assertPresent(id, "id is required");

View File

@@ -22,6 +22,7 @@ import pins from "./pins";
import revisions from "./revisions";
import searches from "./searches";
import shares from "./shares";
import stars from "./stars";
import team from "./team";
import users from "./users";
import utils from "./utils";
@@ -58,6 +59,7 @@ router.use("/", hooks.routes());
router.use("/", apiKeys.routes());
router.use("/", searches.routes());
router.use("/", shares.routes());
router.use("/", stars.routes());
router.use("/", team.routes());
router.use("/", integrations.routes());
router.use("/", notificationSettings.routes());

View File

@@ -0,0 +1,86 @@
import TestServer from "fetch-test-server";
import webService from "@server/services/web";
import { buildUser, buildStar, buildDocument } from "@server/test/factories";
import { flushdb } from "@server/test/support";
const app = webService();
const server = new TestServer(app.callback());
beforeEach(() => flushdb());
afterAll(() => server.close());
describe("#stars.create", () => {
it("should create a star", async () => {
const user = await buildUser();
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const res = await server.post("/api/stars.create", {
body: {
token: user.getJwtToken(),
documentId: document.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.documentId).toEqual(document.id);
});
it("should require authentication", async () => {
const res = await server.post("/api/stars.create");
expect(res.status).toEqual(401);
});
});
describe("#stars.list", () => {
it("should list users stars", async () => {
const user = await buildUser();
await buildStar();
const star = await buildStar({
userId: user.id,
});
const res = await server.post("/api/stars.list", {
body: {
token: user.getJwtToken(),
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.stars.length).toEqual(1);
expect(body.data.stars[0].id).toEqual(star.id);
});
it("should require authentication", async () => {
const res = await server.post("/api/stars.list");
expect(res.status).toEqual(401);
});
});
describe("#stars.delete", () => {
it("should delete users star", async () => {
const user = await buildUser();
const star = await buildStar({
userId: user.id,
});
const res = await server.post("/api/stars.delete", {
body: {
id: star.id,
token: user.getJwtToken(),
},
});
expect(res.status).toEqual(200);
});
it("should require authentication", async () => {
const res = await server.post("/api/stars.delete");
expect(res.status).toEqual(401);
});
});

134
server/routes/api/stars.ts Normal file
View File

@@ -0,0 +1,134 @@
import Router from "koa-router";
import { Sequelize } from "sequelize";
import starCreator from "@server/commands/starCreator";
import starDestroyer from "@server/commands/starDestroyer";
import starUpdater from "@server/commands/starUpdater";
import auth from "@server/middlewares/authentication";
import { Document, Star } from "@server/models";
import { authorize } from "@server/policies";
import {
presentStar,
presentDocument,
presentPolicies,
} from "@server/presenters";
import { starIndexing } from "@server/utils/indexing";
import { assertUuid, assertIndexCharacters } from "@server/validation";
import pagination from "./middlewares/pagination";
const router = new Router();
router.post("stars.create", auth(), async (ctx) => {
const { documentId } = ctx.body;
const { index } = ctx.body;
assertUuid(documentId, "documentId is required");
const { user } = ctx.state;
const document = await Document.findByPk(documentId, {
userId: user.id,
});
authorize(user, "star", document);
if (index) {
assertIndexCharacters(index);
}
const star = await starCreator({
user,
documentId,
ip: ctx.request.ip,
index,
});
ctx.body = {
data: presentStar(star),
policies: presentPolicies(user, [star]),
};
});
router.post("stars.list", auth(), pagination(), async (ctx) => {
const { user } = ctx.state;
const [stars, collectionIds] = await Promise.all([
Star.findAll({
where: {
userId: user.id,
},
order: [
Sequelize.literal('"star"."index" collate "C"'),
["updatedAt", "DESC"],
],
offset: ctx.state.pagination.offset,
limit: ctx.state.pagination.limit,
}),
user.collectionIds(),
]);
const nullIndex = stars.findIndex((star) => star.index === null);
if (nullIndex !== -1) {
const indexedStars = await starIndexing(user.id);
stars.forEach((star) => {
star.index = indexedStars[star.id];
});
}
const documents = await Document.defaultScopeWithUser(user.id).findAll({
where: {
id: stars.map((star) => star.documentId),
collectionId: collectionIds,
},
});
const policies = presentPolicies(user, [...documents, ...stars]);
ctx.body = {
pagination: ctx.state.pagination,
data: {
stars: stars.map(presentStar),
documents: await Promise.all(
documents.map((document: Document) => presentDocument(document))
),
},
policies,
};
});
router.post("stars.update", auth(), async (ctx) => {
const { id, index } = ctx.body;
assertUuid(id, "id is required");
assertIndexCharacters(index);
const { user } = ctx.state;
let star = await Star.findByPk(id);
authorize(user, "update", star);
star = await starUpdater({
user,
star,
ip: ctx.request.ip,
index,
});
ctx.body = {
data: presentStar(star),
policies: presentPolicies(user, [star]),
};
});
router.post("stars.delete", auth(), async (ctx) => {
const { id } = ctx.body;
assertUuid(id, "id is required");
const { user } = ctx.state;
const star = await Star.findByPk(id);
authorize(user, "delete", star);
await starDestroyer({ user, star, ip: ctx.request.ip });
ctx.body = {
success: true,
};
});
export default router;

View File

@@ -5,6 +5,7 @@ import {
User,
Event,
Document,
Star,
Collection,
Group,
GroupUser,
@@ -44,6 +45,30 @@ export async function buildShare(overrides: Partial<Share> = {}) {
});
}
export async function buildStar(overrides: Partial<Star> = {}) {
let user;
if (overrides.userId) {
user = await User.findByPk(overrides.userId);
} else {
user = await buildUser();
overrides.userId = user.id;
}
if (!overrides.documentId) {
const document = await buildDocument({
createdById: overrides.userId,
teamId: user?.teamId,
});
overrides.documentId = document.id;
}
return Star.create({
index: "h",
...overrides,
});
}
export function buildTeam(overrides: Record<string, any> = {}) {
count++;
return Team.create(

View File

@@ -241,15 +241,27 @@ export type PinEvent = {
name: "pins.create" | "pins.update" | "pins.delete";
teamId: string;
modelId: string;
documentId: string;
collectionId?: string;
actorId: string;
ip: string;
};
export type StarEvent = {
name: "stars.create" | "stars.update" | "stars.delete";
teamId: string;
modelId: string;
documentId: string;
userId: string;
actorId: string;
ip: string;
};
export type Event =
| UserEvent
| DocumentEvent
| PinEvent
| StarEvent
| CollectionEvent
| CollectionImportEvent
| CollectionExportAllEvent

View File

@@ -1,44 +0,0 @@
import fractionalIndex from "fractional-index";
import naturalSort from "@shared/utils/naturalSort";
import { Collection } from "@server/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: [Collection, string | null][] = 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;
}

82
server/utils/indexing.ts Normal file
View File

@@ -0,0 +1,82 @@
import fractionalIndex from "fractional-index";
import naturalSort from "@shared/utils/naturalSort";
import { Collection, Document, Star } from "@server/models";
export async function collectionIndexing(
teamId: string
): Promise<{ [id: string]: string }> {
const collections = await Collection.findAll({
where: {
teamId,
// no point in maintaining index of deleted collections.
deletedAt: null,
},
attributes: ["id", "index", "name"],
});
const sortable = naturalSort(collections, (collection) => collection.name);
// for each collection with null index, use previous collection index to create new index
let previousIndex = null;
const promises = [];
for (const collection of sortable) {
if (collection.index === null) {
collection.index = fractionalIndex(previousIndex, null);
promises.push(collection.save());
}
previousIndex = collection.index;
}
await Promise.all(promises);
const indexedCollections = {};
sortable.forEach((collection) => {
indexedCollections[collection.id] = collection.index;
});
return indexedCollections;
}
export async function starIndexing(
userId: string
): Promise<{ [id: string]: string }> {
const stars = await Star.findAll({
where: { userId },
});
const documents = await Document.findAll({
attributes: ["id", "updatedAt"],
where: {
id: stars.map((star) => star.documentId),
},
order: [["updatedAt", "DESC"]],
});
const sortable = stars.sort(function (a, b) {
return (
documents.findIndex((d) => d.id === a.documentId) -
documents.findIndex((d) => d.id === b.documentId)
);
});
let previousIndex = null;
const promises = [];
for (const star of sortable) {
if (star.index === null) {
star.index = fractionalIndex(previousIndex, null);
promises.push(star.save());
}
previousIndex = star.index;
}
await Promise.all(promises);
const indexedStars = {};
sortable.forEach((star) => {
indexedStars[star.id] = star.index;
});
return indexedStars;
}