Proposal: Consolidated Review Queue

(Robin Ward) #18

I haven’t updated in a couple weeks, but I’ve made some good progress on migrating flags over to the review queue. I have been distracted this week with some family emergencies but those should be sorted out soon.

The refactor here is quite major and involves many changes. I am unsure how other team members will be able to review the PR eventually because it will be giant, but we’ll do our best!

I have reached the point in the refactor where I want to implement scoring properly. This means removing a bunch of settings we had for flags (min_flags_staff_visibility, for example) and replace them with score based equivalents. I wanted to jot down my idea for flag scores here and see what people thought before I go too far implementing:

  • a ReviewableFlaggedPost has a score

  • The score is the sum of the ReviewableScore records for that flagged post. Each ReviewableScore record represents a flag a user has applied to the post. The ReviewableScore score is calculated as user_flag_score + flag_type_score_bonus + take_action_bonus.

  • flag_type_score_bonus would be configurable by flag type. For example you could set spam be higher than inappropriate if you desired.

  • take_action_bonus: a value (0.0 or 5.0) depending on whether a staff member “took action”

  • user_flag_score is calculated per user, and is: 1.0 + trust_level + accuracy_bonus

  • trust_level is the user’s trust level (0.0 - 5.0)

  • accuracy_bonus is the percentage of the user’s previous flags that were agreed with * 5, for a value of (0.0 - 5.0). A minimum of 5 flags is required.

So for example, if a post was flagged by two users. One (u0) is TL1 who has never flagged before, and the other (u1) is TL3 whose flags are agreed with 50% of the time. Both are flags whose flag_type_score_bonus are set to 1.5:

# 1.0 + trust_level + (5.0 * agree_percentage)
u0_flag_score = 1.0 + 1.0 + (5.0 * 0.0) = 2.0
u1_flag_score = 1.0 + 3.0 + (5.0 * 0.5) = 6.5

# user_flag_score + default_for_flag_type + take_action_bonus
reviewable_score_u0 = 2.0 + 1.5 + 0.0 = 3.5
reviewable_score_u1 = 6.5 + 1.5 + 0.0 = 8.0

# sum of reviewable scores
flagged_post_score = 3.5 + 8.0 = 11.5 

Reviewable items are sorted by reverse score, so this particular post with a score of 11.5 would show up before a post with a value of 9.3 and so on.

12 Likes
(Rafael dos Santos Silva) #19

One way is to put all action buttons on top, so the click targets are always stable and you render the post below and it can be giant without impacting UX.

That allows people to quickly scan and review/approve a bunch without moving the mouse.

7 Likes
(Kane York) #20

Probably the best way to do this is to implement the rest of the data model migrations, then run your proposed score algorithm against the entire flag history of a few sites, looking for outliers. People can probably survive for a while with a “newest unresolved first” strategy.

1 Like
(Jeff Atwood) #21

This is a bit of a “boil the sea” solution… I think limiting max-height has to be done anyway otherwise the results are crazy.

Reminder: this would need to be the last hundred flags by that user not all time.

4 Likes
(Robin Ward) #22

I forgot to update but this is the solution I went with. I limit the max height of the post and a scrollbar shows up. I think it’s a pretty good solution.

I implemented this feature this morning. agree/disagree/ignore stats are now based on the last 100 flags so the user has a chance to improve.

I wanted to do it in master and the reviewable branch but it was basically 2x the work so it’s only in the reviewable branch right now.

9 Likes
(Robin Ward) #23

Personal stuff continued to get in the way of implementing this feature but I’ve finally got some progress to report.

I’ve hit a milestone of full use of the review queue for flags, with full backwards compatibility for the old flagging REST API. There is still some interface stuff left to do and some site settings left to replace, but I’ll probably be ready for review some time next week.

There are a huge amount of changes here so I do expect some regressions, but the good news is the next Discourse release has quite a long time to go, so there will be time to iron them out.

I ran some benchmarks on the new /review.json endpoint vs the old /admin/flags/active.json and the results are promising:

  • Old code: 51ms (Views: 0.1ms | ActiveRecord: 26.9ms)
  • New code: 21ms (Views: 0.1ms | ActiveRecord: 4.4ms)

So it’s about 2.4x faster overall, and spends about 15% of the time in the DB as it used to!

22 Likes
(Taylor) #24

I think this makes sense here, but if not, I can move it to a separate #feature topic:

There have been several occurrences of a topic or post being accidentally rejected when it was in Queued Posts. Currently, there is nothing in the UI to undo this action. Would it be feasible to include a rejection log with a restore option, or some way to allow admins and mods to restore a post that was rejected by mistake?

11 Likes
(Robin Ward) #25

Yes, absolutely. I haven’t made the “Old Reviewables” tab yet but the idea is you could go there and perform any valid actions on them.

9 Likes
(Robin Ward) #26

It’s been a while since I posted a screenshot. This is a fully working review queue, showing flagged posts.

As you can see, you can now filter by reviewable type (queued posts, users, flags, all) and by minimum score.

The UX still needs a designer pass, but it’s awesome that the interface is working smoothly now.

17 Likes
(Sam Saffron) #27

I have a small structure question.

Can the review queue client UX live in a dedicated JS payload? It would be nice if we only shipped it to users with access to the review queue (ideally when they first navigate to it)

9 Likes
(Robin Ward) #28

One challenge is right now there’s no easy way to know if someone has access to the review queue, because the new data structures support category level moderation. The current user payload comes back with a reviewable_count for all users (usually 0).

It’s not impossible to solve this, but I have a lot of other things on my plate right now to get this feature done. Maybe we can revisit later.

6 Likes
(Robin Ward) #29

I implemented this today. Instead of a tab, you can filter by “Pending, Approved, Rejected, Ignored”. It’s simple and works great. You can also filter by category, in case a moderator only wants to see content from a particular category.

12 Likes
(Joffrey Jaffeux) #30

On this subject, Ember has started some very interesting work on code splitting:

7 Likes
(Robin Ward) #31

Okay, as promised here’s the (large) PR:

I imagine it will take some time to review but it would be good if a couple people at least could look it over.

22 Likes
(Joffrey Jaffeux) #32

I spent a fair amount of time reading this PR today. And damn this is impressive @eviltrout :heart:

You even updated the bot, the import scripts, the user merger, rake tasks, the reports…

And I really like the design so far :heart_eyes_cat:

23 Likes
(Robin Ward) #33

Yes it was a lot of work, but thank you! Honestly our extensive test suite really helped me track down usages of internal APIs and test backwards compatibility. Two months is a long time to spend on a feature like this, but it would have been so much longer if we hadn’t done our jobs properly in the past.

21 Likes
(Robin Ward) #34

The plan is to merge this on Wednesday morning (eastern time), after we cut and deploy a new beta. At that point I’ll probably create a new topic for feedback / bug reports.

15 Likes
(Robin Ward) #35

Update: We didn’t merge this yet. There was a Rails security vulnerability and some other risky changes in core we wanted to avoid. It’s deployed here on Meta and we’re going to dogfood the branch a bit more before we merge it.

10 Likes
(Robin Ward) #36

Okay we’ve merged this in. It’s no longer a proposal, so there is a new topic to discuss the feature.

10 Likes
(Robin Ward) closed #37