Merge branch '6746-info-request-prominence-reason' into develop

app/assets/javascripts/admin/admin.js

@@ -13,7 +13,7 @@
     });
     $('#request_hidden_user_explanation_reasons').on('click', 'input', function() {
       var info_request_id, message;
-      $('#request_hidden_user_subject, #request_hidden_user_explanation, #request_hide_button').show();
+      $('#request_hidden_user_subject, #request_hidden_user_explanation, #request_hidden_user_prominence_reason, #request_hide_button').show();
       info_request_id = $('#hide_request_form').attr('data-info-request-id');
       message = $(this).attr('data-message');
       $('#request_hidden_user_explanation_field').val("[loading default text...]");
@@ -24,7 +24,8 @@
           return $('#request_hidden_user_explanation_field').val("Error: " + textStatus);
         },
         success: function(data, textStatus, jqXHR) {
-          return $('#request_hidden_user_explanation_field').val(data.explanation);
+          $('#request_hidden_user_explanation_field').val(data.explanation);
+          $('#request_hidden_user_prominence_reason_field').val(data.prominence_reason);
         }
       });
     });

app/assets/stylesheets/admin.scss

@@ -159,9 +159,14 @@ body.admin {
     width: 100%;
     height: 15em;
   }
+  #request_hidden_user_prominence_reason_field {
+    width: 100%;
+    height: 5em;
+  }
   #request_hidden_user_subject,
   #request_hide_button,
-  #request_hidden_user_explanation {
+  #request_hidden_user_explanation,
+  #request_hidden_user_prominence_reason {
     display: none;
   }
 

app/controllers/admin_request_controller.rb

@@ -36,6 +36,7 @@ def edit
   def update
     old_title = @info_request.title
     old_prominence = @info_request.prominence
+    old_prominence_reason = @info_request.prominence_reason
     old_described_state = @info_request.described_state
     old_awaiting_description = @info_request.awaiting_description
     old_allow_new_responses_from = @info_request.allow_new_responses_from
@@ -52,6 +53,8 @@ def update
         title: @info_request.title,
         old_prominence: old_prominence,
         prominence: @info_request.prominence,
+        old_prominence_reason: old_prominence_reason,
+        prominence_reason: @info_request.prominence_reason,
         old_described_state: old_described_state,
         described_state: params[:info_request][:described_state],
         old_awaiting_description: old_awaiting_description,
@@ -164,14 +167,22 @@ def hide
     ActiveRecord::Base.transaction do
       subject = params[:subject]
       explanation = params[:explanation]
+
+      old_prominence = @info_request.prominence
+      old_prominence_reason = @info_request.prominence_reason
       @info_request.prominence = "requester_only"
+      @info_request.prominence_reason = params[:prominence_reason]
 
       @info_request.log_event(
         'hide',
         editor: admin_current_user,
         reason: params[:reason],
         subject: subject,
-        explanation: explanation
+        explanation: explanation,
+        old_prominence: old_prominence,
+        prominence: @info_request.prominence,
+        old_prominence_reason: old_prominence_reason,
+        prominence_reason: @info_request.prominence_reason
       )
 
       @info_request.set_described_state(params[:reason])
