Merge branch '7361-not-many-requests-tag' into develop

app/models/info_request.rb

@@ -188,6 +188,7 @@ class << self
   before_create :set_use_notifications
   before_validation :compute_idhash
   before_validation :set_law_used, on: :create
+  after_create :notify_public_body
   after_save :update_counter_cache
   after_update :reindex_request_events, if: :reindexable_attribute_changed?
   before_destroy :expire
@@ -1935,4 +1936,8 @@ def reindexable_attribute_changed?
       saved_change_to_attribute?(attr)
     end
   end
+
+  def notify_public_body
+    public_body.request_created
+  end
 end

app/models/public_body.rb

@@ -62,6 +62,9 @@ def self.admin_title
     ]
   end
 
+  # Set to 0 to prevent application of the not_many_requests tag
+  cattr_accessor :not_many_public_requests_size, default: 5
+
   has_many :info_requests,
            -> { order(created_at: :desc) },
            inverse_of: :public_body
@@ -114,7 +117,7 @@ def self.admin_title
 
   before_save :set_api_key!, unless: :api_key
 
-  after_save :update_missing_email_tag
+  after_save :update_auto_applied_tags
 
   after_update :reindex_requested_from, :invalidate_cached_pages,
                unless: :no_xapian_reindex
@@ -910,6 +913,10 @@ def cached_urls
     ]
   end
 
+  def request_created
+    update_not_many_requests_tag
+  end
+
   private
 
   # If the url_name has changed, then all requested_from: queries will break
