Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions app/assets/stylesheets/admin.scss
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,20 @@ body.admin {
.edit-history .row:nth-child(even) {
background-color: #f9f9f9;
}

#info_request_events .row {
margin-bottom: 1em;
}
#info_request_events .row .details {
display: flex;
margin-left: 7em;
}
#info_request_events .row:nth-child(even) {
background-color: #f9f9f9;
}
#info_request_events .row .details .event-params {
margin-left: 1em;
}
}

/* Debug */
Expand Down
64 changes: 52 additions & 12 deletions app/controllers/admin_info_request_event_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,63 @@
# Email: hello@mysociety.org; WWW: http://www.mysociety.org/

class AdminInfoRequestEventController < AdminController
before_action :set_info_request_event, only: [:update]
before_action :set_info_request_event, only: [:edit, :update]

def index
@page = params[:page] || 1
@query = params[:query] || ""
@info_request_events = InfoRequestEvent.
order(id: :desc).
paginate(page: @page, per_page: 100)

if @query != ""

Check warning on line 17 in app/controllers/admin_info_request_event_controller.rb

View workflow job for this annotation

GitHub Actions / build

[rubocop] reported by reviewdog 🐶 Use a guard clause (`return unless @query != ""`) instead of wrapping the code inside a conditional expression. (https://rubystyle.guide#no-nested-conditionals) Raw Output: app/controllers/admin_info_request_event_controller.rb:17:5: C: Style/GuardClause: Use a guard clause (`return unless @query != ""`) instead of wrapping the code inside a conditional expression. (https://rubystyle.guide#no-nested-conditionals)
@info_request_events =@info_request_events.

Check warning on line 18 in app/controllers/admin_info_request_event_controller.rb

View workflow job for this annotation

GitHub Actions / build

[rubocop] reported by reviewdog 🐶 Surrounding space missing for operator `=`. (https://rubystyle.guide#spaces-operators) Raw Output: app/controllers/admin_info_request_event_controller.rb:18:28: C: Layout/SpaceAroundOperators: Surrounding space missing for operator `=`. (https://rubystyle.guide#spaces-operators)
search_scope(@query,
backend: :postgresql,
admin_mode: true)
end
end

def edit
@params_to_show = JSON.pretty_generate(@info_request_event.params)
end

# used so due dates get fixed
def update
if @info_request_event.event_type != 'response'
raise "can only mark responses as requires clarification"
end
@was_clarification = params[:commit] == "Was clarification request"

if @was_clarification
if @info_request_event.event_type != 'response'
raise "can only mark responses as requires clarification"
end

@info_request_event.described_state = 'waiting_clarification'
@info_request_event.calculated_state = 'waiting_clarification'
# TODO: deliberately don't update described_at so doesn't reenter search?
@info_request_event.save!
# Reset the due dates for the request if necessary
@info_request_event.recheck_due_dates

@info_request_event.described_state = 'waiting_clarification'
@info_request_event.calculated_state = 'waiting_clarification'
# TODO: deliberately don't update described_at so doesn't reenter search?
@info_request_event.save!
# Reset the due dates for the request if necessary
@info_request_event.recheck_due_dates
flash[:notice] = "Old response marked as having been a request for clarification"

Check warning on line 45 in app/controllers/admin_info_request_event_controller.rb

View workflow job for this annotation

GitHub Actions / build

[rubocop] reported by reviewdog 🐶 Line is too long. [87/80] (https://rubystyle.guide#max-line-length) Raw Output: app/controllers/admin_info_request_event_controller.rb:45:81: C: Layout/LineLength: Line is too long. [87/80] (https://rubystyle.guide#max-line-length)
redirect_to admin_request_url(@info_request_event.info_request)
else
@params_to_show = params.dig(:info_request_event, :params_to_show)

flash[:notice] = "Old response marked as having been a request for clarification"
redirect_to admin_request_url(@info_request_event.info_request)
if @params_to_show.strip.empty?

Check warning on line 50 in app/controllers/admin_info_request_event_controller.rb

View workflow job for this annotation

GitHub Actions / build

[rubocop] reported by reviewdog 🐶 Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`. (https://rubystyle.guide#if-as-a-modifier) Raw Output: app/controllers/admin_info_request_event_controller.rb:50:7: C: Style/IfUnlessModifier: Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`. (https://rubystyle.guide#if-as-a-modifier)
raise "Event params cannot be empty"
end

Check warning on line 52 in app/controllers/admin_info_request_event_controller.rb

View workflow job for this annotation

GitHub Actions / build

[rubocop] reported by reviewdog 🐶 Add empty line after guard clause. Raw Output: app/controllers/admin_info_request_event_controller.rb:52:7: C: Layout/EmptyLineAfterGuardClause: Add empty line after guard clause.
begin
@info_request_event.params = JSON.parse(@params_to_show)

@info_request_event.save!
flash[:notice] = "InfoRequestEvent params updated"
redirect_to admin_info_request_events_path
rescue JSON::ParserError => e
flash[:error] = "Invalid JSON content: #{e.message}"
render :edit, status: :unprocessable_entity
end

end
end

private
Expand Down
10 changes: 10 additions & 0 deletions app/models/info_request_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@

attr_accessor :no_xapian_reindex

# we don't want users to find events, they search through requests
# and messages directly. This is only intended for admins to find PII
# inside the JSON logs.
searchable admin_index: {
# call a custom SQL function to extract the relevant bits of JSON into the index,

Check warning on line 116 in app/models/info_request_event.rb

View workflow job for this annotation

GitHub Actions / build

[rubocop] reported by reviewdog 🐶 Line is too long. [85/80] (https://rubystyle.guide#max-line-length) Raw Output: app/models/info_request_event.rb:116:81: C: Layout/LineLength: Line is too long. [85/80] (https://rubystyle.guide#max-line-length)
# to help keep the index size manageable.
# Doing this in SQL is a lot faster than doing the same in ruby.
"cleanup_jsonb_for_search(params)": "A"
}

def self.count_of_hides_by_week
where(event_type: "hide").group("date(date_trunc('week', created_at))").count.sort
end
Expand Down
1 change: 1 addition & 0 deletions app/views/admin_general/_admin_navbar.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
<ul class="dropdown-menu" role="menu">
<li><%= link_to 'Requests', admin_requests_path %></li>
<li><%= link_to 'Comments', admin_comments_path %></li>
<li><%= link_to 'Request events', admin_info_request_events_path %></li>
<% if feature_enabled?(:projects) && can?(:admin, Project) %>
<li><%= link_to 'Projects', admin_projects_path %></li>
<% end %>
Expand Down
30 changes: 30 additions & 0 deletions app/views/admin_info_request_event/_info_request_event.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<div id="info_request_event_<%=info_request_event.id%>" class="row">

<div class="myheading">
<span class="item-title">
<%= link_to(
"Event #{info_request_event.id}:",
edit_admin_info_request_event_path(info_request_event),
title: "Edit event #{info_request_event.id}",
) %>

<strong>
<%= h info_request_event.event_type.humanize %><% if !info_request_event.calculated_state.nil? %>; state:
<%= info_request_event.calculated_state %><% end %>
</strong>
<em>
<%= info_request_event.created_at %>
</em>
</span>
</div>
<div id="event_<%=info_request_event.id%>" class="details">
<div><b>Params:</b></div>
<div class="event-params">
<%= render partial: "admin_request/params",
locals: {
params: info_request_event.params_diff,
} %>
</div>
</div>

</div>
39 changes: 39 additions & 0 deletions app/views/admin_info_request_event/edit.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<h1>Edit Info request event</h1>

<table class="table table-striped table-condensed">
<tbody>
<% @info_request_event.for_admin_column do |name, value| %>
<% if name != 'params' %>
<tr>
<td>
<b><%= name.humanize %></b>
</td>
<td>
<%= admin_value(value) %>
</td>
</tr>
<% end %>
<% end %>
</tbody>
</table>

<%= form_with(
model: @info_request_event,
url: admin_info_request_event_path(@info_request_event),
method: :put,
class: "form form-inline"
) do |f| %>

<div class="field">
<%= f.label :params, "params (JSON):" %>

<%= f.text_area :params_to_show,
value: @params_to_show,
class: "form-control span12",
rows: 25 %>
</div>

<div class="form-actions">
<%= f.submit "Update event", class: "btn btn-success" %>
</div>
<% end %>
30 changes: 30 additions & 0 deletions app/views/admin_info_request_event/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<% @title = "Listing FOI request events" %>

<h1><%= @title %></h1>

<%= form_tag({}, method: 'get', class: 'form form-search') do %>
<div class="input-append">
<%= text_field_tag "query",
params[:query],
size: 40,
class: "input-large search-query" %>
<%= submit_tag "Search", class: "btn" %>
</div>

<span class="help-inline">(search in event metadata)</span>
<% end %>

<div class="accordion" id="info_request_events">
<% if @info_request_events.any? %>
<h2>Found
<%= @info_request_events.count %>
events
</h2>

<%= render partial: "admin_info_request_event/info_request_event",
collection: @info_request_events %>
<%= will_paginate(@info_request_events) %>
<% else %>
<p>Nothing found.</p>
<% end %>
</div>
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ def self.matches?(request)
scope '/admin', :as => 'admin' do
resources :info_request_events,
:controller => 'admin_info_request_event',
:only => [:update]
:only => [:edit, :index, :update]
end

#### AdminIncomingMessage controller
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
class AddInfoRequestEventIndexingFunction < ActiveRecord::Migration[8.0]
def up
execute(
<<-SQL
CREATE OR REPLACE FUNCTION cleanup_jsonb_for_search(json_v jsonb)
-- simplify jsonb columns to keep only values that are likely to be searched.
-- This excludes keys, timestamps and references to other objects.
RETURNS text
LANGUAGE SQL
IMMUTABLE PARALLEL SAFE
AS $$
SELECT string_agg(v, ' ' ORDER BY k)
FROM jsonb_each_text(json_v) AS x(k,v)
WHERE k NOT IN (
'described_state',
'embargo',
'event_created_at',
'incoming_message',
'old_described_state',
'outgoing_message',
'user'
)
$$;
SQL
)
end

def down
execute(
<<-SQL
DROP FUNCTION IF EXISTS cleanup_jsonb_for_search;
SQL
)
end
end
67 changes: 59 additions & 8 deletions spec/controllers/admin_info_request_event_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
require 'spec_helper'

RSpec.describe AdminInfoRequestEventController do
describe 'PUT update' do
describe 'PUT update to mark event as clarification request' do
let(:info_request_event) do
info_request_event = FactoryBot.create(:response_event)
end

describe 'when handling valid data' do
it 'gets the info request event' do
put :update, params: { id: info_request_event }
put :update, params: {
id: info_request_event,
commit: 'Was clarification request'
}
expect(assigns[:info_request_event]).to eq(info_request_event)
end

it 'sets the described and calculated states on the event' do
put :update, params: { id: info_request_event }
put :update, params: {
id: info_request_event,
commit: 'Was clarification request'
}
event = InfoRequestEvent.find(info_request_event.id)
expect(event.described_state).to eq('waiting_clarification')
expect(event.calculated_state).to eq('waiting_clarification')
Expand All @@ -35,34 +41,79 @@
'example.id'
)
outgoing_message.save!
put :update, params: { id: info_request_event }
put :update, params: {
id: info_request_event,
commit: 'Was clarification request'
}
expect(info_request.reload.date_initial_request_last_sent_at).
to eq(Time.zone.now.to_date)
end
end

it 'shows a success notice' do
put :update, params: { id: info_request_event }
put :update, params: {
id: info_request_event,
commit: 'Was clarification request'
}
expect(flash[:notice]).
to eq(
'Old response marked as having been a request for clarification'
)
end

it 'redirects to the request admin page' do
put :update, params: { id: info_request_event }
put :update, params: {
id: info_request_event,
commit: 'Was clarification request'
}
expect(response).
to redirect_to(admin_request_url(info_request_event.info_request))
end
end

it 'raises an exception if the event is not a response' do
put :update, params: { id: info_request_event }
put :update, params: {
id: info_request_event,
commit: 'Was clarification request'
}
info_request_event = FactoryBot.create(:sent_event)
expect {
put :update, params: { id: info_request_event }
put :update, params: {
id: info_request_event,
commit: 'Was clarification request'
}
}.to raise_error(RuntimeError,
"can only mark responses as requires clarification")
end
end

describe 'update InfoRequestEvent to remove PII' do
let(:info_request_event) do
info_request_event = FactoryBot.create(:edit_event)
end

it 'redirects to the info request list page' do
put :update, params: {
id: info_request_event,
commit: 'Update event',
info_request_event: {
params_to_show: '{"key": "value"}'
}
}
expect(response).
to redirect_to(admin_info_request_events_url)
end

it 'shows an error message if invalid JSON is submitted' do
put :update, params: {
id: info_request_event,
commit: 'Update event',
info_request_event: {
params_to_show: '{"key": broken}'
}
}
expect(flash[:error]).
to match(/^Invalid JSON conten/)
end
end
end
Loading