diff --git a/app/controllers/alaveteli_pro/classifications_controller.rb b/app/controllers/alaveteli_pro/classifications_controller.rb index 035974b48c..2fe10f33c5 100644 --- a/app/controllers/alaveteli_pro/classifications_controller.rb +++ b/app/controllers/alaveteli_pro/classifications_controller.rb @@ -10,7 +10,7 @@ def create set_described_state flash[:notice] = _('Your request has been updated!') - redirect_to show_alaveteli_pro_request_path( + redirect_to show_request_path( url_title: @info_request.url_title ) end diff --git a/app/controllers/alaveteli_pro/embargo_extensions_controller.rb b/app/controllers/alaveteli_pro/embargo_extensions_controller.rb index 5faa105ed6..643b4f9197 100644 --- a/app/controllers/alaveteli_pro/embargo_extensions_controller.rb +++ b/app/controllers/alaveteli_pro/embargo_extensions_controller.rb @@ -32,7 +32,7 @@ def create "request's privacy settings, please try again.") end - redirect_to show_alaveteli_pro_request_path( + redirect_to show_request_path( url_title: @info_request.url_title ) end @@ -70,7 +70,7 @@ def create_batch if params[:info_request_id] @info_request = InfoRequest.find(params[:info_request_id]) - redirect_to show_alaveteli_pro_request_path( + redirect_to show_request_path( url_title: @info_request.url_title ) else diff --git a/app/controllers/alaveteli_pro/embargoes_controller.rb b/app/controllers/alaveteli_pro/embargoes_controller.rb index 987fa3d3f0..7dd7b6dcb9 100644 --- a/app/controllers/alaveteli_pro/embargoes_controller.rb +++ b/app/controllers/alaveteli_pro/embargoes_controller.rb @@ -66,7 +66,7 @@ def destroy_batch end if params[:info_request_id] @info_request = InfoRequest.find(params[:info_request_id]) - redirect_to show_alaveteli_pro_request_path( + redirect_to show_request_path( url_title: @info_request.url_title) else redirect_to show_alaveteli_pro_batch_request_path(@info_request_batch) diff --git a/app/controllers/alaveteli_pro/info_requests_controller.rb b/app/controllers/alaveteli_pro/info_requests_controller.rb index 76f5760ca4..6adb6b37fc 100644 --- a/app/controllers/alaveteli_pro/info_requests_controller.rb +++ b/app/controllers/alaveteli_pro/info_requests_controller.rb @@ -47,7 +47,7 @@ def create @embargo.save! if @embargo.present? send_initial_message(@outgoing_message) destroy_draft - redirect_to show_alaveteli_pro_request_path( + redirect_to show_request_path( url_title: @info_request.url_title) else show_errors diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index a5abeec4ef..cb23b6ce70 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -12,8 +12,6 @@ class RequestController < ApplicationController before_action :check_read_only, only: [:new, :upload_response] before_action :set_render_recaptcha, only: [ :new ] before_action :set_info_request, only: [:show] - before_action :redirect_embargoed_requests_for_pro_users, only: [:show] - before_action :redirect_public_requests_from_pro_context, only: [:show] before_action :redirect_new_form_to_pro_version, only: [:select_authority, :new] before_action :set_in_pro_area, only: [:select_authority, :show] @@ -52,8 +50,7 @@ def show # Always show the pro livery if a request is embargoed. This makes it # clear to admins and ex-pro users that the `InfoRequest` is still - # private. Users who are not permitted to view the request are redirected - # so we don't need to consider the `current_user` here. + # private. @in_pro_area = true if @info_request.embargo set_last_request(@info_request) @@ -711,28 +708,6 @@ def set_render_recaptcha (!@user || !@user.confirmed_not_spam?) end - def redirect_embargoed_requests_for_pro_users - # Pro users should see their embargoed requests in the pro page, so that - # if other site functions send them to a request page, they end up back in - # the pro area - if feature_enabled?(:alaveteli_pro) && params[:pro] != "1" && current_user - @info_request = InfoRequest.find_by_url_title!(params[:url_title]) - if @info_request.is_actual_owning_user?(current_user) && @info_request.embargo - redirect_to show_alaveteli_pro_request_url( - url_title: @info_request.url_title) - end - end - end - - def redirect_public_requests_from_pro_context - # Requests which aren't embargoed should always go to the normal request - # page, so that pro's seem them in that context after they publish them - if feature_enabled?(:alaveteli_pro) && params[:pro] == "1" - @info_request = InfoRequest.find_by_url_title!(params[:url_title]) - redirect_to request_url(@info_request) unless @info_request.embargo - end - end - def redirect_new_form_to_pro_version # Pros should use the pro version of the form if feature_enabled?(:alaveteli_pro) && diff --git a/app/services/info_request_batch_metrics.rb b/app/services/info_request_batch_metrics.rb index f90670a186..73d0b2eb6b 100644 --- a/app/services/info_request_batch_metrics.rb +++ b/app/services/info_request_batch_metrics.rb @@ -16,7 +16,7 @@ def initialize(info_request_batch) def metrics @metrics ||= @info_request_batch.info_requests. includes(public_body: :translations).map do |info_request| - url = show_alaveteli_pro_request_url(info_request.url_title) + url = show_request_url(info_request.url_title) status = InfoRequest::State.short_description( info_request.calculate_status(true) ) diff --git a/app/views/admin_request/show.html.erb b/app/views/admin_request/show.html.erb index 4946874df6..191544d993 100644 --- a/app/views/admin_request/show.html.erb +++ b/app/views/admin_request/show.html.erb @@ -34,7 +34,7 @@ <% if @info_request.embargo %> Request page: - <%= link_to show_alaveteli_pro_request_url(@info_request.url_title), show_alaveteli_pro_request_path(@info_request.url_title) %> + <%= link_to show_request_url(@info_request.url_title), show_request_path(@info_request.url_title) %> <% else %> Public page: <%= link_to request_url(@info_request), request_path(@info_request) %> diff --git a/config/routes.rb b/config/routes.rb index 9076e52b31..72d227db92 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -907,13 +907,6 @@ def matches?(request) end scope path: :alaveteli_pro do - # So that we can show a request using the existing controller from the - # pro context - match '/info_requests/:url_title' => 'request#show', - :as => :show_alaveteli_pro_request, - :via => :get, - :defaults => { :pro => '1' } - # So that we can show a batch request using the existing controller from # the pro context match '/info_request_batches/:id' => 'info_request_batch#show', diff --git a/config/routes/redirects.rb b/config/routes/redirects.rb index 1fa67f1fe7..708cae33f2 100644 --- a/config/routes/redirects.rb +++ b/config/routes/redirects.rb @@ -95,3 +95,9 @@ get ':locale/*path', constraints: locale_constraint, to: redirect('%{path}?locale=%{locale}') + +constraints FeatureConstraint.new(:alaveteli_pro) do + get '/alaveteli_pro/info_requests/:url_title', + constraints: { url_title: /(?!new).*/ }, + to: redirect('/request/%{url_title}') +end diff --git a/doc/CHANGES.md b/doc/CHANGES.md index 6cdbfa4b9e..bda5287b7e 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -2,6 +2,8 @@ ## Highlighted Features +* Change use of `/alaveteli_pro/info_requests/{request}` to instead appear as + `/request/{request}` (Alexander Griffen, Graeme Porteous) * Remove locale prefixes from URLs (Alexander Griffen, Graeme Porteous) * Fix missing headers when exporting Project data (Gareth Rees) * Reduce amount of storage related background jobs (Graeme Porteous) diff --git a/spec/controllers/alaveteli_pro/classifications_controller_spec.rb b/spec/controllers/alaveteli_pro/classifications_controller_spec.rb index fc94929d49..814fd3b769 100644 --- a/spec/controllers/alaveteli_pro/classifications_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/classifications_controller_spec.rb @@ -75,7 +75,7 @@ def post_status(status, message: nil) it 'should redirect back to the request' do post_status('successful') expect(response).to redirect_to( - show_alaveteli_pro_request_path(url_title: info_request.url_title) + show_request_path(url_title: info_request.url_title) ) end end @@ -128,7 +128,7 @@ def post_status(status, message: nil) it 'should redirect back to the request' do post_status('error_message', message: 'A message') expect(response).to redirect_to( - show_alaveteli_pro_request_path(url_title: info_request.url_title) + show_request_path(url_title: info_request.url_title) ) end end @@ -169,7 +169,7 @@ def post_status(status, message: nil) it 'should redirect back to the request' do post_status('requires_admin', message: 'A message') expect(response).to redirect_to( - show_alaveteli_pro_request_path(url_title: info_request.url_title) + show_request_path(url_title: info_request.url_title) ) end end diff --git a/spec/controllers/alaveteli_pro/embargo_controller_spec.rb b/spec/controllers/alaveteli_pro/embargo_controller_spec.rb index 60cf5863b3..0cf46fb8c6 100644 --- a/spec/controllers/alaveteli_pro/embargo_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/embargo_controller_spec.rb @@ -306,7 +306,7 @@ end it "redirects to that request, not the batch" do - expected_path = show_alaveteli_pro_request_path( + expected_path = show_request_path( url_title: info_request_batch.info_requests.first.url_title) expect(response).to redirect_to(expected_path) end diff --git a/spec/controllers/alaveteli_pro/embargo_extensions_controller_spec.rb b/spec/controllers/alaveteli_pro/embargo_extensions_controller_spec.rb index 04f0c95346..46e4671109 100644 --- a/spec/controllers/alaveteli_pro/embargo_extensions_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/embargo_extensions_controller_spec.rb @@ -40,7 +40,7 @@ it 'redirects to the request show page' do expect(response). - to redirect_to show_alaveteli_pro_request_path( + to redirect_to show_request_path( url_title: info_request.url_title) end end @@ -71,7 +71,7 @@ it 'redirects to the request show page' do expect(response). - to redirect_to show_alaveteli_pro_request_path( + to redirect_to show_request_path( url_title: info_request.url_title) end end @@ -307,7 +307,7 @@ end it 'redirects to that request, not the batch' do - expected_path = show_alaveteli_pro_request_path( + expected_path = show_request_path( url_title: info_request_batch.info_requests.first.url_title) expect(response).to redirect_to(expected_path) end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 53fc2eb439..f18b69692b 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -113,23 +113,20 @@ end end - describe "redirecting pro users to the pro context" do + describe "livery used", feature: :alaveteli_pro do let(:pro_user) { FactoryBot.create(:pro_user) } + before { sign_in pro_user } + context "when showing pros their own requests" do context "when the request is embargoed" do let(:info_request) do FactoryBot.create(:embargoed_request, user: pro_user) end - it "should always redirect to the pro version of the page" do - with_feature_enabled(:alaveteli_pro) do - sign_in pro_user - get :show, params: { url_title: info_request.url_title } - expect(response).to redirect_to( - show_alaveteli_pro_request_path(url_title: info_request.url_title) - ) - end + it 'uses the pro livery' do + get :show, params: { url_title: info_request.url_title } + expect(assigns[:in_pro_area]).to be true end end @@ -138,12 +135,9 @@ FactoryBot.create(:info_request, user: pro_user) end - it "should not redirect to the pro version of the page" do - with_feature_enabled(:alaveteli_pro) do - sign_in pro_user - get :show, params: { url_title: info_request.url_title } - expect(response).to be_successful - end + it "should not use the pro livery" do + get :show, params: { url_title: info_request.url_title } + expect(assigns[:in_pro_area]).to be false end end end @@ -157,31 +151,16 @@ FactoryBot.create(:embargoed_request, user: pro_user) end - it 'redirects to the pro version of the page' do - with_feature_enabled(:alaveteli_pro) do - sign_in pro_user - get :show, params: { url_title: info_request.url_title } - expect(response).to redirect_to show_alaveteli_pro_request_path( - url_title: info_request.url_title) - end - end - it 'uses the pro livery' do - with_feature_enabled(:alaveteli_pro) do - sign_in pro_user - get :show, params: { url_title: info_request.url_title, pro: '1' } - expect(assigns[:in_pro_area]).to be true - end + get :show, params: { url_title: info_request.url_title } + expect(assigns[:in_pro_area]).to be true end end context "when showing pros a someone else's request" do - it "should not redirect to the pro version of the page" do - with_feature_enabled(:alaveteli_pro) do - sign_in pro_user - get :show, params: { url_title: 'why_do_you_have_such_a_fancy_dog' } - expect(response).to be_successful - end + it "should not user the pro livery" do + get :show, params: { url_title: 'why_do_you_have_such_a_fancy_dog' } + expect(assigns[:in_pro_area]).to be false end end end diff --git a/spec/integration/alaveteli_dsl.rb b/spec/integration/alaveteli_dsl.rb index 8abccda7de..40da2ed5de 100644 --- a/spec/integration/alaveteli_dsl.rb +++ b/spec/integration/alaveteli_dsl.rb @@ -4,7 +4,7 @@ def browse_request(url_title) end def browse_pro_request(url_title) - visit "/alaveteli_pro/info_requests/#{url_title}" + browse_request(url_title) end def create_request(public_body) diff --git a/spec/routing/redirects_spec.rb b/spec/routing/redirects_spec.rb index 74f8c32446..70c8ccac05 100644 --- a/spec/routing/redirects_spec.rb +++ b/spec/routing/redirects_spec.rb @@ -87,4 +87,12 @@ get('/help/about?locale=en_GB') expect(response).to redirect_to('/help/about') end + + it 'redirects old pro request routes', feature: :alaveteli_pro do + get('/alaveteli_pro/info_requests/my-request') + expect(response).to redirect_to('/request/my-request') + + get('/alaveteli_pro/info_requests/new') + expect(response).to_not redirect_to('/request/new') + end end diff --git a/spec/services/info_request_batch_metrics_spec.rb b/spec/services/info_request_batch_metrics_spec.rb index bf111359ec..50dab8e0e0 100644 --- a/spec/services/info_request_batch_metrics_spec.rb +++ b/spec/services/info_request_batch_metrics_spec.rb @@ -8,8 +8,7 @@ subject(:metrics) { described_class.new(batch).metrics } it 'generates info request batch metrics' do - request_url = 'http://test.host/alaveteli_pro/info_requests/' + - request.url_title + request_url = 'http://test.host/request/' + request.url_title authority_name = request.public_body.name is_expected.to match_array( diff --git a/spec/views/admin_request/show.html.erb_spec.rb b/spec/views/admin_request/show.html.erb_spec.rb index 0727ff1368..a12de51a20 100644 --- a/spec/views/admin_request/show.html.erb_spec.rb +++ b/spec/views/admin_request/show.html.erb_spec.rb @@ -25,7 +25,7 @@ it 'links to the pro request page' do render expect(rendered). - to match(show_alaveteli_pro_request_url(info_request.url_title)) + to match(show_request_url(info_request.url_title)) end it 'includes embargo information' do