Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors delete comment #9232

Merged
merged 9 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions libs/model/src/comment/DeleteComment.command.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { InvalidActor, type Command } from '@hicommonwealth/core';
import * as schemas from '@hicommonwealth/schemas';
import { models } from '../database';
import { isAuthorized, type AuthContext } from '../middleware';
import { mustBeAuthorized, mustExist } from '../middleware/guards';

export function DeleteComment(): Command<
typeof schemas.DeleteComment,
AuthContext
> {
return {
...schemas.DeleteComment,
auth: [isAuthorized({})],
timolegros marked this conversation as resolved.
Show resolved Hide resolved
body: async ({ actor, payload, auth }) => {
const { address } = mustBeAuthorized(actor, auth);
const { comment_id, message_id } = payload;

const comment = await models.Comment.findOne({
where: message_id
? { discord_meta: { message_id } }
: { id: comment_id },
include: [
{
model: models.Thread,
attributes: ['community_id'],
required: true,
},
],
});
mustExist('Comment', comment);

if (comment.address_id !== address!.id && address.role === 'member')
timolegros marked this conversation as resolved.
Show resolved Hide resolved
throw new InvalidActor(actor, 'Not authorized author');

// == mutation transaction boundary ==
await models.sequelize.transaction(async (transaction) => {
await models.CommentSubscription.destroy({
where: { comment_id: comment.id },
transaction,
});
await comment.destroy({ transaction });
});
// == end of transaction boundary ==

return {
comment_id: comment.id!,
canvas_signed_data: comment.canvas_signed_data,
canvas_msg_id: comment.canvas_msg_id,
};
},
};
}
1 change: 1 addition & 0 deletions libs/model/src/comment/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './CreateComment.command';
export * from './CreateCommentReaction.command';
export * from './DeleteComment.command';
export * from './GetComments.query';
export * from './SearchComments.query';
export * from './UpdateComment.command';
7 changes: 6 additions & 1 deletion libs/model/src/middleware/authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,19 @@ export const isSuperAdmin: AuthHandler = async (ctx) => {
*
* @param roles specific community roles - all by default
* @param action specific group permission action
* @param author when true, rejects members that are not the author
* @param collaborators authorize thread collaborators
* @throws InvalidActor when not authorized
*/
export function isAuthorized({
roles = ['admin', 'moderator', 'member'],
action,
author = false,
collaborators = false,
}: {
roles?: Role[];
action?: GroupPermissionAction;
author?: boolean;
collaborators?: boolean;
}): AuthHandler {
return async (ctx) => {
Expand Down Expand Up @@ -292,7 +295,9 @@ export function isAuthorized({
throw new InvalidActor(ctx.actor, 'Not authorized collaborator');
}

if (author) throw new InvalidActor(ctx.actor, 'Not authorized member');

// at this point, the address is either a moderator or member
// without any action or collaboration requirements
// without any security requirements for action, author, or collaboration
};
}
55 changes: 55 additions & 0 deletions libs/model/test/thread/thread-lifecycle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const getNamespaceBalanceStub = sinon.stub(

import {
Actor,
InvalidActor,
InvalidInput,
InvalidState,
command,
Expand All @@ -28,6 +29,7 @@ import {
CreateComment,
CreateCommentErrors,
CreateCommentReaction,
DeleteComment,
MAX_COMMENT_DEPTH,
UpdateComment,
} from '../../src/comment';
Expand Down Expand Up @@ -590,6 +592,59 @@ describe('Thread lifecycle', () => {
}),
).rejects.toThrowError(InvalidInput);
});

it('should delete a comment as author', async () => {
const text = 'to be deleted';
const tbd = await command(CreateComment(), {
actor: actors.member,
payload: {
thread_id: thread.id!,
text,
},
});
expect(tbd).to.include({
thread_id: thread!.id,
text,
community_id: thread!.community_id,
});
const deleted = await command(DeleteComment(), {
actor: actors.member,
payload: { comment_id: tbd!.id! },
});
expect(deleted).to.include({ comment_id: tbd!.id! });
});

it('should delete a comment as admin', async () => {
const text = 'to be deleted';
const tbd = await command(CreateComment(), {
actor: actors.member,
payload: {
thread_id: thread.id!,
text,
},
});
expect(tbd).to.include({
thread_id: thread!.id,
text,
community_id: thread!.community_id,
});
const deleted = await command(DeleteComment(), {
actor: actors.admin,
payload: { comment_id: tbd!.id! },
});
expect(deleted).to.include({ comment_id: tbd!.id! });
});

it('should throw delete when user is not author', async () => {
await expect(
command(DeleteComment(), {
actor: actors.rejected,
payload: {
comment_id: comment!.id,
},
}),
).rejects.toThrowError(InvalidActor);
});
timolegros marked this conversation as resolved.
Show resolved Hide resolved
});

