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
Show file tree
Hide file tree
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
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions app/controllers/attachment_masks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/controllers/citations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/prominence_headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions app/controllers/concerns/public_tokenable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
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
Expand Up @@ -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?

Expand Down Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions app/controllers/user_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 9 additions & 9 deletions app/views/request/_after_actions.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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 %>

Expand All @@ -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>
Expand Down
2 changes: 2 additions & 0 deletions doc/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions spec/controllers/attachment_masks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
24 changes: 12 additions & 12 deletions spec/controllers/attachments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions spec/controllers/citations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/public_tokens_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions spec/controllers/request_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
6 changes: 6 additions & 0 deletions spec/controllers/user_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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