Skip to content

Commit

Permalink
Automatically tag bodies with few public requests
Browse files Browse the repository at this point in the history
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 #7361
  • Loading branch information
garethrees committed Nov 10, 2022
1 parent c2b1b37 commit 99d0917
Show file tree
Hide file tree
Showing 9 changed files with 114 additions and 7 deletions.
5 changes: 5 additions & 0 deletions app/models/info_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,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
Expand Down Expand Up @@ -1899,4 +1900,8 @@ def reindexable_attribute_changed?
saved_change_to_attribute?(attr)
end
end

def notify_public_body
public_body.request_created
end
end
22 changes: 21 additions & 1 deletion app/models/public_body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,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
Expand Down Expand Up @@ -113,7 +116,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

Expand Down Expand Up @@ -927,6 +930,10 @@ def questions
PublicBodyQuestion.fetch(self)
end

def request_created
update_not_many_requests_tag
end

private

# If the url_name has changed, then all requested_from: queries will break
Expand Down Expand Up @@ -977,6 +984,11 @@ def self.get_public_body_list_translated_condition(table, has_first_letter=false
result
end

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')
Expand All @@ -988,4 +1000,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
1 change: 1 addition & 0 deletions app/views/admin_public_body/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
<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>
<li><code>not_many_requests</code> 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') %>.</li>
</ul>
</div>
</div>
Expand Down
9 changes: 9 additions & 0 deletions doc/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
* Fix categorisation game total requests count (Gareth Rees)
* Add count of requests in each prominence state to body and user admin pages
(Gareth Rees)
Expand Down Expand Up @@ -37,6 +40,12 @@

bin/rails runner "PublicBody.without_request_email.each(&:save)"

* _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)"

* The crontab needs to be regenerated to include the new modifications:
http://alaveteli.org/docs/installing/manual_install/#generate-crontab

Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/admin_public_body_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@
end
end

describe "DELETE #mass_tag" do
describe "DELETE #mass_tag", not_many_requests_tag: false do
it "mass removed tags" do
body = FactoryBot.create(:public_body, tag_string: 'department foo')
expect(PublicBody.find_by_tag("department").count).to eq(1)
Expand Down
3 changes: 1 addition & 2 deletions spec/lib/public_body_csv_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
require 'spec_helper'

RSpec.describe PublicBodyCSV do

RSpec.describe PublicBodyCSV, not_many_requests_tag: false do
describe '.default_fields' do

it 'has a default set of fields' do
Expand Down
11 changes: 11 additions & 0 deletions spec/models/info_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3307,6 +3307,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
Expand Down
60 changes: 57 additions & 3 deletions spec/models/public_body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,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
Expand Down Expand Up @@ -979,8 +1004,7 @@

end

describe 'when generating json for the api' do

describe 'when generating json for the api', not_many_requests_tag: false do
let(:public_body) do
FactoryBot.create(:public_body,
:name => 'Marmot Appreciation Society',
Expand Down Expand Up @@ -1344,7 +1368,7 @@ def set_default_attributes(public_body)

end

RSpec.describe PublicBody, " when loading CSV files" do
RSpec.describe PublicBody, " when loading CSV files", not_many_requests_tag: false do
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
Expand Down Expand Up @@ -2355,6 +2379,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
Expand Down
8 changes: 8 additions & 0 deletions spec/support/shared_context_for_not_many_requests_tag.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
RSpec.shared_context 'disable not_many_requests tagging', not_many_requests_tag: false do
around do |example|
orig = PublicBody.not_many_public_requests_size
PublicBody.not_many_public_requests_size = 0
example.run
PublicBody.not_many_public_requests_size = orig
end
end

0 comments on commit 99d0917

Please sign in to comment.