Skip to content

Commit 991ffea

Browse files
committed
WIP
1 parent 0f0998f commit 991ffea

6 files changed

Lines changed: 186 additions & 46 deletions

File tree

app/backend/src/couchers/migrations/versions/0151_add_edit_delete_to_discussions_and_comments.py renamed to app/backend/src/couchers/migrations/versions/0155_add_edit_delete_to_discussions_and_comments.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Add edit/delete support to discussions, comments, and replies
22
3-
Revision ID: 0151
4-
Revises: 0150
3+
Revision ID: 0155
4+
Revises: 0154
55
Create Date: 2026-05-13 00:00:00.000000
66
77
"""
@@ -10,8 +10,8 @@
1010
from alembic import op
1111

1212
# revision identifiers, used by Alembic.
13-
revision = "0151"
14-
down_revision = "0150"
13+
revision = "0155"
14+
down_revision = "0154"
1515
branch_labels = None
1616
depends_on = None
1717

app/backend/src/couchers/servicers/admin.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,8 @@ def DeleteDiscussion(
916916
).scalar_one_or_none()
917917
if not discussion:
918918
context.abort_with_error_code(grpc.StatusCode.NOT_FOUND, "discussion_not_found")
919+
if discussion.deleted is not None:
920+
return empty_pb2.Empty()
919921
discussion.deleted = now()
920922
return empty_pb2.Empty()
921923

app/backend/src/couchers/servicers/communities.py

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import grpc
66
from google.protobuf import empty_pb2
7-
from sqlalchemy import exists, select
7+
from sqlalchemy import select
88
from sqlalchemy.orm import Session, selectinload
99
from sqlalchemy.sql import delete, func, or_
1010

@@ -33,7 +33,7 @@
3333
from couchers.servicers.events import event_to_pb
3434
from couchers.servicers.groups import group_to_pb
3535
from couchers.servicers.pages import page_to_pb
36-
from couchers.sql import to_bool, users_visible, where_moderated_content_visible
36+
from couchers.sql import to_bool, users_visible, where_moderated_content_visible, where_users_column_visible
3737
from couchers.utils import Timestamp_from_datetime, dt_from_millis, millis_from_dt, now
3838

3939
logger = logging.getLogger(__name__)
@@ -466,22 +466,46 @@ def ListDiscussions(
466466
self, request: communities_pb2.ListDiscussionsReq, context: CouchersContext, session: Session
467467
) -> communities_pb2.ListDiscussionsRes:
468468
page_size = min(MAX_PAGINATION_LENGTH, request.page_size or MAX_PAGINATION_LENGTH)
469-
next_page_id = int(request.page_token) if request.page_token else 0
469+
next_page_id = int(request.page_token) if request.page_token else 2**63 - 1
470470
node = session.execute(select(Node).where(Node.id == request.community_id)).scalar_one_or_none()
471471
if not node:
472472
context.abort_with_error_code(grpc.StatusCode.NOT_FOUND, "community_not_found")
473473
if not node.official_cluster.small_community_features_enabled:
474474
context.abort_with_error_code(grpc.StatusCode.FAILED_PRECONDITION, "discussions_not_enabled")
475-
has_comments = exists().where((Comment.thread_id == Discussion.thread_id) & (Comment.deleted == None))
476-
query = (
477-
node.official_cluster.owned_discussions.where((Discussion.deleted == None) | has_comments)
478-
.where(or_(Discussion.id <= next_page_id, to_bool(next_page_id == 0)))
479-
.order_by(Discussion.id.desc())
480-
.limit(page_size + 1)
475+
has_visible_comments = (
476+
where_moderated_content_visible(
477+
where_users_column_visible(
478+
select(func.count())
479+
.select_from(Comment)
480+
.where(Comment.thread_id == Discussion.thread_id)
481+
.where(Comment.deleted == None),
482+
context,
483+
Comment.author_user_id,
484+
),
485+
context,
486+
Comment,
487+
is_list_operation=True,
488+
)
489+
.correlate(Discussion)
490+
.scalar_subquery()
491+
)
492+
discussions = (
493+
session.execute(
494+
where_moderated_content_visible(
495+
select(Discussion)
496+
.where(Discussion.owner_cluster_id == node.official_cluster.id)
497+
.where((Discussion.deleted == None) | (has_visible_comments > 0))
498+
.where(Discussion.id <= next_page_id)
499+
.order_by(Discussion.id.desc())
500+
.limit(page_size + 1),
501+
context,
502+
Discussion,
503+
is_list_operation=True,
504+
)
505+
)
506+
.scalars()
481507
.all()
482508
)
483-
query = where_moderated_content_visible(query, context, Discussion, is_list_operation=True)
484-
discussions = session.execute(query.order_by(Discussion.id.desc()).limit(page_size + 1)).scalars().all()
485509
return communities_pb2.ListDiscussionsRes(
486510
discussions=[discussion_to_pb(session, discussion, context) for discussion in discussions[:page_size]],
487511
next_page_token=str(discussions[-1].id) if len(discussions) > page_size else None,

app/backend/src/couchers/servicers/discussions.py

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import logging
22

33
import grpc
4+
from google.protobuf import empty_pb2
45
from sqlalchemy import select
56
from sqlalchemy.orm import Session
67

@@ -18,7 +19,7 @@
1819
from couchers.servicers.blocking import is_not_visible
1920
from couchers.servicers.threads import thread_to_pb
2021
from couchers.sql import where_moderated_content_visible
21-
from couchers.utils import Timestamp_from_datetime
22+
from couchers.utils import Timestamp_from_datetime, now
2223

2324
logger = logging.getLogger(__name__)
2425

@@ -33,6 +34,17 @@ def discussion_to_pb(session: Session, discussion: Discussion, context: Couchers
3334
else:
3435
owner_group_id = discussion.owner_cluster.id
3536

37+
if discussion.deleted is not None:
38+
return discussions_pb2.Discussion(
39+
discussion_id=discussion.id,
40+
slug=discussion.slug,
41+
deleted=True,
42+
owner_community_id=owner_community_id,
43+
owner_group_id=owner_group_id,
44+
owner_title=discussion.owner_cluster.name,
45+
thread=thread_to_pb(session, context, discussion.thread_id),
46+
)
47+
3648
can_moderate = can_moderate_node(session, context.user_id, discussion.owner_cluster.parent_node_id)
3749

3850
return discussions_pb2.Discussion(
@@ -47,6 +59,8 @@ def discussion_to_pb(session: Session, discussion: Discussion, context: Couchers
4759
content=discussion.content,
4860
thread=thread_to_pb(session, context, discussion.thread_id),
4961
can_moderate=can_moderate,
62+
can_edit=(context.user_id == discussion.creator_user_id),
63+
last_edited=Timestamp_from_datetime(discussion.last_edited) if discussion.last_edited else None,
5064
)
5165

5266

@@ -169,6 +183,78 @@ def GetDiscussion(
169183

170184
return discussion_to_pb(session, discussion, context)
171185

186+
def UpdateDiscussion(
187+
self, request: discussions_pb2.UpdateDiscussionReq, context: CouchersContext, session: Session
188+
) -> discussions_pb2.Discussion:
189+
discussion = session.execute(
190+
select(Discussion).where(Discussion.id == request.discussion_id)
191+
).scalar_one_or_none()
192+
if not discussion:
193+
context.abort_with_error_code(grpc.StatusCode.NOT_FOUND, "discussion_not_found")
194+
if discussion.deleted is not None:
195+
context.abort_with_error_code(grpc.StatusCode.FAILED_PRECONDITION, "discussion_deleted")
196+
if context.user_id != discussion.creator_user_id:
197+
context.abort_with_error_code(grpc.StatusCode.PERMISSION_DENIED, "discussion_edit_permission_denied")
198+
199+
updated = False
200+
201+
if request.HasField("title"):
202+
new_title = request.title.value.strip()
203+
if not new_title:
204+
context.abort_with_error_code(grpc.StatusCode.INVALID_ARGUMENT, "missing_discussion_title")
205+
discussion.title = new_title
206+
updated = True
207+
208+
if request.HasField("content"):
209+
new_content = request.content.value.strip()
210+
if not new_content:
211+
context.abort_with_error_code(grpc.StatusCode.INVALID_ARGUMENT, "missing_discussion_content")
212+
discussion.content = new_content
213+
updated = True
214+
215+
if not updated:
216+
return discussion_to_pb(session, discussion, context)
217+
218+
discussion.last_edited = now()
219+
220+
log_event(
221+
context,
222+
session,
223+
"discussion.updated",
224+
{
225+
"discussion_id": discussion.id,
226+
},
227+
)
228+
229+
return discussion_to_pb(session, discussion, context)
230+
231+
def DeleteDiscussion(
232+
self, request: discussions_pb2.DeleteDiscussionReq, context: CouchersContext, session: Session
233+
) -> empty_pb2.Empty:
234+
discussion = session.execute(
235+
select(Discussion).where(Discussion.id == request.discussion_id)
236+
).scalar_one_or_none()
237+
if not discussion:
238+
context.abort_with_error_code(grpc.StatusCode.NOT_FOUND, "discussion_not_found")
239+
if discussion.deleted is not None:
240+
context.abort_with_error_code(grpc.StatusCode.FAILED_PRECONDITION, "discussion_deleted")
241+
242+
if context.user_id != discussion.creator_user_id:
243+
context.abort_with_error_code(grpc.StatusCode.PERMISSION_DENIED, "discussion_delete_permission_denied")
244+
245+
discussion.deleted = now()
246+
247+
log_event(
248+
context,
249+
session,
250+
"discussion.deleted",
251+
{
252+
"discussion_id": discussion.id,
253+
},
254+
)
255+
256+
return empty_pb2.Empty()
257+
172258
def ListMyCommunitiesDiscussions(
173259
self, request: discussions_pb2.ListMyCommunitiesDiscussionsReq, context: CouchersContext, session: Session
174260
) -> discussions_pb2.ListMyCommunitiesDiscussionsRes:

app/backend/src/couchers/servicers/threads.py

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -289,21 +289,30 @@ def GetThread(
289289
is_list_operation=True,
290290
)
291291
).all()
292-
replies = [
293-
threads_pb2.Reply(
294-
thread_id=pack_thread_id(r.id, 1),
295-
deleted=r.deleted is not None,
296-
content=r.content if r.deleted is None else "",
297-
author_user_id=r.author_user_id if r.deleted is None else 0,
298-
created_time=Timestamp_from_datetime(r.created) if r.deleted is None else None,
299-
num_replies=n,
300-
can_edit=(context.user_id == r.author_user_id) if r.deleted is None else False,
301-
last_edited=Timestamp_from_datetime(r.last_edited)
302-
if (r.last_edited and r.deleted is None)
303-
else None,
304-
)
305-
for r, n in res[:page_size]
306-
]
292+
# Deleted comments are shown as stubs (thread_id, deleted, num_replies only)
293+
# to preserve thread structure, but content and author are stripped.
294+
replies = []
295+
for r, n in res[:page_size]:
296+
if r.deleted is not None:
297+
replies.append(
298+
threads_pb2.Reply(
299+
thread_id=pack_thread_id(r.id, 1),
300+
deleted=True,
301+
num_replies=n,
302+
)
303+
)
304+
else:
305+
replies.append(
306+
threads_pb2.Reply(
307+
thread_id=pack_thread_id(r.id, 1),
308+
content=r.content,
309+
author_user_id=r.author_user_id,
310+
created_time=Timestamp_from_datetime(r.created),
311+
num_replies=n,
312+
can_edit=(context.user_id == r.author_user_id),
313+
last_edited=Timestamp_from_datetime(r.last_edited) if r.last_edited else None,
314+
)
315+
)
307316

308317
elif depth == 1:
309318
if not session.execute(

app/backend/src/tests/test_discussions.py

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,13 @@ def test_update_discussion(db):
183183
assert res.title == "Updated title"
184184
assert res.content == "Updated content"
185185
assert res.can_edit
186+
assert res.last_edited is not None
187+
188+
with discussions_session(token) as api:
189+
res = api.GetDiscussion(discussions_pb2.GetDiscussionReq(discussion_id=discussion_id))
190+
assert res.title == "Updated title"
191+
assert res.content == "Updated content"
192+
assert res.last_edited is not None
186193

187194

188195
def test_update_discussion_permission_denied(db):
@@ -263,18 +270,20 @@ def test_delete_discussion_by_creator(db):
263270
assert res.thread.thread_id != 0
264271

265272

266-
def test_delete_discussion_by_moderator(db):
273+
def test_delete_discussion_permission_denied_for_non_creator(db):
267274
creator, creator_token = generate_user()
268-
moderator, moderator_token = generate_user()
275+
community_admin, community_admin_token = generate_user()
269276
random_user, random_token = generate_user()
270277
with session_scope() as session:
271-
community = create_community(session, 0, 1, "Testing Community", [moderator], [creator, random_user], None)
278+
community = create_community(
279+
session, 0, 1, "Testing Community", [community_admin], [creator, random_user], None
280+
)
272281
community_id = community.id
273282

274283
with discussions_session(creator_token) as api:
275284
res = api.CreateDiscussion(
276285
discussions_pb2.CreateDiscussionReq(
277-
title="To be moderated",
286+
title="Only creator can delete",
278287
content="Some content",
279288
owner_community_id=community_id,
280289
)
@@ -287,10 +296,10 @@ def test_delete_discussion_by_moderator(db):
287296
api.DeleteDiscussion(discussions_pb2.DeleteDiscussionReq(discussion_id=discussion_id))
288297
assert e.value.code() == grpc.StatusCode.PERMISSION_DENIED
289298

290-
with discussions_session(moderator_token) as api:
291-
api.DeleteDiscussion(discussions_pb2.DeleteDiscussionReq(discussion_id=discussion_id))
292-
res = api.GetDiscussion(discussions_pb2.GetDiscussionReq(discussion_id=discussion_id))
293-
assert res.deleted
299+
with discussions_session(community_admin_token) as api:
300+
with pytest.raises(grpc.RpcError) as e:
301+
api.DeleteDiscussion(discussions_pb2.DeleteDiscussionReq(discussion_id=discussion_id))
302+
assert e.value.code() == grpc.StatusCode.PERMISSION_DENIED
294303

295304

296305
def test_deleted_discussion_not_in_list(db):
@@ -326,7 +335,7 @@ def test_deleted_discussion_not_in_list(db):
326335
assert deleted_id not in ids
327336

328337

329-
def test_deleted_discussion_thread_still_accessible(db):
338+
def test_deleted_discussion_thread_still_accessible(db, moderator: Moderator):
330339
user, token = generate_user()
331340
commenter, commenter_token = generate_user()
332341
with session_scope() as session:
@@ -347,7 +356,8 @@ def test_deleted_discussion_thread_still_accessible(db):
347356
process_jobs()
348357

349358
with threads_session(commenter_token) as api:
350-
api.PostReply(threads_pb2.PostReplyReq(thread_id=thread_id, content="a comment"))
359+
comment_thread_id = api.PostReply(threads_pb2.PostReplyReq(thread_id=thread_id, content="a comment")).thread_id
360+
moderator.approve_thread_post(comment_thread_id)
351361

352362
with discussions_session(token) as api:
353363
api.DeleteDiscussion(discussions_pb2.DeleteDiscussionReq(discussion_id=discussion_id))
@@ -360,7 +370,7 @@ def test_deleted_discussion_thread_still_accessible(db):
360370
assert len(res.replies) == 1
361371

362372

363-
def test_delete_comment_shows_placeholder_with_replies(db):
373+
def test_delete_comment_shows_placeholder_with_replies(db, moderator: Moderator):
364374
"""Deleting a top-level comment that has nested replies should show a deleted
365375
placeholder so the replies remain visible."""
366376
user, token = generate_user()
@@ -383,13 +393,22 @@ def test_delete_comment_shows_placeholder_with_replies(db):
383393

384394
# commenter adds a top-level comment
385395
with threads_session(commenter_token) as api:
386-
comment_res = api.PostReply(threads_pb2.PostReplyReq(thread_id=thread_id, content="top-level comment"))
387-
comment_thread_id = comment_res.thread_id
396+
comment_thread_id = api.PostReply(
397+
threads_pb2.PostReplyReq(thread_id=thread_id, content="top-level comment")
398+
).thread_id
388399

389400
# replier adds two nested replies
390401
with threads_session(replier_token) as api:
391-
api.PostReply(threads_pb2.PostReplyReq(thread_id=comment_thread_id, content="nested reply 1"))
392-
api.PostReply(threads_pb2.PostReplyReq(thread_id=comment_thread_id, content="nested reply 2"))
402+
reply_thread_id_1 = api.PostReply(
403+
threads_pb2.PostReplyReq(thread_id=comment_thread_id, content="nested reply 1")
404+
).thread_id
405+
reply_thread_id_2 = api.PostReply(
406+
threads_pb2.PostReplyReq(thread_id=comment_thread_id, content="nested reply 2")
407+
).thread_id
408+
409+
moderator.approve_thread_post(comment_thread_id)
410+
moderator.approve_thread_post(reply_thread_id_1)
411+
moderator.approve_thread_post(reply_thread_id_2)
393412

394413
# commenter deletes their top-level comment
395414
with threads_session(commenter_token) as api:

0 commit comments

Comments
 (0)