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

[#8132] Update actions and pages which set "noindex", "nofollow" crawler directives #8223

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -32,6 +32,7 @@ class RouteNotFound < StandardError

include FastGettext::Translation # make functions like _, n_, N_ etc available)
include AlaveteliPro::PostRedirectHandler
include RobotsHeaders

# NOTE: a filter stops the chain if it redirects or renders something
before_action :html_response
4 changes: 0 additions & 4 deletions app/controllers/attachment_masks_controller.rb
Original file line number Diff line number Diff line change
@@ -32,10 +32,6 @@ def done

private

def set_no_crawl_headers
headers['X-Robots-Tag'] = 'noindex'
end

def decode_referer
@referer = verifier.verified(params[:referer])
end
1 change: 1 addition & 0 deletions app/controllers/citations_controller.rb
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ class CitationsController < ApplicationController
before_action :authenticate
before_action :load_info_request_and_authorise
before_action :set_in_pro_area
before_action :set_no_crawl_headers

def new
@citation = current_user.citations.build
2 changes: 1 addition & 1 deletion app/controllers/concerns/prominence_headers.rb
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ def set_normal_headers
end

def set_backpage_headers
headers['X-Robots-Tag'] = 'noindex'
set_no_crawl_headers
end

def set_requester_only_headers
6 changes: 1 addition & 5 deletions app/controllers/concerns/public_tokenable.rb
Original file line number Diff line number Diff line change
@@ -8,16 +8,12 @@ module PublicTokenable
extend ActiveSupport::Concern

included do
before_action :set_no_crawl_headers
before_action :set_no_crawl_headers, if: -> { public_token }
helper_method :public_token
end

private

def set_no_crawl_headers
headers['X-Robots-Tag'] = 'noindex' if public_token
end

def public_token
params[:public_token]
end
17 changes: 17 additions & 0 deletions app/controllers/concerns/robots_headers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
##
# Set robots noindex headers.
#
module RobotsHeaders
extend ActiveSupport::Concern

included do
before_action :set_no_crawl_headers, if: -> { params[:page].to_i > 1 }
end

private

def set_no_crawl_headers
@no_crawl = true
headers['X-Robots-Tag'] = 'noindex, nofollow'
end
end
4 changes: 1 addition & 3 deletions app/controllers/request_controller.rb
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@ class RequestController < ApplicationController
before_action :redirect_new_form_to_pro_version, only: [:select_authority, :new]
before_action :set_in_pro_area, only: [:select_authority, :show]
before_action :setup_results_pagination, only: [:list, :similar]
before_action :set_no_crawl_headers, only: [:new, :details, :similar]

helper_method :state_transitions_empty?

@@ -470,9 +471,6 @@ def setup_results_pagination
@per_page = PER_PAGE
@max_results = MAX_RESULTS

# Don't let robots go more than 20 pages in
@no_crawl = true if @page > 20

# Later pages are very expensive to load
return if @page <= MAX_RESULTS / PER_PAGE

1 change: 1 addition & 0 deletions app/controllers/user_controller.rb
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ class UserController < ApplicationController
before_action :work_out_post_redirect, only: [ :signup ]
before_action :set_request_from_foreign_country, only: [ :signup ]
before_action :set_in_pro_area, only: [ :signup ]
before_action :set_no_crawl_headers, only: :wall

# Normally we wouldn't be verifying the authenticity token on these actions
# anyway as there shouldn't be a user_id in the session when the before
18 changes: 9 additions & 9 deletions app/views/request/_after_actions.html.erb
Original file line number Diff line number Diff line change
@@ -14,20 +14,20 @@
<ul class="action-menu__menu__submenu owner_actions">
<li>
<% if last_response.nil? %>
<%= link_to _('Send a followup'), new_request_followup_path(info_request.url_title) %>
<%= link_to _('Send a followup'), new_request_followup_path(info_request.url_title), rel: 'nofollow' %>
<% else %>
<%= link_to _('Write a reply'), new_request_incoming_followup_path(info_request.url_title, incoming_message_id: last_response.id) %>
<%= link_to _('Write a reply'), new_request_incoming_followup_path(info_request.url_title, incoming_message_id: last_response.id), rel: 'nofollow' %>
<% end %>
</li>

<% if show_owner_update_status_action %>
<li>
<%= link_to _('Update the status of this request'), request_path(info_request, update_status: 1) %>
<%= link_to _('Update the status of this request'), request_path(info_request, update_status: 1), rel: 'nofollow' %>
</li>
<% end %>

<li>
<%= link_to _('Request an internal review'), new_request_followup_path(info_request.url_title, internal_review: '1') %>
<%= link_to _('Request an internal review'), new_request_followup_path(info_request.url_title, internal_review: '1'), rel: 'nofollow' %>
</li>
</ul>
</li>
@@ -42,7 +42,7 @@

<ul class="action-menu__menu__submenu public_body_actions">
<li>
<%= link_to _("Respond to request"), upload_response_path(:url_title => info_request.url_title) %>
<%= link_to _("Respond to request"), upload_response_path(:url_title => info_request.url_title), rel: 'nofollow' %>
</li>
</ul>
</li>
@@ -61,15 +61,15 @@
</li>
<% else %>
<li>
<%= link_to _("Report this request"), new_request_report_path(info_request.url_title) %><span class="action-menu__info-link">
<%= link_to _("Report this request"), new_request_report_path(info_request.url_title), rel: 'nofollow' %><span class="action-menu__info-link">
<%= link_to _("Help"), help_about_path(:anchor => "reporting") %>
</span>
</li>
<% end %>

<% if feature_enabled?(:annotations) && info_request.comments_allowed? %>
<li>
<%= link_to _('Add an annotation'), new_comment_path(:url_title => info_request.url_title) %>
<%= link_to _('Add an annotation'), new_comment_path(:url_title => info_request.url_title), rel: 'nofollow' %>
</li>
<% end %>

@@ -80,11 +80,11 @@
<% end %>

<li>
<%= link_to _("Download a zip file of all correspondence"), download_entire_request_path(:url_title => info_request.url_title) %>
<%= link_to _("Download a zip file of all correspondence"), download_entire_request_path(:url_title => info_request.url_title), rel: 'nofollow' %>
</li>

<li>
<%= link_to _('View event history details'), request_details_path(info_request) %>
<%= link_to _('View event history details'), request_details_path(info_request), rel: 'nofollow' %>
</li>

<li>
2 changes: 2 additions & 0 deletions doc/CHANGES.md
Original file line number Diff line number Diff line change
@@ -2,6 +2,8 @@

## Highlighted Features

* Update actions and pages which set "noindex", "nofollow" crawler directives
(Graeme Porteous)
* Add default value and not null constraint to `CensorRule#regexp` (Gareth Rees)
* Allow requests to be listed and filtered by tag (Graeme Porteous)
* Fix admin error when authority are missing an email address (Graeme Porteous)
8 changes: 4 additions & 4 deletions spec/controllers/attachment_masks_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -54,9 +54,9 @@ def wait
expect(response).to render_template(:wait)
end

it 'sets noindex header' do
it 'sets noindex, nofollow header' do
wait
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end

@@ -118,9 +118,9 @@ def done
expect(response).to render_template(:done)
end

it 'sets noindex header' do
it 'sets noindex, nofollow header' do
done
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end

24 changes: 12 additions & 12 deletions spec/controllers/attachments_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -198,13 +198,13 @@ def show(params = {})
expect(assigns(:info_request)).to eq(info_request)
end

it 'adds noindex header when using public token' do
it 'adds noindex, nofollow header when using public token' do
expect(InfoRequest).to receive(:find_by!).with(public_token: 'ABC').
and_return(info_request)

show(public_token: 'ABC', id: nil)

expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end

it 'passes public token to current ability' do
@@ -287,14 +287,14 @@ def show(params = {})
context 'when the request is backpage' do
let(:request_prominence) { 'backpage' }

it 'sets a noindex header when viewing' do
it 'sets a noindex, nofollow header when viewing' do
show
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end

it 'sets a noindex header when viewing a cached copy' do
it 'sets a noindex, nofollow header when viewing a cached copy' do
show
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end

context 'when logged in as requester' do
@@ -449,13 +449,13 @@ def show_as_html(params = {})
expect(assigns(:info_request)).to eq(info_request)
end

it 'adds noindex header when using public token' do
it 'adds noindex, nofollow header when using public token' do
expect(InfoRequest).to receive(:find_by!).with(public_token: '123').
and_return(info_request)

show_as_html(public_token: '123', id: nil)

expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end

context 'when attachment has not been masked' do
@@ -560,14 +560,14 @@ def show_as_html(params = {})
context 'when the request is backpage' do
let(:request_prominence) { 'backpage' }

it 'sets a noindex header when viewing a HTML version' do
it 'sets a noindex, nofollow header when viewing a HTML version' do
show_as_html
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end

it 'sets a noindex header when viewing a cached HTML version' do
it 'sets a noindex, nofollow header when viewing a cached HTML version' do
show_as_html
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end

5 changes: 5 additions & 0 deletions spec/controllers/citations_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -37,6 +37,11 @@
expect(response).to be_successful
end
end

it 'adds noindex, nofollow header' do
action
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end

# when requester
4 changes: 2 additions & 2 deletions spec/controllers/public_tokens_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -25,9 +25,9 @@
expect(assigns(:info_request)).to eq info_request
end

it 'adds noindex header' do
it 'adds noindex, nofollow header' do
get :show, params: { public_token: 'TOKEN' }
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end

it 'returns http success' do
19 changes: 17 additions & 2 deletions spec/controllers/request_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -603,9 +603,9 @@ def expect_hidden(hidden_template)
expect(response).to render_template('show')
end

it 'sets a noindex header' do
it 'sets a noindex, nofollow header' do
get :show, params: { url_title: info_request.url_title }
expect(response.headers['X-Robots-Tag']).to eq 'noindex'
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end
end
@@ -734,6 +734,11 @@ def expect_hidden(hidden_template)
expect(response).to render_template('new_bad_contact')
end

it 'adds noindex, nofollow header' do
get :new, params: { public_body_id: @body.id }
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end

context "the outgoing message includes an email address" do
context "there is no logged in user" do
it "displays a flash error message without escaping the HTML" do
@@ -1964,6 +1969,11 @@ def send_request
get :similar, params: { url_title: badger_request.url_title }
}.to raise_error(ActiveRecord::RecordNotFound)
end

it 'adds noindex, nofollow header' do
get :similar, params: { url_title: badger_request.url_title }
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end

RSpec.describe RequestController, "when the site is in read_only mode" do
@@ -2041,6 +2051,11 @@ def send_request
}.to raise_error(ActiveRecord::RecordNotFound)
end
end

it 'adds noindex, nofollow header' do
get :details, params: { url_title: info_request.url_title }
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end
end

6 changes: 6 additions & 0 deletions spec/controllers/user_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1351,4 +1351,10 @@ def make_request
get :wall, params: { url_name: user.url_name }
expect(assigns[:feed_results]).to be_empty
end

it 'adds noindex, nofollow header' do
user = FactoryBot.create(:user)
get :wall, params: { url_name: user.url_name }
expect(response.headers['X-Robots-Tag']).to eq 'noindex, nofollow'
end
end
Loading