Implement reports for private messages #2433

Nutomic · 2022-09-07T12:36:15Z

No description provided.

Nutomic · 2022-09-08T09:48:37Z

Should be finished, though i havent done any testing of the api code.

I really dislike how much copy-pasting of code this involves. I think at least all the ListReports API calls should be merged into one, because they wont be used separately (similar to how mod log entries of all types are listed with a single api call).

Edit: Merged the list report endpoints into a single one now, and moved report count to site. I would also consider merging create/resolve report endpoints into one (with a type param) to simplify the api, but that probably wouldnt simplify the code.

Nutomic · 2022-09-08T20:31:49Z

Merging list report endpoints broke api tests.

dessalines · 2022-09-09T21:50:28Z

crates/api/src/site/reports/list.rs

+///
+/// If called by an admin, private message reports are also included.
+#[async_trait::async_trait(?Send)]
+impl Perform for ListReports {


I'm not totally sure I agree with combining all of them, as that would take away this functionality. Its kinda nice having individual ones in addition to all. Esp in the future where admins have to prioritize and need to care more about post reports than comments.

Okay, reverted the last commit. Still i think the api should look somewhat different.

dessalines · 2022-09-09T21:55:07Z

src/api_routes.rs

@@ -18,7 +26,9 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimit) {
          .route("", web::get().to(route_get_crud::<GetSite>))
          // Admin Actions
          .route("", web::post().to(route_post_crud::<CreateSite>))
-          .route("", web::put().to(route_post_crud::<EditSite>)),
+          .route("", web::put().to(route_post_crud::<EditSite>))
+          .route("/report_count", web::get().to(route_get::<GetReportCount>))


Ah so report_count is just getting moved from user -> admin scope.

also reverted

dessalines · 2022-09-09T21:56:35Z

src/api_routes.rs

@@ -96,7 +106,6 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimit) {
            "/report/resolve",
            web::put().to(route_post::<ResolvePostReport>),
          )
-          .route("/report/list", web::get().to(route_get::<ListPostReports>))


See comment above, but yeah I think its probably best to preserve these as individual endpoints, so that the front end can still filter on the types.

dessalines · 2022-09-09T21:58:04Z

migrations/2022-09-07-114618_pm-reports/up.sql

+  unique(private_message_id, creator_id) -- users should only be able to report a pm once
+);
+
+create or replace view private_message_report_view as


No SQL views anymore, this must be done in diesel. There are already examples for how this works for post and comment report views.

edit: ah I see you maybe just copied this as a template from the other examples, but its not necessary.

True, i copied this from other report migrations, but didnt notice that its not used anymore. Removed.

migrations/2022-09-07-114618_pm-reports/down.sql

dessalines · 2022-09-09T22:05:08Z

crates/api/src/private_message_report/create.rs

+    };
+
+    // TODO: should send message about new report to admins via websocket, but there is only op
+    //       for community mods


You can just copy paste from the others, 0 is the admin room for "SendModRoomMessage". I don't think I fully tested that on the front end, but might as well have it here to be consistent.

Ah, that wasnt documented anywhere. Btw, as far as i can tell there arent any checks in the code to ensure that JoinModRoom can only be called by moderators for that community (or admin in case of 0). Am i missing something? Otherwise this would be a security issue.

dessalines · 2022-09-09T22:08:13Z

crates/api/src/private_message_report/resolve.rs

+      private_message_report_view,
+    };
+
+    // TODO: should send message about new report to admins via websocket, but there is only op


Same here, copy paste from the createpostreport example.

dessalines · 2022-09-09T22:10:57Z

crates/db_views/src/private_message_report_view.rs

+    assert_eq!(
+      inserted_admin.name,
+      reports[0].resolver.as_ref().unwrap().name
+    );


Perfect, this looks good.

dessalines · 2022-09-09T22:11:55Z

crates/db_views/src/structs.rs

+  pub private_message_report: PrivateMessageReport,
+  pub private_message: PrivateMessage,
+  pub private_message_creator: PersonSafe,
+  pub creator: PersonSafeAlias1,


Once I have some time to work on diesel 2.0, we won't need these aliases anymore :)

crates/db_schema/src/schema.rs

dessalines

Looks good, a few suggestions.

crates/api/src/comment_report/create.rs

migrations/2022-09-07-114618_pm-reports/down.sql

crates/websocket/src/messages.rs

crates/websocket/src/chat_server.rs

crates/api_common/src/person.rs

Nutomic · 2022-09-19T21:06:02Z

Adressed all the comments.

This reverts commit 3bf3b06.

Nutomic force-pushed the pm-reports branch 2 times, most recently from 79df8c2 to 5f6f33e Compare September 8, 2022 09:47

Nutomic marked this pull request as ready for review September 8, 2022 09:47

Nutomic requested a review from dessalines as a code owner September 8, 2022 09:47

Nutomic force-pushed the pm-reports branch 2 times, most recently from 5a6dd39 to 3bf3b06 Compare September 8, 2022 10:17

dessalines reviewed Sep 9, 2022

View reviewed changes

dessalines reviewed Sep 19, 2022

View reviewed changes

dessalines mentioned this pull request Sep 19, 2022

Add private message reporting LemmyNet/lemmy-ui#782

Closed

Nutomic added 10 commits September 19, 2022 18:35

Implement reports for private messages

0516fef

finish private message report view + test

a886587

implement api for pm reports

c39acd6

merge list report api calls into one, move report count to site

40947aa

fix compile error

0a13e93

Revert "merge list report api calls into one, move report count to site"

35c8e05

This reverts commit 3bf3b06.

add websocket messages for pm report created/resolved

54c6ad5

remove private_message_report_view

dfe30f2

add joinable private_message_report -> person_alias_1

33189ae

Address review comments

b6c14d6

dessalines force-pushed the pm-reports branch from 13b5913 to b6c14d6 Compare September 19, 2022 22:35

dessalines enabled auto-merge (squash) September 19, 2022 22:35

dessalines approved these changes Sep 19, 2022

View reviewed changes

dessalines merged commit 004efd5 into main Sep 19, 2022

Nutomic mentioned this pull request Dec 2, 2022

Reports for Private Messages #1543

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement reports for private messages #2433

Implement reports for private messages #2433

Nutomic commented Sep 7, 2022

Nutomic commented Sep 8, 2022 •

edited

Nutomic commented Sep 8, 2022

dessalines Sep 9, 2022

Nutomic Sep 12, 2022

dessalines Sep 9, 2022

Nutomic Sep 12, 2022

dessalines Sep 9, 2022

dessalines Sep 9, 2022

Nutomic Sep 12, 2022

dessalines Sep 9, 2022

Nutomic Sep 12, 2022

dessalines Sep 9, 2022

dessalines Sep 9, 2022

dessalines Sep 9, 2022

dessalines left a comment

Nutomic commented Sep 19, 2022

Implement reports for private messages #2433

Implement reports for private messages #2433

Conversation

Nutomic commented Sep 7, 2022

Nutomic commented Sep 8, 2022 • edited

Nutomic commented Sep 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dessalines left a comment

Choose a reason for hiding this comment

Nutomic commented Sep 19, 2022

Nutomic commented Sep 8, 2022 •

edited