Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1005] hide attachments prep #7387

Merged
merged 12 commits into from
Oct 24, 2022
34 changes: 34 additions & 0 deletions app/controllers/admin/foi_attachments_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
##
# Controller to manage FoiAttachment instances
#
class Admin::FoiAttachmentsController < AdminController
before_action :set_foi_attachment, :set_incoming_message, :set_info_request
before_action :check_info_request

def edit
end

def update
redirect_to edit_admin_incoming_message_path(@incoming_message)
end

private

def set_foi_attachment
@foi_attachment = FoiAttachment.find(params[:id])
end

def set_incoming_message
@incoming_message = @foi_attachment&.incoming_message
end

def set_info_request
@info_request = @incoming_message&.info_request
end

def check_info_request
return if can? :admin, @info_request

raise ActiveRecord::RecordNotFound
end
end
22 changes: 12 additions & 10 deletions app/controllers/attachments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ class AttachmentsController < ApplicationController

include ProminenceHeaders

around_action :cache_attachments

before_action :authenticate_attachment
before_action :authenticate_attachment_as_html, only: :show_as_html

around_action :cache_attachments

def show
# Prevent spam to magic request address. Note that the binary
# substitution method used depends on the content type
Expand Down Expand Up @@ -111,8 +111,12 @@ def authenticate_attachment

return if @attachment

# If we can't find the right attachment, redirect to the incoming message:
redirect_to incoming_message_url(@incoming_message), status: 303
if params[:file_name]
# If we can't find the right attachment, redirect to the incoming message:
redirect_to incoming_message_url(@incoming_message), status: 303
else
render plain: 'Directory listing not allowed', status: 403
end
end

def authenticate_attachment_as_html
Expand All @@ -132,12 +136,8 @@ def cache_attachments
if foi_fragment_cache_exists?(cache_key_path)
logger.info("Reading cache for #{cache_key_path}")

if File.directory?(cache_key_path)
render plain: 'Directory listing not allowed', status: 403
else
render body: foi_fragment_cache_read(cache_key_path),
content_type: content_type
end
render body: foi_fragment_cache_read(cache_key_path),
content_type: content_type
return
end

Expand All @@ -162,6 +162,8 @@ def part_number

def original_filename
filename = params[:file_name]
return unless filename

if action_name == 'show_as_html'
filename.gsub(/\.html$/, '')
else
Expand Down
11 changes: 11 additions & 0 deletions app/helpers/admin/link_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ def incoming_message_both_links(incoming_message)
title: admin_title)
end

def foi_attachment_both_links(foi_attachment)
title = 'View attachment on public website'
icon = prominence_icon(foi_attachment)
info_request = foi_attachment.incoming_message.info_request

link_to(icon, foi_attachment_path(foi_attachment), title: title) + ' ' +
link_to("#{info_request.title} #{foi_attachment.filename}",
edit_admin_foi_attachment_path(foi_attachment),
title: admin_title)
end

def info_request_batch_both_links(batch)
title = 'View batch on public website'
icon = prominence_icon(batch)
Expand Down
14 changes: 13 additions & 1 deletion app/helpers/link_to_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ def outgoing_message_path(outgoing_message, options = {})
message_path(outgoing_message, options)
end

def foi_attachment_url(foi_attachment, options = {})
incoming_message_url(
foi_attachment.incoming_message,
options.merge(anchor: dom_id(foi_attachment))
)
end

def foi_attachment_path(foi_attachment, options = {})
foi_attachment_url(foi_attachment, options.merge(only_path: true))
end

def comment_url(comment, options = {})
request_url(comment.info_request, options.merge(:anchor => "comment-#{comment.id}"))
end
Expand Down Expand Up @@ -299,7 +310,8 @@ def incoming_message_dom_id(incoming_message)

# Private: Generate a request_url linking to the new correspondence
def message_url(message, options = {})
anchor = dom_id(message)
anchor = options[:anchor]
anchor ||= dom_id(message)

