Issue 86 - Submission and comment violation reporting #158

tstiegler · 2018-04-03T18:07:07Z

This PR allows a logged in user to report both submissions or comments. As per the issue, no report "reason" is required.

To test this issue:

Log in as a normal user
Click the "Report" link which is displayed underneath each submission.
Click "Yes"/"Ok" to the prompt.
Log out of the account and log in with a mod/admin
Navigate to the mod forum for the forum that the submission was in
You should see a "Submission Report" that links to the submission that was reported.
Log in as a normal user
Navigate to a submission with comments (or create one)
Click the "Report" link which is displayed underneath each comment.
Click "Yes"/"Ok" to the prompt.
Log out of the account and log in with a mod/admin
Navigate to the mod forum for the forum that the submission was in
You should see a "Comment Report" that links to the comment that was reported.

…inig-issue-86

nationwide13

This has security concerns.

The mod threads are hidden from standard users, but if the thread is created by a non-mod they will still probably get reply notifications/other information.

We could create a migration that creates a user with ID=0 and name something like nautbot, and use this user to submit these reports.

Or we can filter the notifications and other things

tstiegler · 2018-04-03T18:19:59Z

I didn't think about that. We will want a "nautbot" user for other reasons too wouldn't we? I'll add a migration to create that user and use that for the reports.

nationwide13 · 2018-04-03T18:32:50Z

Yes, it'll definitely be used for other things as well.

You drew the short straw, and created the first feature that needs it haha

…eports.

tstiegler · 2018-04-03T19:32:35Z

Added a db migration for adding nautbot user, made the comment and submission report method use nautbot for the mod thread.

nationwide13

It looks good.

Did you test login as nautbot?
username: nautbot
password blank

Does it fail?

tstiegler · 2018-04-03T20:57:34Z

Doesnt look like you can log in as nautbot

psineur

🏆

This is freakin' awesome.
Few minor things:

Improve error translation
refactor controller methods into one

Feel free to add to this PR and then Squash + Merge
OR
Squash + Merge this now and follow up with another PR to address the refactor/translation.

Also - let's create an issue for "Users without Password" and mention any Symfony articles / links to codelines directly that make it explicit that empty password is OK measure to avoid login.

psineur · 2018-04-03T22:10:23Z

config/app_routes/submission.yaml

+    controller: App\Controller\SubmissionController::report
+    path: /f/{forum_name}/{submission_id}/report
+    methods: [POST]
+    requirements: { id: \d+ }


there's no 'id' param here though, only submission_id and forum_name

psineur · 2018-04-03T22:11:23Z

src/Controller/CommentController.php

+        $this->validateCsrf('report_comment', $request->request->get('token'));
+
+        // Find nautbot.
+        $nautbot = $em->find("App\\Entity\\User", 0);


this needs to be backed up by SQL migration creating a nautbot user with id = 0

psineur · 2018-04-03T22:13:30Z

src/Controller/CommentController.php

+
+            $this->addFlash('success', 'flash.comment_reported');
+        } else {
+            $this->addFlash('notice', 'flash.generic_error');


Let's have explicit error - "flash.comment_report_fail" and something along the lines -
"Reporting system is unavailable, please contact Moderators directly"

psineur · 2018-04-03T22:14:49Z

src/Controller/SubmissionController.php

+            'submission_id' => $submission->getId(),
+            'slug' => Slugger::slugify($submission->getTitle()),
+        ]);
+    }


This is almost a complete copy-paste of comment report, let's extract this method somewhere so controllers can be calling it with few different params and do the same thing.
Could live in utils

I'm working on a new Utils class for NautBot functions
src/Utils/NautBot.php
It'd be sweet if you used the same and build something like

