Merge ab5645b into 0ee6a01

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

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

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)

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

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) &&

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)
       )

app/views/admin_request/show.html.erb

@@ -34,7 +34,7 @@
             <tr>
               <% if @info_request.embargo %>
                 <th>Request page:</th>
-                <td><%= link_to show_alaveteli_pro_request_url(@info_request.url_title), show_alaveteli_pro_request_path(@info_request.url_title) %></td>
+                <td><%= link_to show_request_url(@info_request.url_title), show_request_path(@info_request.url_title) %></td>
               <% else %>
                 <th>Public page:</th>
                 <td><%= link_to request_url(@info_request), request_path(@info_request) %></td>

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',

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

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)

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

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

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

spec/controllers/request_controller_spec.rb

@@ -122,13 +122,11 @@
           FactoryBot.create(:embargoed_request, user: pro_user)
         end
 
-        it "should always redirect to the pro version of the page" do
+        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 }
-            expect(response).to redirect_to(
-              show_alaveteli_pro_request_path(url_title: info_request.url_title)
-            )
+            get :show, params: { url_title: info_request.url_title, pro: '1' }
+            expect(assigns[:in_pro_area]).to be true
           end
         end
       end
@@ -157,15 +155,6 @@
         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

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}"
+    visit "/request/#{url_title}"
   end
 
   def create_request(public_body)

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

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(

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