return "##{anchor}" if options[:anchor_only]
default_options = { anchor: anchor }
Expand Down
1 change: 1 addition & 0 deletions app/models/concerns/message_prominence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module MessageProminence
extend ActiveSupport::Concern

included do
strip_attributes only: [:prominence_reason]
validates_inclusion_of :prominence, in: self.prominence_states
end

Expand Down
8 changes: 7 additions & 1 deletion app/models/foi_attachment.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# == Schema Information
# Schema version: 20210114161442
# Schema version: 20220916134847
#
# Table name: foi_attachments
#
Expand All @@ -14,6 +14,8 @@
# hexdigest :string(32)
# created_at :datetime
# updated_at :datetime
# prominence :string default("normal")
# prominence_reason :text
#

# models/foi_attachment.rb:
Expand All @@ -26,6 +28,8 @@
require 'digest'

class FoiAttachment < ApplicationRecord
include MessageProminence

belongs_to :incoming_message,
:inverse_of => :foi_attachments

Expand All @@ -40,6 +44,8 @@ class FoiAttachment < ApplicationRecord

scope :binary, -> { where.not(content_type: AlaveteliTextMasker::TextMask) }

admin_columns exclude: %i[url_part_number within_rfc822_subject hexdigest]

BODY_MAX_TRIES = 3
BODY_MAX_DELAY = 5

Expand Down
2 changes: 0 additions & 2 deletions app/models/incoming_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ class IncomingMessage < ApplicationRecord
include CacheAttributesFromRawEmail
include Taggable

strip_attributes only: [:prominence_reason]

MAX_ATTACHMENT_TEXT_CLIPPED = 1000000 # 1Mb ish

belongs_to :info_request,
Expand Down
2 changes: 0 additions & 2 deletions app/models/outgoing_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ class OutgoingMessage < ApplicationRecord
include LinkToHelper
include Taggable

strip_attributes only: [:prominence_reason]

