From b12b788ce7dda109166cc507bf92785f95e6f085 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Fri, 14 Oct 2022 09:06:55 +0100 Subject: [PATCH 1/3] Automatically tag bodies with few public requests Automatically apply and remove a tag to bodies that have a low number of public requests, so that users can find them via a public search to help bootstrap basic transparency/site content, and admins can add a tag-based note to suggest generally useful first requests. The changelog suggests a one-shot script to mass-apply the tag to existing bodies that are missing an email. Fixes https://github.com/mysociety/alaveteli/issues/7361 --- app/models/info_request.rb | 5 ++ app/models/public_body.rb | 22 ++++++- app/views/admin_public_body/_form.html.erb | 1 + doc/CHANGES.md | 12 ++++ .../admin_public_body_controller_spec.rb | 4 ++ spec/lib/public_body_csv_spec.rb | 3 + spec/models/info_request_spec.rb | 11 ++++ spec/models/public_body_spec.rb | 62 +++++++++++++++++++ spec/support/not_many_requests_helper.rb | 16 +++++ ...hared_context_for_not_many_requests_tag.rb | 0 10 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 spec/support/not_many_requests_helper.rb create mode 100644 spec/support/shared_context_for_not_many_requests_tag.rb diff --git a/app/models/info_request.rb b/app/models/info_request.rb index f8892a742b..7bb920109b 100644 --- a/app/models/info_request.rb +++ b/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 diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 72a48a22e4..306b0c3ab3 100644 --- a/app/models/public_body.rb +++ b/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 @@ -909,6 +912,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 @@ -959,6 +966,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') @@ -970,4 +982,12 @@ def update_missing_email_tag def missing_email? !has_request_email? end + + def update_not_many_requests_tag + if info_requests.is_searchable.size < not_many_public_requests_size + add_tag_if_not_already_present('not_many_requests') + else + remove_tag('not_many_requests') + end + end end diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 8bb453ba4d..978f7b5724 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -63,6 +63,7 @@
  • charity:NUMBER if a registered charity
  • important_notes if the notes have major implications on making a request to this authority
  • missing_email 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') %>.
    • +
  • not_many_requests is automatically applied (and removed) so that users can find low-transparency bodies via a <%= link_to 'public search', list_public_bodies_by_tag_path('not_many_requests') %>.
    • diff --git a/doc/CHANGES.md b/doc/CHANGES.md index af7cd4fba7..82cda1cf7d 100644 --- a/doc/CHANGES.md +++ b/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) * Signpost key user administration contributions for requests on request list pages (Gareth Rees) * Signpost users to find new contact details for requests with delivery errors @@ -10,6 +13,15 @@ * Add internal ID number to authority CSV download (Alex Parsons, Graeme Porteous) + +## 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 diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index ccd6033253..ffaa8e633e 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/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) diff --git a/spec/lib/public_body_csv_spec.rb b/spec/lib/public_body_csv_spec.rb index 3c6c431ec0..c31474588d 100644 --- a/spec/lib/public_body_csv_spec.rb +++ b/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 diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index b3591271bc..b5f1ea4b40 100644 --- a/spec/models/info_request_spec.rb +++ b/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 diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 480ed98211..02c28ec283 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -242,6 +242,31 @@ 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 end describe '#name' do @@ -956,6 +981,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 +1359,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 +2400,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 diff --git a/spec/support/not_many_requests_helper.rb b/spec/support/not_many_requests_helper.rb new file mode 100644 index 0000000000..54604e9344 --- /dev/null +++ b/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 diff --git a/spec/support/shared_context_for_not_many_requests_tag.rb b/spec/support/shared_context_for_not_many_requests_tag.rb new file mode 100644 index 0000000000..e69de29bb2 From cc6cc3321bad3b31f95f7676eb0741405ffdea51 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Fri, 14 Oct 2022 09:20:33 +0100 Subject: [PATCH 2/3] Organise automatically managed tags Separates out automatically managed tags (that don't change site behaviour) from tags that change behaviour of the body in some way. --- app/views/admin_public_body/_form.html.erb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 978f7b5724..7f4b799d55 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -54,7 +54,9 @@ <%= f.text_field :tag_string, :class => "span4" %>

    Space separated – see list of tags on the right.

    -

    Special tags:

    + +

    Special tags that change behaviour:

    +
    • not_apply: if FOI and EIR no longer apply to authority and prevents further requests from being sent
    • foi_no: FOI does not apply but we list the authority to campaign for it to be subject to FOI
      • @@ -62,8 +64,13 @@
    • defunct if the authority no longer exists
    • charity:NUMBER if a registered charity
    • important_notes if the notes have major implications on making a request to this authority
      • -
    • missing_email 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') %>.
      • -
    • not_many_requests is automatically applied (and removed) so that users can find low-transparency bodies via a <%= link_to 'public search', list_public_bodies_by_tag_path('not_many_requests') %>.
      • +
    + +

    Automatically applied and removed tags:

    + +
      +
    • missing_email users can help source missing request addresses via a <%= link_to 'public search', list_public_bodies_by_tag_path('missing_email') %>.
      • +
    • not_many_requests users can find low-transparency bodies via a <%= link_to 'public search', list_public_bodies_by_tag_path('not_many_requests') %>.
    From e3aee8ca917ca40b464fc717ca783609d879a698 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Fri, 14 Oct 2022 09:22:15 +0100 Subject: [PATCH 3/3] Only apply not many requests to requestable bodies Our main intent with this tag is for it to act as a hook for suggesting some good basic requests to "bootstrap transparency". There's no point in doing this if users can't make requests to the body. --- app/models/public_body.rb | 6 +++++- app/views/admin_public_body/_form.html.erb | 2 +- spec/models/public_body_spec.rb | 12 ++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 306b0c3ab3..b759dec220 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -984,10 +984,14 @@ def missing_email? end def update_not_many_requests_tag - if info_requests.is_searchable.size < not_many_public_requests_size + 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 diff --git a/app/views/admin_public_body/_form.html.erb b/app/views/admin_public_body/_form.html.erb index 7f4b799d55..b7760a629c 100644 --- a/app/views/admin_public_body/_form.html.erb +++ b/app/views/admin_public_body/_form.html.erb @@ -70,7 +70,7 @@
    • missing_email users can help source missing request addresses via a <%= link_to 'public search', list_public_bodies_by_tag_path('missing_email') %>.
      • -
    • not_many_requests users can find low-transparency bodies via a <%= link_to 'public search', list_public_bodies_by_tag_path('not_many_requests') %>.
      • +
    • not_many_requests 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.
    diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 02c28ec283..562cdc4eac 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -267,6 +267,18 @@ 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