Feedback on the new Review Queue

Yes, when a user is deleted, We remove them from any events they’ve rsvp’d for. Maybe that code is causing this.

3 Likes

Because all the topic IDs in the error are topics that use Events.

Hmm, I’m fairly certain that these specific users that I’m deleting have not RSVP’ed though., but it could be that these are the only topics that have RSVP enabled?

3 Likes

Oh oh got it.

https://github.com/paviliondev/discourse-events/blob/85cc5a5c01e67135d8b06d5a386aa56192fb294c/plugin.rb#L301

@fzngagan this is using unsafe .find(?

topics = Topic.find(topic_ids) if topic_ids

See above fix I just pushed to Follow (though solution here may need to be different as multiple ids - use where?)

3 Likes

I just updated but still get the 404.

1 Like

Bart, hold off for @fzngagan to apply and confirm a fix.

5 Likes

I’ve pushed a fix. It should hopefully resolve the issue. Please check and confirm.

5 Likes

That fixed it, thanks!!

6 Likes

I have seen now multiple times posts in the review queue with the reason “New user typed their first post suspiciously fast”.
Upon further checking I noticed that those posts contained a watched word, but this was not mentioned in the review queue.

Since the “New user typed their first post suspiciously fast” flagging is 96% of the time wrong, and the “watched words” flaggings are 100% of the time correct, i.e. if a post gets into the review queue due to a watched word, it is 100% of the time a post that really needs attention, I feel that the “watched words” should have precedence over the “new user typed too fast”.

Imagine the following situations:

  1. Post gets into review queue due to “New user typed their first post suspiciously fast”. This post contains an invisible spam link which is in the “watched words” list. -> Post gets approved, since nobody notices the invisible link (e.g. link behind a “.”). -> Fail!
  2. Post gets into review queue due to a watched word (“watched words” having precedence over “typed too fast”). This post contains an invisible spam link which is in the “watched words” list. -> Post gets rejected and spammer gets deleted due to “watched words” -> Win!
5 Likes

Absolutely, this is a borderline bug, could we add this to someone’s list @eviltrout? I see @Roman_Rizzi is still assigned, perhaps?

4 Likes

Fixed here:

8 Likes

On 2.6.0.beta2. Just a heads up, I have queued topics that seem to be deleted. So I think how this happens is something along the lines of:

  • User’s post gets put into the review queue due to fast typing.
  • They delete their topic (if that is possible), perhaps to resubmit it.

Not sure if this is as intended or not. In the review queue the title is blank, but the post body is there and the user is silenced. Can’t see any topics/posts in the user’s public profile.


Unrelated to the above. I have a suggestion for the filtering options. A feature that would be pretty great imo, would be a more granular filter for post/topic types. Right now for Types we have:

discourse-types

Flagged Post and Queued Post include both Topics and Posts. I think it would be pretty useful if this was changed to just:

Review Type:

  • Flagged
  • Queued

Could consider splitting flagged into User Flagged and System Flagged also perhaps.

Then also have another filter for the content type.

Content Type:

  • Topic
  • Post
  • User

I think this would be pretty useful. It could be quite fast to prioritise Topic approval over post/reply approval for example. There isn’t really a way to differentiate between Topics and Posts with the current filters besides Grouped by Topic.


Another suggestion would be to tweak the review queue UI so that Topics and Posts are a bit easier to differentiate. Right now this information is showed as some small grey text (i.e. Queued Post, Queued Topic, Flagged Post, Flagged Topic). The text size is the same size as the post/topic’s categories and tags.

For me, it’s not very immediately obvious if the item is a topic or post/reply and I’m regularly confusing the two.

Some ideas:

  • Tweak the Topic Title section of the post items to have a reply icon and be smaller than for topic items and perhaps include the text RE:
  • Increase the text size/emphasis on the item type (Topic/Post).
2 Likes

When attempting to approve topics and posts, I’m getting a 500 error. Currently using 2.6.0.beta3. Here is the log:

ActiveRecord::RangeError (PG::NumericValueOutOfRange: ERROR:  integer out of range
)
app/models/reviewable_queued_post.rb:97:in `perform_approve_post'
app/models/reviewable.rb:355:in `public_send'
app/models/reviewable.rb:355:in `block in perform'
app/models/reviewable.rb:353:in `perform'
app/controllers/reviewables_controller.rb:192:in `perform'
app/controllers/application_controller.rb:347:in `block in with_resolved_locale'
app/controllers/application_controller.rb:347:in `with_resolved_locale'
lib/middleware/omniauth_bypass_middleware.rb:68:in `call'
lib/content_security_policy/middleware.rb:12:in `call'
lib/middleware/anonymous_cache.rb:336:in `call'
config/initializers/100-quiet_logger.rb:23:in `call'
config/initializers/100-silence_logger.rb:31:in `call'
lib/middleware/enforce_hostname.rb:22:in `call'
lib/middleware/request_tracker.rb:176:in `call'

Some potentially relevant information is that I did have Akismet installed in the past and removed it (did the rake task also) as detailed here: Feedback on the new Review Queue

The items are from within the last 60 days (auto_handle_queued_age: 60). I’ve tried both with old posts (~2mo old) and recent (within the last 3 hours).

I can approve users (Type: User) and the ‘Delete User’ option on queued topics/posts works.

2 Likes

@Roman_Rizzi this one is super weird - how are we getting a huge integer here?

6 Likes

The error is thrown when attempting to create a notification:

https://github.com/discourse/discourse/blob/master/app/models/reviewable_queued_post.rb#L97-100

I suspect we’re probably storing

Maybe we are storing some IDs using the int type? Rails started using BIGINT(8) for primary keys in 5.1.

https://github.com/rails/rails/pull/26266

@markersorcial Could you please share with us the results of these queries?

Notification.count
User.count
Topic.count
4 Likes

Thanks for looking into this :slight_smile:

Sure, here are the query results:

Notification.count: 1506604
User.count: 238083
Topic.count: 936067
3 Likes

Update: Looking in Sidekiq, I see a lot of failed tasks with this error which seems similar:

Jobs::HandledExceptionWrapper: Wrapped ActiveRecord::RangeError: PG::NumericValueOutOfRange: ERROR: integer out of range

For these Task types (that I’ve noticed so far):
Jobs::PostAlert (most common)
Jobs::ProcessPost
Jobs::NotifyCategoryChange

1 Like

None of those numbers should be breaking the maximum size. I wonder if something else is overwriting an integer value @Roman_Rizzi.

3 Likes

It has to be something else then. There’s definitely something going on and it’s not specific to the review queue.

These jobs create notifications and they’re also failing:

Also, if I try doing something like this:

Notification.create!(
      notification_type: Notification.types[:post_approved],
      user_id: 1,
      data: {},
      topic_id: 1,
      post_number: 1311344111111
)

I get a different error:

ActiveModel::RangeError: 1311344111111 is out of range for ActiveModel::Type::Integer with limit 4 bytes

Got the same error doing this:

DB.exec <<~SQL
      INSERT INTO notifications(user_id, topic_id, post_number)
      VALUES (1, 1, 1311344111111)
    SQL

PG::NumericValueOutOfRange: ERROR: integer out of range

6 Likes

@markersocial I wonder if the PostgreSQL logs could help us have more information about the error. Could you please check?

If you don’t know where to find the logs see:

5 Likes

Our moderators appreciate the ability to easily discuss Flagged Posts among themselves and have the history maintained in the Review Queue. Would it be possible to add the same ability to Queued Posts? We use the approve_post_count setting to require approval for a new users’ first 5 posts. Those first 5 posts become Queued Posts, but if it requires moderator discussion, they have to start a PM which is detached from the Review Queue and the history is lost.

3 Likes