newNautBotNotice($forum, $title, $body): Submission { // build nautbot submission and return }

That returned a new submission. This would get used a lot and be a huge help!

Not seeing that class. Is it in another branch?

It's not pushed up yet, still working on this feature. Sorry I should've been more clear.

psineur · 2018-04-03T22:15:02Z

src/Migrations/Version20180403185827.php

+    public function up(Schema $schema)
+    {
+        // this up() migration is auto-generated, please modify it to your needs
+        $this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.');


psineur · 2018-04-03T22:16:12Z

translations/messages.en.yml

@@ -128,6 +129,9 @@ flash:
    page_unlocked: The page was unlocked.
    no_cookies_verification: Does the image not load correctly? Please make sure you aren't blocking cookies and try again.
    no_password_resets: Password resets have been disabled by the administrator.
+    thread_reported: The submission has been reported.
+    comment_reported: The comment has been reported.
+    generic_error: An error had occurred, please try again later.


If nautbot goes down - we want users to:

know reporting is fucked for the moment

tell mods/admins about it - so we can fix it

tell mods to do something about comment/submission manually

…to messages, route fix.

tstiegler · 2018-04-04T13:46:18Z

Added some code to our UserChecker to force nautbot to not be able to be logged in, even if it had a password. Made the requested changes.

nationwide13 · 2018-04-04T18:54:19Z

We might have missed a use case with this.

Each report will be a new thread, so there's no easy way for mods to see how many times a submission/comment has been reported, which is decently important I think.

We may want to talk to the former mods about how important this is, but I think this will be needed, because previously, after 5 reports a thread was removed until a mod looked at it.

I think it'd also be useful for mods to see the number of reports when they view a thread, but again, we'd have to ask the mods for sure.

This use case may not be urgent, and this could potentially be pushed to v2/3, but I think it'd be good functionality to have down the line.

The below is just a suggestion, kind of spitballing how this could be accomplished

For submissions this probably wouldn't be too hard to do based on what you've built so far.
I'm not sure the report threads need the title in their title, that information can be contained in the report itself

"Submission Report: " . $submission->getId(),
"Comment Report: " . $comment->getId(),

On submissions and comment entities you can create a report count, and increment it each time a new report is filed.

"Submission Report: " . $submission->getId() . ' Report Count: ' . $submission->getReports(),
"Comment Report: " . $comment->getId()  . ' Report Count: ' . $comment->getReports(),

Then you can get the old thread:

if ($submission->getReports() > 0) {
  $reportSubmission = $sr->getSubmissionByTitle("Submission Report: " . $submission->getId() . ' Report Count: ' . $submission->getReports()");
  // this function could also update the submissions "Created At" timestamp so that it gets bumped to the top of the "new" list
  $submission->incrementReports();
  $reportSubmission->setTitle("Submission Report: " . $submission->getId() . ' Report Count: ' . $submission->getReports());
} else {
  $reportSubmission = ReportHelper::createReportSubmission(...);
}
$em->persist($reportSubmission);

nautbot · 2018-04-04T23:32:01Z

Seeing the number of accrued reports on a single item is helpful - several of us are looking at the modqueue throughout the day, and seeing something with multiple reports is usually a sign that we need to jump it immediately.

The count would be nice for us to have and would be needed by nautbot to implement auto-moderation rules, but not immediately critical for the mods so long as there is a basic flagging/reporting mechanism.

nautbot · 2018-04-05T02:21:04Z

Created a more detailed reporting/moderation queue user story in issue #160

…inig-issue-86 # Conflicts: # config/app_routes/comment.yaml # src/Entity/Comment.php

psineur

This is awesome!
@nautbot just created #160 - take a look if you want to see what's next on this front. It's possible that you just implemented all of it ;)

psineur · 2018-04-05T06:32:13Z

src/Migrations/Version20180405030642.php

+        // this down() migration is auto-generated, please modify it to your needs
+        $this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.');
+
+        $this->addSql('CREATE SCHEMA public');


oh wow, doctrine:migrations:diff created that?
delete that line - we should use existing schema all the time - it's part of DBA, not code/dev

Yeah I'll get rid of that. migrations:diff also adds a ton of "alter tables" that don't actually do anything. You can see them in some of the other more recent migrations.

Version20180401054730 specifically

I've been manually removing those lines from the migrations.

psineur · 2018-04-05T06:37:53Z

@tstiegler approved, please squash when ready and notify Jenkins/dev to run migrations.
This is huge
💥

psineur · 2018-04-05T06:40:52Z

Also: https://github.com/TheRealGD/therealgd/wiki/Core-Product-Feature-List

tstiegler · 2018-04-05T11:36:35Z

Theres actually still one thing I wanted to do for this issue and that was to make it so that multiple reports don't get jumbled into multiple threads. Part of @nationwide13 suggestion was some code that did that.

…inig-issue-86 # Conflicts: # src/Controller/SubmissionController.php

tstiegler · 2018-04-07T04:38:44Z

Need a new review on this, I'm going to hold off my change to de-dupe reporting threads since issue #160 will nullify most of this stuff.

psineur

🔥 it 🔽

tstiegler added 8 commits April 1, 2018 17:30

Issue 86 - Thread report link

e01fa3a

Merge branch 'develop' into feature-post-and-comment-violation-report…

9debc3c

…inig-issue-86

Issue 86 - Thread report link

36fafce

Merge branch 'develop' into feature-post-and-comment-violation-report…

23c079c

…inig-issue-86

Issue 84 - Thread reporting

11bb650

Merge branch 'develop' into feature-post-and-comment-violation-report…

51e8e6f

…inig-issue-86

Issue 86 - Comment reporting

1c345e3

Merge branch 'develop' into feature-post-and-comment-violation-report…

d0f97d1

…inig-issue-86

tstiegler requested a review from nationwide13 April 3, 2018 18:07

nationwide13 reviewed Apr 3, 2018

View reviewed changes

Issue 86 - Add nautbot user, use nautbot for submission and comment r…

ea8c08b

…eports.

nationwide13 previously approved these changes Apr 3, 2018

View reviewed changes

Issue 86 - Change error alert type.

26d07a7

tstiegler dismissed nationwide13’s stale review via 26d07a7 April 3, 2018 19:36

psineur previously approved these changes Apr 3, 2018

View reviewed changes

Issue 86 - Force disable nautbot user, refactor report code, changes …

510d9d0

…to messages, route fix.

tstiegler dismissed psineur’s stale review via 510d9d0 April 4, 2018 13:28

tstiegler added 2 commits April 4, 2018 23:21

Issue 86 - Report counting

c327014

Merge branch 'develop' into feature-post-and-comment-violation-report…

97f85f4

…inig-issue-86 # Conflicts: # config/app_routes/comment.yaml # src/Entity/Comment.php

psineur assigned psineur and tstiegler Apr 5, 2018

psineur previously approved these changes Apr 5, 2018

View reviewed changes

psineur added the AWAITING CLOSURE label Apr 5, 2018

psineur added this to the v.1 - MVP - Public Release milestone Apr 5, 2018

psineur removed their assignment Apr 5, 2018

Issue 86 - Remove create schema from migration.

55e20a8

tstiegler dismissed psineur’s stale review via 55e20a8 April 7, 2018 03:21

tstiegler added 2 commits April 6, 2018 23:28

Merge branch 'develop' into feature-post-and-comment-violation-report…

9443830

…inig-issue-86 # Conflicts: # src/Controller/SubmissionController.php

Issue 86 - Fix submission controller, missing use statement.

a7cabf5

psineur approved these changes Apr 8, 2018

View reviewed changes

tstiegler merged commit b30f8e2 into develop Apr 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 86 - Submission and comment violation reporting #158

Issue 86 - Submission and comment violation reporting #158

tstiegler commented Apr 3, 2018

nationwide13 left a comment

tstiegler commented Apr 3, 2018 •

edited

Loading

nationwide13 commented Apr 3, 2018

tstiegler commented Apr 3, 2018

nationwide13 left a comment

tstiegler commented Apr 3, 2018

psineur left a comment

psineur Apr 3, 2018

psineur Apr 3, 2018

psineur Apr 3, 2018

psineur Apr 3, 2018

nationwide13 Apr 4, 2018

tstiegler Apr 4, 2018

nationwide13 Apr 4, 2018

psineur Apr 3, 2018

psineur Apr 3, 2018

tstiegler commented Apr 4, 2018

nationwide13 commented Apr 4, 2018

nautbot commented Apr 4, 2018

nautbot commented Apr 5, 2018

psineur left a comment

psineur Apr 5, 2018

tstiegler Apr 5, 2018

psineur commented Apr 5, 2018

psineur commented Apr 5, 2018

tstiegler commented Apr 5, 2018

tstiegler commented Apr 7, 2018

psineur left a comment •

edited

Loading

Issue 86 - Submission and comment violation reporting #158

Issue 86 - Submission and comment violation reporting #158

Conversation

tstiegler commented Apr 3, 2018

nationwide13 left a comment

Choose a reason for hiding this comment

tstiegler commented Apr 3, 2018 • edited Loading

nationwide13 commented Apr 3, 2018

tstiegler commented Apr 3, 2018

nationwide13 left a comment

Choose a reason for hiding this comment

tstiegler commented Apr 3, 2018

psineur left a 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

tstiegler commented Apr 4, 2018

nationwide13 commented Apr 4, 2018

The below is just a suggestion, kind of spitballing how this could be accomplished

nautbot commented Apr 4, 2018

nautbot commented Apr 5, 2018

psineur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psineur commented Apr 5, 2018

psineur commented Apr 5, 2018

tstiegler commented Apr 5, 2018

tstiegler commented Apr 7, 2018

psineur left a comment • edited Loading

Choose a reason for hiding this comment

tstiegler commented Apr 3, 2018 •

edited

Loading

psineur left a comment •

edited

Loading