feat: Comment resolving (#7115)

This commit is contained in:
Tom Moor
2024-07-02 06:55:16 -04:00
committed by GitHub
parent f34557337d
commit 117c4f5009
38 changed files with 1126 additions and 291 deletions

View File

@@ -0,0 +1,19 @@
import { Next } from "koa";
import { TeamPreference } from "@shared/types";
import { ValidationError } from "@server/errors";
import { APIContext } from "@server/types";
/**
* Middleware to check if a feature is enabled for the team.
*
* @param preference The preference to check
* @returns The middleware function
*/
export function feature(preference: TeamPreference) {
return async function featureEnabledMiddleware(ctx: APIContext, next: Next) {
if (!ctx.state.auth.user.team.getPreference(preference)) {
throw ValidationError(`${preference} is currently disabled`);
}
return next();
};
}

View File

@@ -13,6 +13,7 @@ import type { ProsemirrorData } from "@shared/types";
import { ProsemirrorHelper } from "@shared/utils/ProsemirrorHelper";
import { CommentValidation } from "@shared/validations";
import { schema } from "@server/editor";
import { ValidationError } from "@server/errors";
import Document from "./Document";
import User from "./User";
import ParanoidModel from "./base/ParanoidModel";
@@ -26,6 +27,11 @@ import TextLength from "./validators/TextLength";
as: "createdBy",
paranoid: false,
},
{
model: User,
as: "resolvedBy",
paranoid: false,
},
],
}))
@Table({ tableName: "comments", modelName: "comment" })
@@ -54,12 +60,15 @@ class Comment extends ParanoidModel<
@Column(DataType.UUID)
createdById: string;
@Column(DataType.DATE)
resolvedAt: Date | null;
@BelongsTo(() => User, "resolvedById")
resolvedBy: User;
resolvedBy: User | null;
@ForeignKey(() => User)
@Column(DataType.UUID)
resolvedById: string;
resolvedById: string | null;
@BelongsTo(() => Document, "documentId")
document: Document;
@@ -75,6 +84,51 @@ class Comment extends ParanoidModel<
@Column(DataType.UUID)
parentCommentId: string;
// methods
/**
* Resolve the comment. Note this does not save the comment to the database.
*
* @param resolvedBy The user who resolved the comment
*/
public resolve(resolvedBy: User) {
if (this.isResolved) {
throw ValidationError("Comment is already resolved");
}
if (this.parentCommentId) {
throw ValidationError("Cannot resolve a reply");
}
this.resolvedById = resolvedBy.id;
this.resolvedBy = resolvedBy;
this.resolvedAt = new Date();
}
/**
* Unresolve the comment. Note this does not save the comment to the database.
*/
public unresolve() {
if (!this.isResolved) {
throw ValidationError("Comment is not resolved");
}
this.resolvedById = null;
this.resolvedBy = null;
this.resolvedAt = null;
}
/**
* Whether the comment is resolved
*/
public get isResolved() {
return !!this.resolvedAt;
}
/**
* Convert the comment data to plain text
*
* @returns The plain text representation of the comment data
*/
public toPlainText() {
const node = Node.fromJSON(schema, this.data);
return ProsemirrorHelper.toPlainText(node, schema);

View File

@@ -73,6 +73,10 @@ export default function onerror(app: Koa) {
requestErrorHandler(err, this);
if (!(err instanceof InternalError)) {
if (env.ENVIRONMENT === "test") {
// eslint-disable-next-line no-console
console.error(err);
}
err = InternalError();
}
}

View File

@@ -8,6 +8,22 @@ allow(User, "read", Comment, (actor, comment) =>
isTeamModel(actor, comment?.createdBy)
);
allow(User, "resolve", Comment, (actor, comment) =>
and(
isTeamModel(actor, comment?.createdBy),
comment?.parentCommentId === null,
comment?.resolvedById === null
)
);
allow(User, "unresolve", Comment, (actor, comment) =>
and(
isTeamModel(actor, comment?.createdBy),
comment?.parentCommentId === null,
comment?.resolvedById !== null
)
);
allow(User, ["update", "delete"], Comment, (actor, comment) =>
and(
isTeamModel(actor, comment?.createdBy),

View File

@@ -9,6 +9,9 @@ export default function present(comment: Comment) {
parentCommentId: comment.parentCommentId,
createdBy: presentUser(comment.createdBy),
createdById: comment.createdById,
resolvedAt: comment.resolvedAt,
resolvedBy: comment.resolvedBy ? presentUser(comment.resolvedBy) : null,
resolvedById: comment.resolvedById,
createdAt: comment.createdAt,
updatedAt: comment.updatedAt,
};

View File

@@ -424,6 +424,7 @@ export default class WebsocketsProcessor {
case "comments.delete": {
const comment = await Comment.findByPk(event.modelId, {
paranoid: false,
include: [
{
model: Document.scope(["withoutState", "withDrafts"]),

View File

@@ -26,3 +26,30 @@ exports[`#comments.list should require authentication 1`] = `
"status": 401,
}
`;
exports[`#comments.resolve should require authentication 1`] = `
{
"error": "authentication_required",
"message": "Authentication required",
"ok": false,
"status": 401,
}
`;
exports[`#comments.unresolve should require authentication 1`] = `
{
"error": "authentication_required",
"message": "Authentication required",
"ok": false,
"status": 401,
}
`;
exports[`#comments.update should require authentication 1`] = `
{
"error": "authentication_required",
"message": "Authentication required",
"ok": false,
"status": 401,
}
`;

View File

@@ -1,8 +1,10 @@
import { CommentStatusFilter } from "@shared/types";
import {
buildAdmin,
buildCollection,
buildComment,
buildDocument,
buildResolvedComment,
buildTeam,
buildUser,
} from "@server/test/factories";
@@ -10,6 +12,73 @@ import { getTestServer } from "@server/test/support";
const server = getTestServer();
describe("#comments.info", () => {
it("should require authentication", async () => {
const res = await server.post("/api/comments.info");
const body = await res.json();
expect(res.status).toEqual(401);
expect(body).toMatchSnapshot();
});
it("should return comment info", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const user2 = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const comment = await buildComment({
userId: user2.id,
documentId: document.id,
});
const res = await server.post("/api/comments.info", {
body: {
token: user.getJwtToken(),
id: comment.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.id).toEqual(comment.id);
expect(body.data.data).toEqual(comment.data);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
expect(body.policies[0].abilities.update).toEqual(false);
expect(body.policies[0].abilities.delete).toEqual(false);
});
it("should return comment info for admin", async () => {
const team = await buildTeam();
const user = await buildAdmin({ teamId: team.id });
const user2 = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const comment = await buildComment({
userId: user2.id,
documentId: document.id,
});
const res = await server.post("/api/comments.info", {
body: {
token: user.getJwtToken(),
id: comment.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.id).toEqual(comment.id);
expect(body.data.data).toEqual(comment.data);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
expect(body.policies[0].abilities.update).toEqual(true);
expect(body.policies[0].abilities.delete).toEqual(true);
});
});
describe("#comments.list", () => {
it("should require authentication", async () => {
const res = await server.post("/api/comments.list");
@@ -29,6 +98,10 @@ describe("#comments.list", () => {
userId: user.id,
documentId: document.id,
});
await buildResolvedComment(user, {
userId: user.id,
documentId: document.id,
});
const res = await server.post("/api/comments.list", {
body: {
token: user.getJwtToken(),
@@ -38,13 +111,14 @@ describe("#comments.list", () => {
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.length).toEqual(1);
expect(body.data[0].id).toEqual(comment.id);
expect(body.policies.length).toEqual(1);
expect(body.data.length).toEqual(2);
expect(body.data[1].id).toEqual(comment.id);
expect(body.policies.length).toEqual(2);
expect(body.policies[0].abilities.read).toEqual(true);
expect(body.policies[1].abilities.read).toEqual(true);
});
it("should return all comments for a collection", async () => {
it("should return unresolved comments for a collection", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const collection = await buildCollection({
@@ -75,7 +149,71 @@ describe("#comments.list", () => {
expect(body.policies[0].abilities.read).toEqual(true);
});
it("should return all comments", async () => {
it("should return unresolved comments for a parentCommentId", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const comment = await buildComment({
userId: user.id,
documentId: document.id,
});
const childComment = await buildComment({
userId: user.id,
documentId: document.id,
parentCommentId: comment.id,
});
const res = await server.post("/api/comments.list", {
body: {
token: user.getJwtToken(),
parentCommentId: comment.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.length).toEqual(1);
expect(body.data[0].id).toEqual(childComment.id);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
});
it("should return resolved comments for a statusFilter", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
await buildComment({
userId: user.id,
documentId: document.id,
});
const resolved = await buildResolvedComment(user, {
userId: user.id,
documentId: document.id,
});
const res = await server.post("/api/comments.list", {
body: {
token: user.getJwtToken(),
documentId: document.id,
statusFilter: [CommentStatusFilter.Resolved],
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.length).toEqual(1);
expect(body.data[0].id).toEqual(resolved.id);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
expect(body.policies[0].abilities.unresolve).toEqual(true);
expect(body.policies[0].abilities.resolve).toEqual(false);
});
it("should return all unresolved comments", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const collection1 = await buildCollection({
@@ -310,65 +448,37 @@ describe("#comments.create", () => {
});
});
describe("#comments.info", () => {
describe("#comments.update", () => {
it("should require authentication", async () => {
const res = await server.post("/api/comments.info");
const res = await server.post("/api/comments.update");
const body = await res.json();
expect(res.status).toEqual(401);
expect(body).toMatchSnapshot();
});
it("should return comment info", async () => {
it("should update an existing comment", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const user2 = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const comment = await buildComment({
userId: user2.id,
userId: user.id,
documentId: document.id,
});
const res = await server.post("/api/comments.info", {
const res = await server.post("/api/comments.update", {
body: {
token: user.getJwtToken(),
id: comment.id,
data: comment.data,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.id).toEqual(comment.id);
expect(body.data.data).toEqual(comment.data);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
expect(body.policies[0].abilities.update).toEqual(false);
expect(body.policies[0].abilities.delete).toEqual(false);
});
it("should return comment info for admin", async () => {
const team = await buildTeam();
const user = await buildAdmin({ teamId: team.id });
const user2 = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const comment = await buildComment({
userId: user2.id,
documentId: document.id,
});
const res = await server.post("/api/comments.info", {
body: {
token: user.getJwtToken(),
id: comment.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.id).toEqual(comment.id);
expect(body.data.data).toEqual(comment.data);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
@@ -376,3 +486,115 @@ describe("#comments.info", () => {
expect(body.policies[0].abilities.delete).toEqual(true);
});
});
describe("#comments.resolve", () => {
it("should require authentication", async () => {
const res = await server.post("/api/comments.resolve");
const body = await res.json();
expect(res.status).toEqual(401);
expect(body).toMatchSnapshot();
});
it("should allow resolving a comment thread", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const comment = await buildComment({
userId: user.id,
documentId: document.id,
});
const res = await server.post("/api/comments.resolve", {
body: {
token: user.getJwtToken(),
id: comment.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.resolvedAt).toBeTruthy();
expect(body.data.resolvedById).toEqual(user.id);
expect(body.data.resolvedBy.id).toEqual(user.id);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
expect(body.policies[0].abilities.update).toEqual(true);
expect(body.policies[0].abilities.delete).toEqual(true);
expect(body.policies[0].abilities.unresolve).toEqual(true);
expect(body.policies[0].abilities.resolve).toEqual(false);
});
it("should not allow resolving a child comment", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const parentComment = await buildComment({
userId: user.id,
documentId: document.id,
});
const comment = await buildComment({
userId: user.id,
documentId: document.id,
parentCommentId: parentComment.id,
});
const res = await server.post("/api/comments.resolve", {
body: {
token: user.getJwtToken(),
id: comment.id,
},
});
expect(res.status).toEqual(403);
});
});
describe("#comments.unresolve", () => {
it("should require authentication", async () => {
const res = await server.post("/api/comments.unresolve");
const body = await res.json();
expect(res.status).toEqual(401);
expect(body).toMatchSnapshot();
});
it("should allow unresolving a comment", async () => {
const team = await buildTeam();
const user = await buildUser({ teamId: team.id });
const document = await buildDocument({
userId: user.id,
teamId: user.teamId,
});
const comment = await buildResolvedComment(user, {
userId: user.id,
documentId: document.id,
});
const res = await server.post("/api/comments.unresolve", {
body: {
token: user.getJwtToken(),
id: comment.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.resolvedAt).toEqual(null);
expect(body.data.resolvedBy).toEqual(null);
expect(body.data.resolvedById).toEqual(null);
expect(body.policies.length).toEqual(1);
expect(body.policies[0].abilities.read).toEqual(true);
expect(body.policies[0].abilities.update).toEqual(true);
expect(body.policies[0].abilities.delete).toEqual(true);
expect(body.policies[0].abilities.resolve).toEqual(true);
expect(body.policies[0].abilities.unresolve).toEqual(false);
});
});

View File

@@ -1,16 +1,15 @@
import { Next } from "koa";
import Router from "koa-router";
import { FindOptions, Op } from "sequelize";
import { TeamPreference } from "@shared/types";
import { FindOptions, Op, WhereOptions } from "sequelize";
import { CommentStatusFilter, TeamPreference } from "@shared/types";
import commentCreator from "@server/commands/commentCreator";
import commentDestroyer from "@server/commands/commentDestroyer";
import commentUpdater from "@server/commands/commentUpdater";
import { ValidationError } from "@server/errors";
import auth from "@server/middlewares/authentication";
import { feature } from "@server/middlewares/feature";
import { rateLimiter } from "@server/middlewares/rateLimiter";
import { transaction } from "@server/middlewares/transaction";
import validate from "@server/middlewares/validate";
import { Document, Comment, Collection } from "@server/models";
import { Document, Comment, Collection, Event } from "@server/models";
import { authorize } from "@server/policies";
import { presentComment, presentPolicies } from "@server/presenters";
import { APIContext } from "@server/types";
@@ -24,7 +23,7 @@ router.post(
"comments.create",
rateLimiter(RateLimiterStrategy.TenPerMinute),
auth(),
checkCommentingEnabled(),
feature(TeamPreference.Commenting),
validate(T.CommentsCreateSchema),
transaction(),
async (ctx: APIContext<T.CommentsCreateReq>) => {
@@ -58,7 +57,7 @@ router.post(
router.post(
"comments.info",
auth(),
checkCommentingEnabled(),
feature(TeamPreference.Commenting),
validate(T.CommentsInfoSchema),
async (ctx: APIContext<T.CommentsInfoReq>) => {
const { id } = ctx.input.body;
@@ -67,14 +66,11 @@ router.post(
const comment = await Comment.findByPk(id, {
rejectOnEmpty: true,
});
const document = await Document.findByPk(comment.documentId, {
userId: user.id,
});
authorize(user, "read", comment);
if (comment.documentId) {
const document = await Document.findByPk(comment.documentId, {
userId: user.id,
});
authorize(user, "read", document);
}
authorize(user, "read", document);
ctx.body = {
data: presentComment(comment),
@@ -87,13 +83,45 @@ router.post(
"comments.list",
auth(),
pagination(),
checkCommentingEnabled(),
feature(TeamPreference.Commenting),
validate(T.CommentsListSchema),
async (ctx: APIContext<T.CommentsListReq>) => {
const { sort, direction, documentId, collectionId } = ctx.input.body;
const {
sort,
direction,
documentId,
parentCommentId,
statusFilter,
collectionId,
} = ctx.input.body;
const { user } = ctx.state.auth;
const statusQuery = [];
if (statusFilter?.includes(CommentStatusFilter.Resolved)) {
statusQuery.push({ resolvedById: { [Op.not]: null } });
}
if (statusFilter?.includes(CommentStatusFilter.Unresolved)) {
statusQuery.push({ resolvedById: null });
}
const where: WhereOptions<Comment> = {
[Op.and]: [],
};
if (documentId) {
// @ts-expect-error ignore
where[Op.and].push({ documentId });
}
if (parentCommentId) {
// @ts-expect-error ignore
where[Op.and].push({ parentCommentId });
}
if (statusQuery.length) {
// @ts-expect-error ignore
where[Op.and].push({ [Op.or]: statusQuery });
}
const params: FindOptions<Comment> = {
where,
order: [[sort, direction]],
offset: ctx.state.pagination.offset,
limit: ctx.state.pagination.limit,
@@ -103,12 +131,7 @@ router.post(
if (documentId) {
const document = await Document.findByPk(documentId, { userId: user.id });
authorize(user, "read", document);
comments = await Comment.findAll({
where: {
documentId: document.id,
},
...params,
});
comments = await Comment.findAll(params);
} else if (collectionId) {
const collection = await Collection.findByPk(collectionId);
authorize(user, "read", collection);
@@ -153,7 +176,7 @@ router.post(
router.post(
"comments.update",
auth(),
checkCommentingEnabled(),
feature(TeamPreference.Commenting),
validate(T.CommentsUpdateSchema),
transaction(),
async (ctx: APIContext<T.CommentsUpdateReq>) => {
@@ -194,7 +217,7 @@ router.post(
router.post(
"comments.delete",
auth(),
checkCommentingEnabled(),
feature(TeamPreference.Commenting),
validate(T.CommentsDeleteSchema),
transaction(),
async (ctx: APIContext<T.CommentsDeleteReq>) => {
@@ -226,19 +249,98 @@ router.post(
}
);
function checkCommentingEnabled() {
return async function checkCommentingEnabledMiddleware(
ctx: APIContext,
next: Next
) {
if (!ctx.state.auth.user.team.getPreference(TeamPreference.Commenting)) {
throw ValidationError("Commenting is currently disabled");
}
return next();
};
}
router.post(
"comments.resolve",
auth(),
feature(TeamPreference.Commenting),
validate(T.CommentsResolveSchema),
transaction(),
async (ctx: APIContext<T.CommentsResolveReq>) => {
const { id } = ctx.input.body;
const { user } = ctx.state.auth;
const { transaction } = ctx.state;
// router.post("comments.resolve", auth(), async (ctx) => {
// router.post("comments.unresolve", auth(), async (ctx) => {
const comment = await Comment.findByPk(id, {
transaction,
rejectOnEmpty: true,
lock: {
level: transaction.LOCK.UPDATE,
of: Comment,
},
});
const document = await Document.findByPk(comment.documentId, {
userId: user.id,
});
authorize(user, "resolve", comment);
authorize(user, "update", document);
comment.resolve(user);
const changes = comment.changeset;
await comment.save({ transaction });
await Event.createFromContext(
ctx,
{
name: "comments.update",
modelId: comment.id,
documentId: comment.documentId,
changes,
},
{ transaction }
);
ctx.body = {
data: presentComment(comment),
policies: presentPolicies(user, [comment]),
};
}
);
router.post(
"comments.unresolve",
auth(),
feature(TeamPreference.Commenting),
validate(T.CommentsUnresolveSchema),
transaction(),
async (ctx: APIContext<T.CommentsUnresolveReq>) => {
const { id } = ctx.input.body;
const { user } = ctx.state.auth;
const { transaction } = ctx.state;
const comment = await Comment.findByPk(id, {
transaction,
rejectOnEmpty: true,
lock: {
level: transaction.LOCK.UPDATE,
of: Comment,
},
});
const document = await Document.findByPk(comment.documentId, {
userId: user.id,
});
authorize(user, "unresolve", comment);
authorize(user, "update", document);
comment.unresolve();
const changes = comment.changeset;
await comment.save({ transaction });
await Event.createFromContext(
ctx,
{
name: "comments.update",
modelId: comment.id,
documentId: comment.documentId,
changes,
},
{ transaction }
);
ctx.body = {
data: presentComment(comment),
policies: presentPolicies(user, [comment]),
};
}
);
export default router;

View File

@@ -1,4 +1,5 @@
import { z } from "zod";
import { CommentStatusFilter } from "@shared/types";
import { BaseSchema, ProsemirrorSchema } from "@server/routes/api/schema";
const BaseIdSchema = z.object({
@@ -57,7 +58,12 @@ export const CommentsListSchema = BaseSchema.extend({
body: CommentsSortParamsSchema.extend({
/** Id of a document to list comments for */
documentId: z.string().optional(),
collectionId: z.string().uuid().optional(),
/** Id of a collection to list comments for */
collectionId: z.string().optional(),
/** Id of a parent comment to list comments for */
parentCommentId: z.string().uuid().optional(),
/** Comment statuses to include in results */
statusFilter: z.nativeEnum(CommentStatusFilter).array().optional(),
}),
});
@@ -68,3 +74,15 @@ export const CommentsInfoSchema = z.object({
});
export type CommentsInfoReq = z.infer<typeof CommentsInfoSchema>;
export const CommentsResolveSchema = z.object({
body: BaseIdSchema,
});
export type CommentsResolveReq = z.infer<typeof CommentsResolveSchema>;
export const CommentsUnresolveSchema = z.object({
body: BaseIdSchema,
});
export type CommentsUnresolveReq = z.infer<typeof CommentsUnresolveSchema>;

View File

@@ -403,8 +403,12 @@ export async function buildDocument(
export async function buildComment(overrides: {
userId: string;
documentId: string;
parentCommentId?: string;
resolvedById?: string;
}) {
const comment = await Comment.create({
resolvedById: overrides.resolvedById,
parentCommentId: overrides.parentCommentId,
documentId: overrides.documentId,
data: {
type: "doc",
@@ -427,6 +431,16 @@ export async function buildComment(overrides: {
return comment;
}
export async function buildResolvedComment(
user: User,
overrides: Parameters<typeof buildComment>[0]
) {
const comment = await buildComment(overrides);
comment.resolve(user);
await comment.save();
return comment;
}
export async function buildFileOperation(
overrides: Partial<FileOperation> = {}
) {