@@ -203,6 +214,7 @@ def info_request_params
     if params[:info_request]
       params.require(:info_request).permit(:title,
                                            :prominence,
+                                           :prominence_reason,
                                            :described_state,
                                            :awaiting_description,
                                            :allow_new_responses_from,

app/controllers/services_controller.rb

@@ -57,15 +57,21 @@ def other_country_message
   end
 
   def hidden_user_explanation
+    reason = render_to_string(
+      partial: "admin_request/hidden_user_explanation/#{params[:message]}",
+      formats: [:text]
+    )
+
     render json: {
+      prominence_reason: reason,
       explanation: render_to_string(
         template: "admin_request/hidden_user_explanation",
         formats: [:text],
         layout: false,
         locals: {
           name_to: @info_request.user_name.html_safe,
           info_request: @info_request,
-          message: params[:message],
+          reason: reason,
           info_request_url: request_url(@info_request),
           site_name: site_name.html_safe
         }

app/helpers/prominence_helper.rb

@@ -25,6 +25,11 @@ class Base # :nodoc:
     delegate :current_user, :request, :link_to,
              :help_contact_path, :signin_path, to: :@helper
 
+    def self.default_prominence_reason
+      _("There are various reasons why we might have done this, sorry we " \
+        "can't be more specific here.")
+    end
+
     def initialize(helper, prominenceable)
       @helper = helper
       @prominenceable = prominenceable
@@ -83,18 +88,12 @@ def sign_in_notice(*args)
     end
 
     def reason
-      if prominenceable.respond_to?(:prominence_reason) &&
-         prominenceable.prominence_reason.present?
-        prominenceable.prominence_reason
-      else
-        default_prominence_reason
-      end
+      prominenceable.prominence_reason.presence || default_prominence_reason
     end
 
     def default_prominence_reason
       return '' if current_user&.is_admin?
-      _("There are various reasons why we might have done this, sorry we " \
-        "can't be more specific here.")
+      self.class.default_prominence_reason
     end
   end
 

app/models/concerns/admin_column.rb

@@ -6,11 +6,27 @@ class << self
       def admin_columns(exclude: nil, include: nil)
         @excluded_admin_columns = exclude || @excluded_admin_columns
         @included_admin_columns = include || @included_admin_columns
+        ordered_columns
+      end
+
+      # Ensure prominence_reason immediately follows prominence
+      def ordered_columns
+        return all_columns unless prominenceable_admin_columns?
+
+        columns = all_columns
+        index = columns.index('prominence') + 1
+        columns.insert(index, columns.delete('prominence_reason'))
+      end
+
+      def prominenceable_admin_columns?
+        all_columns.prominence? && all_columns.prominence_reason?
+      end
 
-        translated_columns +
+      def all_columns
+        (translated_columns +
           content_columns_names +
           included_admin_columns -
-          excluded_admin_columns
+          excluded_admin_columns).inquiry
       end
 
       def translated_columns

app/models/info_request.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220928093559
 #
 # Table name: info_requests
 #
@@ -33,6 +33,7 @@
 #  last_event_time                       :datetime
 #  incoming_messages_count               :integer          default(0)
 #  public_token                          :string
+#  prominence_reason                     :text
 #
 
 require 'digest/sha1'

app/views/admin_request/edit.html.erb

@@ -64,6 +64,15 @@
     </span>
   </p>
 
+  <p><label for="info_request_prominence_reason"><strong>Reason for prominence</strong></label>
+    <%= text_area 'info_request', 'prominence_reason', rows: 5, class: 'span6' %>
+
+    <span class="help-block">
+      This reason is shown in public. If left blank a generic reason of
+      "<%= ProminenceHelper::Base.default_prominence_reason %>" will be shown.
+    </span>
+  </p>
+
   <p><label for="info_request_described_state"><strong>Described state</strong></label>
     <%= select( 'info_request', "described_state", InfoRequest::State.all) %>
     <label for="info_request_awaiting_description"><strong>Awaiting description</strong></label>

app/views/admin_request/hidden_user_explanation.text.erb

@@ -4,7 +4,7 @@
       request: info_request.title.html_safe,
       url: info_request_url) %>
 
-<%= render partial: "admin_request/hidden_user_explanation/#{ message }" -%>
+<%= reason -%>
 
 <%= _('You will still be able to view it while logged in to the site. ' \
       'Please reply to this email if you would like to discuss this decision ' \

app/views/admin_request/show.html.erb

@@ -200,6 +200,20 @@
           </div>
         </div>
 
+        <div class="control-group" id="request_hidden_user_prominence_reason">
+          <label for="request_hidden_user_prominence_reason_field" class="control-label">Prominence Reason:</label>
+
+          <div class="controls">
+            <%= text_area_tag "prominence_reason", "", {:id => "request_hidden_user_prominence_reason_field"} %>
+
+            <span class="help-block">
+              This reason is shown in public. If left blank a generic reason of
+              "<%= ProminenceHelper::Base.default_prominence_reason %>" will be
+              shown.
+            </span>
+          </div>
+        </div>
+
       <% end %>
       <div class="form-actions" id="request_hide_button">
         <%= submit_tag 'Hide request', class: 'btn' %>

db/migrate/20220928093559_add_prominence_reason_to_info_request.rb

@@ -0,0 +1,5 @@
+class AddProminenceReasonToInfoRequest < ActiveRecord::Migration[6.1]
+  def change
+    add_column :info_requests, :prominence_reason, :text
+  end
+end

spec/factories/info_requests.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220928093559
 #
 # Table name: info_requests
 #
@@ -33,6 +33,7 @@
 #  last_event_time                       :datetime
 #  incoming_messages_count               :integer          default(0)
 #  public_token                          :string
+#  prominence_reason                     :text
 #
 
 FactoryBot.define do

spec/fixtures/info_requests.yml

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220928093559
 #
 # Table name: info_requests
 #
@@ -33,6 +33,7 @@
 #  last_event_time                       :datetime
 #  incoming_messages_count               :integer          default(0)
 #  public_token                          :string
+#  prominence_reason                     :text
 #
 
 fancy_dog_request:

spec/helpers/prominence_helper_spec.rb

@@ -13,8 +13,8 @@
     FactoryBot.build(
       :info_request,
       user: requester,
-      prominence: prominence
-      # prominence_reason: prominence_reason
+      prominence: prominence,
+      prominence_reason: prominence_reason
     )
   end
 
@@ -132,8 +132,7 @@
       end
     end
 
-    # Enable with #6746
-    xcontext 'request with hidden prominence and reason as admin' do
+    context 'request with hidden prominence and reason as admin' do
       let(:object) { info_request }
       let(:prominence) { 'hidden' }
       let(:prominence_reason) { 'Spam.' }

spec/models/info_request_spec.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220928093559
 #
 # Table name: info_requests
 #
@@ -33,6 +33,7 @@
 #  last_event_time                       :datetime
 #  incoming_messages_count               :integer          default(0)
 #  public_token                          :string
+#  prominence_reason                     :text
 #
 
 require 'spec_helper'

spec/views/admin_request/hidden_user_explanation.text.erb_spec.rb

@@ -1,7 +1,11 @@
 require 'spec_helper'
 
 RSpec.describe 'admin_request/hidden_user_explanation' do
-  let(:message) { 'vexatious' }
+  let(:reason) do
+    "We consider it to be vexatious, and have therefore hidden it from other " \
+    "users.\n"
+  end
+
   let(:template) do
     'admin_request/hidden_user_explanation'
   end
@@ -12,20 +16,11 @@
            locals: { name_to: 'Bob Smith',
                      info_request: double(title: 'Foo'),
                      info_request_url: 'https://test.host/request/foo',
-                     message: message,
+                     reason: reason,
                      site_name: 'Alaveteli' }
   end
 
   it 'interpolates the locals' do
     expect(rendered).to eq(read_described_template_fixture)
   end
-
-  context 'when not_foi message' do
-    let(:message) { 'not_foi' }
-
-    it 'renders the correct message partial' do
-      expected = 'admin_request/hidden_user_explanation/_not_foi'
-      expect(rendered).to render_template(partial: expected)
-    end
-  end
 end