STATUS_TYPES = %w(ready sent failed).freeze
MESSAGE_TYPES = %w(initial_request followup).freeze
WHAT_DOING_VALUES = %w(normal_sort
Expand Down
8 changes: 8 additions & 0 deletions app/views/admin/foi_attachments/_intro.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<% @title = "Attachment #{foi_attachment.id} of FOI request '#{info_request.title}'" %>
<h1>Attachment <%= foi_attachment.id %></h1>

<ul class="breadcrumb">
<li>FOI request: <%= both_links(info_request) %> <span class="divider">/</span></li>
<li>Message: <%= both_links(incoming_message) %> <span class="divider">/</span></li>
<li class="active">Attachment: <%= both_links(foi_attachment) %></li>
</ul>
26 changes: 26 additions & 0 deletions app/views/admin/foi_attachments/edit.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<%= render partial: 'intro', locals: {
info_request: @info_request,
incoming_message: @incoming_message,
foi_attachment: @foi_attachment
} %>

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

<%= form_tag admin_foi_attachment_path(@foi_attachment), method: 'put', class: 'form form-inline' do %>
<div class="form-actions">
<%= submit_tag 'Save', class: 'btn btn-success' %>
</div>
<% end %>
6 changes: 2 additions & 4 deletions app/views/admin_incoming_message/_foi_attachments.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
<table class="table">
<thead>
<tr>
<th>ID</th>
<th>Filename</th>
<th>Attachment</th>
<th>Content Type</th>
<th>Hexdigest</th>
<th>Display Size</th>
Expand All @@ -15,8 +14,7 @@
<tbody>
<% foi_attachments.each do |attachment| %>
<tr>
<td><tt><%= attachment.id %></tt></td>
<td><tt><%= attachment.filename %></tt></td>
<td><%= both_links attachment %></td>
<td><tt><%= attachment.content_type %></tt></td>
<td><tt><%= attachment.hexdigest %></tt></td>
<td><tt><%= attachment.display_size %></tt></td>
Expand Down
2 changes: 1 addition & 1 deletion app/views/request/_incoming_correspondence.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<% end %>
</div>

<%= render partial: 'request/prominence', locals: { message: incoming_message } %>
<%= render partial: 'request/prominence', locals: { prominenceable: incoming_message } %>

<%- if can?(:read, incoming_message) %>
<% if feature_enabled?(:refusal_identification) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/request/_incoming_correspondence.text.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%= render partial: 'request/prominence',
formats: [:text],
locals: { message: incoming_message } %>
locals: { prominenceable: incoming_message } %>
<% if can?(:read, incoming_message) %>
<%= _('From:') %><% if incoming_message.specific_from_name? %> <%= incoming_message.safe_from_name %><% end %><% if incoming_message.from_public_body? %>, <%= @info_request.public_body.name %><% end %>
<%= _('To:') %> <% if @info_request.user_name %><%= @info_request.user_name %><% else %><%= "[#{_('An anonymous user')}]"%><% end %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/request/_outgoing_correspondence.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div class="outgoing correspondence js-collapsable" id="<%= dom_id(outgoing_message) %>">
<%= render partial: 'request/prominence', locals: { message: outgoing_message } %>
<%= render partial: 'request/prominence', locals: { prominenceable: outgoing_message } %>

<%- if can?(:read, outgoing_message) %>
<% delivery_status = outgoing_message.delivery_status %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/request/_outgoing_correspondence.text.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%= render partial: 'request/prominence',
formats: [:text],
locals: { message: outgoing_message } %>
locals: { prominenceable: outgoing_message } %>
<% if can?(:read, outgoing_message) %>
<%= _('From:') %> <% if @info_request.user_name %><%= @info_request.user_name %><% else %><%= "[#{_('An anonymous user')}]"%><% end %>
<%= _('To:') %> <%= @info_request.public_body.name %>
Expand Down
4 changes: 2 additions & 2 deletions app/views/request/_prominence.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<% if conceled_prominence?(message) %>
<% if conceled_prominence?(prominenceable) %>
<p id="hidden_message" class="hidden_message">
<%= render_prominence(message) %>
<%= render_prominence(prominenceable) %>
</p>
<% end %>
2 changes: 1 addition & 1 deletion app/views/request/_prominence.text.erb
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<%= render_prominence(message, format: :text) %>
<%= render_prominence(prominenceable, format: :text) %>
2 changes: 1 addition & 1 deletion app/views/request/hidden_correspondence.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% @title = _("Message has been removed") %>

<h1><%=@title%></h1>
<%= render partial: 'request/prominence', locals: { message: @incoming_message } %>
<%= render partial: 'request/prominence', locals: { prominenceable: @incoming_message } %>
16 changes: 13 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,17 @@
:as => :similar_request,
:via => :get

match '/request/:id/response/:incoming_message_id/attach/html/:part/*file_name' => 'attachments#show_as_html',
match '/request/:id/response/:incoming_message_id/attach/html' \
'/(:part(/*file_name))' => 'attachments#show_as_html',
:format => false,
:as => :get_attachment_as_html,
:via => :get
:via => :get,
:constraints => { :part => /\d+/ }
match '/request/:id/response/:incoming_message_id/attach/:part(/*file_name)' => 'attachments#show',
:format => false,
:as => :get_attachment,
:via => :get
:via => :get,
:constraints => { :part => /\d+/ }

match '/request_event/:info_request_event_id' => 'request#show_request_event',
:as => :info_request_event,
Expand Down Expand Up @@ -647,6 +650,13 @@
end
####

#### AdminFoiAttachment controller
namespace :admin do
resources :foi_attachments, path: :attachments,
only: [:edit, :update]
end
####

#### AdminUser controller
scope '/admin', :as => 'admin' do
resources :users,
Expand Down
6 changes: 6 additions & 0 deletions db/migrate/20220916134847_add_prominence_to_foi_attachment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddProminenceToFoiAttachment < ActiveRecord::Migration[6.1]
def change
add_column :foi_attachments, :prominence, :string, default: 'normal'
add_column :foi_attachments, :prominence_reason, :text
end
end
Loading