describe('thread reaction', () => {
Expand Down
15 changes: 15 additions & 0 deletions libs/schemas/src/commands/comment.schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,18 @@ export const CreateCommentReaction = {
input: CommentCanvasReaction,
output: Reaction.extend({ community_id: z.string() }),
};

export const DeleteComment = {
input: z.object({
comment_id: PG_INT,

// discord integration
thread_id: PG_INT.optional(),
message_id: z.string().optional(),
}),
output: z.object({
comment_id: PG_INT,
canvas_signed_data: z.string().nullish(),
canvas_msg_id: z.string().nullish(),
}),
};
Original file line number Diff line number Diff line change
@@ -1,61 +1,12 @@
import { toCanvasSignedDataApiArgs } from '@hicommonwealth/shared';
import { useMutation, useQueryClient } from '@tanstack/react-query';
import axios from 'axios';
import { signDeleteComment } from 'controllers/server/sessions';
import { useQueryClient } from '@tanstack/react-query';
import { trpc } from 'client/scripts/utils/trpcClient';
import Comment from 'models/Comment';
import { IUniqueId } from 'models/interfaces';
import { ApiEndpoints, SERVER_URL } from 'state/api/config';
import { ApiEndpoints } from 'state/api/config';
import { useAuthModalStore } from '../../ui/modals';
import { userStore } from '../../ui/user';
import { updateThreadInAllCaches } from '../threads/helpers/cache';
import useFetchCommentsQuery from './fetchComments';

interface DeleteCommentProps {
address: string;
communityId: string;
commentMsgId: string;
commentId: number;
existingNumberOfComments: number;
}

const deleteComment = async ({
address,
communityId,
commentId,
commentMsgId,
}: DeleteCommentProps) => {
const canvasSignedData = await signDeleteComment(
userStore.getState().activeAccount?.address || '',
{
comment_id: commentMsgId,
},
);

await axios.delete(`${SERVER_URL}/comments/${commentId}`, {
data: {
author_community_id: communityId,
address: address,
community_id: communityId,
jwt: userStore.getState().jwt,
...toCanvasSignedDataApiArgs(canvasSignedData),
},
});

// Important: we render comments in a tree, if the deleted comment is a
// leaf node, remove it, but if it has replies, then preserve it with
// [deleted] msg.
return {
softDeleted: {
id: commentId,
deleted: true,
text: '[deleted]',
plaintext: '[deleted]',
versionHistory: [],
...toCanvasSignedDataApiArgs(canvasSignedData),
},
};
};

interface UseDeleteCommentMutationProps {
communityId: string;
threadId: number;
Expand All @@ -75,18 +26,30 @@ const useDeleteCommentMutation = ({

const { checkForSessionKeyRevalidationErrors } = useAuthModalStore();

return useMutation({
mutationFn: deleteComment,
return trpc.comment.deleteComment.useMutation({
onSuccess: async (response) => {
// Important: we render comments in a tree, if the deleted comment is a
// leaf node, remove it, but if it has replies, then preserve it with
// [deleted] msg.
const softDeleted = {
id: response.comment_id,
deleted: true,
text: '[deleted]',
plaintext: '[deleted]',
versionHistory: [],
canvas_signed_data: response.canvas_signed_data,
canvas_msg_id: response.canvas_msg_id,
};

// find the existing comment index
const foundCommentIndex = comments.findIndex(
(x) => x.id === response.softDeleted.id,
(x) => x.id === softDeleted.id,
);

if (foundCommentIndex > -1) {
const softDeletedComment = Object.assign(
{ ...comments[foundCommentIndex] },
{ ...response.softDeleted },
{ ...softDeleted },
);

// update fetch comments query state
Expand All @@ -105,13 +68,11 @@ const useDeleteCommentMutation = ({
communityId,
threadId,
{
recentComments: [
{ id: response.softDeleted.id },
] as Comment<IUniqueId>[],
recentComments: [{ id: softDeleted.id }] as Comment<IUniqueId>[],
},
'removeFromExisting',
);
return response;
return softDeleted;
},
onError: (error) => checkForSessionKeyRevalidationErrors(error),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,19 +195,13 @@ export const CommentTree = ({
buttonHeight: 'sm',
onClick: async () => {
try {
await deleteComment({
communityId,
commentId: comment.id,
commentMsgId: comment.canvasMsgId,
address: user.activeAccount?.address || '',
existingNumberOfComments: thread.numberOfComments,
});
await deleteComment({ comment_id: comment.id });
} catch (err) {
if (err instanceof SessionKeyError) {
checkForSessionKeyRevalidationErrors(err);
return;
}
console.error(err.response.data.error || err?.message);
console.error(err.message);
notifyError('Failed to delete comment');
}
},
Expand Down
1 change: 1 addition & 0 deletions packages/commonwealth/server/api/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ export const trpcRouter = trpc.router({
),
searchComments: trpc.query(Comment.SearchComments, trpc.Tag.Comment),
getComments: trpc.query(Comment.GetComments, trpc.Tag.Comment),
deleteComment: trpc.command(Comment.DeleteComment, trpc.Tag.Comment),
});
10 changes: 8 additions & 2 deletions packages/commonwealth/server/api/external-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ const {
getMembers,
} = community.trpcRouter;
const { createThread, updateThread, createThreadReaction } = thread.trpcRouter;
const { createComment, createCommentReaction, updateComment, getComments } =
comment.trpcRouter;
const {
createComment,
createCommentReaction,
updateComment,
getComments,
deleteComment,
} = comment.trpcRouter;

const api = {
createCommunity,
Expand All @@ -31,6 +36,7 @@ const api = {
createComment,
updateComment,
createCommentReaction,
deleteComment,
};

const PATH = '/api/v1';
Expand Down
7 changes: 1 addition & 6 deletions packages/commonwealth/server/api/integration-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { RequestHandler, Router, raw } from 'express';

// TODO: remove as we migrate to tRPC commands
import DatabaseValidationService from 'server/middleware/databaseValidationService';
import { deleteBotCommentHandler } from 'server/routes/comments/delete_comment_bot_handler';
import { deleteBotThreadHandler } from 'server/routes/threads/delete_thread_bot_handler';
import { ServerControllers } from 'server/routing/router';

Expand All @@ -18,9 +17,6 @@ function build(
const isBotUser: RequestHandler = (req, res, next) => {
validator.validateBotUser(req, res, next).catch(next);
};
const isAuthor: RequestHandler = (req, res, next) => {
validator.validateAuthor(req, res, next).catch(next);
};

const router = Router();
router.use(express.statsMiddleware);
Expand Down Expand Up @@ -75,8 +71,7 @@ function build(
router.delete(
'/bot/comments/:message_id',
isBotUser,
isAuthor,
deleteBotCommentHandler.bind(this, controllers),
express.command(Comment.DeleteComment()),
);

return router;
Expand Down
Loading
Loading