mysociety / alaveteli

Provide a Freedom of Information request system for your jurisdiction
https://alaveteli.org
Other
385 stars 195 forks source link

Hide all requests from a user and ban them from the admin interface #3580

Open garethrees opened 7 years ago

garethrees commented 7 years ago

Requires https://github.com/mysociety/alaveteli/issues/3578

RichardTaylor commented 6 years ago

Just adding a +1 here. On WhatDoTheyKnow we've had a banned user creating new accounts to evade a ban and they've been making 2-4 requests plus annotations on the accounts. Hiding the requests and annotations at the moment is taking quite a number of clicks.

garethrees commented 4 years ago

Here's a bit of code to do this through the console:

def hide_all_requests(user_id, admin_url_name)
  InfoRequest.where(user_id: user_id).find_each do |info_request|
    old_prominence = info_request.prominence

    next if old_prominence == 'hidden'

    info_request.update!(prominence: 'hidden')

    params =
      { old_prominence: old_prominence,
        prominence: info_request.prominence,
        editor: admin_url_name }

    info_request.log_event('edit', params)
  end
end

# Example:
#
# hide_all_requests(181416, 'gareth_rees_3')

Can obviously tweak the prominence to whatever final state is most relevant.

RichardTaylor commented 3 years ago

The desired admin action might be backpage rather than hide so both options should be available to apply en-masse to a user's requests.

itsaphel commented 2 years ago

Would it be safe to automatically do this for any "Ban for spamming" ban, rather than make it a separate function?

Something like this: https://github.com/itsaphel/alaveteli/commit/539a3c3efc358ebb60b09812731f904ba0cfcf84

garethrees commented 2 years ago

It's very unusual for us to use the "Ban for spamming" button for human accounts; it's more for automated signups from bots.

I don't think I'd want to connect something like this to an account suspension action because it reduces our flexibility in decision-making. For example, we might decide to hide all a user's existing requests and send a warning not to continue as they were before. We wouldn't necessarily also ban them (despite what the issue title says). It does add a little extra work for admins, (update ban text; then click hide all requests) but for that we gain optionality, which is good.

I think the ideal UI for this would be similar to the comments (https://github.com/mysociety/alaveteli/pull/1398), where we can select a number of requests, and then have a variety of buttons (hide all, backpage all, etc) that submits to, say, Admin::InfoRequestsBulkProminenceController#update with the IDs of the requests and a param for the prominence value. We'd also want to add a javascript-based "Select All" link (a la https://github.com/mysociety/alaveteli/issues/3324).

Screenshot 2021-07-28 at 09 30 08

That's quite a bit of work, so I think we could simplify by implementing the controller as described, and then just adding a variety of buttons ("Backpage All", "Requester Only All", "Hide all") that set up the params along the lines of:

<%= form_for … %>
  <%= hidden_field_tag :request_ids, @admin_user.info_requests.pluck(:id) %>
  <%= hidden_field_tag :prominence, 'hidden' %>
  <%= submit_tag 'Hide all requests' %>
<% end %>

<%= form_for … %>
  <%= hidden_field_tag :request_ids, @admin_user.info_requests.pluck(:id) %>
  <%= hidden_field_tag :prominence, 'backpage' %>
  <%= submit_tag 'Backpage all requests' %>
<% end %>

<!-- etc -->

I think we'd want a model-layer method of InfoRequest#set_prominence_with_event and pass the prominence value (:hidden, :backpage, :requester_only), prominence_reason ("Bulk prominence update") and the current admin user.

itsaphel commented 2 years ago

Thanks @garethrees! I think I understand the use case better now too. Will give that implementation you mention a shot at some point.

garethrees commented 2 years ago

No problem, and thank you for looking in to this! 🙇 Feel free to post a spike if you want any early feedback.

garethrees commented 2 years ago

Can now also hide comments programatically from the console or app https://github.com/mysociety/alaveteli/issues/6812.

garethrees commented 2 years ago

Bit of an update on https://github.com/mysociety/alaveteli/issues/3580#issuecomment-557102002 (minor tweaks to hide_all_requests and incorporating comments:

def hide_all_requests(user, editor)
  InfoRequest.where(user_id: user.id).find_each do |info_request|
    old_prominence = info_request.prominence

    next if %w[hidden requester_only].include?(old_prominence)

    info_request.update!(prominence: 'hidden')

    params =
      { old_prominence: old_prominence,
        prominence: info_request.prominence,
        editor: editor.url_name }

    info_request.log_event('hide', params)
  end
end

def hide_all_comments(user, editor)
  user.comments.find_each do |comment|
    next if comment.hidden?
    comment.hide(editor: editor)
    comment.update(attention_requested: false)
  end
end

editor = User.find(USER_ID)

bad_users =
  %w[foo@example.com
     bar@example.com]

bad_users.map { |email| User.find_user_by_email(email) }.each do |user|
  hide_all_requests(user, editor)
  hide_all_comments(user, editor)
end
itsaphel commented 2 years ago

@garethrees if I understand right, there's currently no pagination for comments/annotations? If pagination was added in the future, then I imagine the JS 'select all' approach might not work?

garethrees commented 2 years ago

if I understand right, there's currently no pagination for comments/annotations?

That's correct, though it's been suggested as a solution to https://github.com/mysociety/alaveteli/issues/6980.

If pagination was added in the future, then I imagine the JS 'select all' approach might not work?

In this situation I'd imagine the process being:

If we implemented the select boxes for requests, we'd probably then phrase the buttons "Hide selected requests" rather than "Hide all requests" etc.

garethrees commented 1 year ago

Linking to https://github.com/mysociety/alaveteli/issues/843, since more use of this would lead to more support requests for copies of the requests.