Plots2: Remove emoji left by banned user accounts

[ebarry]

Please describe the problem (or idea)

I was reading a post on publiclab.org, and was inspired to leave an emoji. When i scrolled down to see if the emoji was there, i noticed a couple other people had left emoji, and one of them had a suspiciously spammy username: dariensmith000

When i looked up that username, indeed another moderator had already banned this account:

What did you expect to see that you didn't?

I did not expect to see emoji left by banned accounts on publiclab.org content

Please show us where to look

https://publiclab.org/questions/karunv/10-12-2020/ndvi-with-a-2-camera-setup#c27483
https://publiclab.org/p/dariensmith000 (if your account is not a moderator level, you may not be able to see this page.

What's your PublicLab.org username?

This can help us diagnose the issue:
liz

[cesswairimu]

@RuthNjeri would you like to give this a try?

[RuthNjeri]

Thanks @cesswairimu, I am currently working on this one https://github.com/publiclab/plots2/pull/8620
I'll leave this one for someone else to pick up, if I'll finish the one I am working on, I'll pick this up if it is not yet assigned.

[noi5e]

@cesswairimu @jywarren I'd like to give this a try if possible!

I could use a little bit of guidance since I'm still learning the ropes here. If you could point me toward the right group of files, I could look it over and do some research.

[cesswairimu]

Go ahead @noi5e, thanks

[noi5e]

@jywarren @cesswairimu

Hi! I've been researching this issue and would like some feedback on the best way to move forward please.

To start, it looks like users are banned in app/models/user.rb:

https://github.com/publiclab/plots2/blob/45fe39a6439e4f3683ccaa9f34ab1970a0f2199d/app/models/user.rb#L300-L306

The method decrease_likes_banned that's called above only decrements cached_likes, and nothing else. I'm assuming cached_likes is, well, a cached integer for the front-end to quickly read a comment's number of likes:

https://github.com/publiclab/plots2/blob/45fe39a6439e4f3683ccaa9f34ab1970a0f2199d/app/models/user.rb#L498-L503

So that probably explains why you can still see banned usernames in the list of likes.

Wondering if it would be okay to simply destroy all the banned user's likes when they are banned. Something like so:

# rough draft!
# 
# still getting used to Ruby syntax

likes = Like.where(uid: @user_id) # I'm not sure how to query all a particular user's likes, help?
likes.each do |like|
  like.destroy
end

The downside here is that it would be permanent. When the user is unbanned, I think their likes wouldn't be recoverable, which isn't ideal. The code below would have to be changed to account for this: ie. don't run increase_likes_banned anymore.

https://github.com/publiclab/plots2/blob/45fe39a6439e4f3683ccaa9f34ab1970a0f2199d/app/models/user.rb#L308-L313

Unless you all think it's feasible to cache a user's likes somewhere in the database so they can be recoverable in the event that they're unbanned? That would require changes in the database models. What do you all think?

[cesswairimu]

Great investigation @noi5e :1st_place_medal:
to get all a users likes you could use self.likes on the user_model

I agree it would be good to keep the users likes - there was a recent case where a number of contributors were unintentionally banned so it would be safe to not delete these. Maybe we could soft-delete using the paranoia gem then after unbanning we could call a restore for the users likes... I bet that would be simple and not a lot changes need to be done on the model, thoughts?

[jywarren]

Hi all - this is a complex one. Just offering some maybe helpful links into code segments that could be relevant:

• tests for this collection of names: https://github.com/publiclab/plots2/search?q=reacted%20with
• user reactions map: https://github.com/publiclab/plots2/blob/f04bd05f08cb8f60a25c22f3fcda5d8a141cf378/app/models/comment.rb#L220-L234

Note how that last bit of code relies on a likes method in the comment model... that's a relation between the comment and another database table --

https://github.com/publiclab/plots2/blob/f04bd05f08cb8f60a25c22f3fcda5d8a141cf378/app/models/comment.rb#L8

so, we can add a .where(status: 1) filter somewhere there. Maybe we need to join the User model to do that? Ah, it looks like we already include that, so the where filter should work!