@@ -960,6 +967,11 @@ def self.get_public_body_list_translated_condition(table, has_first_letter=false
   end
   private_class_method :get_public_body_list_translated_condition
 
+  def update_auto_applied_tags
+    update_missing_email_tag
+    update_not_many_requests_tag
+  end
+
   def update_missing_email_tag
     if missing_email? && !defunct?
       add_tag_if_not_already_present('missing_email')
@@ -971,4 +983,16 @@ def update_missing_email_tag
   def missing_email?
     !has_request_email?
   end
+
+  def update_not_many_requests_tag
+    if is_requestable? && not_many_public_requests?
+      add_tag_if_not_already_present('not_many_requests')
+    else
+      remove_tag('not_many_requests')
+    end
+  end
+
+  def not_many_public_requests?
+    info_requests.is_searchable.size < not_many_public_requests_size
+  end
 end

app/views/admin_public_body/_form.html.erb

@@ -54,15 +54,23 @@
     <%= f.text_field :tag_string, :class => "span4" %>
     <div class="help-block">
       <p>Space separated – see list of tags on the right.</p>
-      <p>Special tags:</p>
+
+      <p>Special tags that change behaviour:</p>
+
       <ul>
         <li><code>not_apply</code>: if FOI and EIR no longer apply to authority and prevents further requests from being sent</li>
         <li><code>foi_no</code>: FOI does not apply but we list the authority to campaign for it to be subject to FOI</li>
         <li><code>eir_only</code> if EIR but not FOI applies to authority</li>
         <li><code>defunct</code> if the authority no longer exists</li>
         <li><code>charity:NUMBER</code> if a registered charity</li>
         <li><code>important_notes</code> if the notes have major implications on making a request to this authority</li>
-        <li><code>missing_email</code> is automatically applied (and removed) so that users can help source missing request addresses via a <%= link_to 'public search', list_public_bodies_by_tag_path('missing_email') %>.</li>
+      </ul>
+
+      <p>Automatically applied and removed tags:</p>
+
+      <ul>
+        <li><code>missing_email</code> users can help source missing request addresses via a <%= link_to 'public search', list_public_bodies_by_tag_path('missing_email') %>.</li>
+        <li><code>not_many_requests</code> users can find low-transparency bodies via a <%= link_to 'public search', list_public_bodies_by_tag_path('not_many_requests') %>. Only applied to bodies that are requestable.</li>
       </ul>
     </div>
   </div>

doc/CHANGES.md

@@ -2,6 +2,9 @@
 
 ## Highlighted Features
 
+* Automatically apply `not_many_requests` tag to bodies who don't have many
+  public requests so that they can be found in a public list or have tag-based
+  notes applied (Gareth Rees)
 * Improve background job performance by limiting the number `NotifyCacheJob`
   jobs created (Graeme Porteous)
 * Signpost key user administration contributions for requests on request list
@@ -14,6 +17,15 @@
 * Don't show users that have closed their account or been banned on leaderboards
   (Chris Mytton)
 
+
+## Upgrade Notes
+
+* _Optional:_ Bodies with not many requests will automatically get tagged
+  `not_many_requests` as they are updated. If you want to automatically tag them
+  all in one go, run the following from the app root directory:
+
+      bin/rails runner "PublicBody.where('info_requests_visible_count < ?', PublicBody.not_many_public_requests_size).each(&:save)"
+
 # 0.44.0.0
 
 ## Highlighted Features

spec/controllers/admin_public_body_controller_spec.rb

@@ -673,6 +673,10 @@
   end
 
   describe "DELETE #mass_tag" do
+    around do |example|
+      disable_not_many_requests_auto_tagging { example.run }
+    end
+
     it "mass removed tags" do
       body = FactoryBot.create(:public_body, tag_string: 'department foo')
       expect(PublicBody.find_by_tag("department").count).to eq(1)

spec/lib/public_body_csv_spec.rb

@@ -1,6 +1,9 @@
 require 'spec_helper'
 
 RSpec.describe PublicBodyCSV do
+  around do |example|
+    disable_not_many_requests_auto_tagging { example.run }
+  end
 
   describe '.default_fields' do
 

spec/models/info_request_spec.rb

@@ -3655,6 +3655,17 @@ def create_old_unclassified_holding_pen
       expect(info_request).to receive(:update_counter_cache)
       info_request.save!
     end
+
+    it 'notifies the public body when created' do
+      expect(info_request.public_body).to receive(:request_created)
+      info_request.save!
+    end
+
+    it 'does not notify the public body when updated' do
+      info_request.save!
+      expect(info_request.public_body).not_to receive(:request_created)
+      info_request.save!
+    end
   end
 
   describe 'when changing a described_state' do

spec/models/public_body_spec.rb

@@ -242,6 +242,43 @@
         expect(public_body).not_to be_tagged('missing_email')
       end
     end
+
+    context 'when there are not many public requests' do
+      let!(:public_body) { FactoryBot.create(:public_body) }
+
+      it 'adds the not many requests tag' do
+        subject
+        expect(public_body).to be_tagged('not_many_requests')
+      end
+    end
+
+    context 'when a request email is removed' do
+      let!(:public_body) { FactoryBot.create(:public_body) }
+
+      # Reduce size for test so we have to create fewer records
+      before { public_body.not_many_public_requests_size = 2 }
+
+      before do
+        FactoryBot.create_list(:info_request, 2, public_body: public_body)
+      end
+
+      it 'removes the not many requests tag' do
+        subject
+        expect(public_body).not_to be_tagged('not_many_requests')
+      end
+    end
+
+    context 'when a not_requestable body is is tagged not_many_requests' do
+      let!(:public_body) { FactoryBot.create(:public_body) }
+
+      before { public_body.add_tag_if_not_already_present('not_many_requests') }
+      before { public_body.update(request_email: '') }
+
+      it 'removes the not many requests tag' do
+        subject
+        expect(public_body).not_to be_tagged('not_many_requests')
+      end
+    end
   end
 
   describe '#name' do
@@ -956,6 +993,9 @@
   end
 
   describe 'when generating json for the api' do
+    around do |example|
+      disable_not_many_requests_auto_tagging { example.run }
+    end
 
     let(:public_body) do
       FactoryBot.create(:public_body,
@@ -1331,6 +1371,10 @@ def set_default_attributes(public_body)
 end
 
 RSpec.describe PublicBody, " when loading CSV files" do
+  around do |example|
+    disable_not_many_requests_auto_tagging { example.run }
+  end
+
   before(:each) do
     # InternalBody is created the first time it's accessed, which happens sometimes during imports,
     # depending on the tag used. By accessing it here before every test, it doesn't disturb our checks later on
@@ -2368,6 +2412,36 @@ def set_default_attributes(public_body)
         to change { public_body.info_requests_visible_count }.from(1).to(0)
     end
   end
+
+  describe '#request_created' do
+    subject { public_body.request_created }
+
+    context 'when there are not many public requests' do
+      let!(:public_body) { FactoryBot.create(:public_body) }
+
+      before { public_body.not_many_public_requests_size = 2 }
+
+      it 'adds the not many requests tag' do
+        subject
+        expect(public_body).to be_tagged('not_many_requests')
+      end
+    end
+
+    context 'when a request email is removed' do
+      let!(:public_body) { FactoryBot.create(:public_body) }
+
+      before { public_body.not_many_public_requests_size = 2 }
+
+      before do
+        FactoryBot.create_list(:info_request, 3, public_body: public_body)
+      end
+
+      it 'removes the not many requests tag' do
+        subject
+        expect(public_body).not_to be_tagged('not_many_requests')
+      end
+    end
+  end
 end
 
 RSpec.describe PublicBody::Translation do

spec/support/not_many_requests_helper.rb

@@ -0,0 +1,16 @@
+# Depending on spec run ordering an authority may or may not meet the criteria
+# to have the tag auto-applied, and as such breaks the expectations (e.g.
+# sometimes the tag string will be "foo", sometimes "foo not_many_requests").
+# Use this in an `around` block to disable the tagging for specs that match a
+# defined set of tags:
+#
+#   around do |example|
+#     disable_not_many_requests_auto_tagging { example.run }
+#   end
+#
+def disable_not_many_requests_auto_tagging
+  orig = PublicBody.not_many_public_requests_size
+  PublicBody.not_many_public_requests_size = 0
+  yield
+  PublicBody.not_many_public_requests_size = orig
+end