https://github.com/publiclab/plots2/blob/f04bd05f08cb8f60a25c22f3fcda5d8a141cf378/app/models/comment.rb#L221

[noi5e]

Coming back to this issue just now... I know it may be only tangentially related to the Comment Editor :sweat_smile: (I picked this issue up before I was selected as an Outreachy Intern). Thanks for the thorough and helpful comments as always @jywarren!

It does seem like the fix is best done in the definition for user_reactions_map mentioned above: https://github.com/publiclab/plots2/blob/f04bd05f08cb8f60a25c22f3fcda5d8a141cf378/app/models/comment.rb#L220-L234

If I add conditionals to filter out users with :status: 0 I can get somewhere:

likes_map.each do |reaction, likes|
      users = []

      likes.each do |like|

        # NEW CONDITIONAL #1: only push into users if user isn't banned
        if like.user.status != 0
          users << like.user.name
        end
      end

      # NEW CONDITIONAL #2:
      # if the comment only has one like, and that user is banned, 
      # it will throw an error when users_string is assigned.

      # specifically, users[0] will be nil, and nil values can't be
      # part of a string like 'users[0] + string'

      if users.size > 0
        emoji_type = reaction.underscore.humanize.downcase
        users_string = (users.size > 1 ? users[0..-2].join(", ") + " and " + users[-1] : users[0]) + " reacted with " + emoji_type + " emoji" # if users.size = 0, an error is thrown here
        user_like_map[reaction] = users_string
      end
    end

However, this fix doesn't fully work. The banned username is no longer visible in the hover tooltip, but the emoji and the emoji count are still visible! (see below: normal behavior, and buggy behavior)

_I could explain why this happens, but maybe that's too much detail in a comment!_

[noi5e]

Is there a way to filter out users with status:0 from the beginning in this database query?
https://github.com/publiclab/plots2/blob/f04bd05f08cb8f60a25c22f3fcda5d8a141cf378/app/models/comment.rb#L221

I've tried this query, but it throws an error:
likes_map = likes.where.not(emoji_type: nil).joins(:user).where.not(status: 0).group_by(&:emoji_type)

Otherwise, I'll probably do this fix the long way around by filtering likes. Asking because I'm still learning about Rails DB queries. Thanks! @sagarpreet-chadha @cesswairimu

[noi5e]

Read through some docs, brushed up on my queries, and now I seem to be getting somewhere with this:

likes_map = likes.joins(:user).select(:emoji_type, :username).where("emoji_type IS NOT NULL AND status = 1").group_by(&:emoji_type)

I know that really it should be user.status != 0, but Rails throws an error on that for some reason.

The full query which needs to retrieve:

• :emoji_type and :user_id from all of a comment's likes
  
  
  
  • :emoji_type must not be nil
  
  
• join :user table, and look up user.status there:
  
  
  
  • status must not be 0 (banned)
  
  

Seems relatively simple, but I'm getting stuck translating that query into Rails without throwing errors. Any ideas?

EDIT: the above query gets me here:

The tooltip displays admin's name, but not banned_user's name. So I think the query is still including likes with banned users... but not their usernames.

[noi5e]

Making good progress here:
likes_map = likes.joins(:user).select(:emoji_type, :username, :status).where("emoji_type IS NOT NULL").where("status IS NOT 0").group_by(&:emoji_type)

I was getting confused earlier because of slight differences between .joins and .includes that I was previously unaware of. For example, you can refer to like.user on a query made with .includes. However with queries made with .joins, you refer to the column name instead, eg. like.username. I also learned that in either case, if you log the ActiveRecord query to the console with p, it WILL NOT display the user columns you've joined. It will ONLY display the like record. Very confusing!!!

So it turns out that I was pretty close all along. I wrote a simple test and it seems like this query works as intended.

There's another significant change that needs to be made, also in comment.rb. This method displays the number of emojis in the comment view. Another DB query could go here to check the user status of each emoji:

  def emoji_likes
    likes.group(:emoji_type).size
  end

Or maybe that's too many queries and will affect performance? Maybe the best way would be to rewrite the view and controller methods a bit, so that only one query is done and one bit of data returned. This data bit would include both the "username reacted" string and the number of emoji likes.