Merge branch '7784-url-request-id-permutations' into develop

app/controllers/application_controller.rb

@@ -279,10 +279,7 @@ def ask_to_login(as: nil, **reason_params)
   # Return logged in user
   def authenticated_user
     return unless session[:user_id]
-
-    @user ||= User.find_by(
-      id: session[:user_id], login_token: session[:user_login_token]
-    )
+    @user ||= User.authenticate_from_session(session)
   end
 
   # For CanCanCan and other libs which need a Devise-like current_user method

app/controllers/attachments_controller.rb

@@ -58,7 +58,7 @@ def find_info_request
       if public_token?
         InfoRequest.find_by!(public_token: public_token)
       else
-        InfoRequest.find(params[:id])
+        InfoRequest.find_by!(url_title: params[:request_url_title])
       end
   end
 

app/controllers/citations_controller.rb

@@ -16,7 +16,7 @@ def create
 
     if @citation.save
       notice = _('Citation successfully created.')
-      redirect_to show_request_path(url_title: info_request.url_title),
+      redirect_to show_request_path(info_request.url_title),
                   notice: notice
     else
       render :new

app/controllers/followups_controller.rb

@@ -190,11 +190,12 @@ def set_incoming_message
 
   def set_info_request
     if current_user
-      @info_request =
-        current_user.info_requests.find_by(id: params[:request_id].to_i)
+      @info_request = current_user.info_requests.
+        find_by(url_title: params[:request_url_title])
     end
 
-    @info_request ||= InfoRequest.not_embargoed.find(params[:request_id].to_i)
+    @info_request ||= InfoRequest.not_embargoed.
+      find_by!(url_title: params[:request_url_title])
   end
 
   def set_last_request_data

app/controllers/refusal_advice_controller.rb

@@ -39,10 +39,10 @@ def internal_redirect_to
 
     case action.target[:internal]
     when 'followup'
-      redirect_to new_request_followup_path(request_id: info_request.id)
+      redirect_to new_request_followup_path(info_request.url_title)
     when 'internal_review'
       redirect_to new_request_followup_path(
-        request_id: info_request.id, internal_review: '1'
+        info_request.url_title, internal_review: '1'
       )
     when 'new_request'
       redirect_to new_request_to_body_path(

app/controllers/reports_controller.rb

@@ -49,9 +49,8 @@ def new
   private
 
   def set_info_request
-    @info_request = InfoRequest
-                      .not_embargoed
-                        .find_by_url_title!(params[:request_id])
+    @info_request = InfoRequest.not_embargoed.
+      find_by_url_title!(params[:request_url_title])
   end
 
   def set_comment

app/controllers/request_controller.rb

@@ -11,7 +11,6 @@ class RequestController < ApplicationController
 
   before_action :check_read_only, only: [:new, :upload_response]
   before_action :set_render_recaptcha, only: [ :new ]
-  before_action :redirect_numeric_id_to_url_title, only: [:show]
   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]
@@ -331,7 +330,7 @@ def new
       end
     end
 
-    redirect_to show_request_path(url_title: @info_request.url_title)
+    redirect_to show_request_path(@info_request.url_title)
   end
 
   # Used for links from polymorphic URLs e.g. in Atom feeds - just redirect to
@@ -712,18 +711,6 @@ def set_render_recaptcha
                         (!@user || !@user.confirmed_not_spam?)
   end
 
-  def redirect_numeric_id_to_url_title
-    # Look up by old style numeric identifiers
-    if params[:url_title].match(/^[0-9]+$/)
-      @info_request = InfoRequest.find(params[:url_title].to_i)
-      # We don't want to leak the title of embargoed or hidden requests, so
-      # don't even redirect on if the user can't access the request
-      return render_hidden if cannot?(:read, @info_request)
-
-      redirect_to request_url(@info_request, format: params[:format])
-    end
-  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

app/controllers/request_game_controller.rb

@@ -52,7 +52,7 @@ def show
       )
       return
     end
-    redirect_to show_request_url(url_title: url_title)
+    redirect_to show_request_url(url_title)
   end
 
   def stop

app/controllers/widget_votes_controller.rb

@@ -37,7 +37,8 @@ def check_widget_config
   end
 
   def find_info_request
-    @info_request = InfoRequest.not_embargoed.find(params[:request_id])
+    @info_request = InfoRequest.not_embargoed.
+      find_by!(url_title: params[:request_url_title])
   end
 
   def check_prominence

app/controllers/widgets_controller.rb

@@ -42,7 +42,7 @@ def check_widget_config
   end
 
   def find_info_request
-    @info_request = InfoRequest.find(params[:request_id])
+    @info_request = InfoRequest.find_by!(url_title: params[:request_url_title])
   end
 
   def check_prominence

app/helpers/info_request_helper.rb

@@ -112,9 +112,12 @@ def status_text_waiting_response_very_overdue(info_request, _opts = {})
       str += ' '
       str += _('You can <strong>complain</strong> by')
       str += ' '
-      str += link_to _('requesting an internal review'),
-                    new_request_followup_path(request_id: info_request.id) +
-                    '?internal_review=1'
+      str += link_to(
+        _('requesting an internal review'),
+        new_request_followup_path(
+          info_request.url_title, internal_review: 1
+        )
+      )
       str += '.'
     end
 
@@ -286,13 +289,13 @@ def attachment_url(attachment, options = {})
     attach_params = attachment_params(attachment, options)
 
     if options[:html] && public_token?
-      share_attachment_as_html_url(attach_params)
+      share_attachment_as_html_url(*attach_params)
     elsif options[:html]
-      get_attachment_as_html_url(attach_params)
+      get_attachment_as_html_url(*attach_params)
     elsif public_token?
-      share_attachment_url(attach_params)
+      share_attachment_url(*attach_params)
     else
-      get_attachment_url(attach_params)
+      get_attachment_url(*attach_params)
     end
   end
 
@@ -312,7 +315,8 @@ def attachment_params(attachment, options = {})
     if public_token?
       attach_params[:public_token] = public_token
     else
-      attach_params[:id] = attachment.incoming_message.info_request_id
+      attach_params[:request_url_title] = attachment.incoming_message.
+        info_request.url_title
     end
     if options[:html]
       attach_params[:file_name] = "#{attachment.display_filename}.html"
@@ -325,7 +329,7 @@ def attachment_params(attachment, options = {})
 
     attach_params[:project_id] = @project.id if @project
 
-    attach_params
+    [attach_params.delete(:id), attach_params]
   end
 
   def public_token?

app/helpers/link_to_helper.rb

@@ -10,7 +10,7 @@ module LinkToHelper
 
   # Requests
   def request_url(info_request, options = {})
-    show_request_url({ url_title: info_request.url_title }.merge(options))
+    show_request_url(info_request.url_title, options)
   end
 
   def request_path(info_request, options = {})
@@ -80,9 +80,12 @@ def new_response_url(info_request, incoming_message)
   def respond_to_last_url(info_request, options = {})
     last_response = info_request.get_last_public_response
     if last_response.nil?
-      new_request_followup_url(options.merge(request_id: info_request.id))
+      new_request_followup_url(info_request.url_title, options)
     else
-      new_request_incoming_followup_url(options.merge(request_id: info_request.id, incoming_message_id: last_response.id))
+      new_request_incoming_followup_url(
+        info_request.url_title,
+        options.merge(incoming_message_id: last_response.id)
+      )
     end
   end
 

app/mailers/request_mailer.rb

@@ -133,7 +133,7 @@ def very_overdue_alert(info_request, user)
   # Tell the requester that they need to say if the new response
   # contains info or not
   def new_response_reminder_alert(info_request, incoming_message)
-    target = show_request_url(info_request,
+    target = show_request_url(info_request.url_title,
                               anchor: 'describe_state_form_1',
                               only_path: true)
     @url = signin_url(r: target)
@@ -162,9 +162,11 @@ def old_unclassified_updated(info_request)
 
   # Tell the requester that they need to clarify their request
   def not_clarified_alert(info_request, incoming_message)
-    respond_url = new_request_incoming_followup_url(request_id: info_request.id,
-                                                    incoming_message_id: incoming_message.id,
-                                                    anchor: 'followup')
+    respond_url = new_request_incoming_followup_url(
+      info_request.url_title,
+      incoming_message_id: incoming_message.id,
+      anchor: 'followup'
+    )
     @url = respond_url
     @incoming_message = incoming_message
     @info_request = info_request

app/models/alaveteli_pro/activity_list/overdue.rb

@@ -10,7 +10,9 @@ def call_to_action
       end
 
       def call_to_action_url
-        new_request_followup_path(request_id: event.info_request.id, anchor: 'followup')
+        new_request_followup_path(
+          event.info_request.url_title, anchor: 'followup'
+        )
       end
     end
   end

app/models/alaveteli_pro/activity_list/very_overdue.rb

@@ -10,9 +10,11 @@ def call_to_action
       end
 
       def call_to_action_url
-        new_request_followup_path(request_id: event.info_request.id,
-                                  anchor: 'followup',
-                                  internal_review: 1)
+        new_request_followup_path(
+          event.info_request.url_title,
+          anchor: 'followup',
+          internal_review: 1
+        )
       end
     end
   end

app/models/user.rb

@@ -239,6 +239,12 @@ def self.authenticate_from_form(params, specific_user_login = false)
     user
   end
 
+  def self.authenticate_from_session(session)
+    return unless session[:user_id]
+
+    find_by(id: session[:user_id], login_token: session[:user_login_token])
+  end
+
   # Case-insensitively find a user from their email
   def self.find_user_by_email(email)
     return nil if email.blank?

app/views/alaveteli_pro/info_requests/_after_actions.html.erb

@@ -8,14 +8,14 @@
           <ul class="action-menu__menu__submenu">
             <li>
               <% if last_response.nil? %>
-                <%= link_to _("Send a followup"), new_request_followup_path(:request_id => info_request.id, :anchor => 'followup') %>
+                <%= link_to _("Send a followup"), new_request_followup_path(info_request.url_title, :anchor => 'followup') %>
               <% else %>
-                <%= link_to _("Write a reply"), new_request_incoming_followup_path(:request_id => info_request.id, :incoming_message_id => last_response.id, :anchor => 'followup') %>
+                <%= link_to _("Write a reply"), new_request_incoming_followup_path(info_request.url_title, :incoming_message_id => last_response.id, :anchor => 'followup') %>
               <% end %>
             </li>
 
             <li>
-              <%= link_to _("Request an internal review"), new_request_followup_path(:request_id => info_request.id, :internal_review => '1', :anchor => 'followup') %>
+              <%= link_to _("Request an internal review"), new_request_followup_path(info_request.url_title, :internal_review => '1', :anchor => 'followup') %>
             </li>
 
             <li>

app/views/comment/_single_comment.html.erb

@@ -35,7 +35,7 @@
     <% link_to_this_url = comment_url(comment) %>
 
     <% report_path =
-         new_request_report_path(request_id: @info_request.url_title,
+         new_request_report_path(@info_request.url_title,
                                  comment_id: comment.id) %>
 
     <%= render partial: 'request/correspondence_footer',

app/views/followups/_choose_recipient.html.erb

@@ -10,29 +10,27 @@
         <li>
           <%= link_to(_('the main FOI contact address for {{public_body}}',
                         public_body: name),
-                      new_request_followup_path(request_id: @info_request.id)) %>
+                      new_request_followup_path(@info_request.url_title)) %>
         </li>
       <% else %>
         <% if id.present? %>
           <% if @info_request.public_body.request_email == email %>
             <li>
               <%= link_to(_('the main FOI contact address for {{public_body}}',
                             public_body: name),
-                          new_request_followup_path(request_id: @info_request.id)) %>
+                          new_request_followup_path(@info_request.url_title)) %>
             </li>
           <% else %>
             <li>
               <%= link_to name,
-                          new_request_incoming_followup_path(
-                            request_id: @info_request.id,
-                            incoming_message_id: id) %>
+                          new_request_incoming_followup_path(@info_request.url_title, incoming_message_id: id) %>
             </li>
           <% end %>
         <% else %>
           <li>
             <%= link_to(_('the main FOI contact address for {{public_body}}',
                           public_body: name),
-                        new_request_followup_path(request_id: @info_request.id)) %>
+                        new_request_followup_path(@info_request.url_title)) %>
           </li>
         <% end %>
       <% end %>

app/views/followups/_followup.html.erb

@@ -117,11 +117,14 @@
 
     <% form_url =
          if incoming_message.nil?
-           preview_request_followups_url(request_id: @info_request.id)
+           preview_request_followups_url(
+             request_url_title: @info_request.url_title
+           )
          else
            preview_request_followups_url(
-             request_id: @info_request.id,
-             incoming_message_id: incoming_message.id)
+             request_url_title: @info_request.url_title,
+             incoming_message_id: incoming_message.id
+           )
          end -%>
     <%= form_for @outgoing_message,
                  html: { id: 'followup_form' },

app/views/reports/new.html.erb

@@ -11,7 +11,7 @@
     </p>
     <p><%= _("Why specifically do you consider this request unsuitable?") %></p>
 
-    <%= form_tag request_report_path(:request_id => @info_request.url_title) do %>
+    <%= form_tag request_report_path(@info_request.url_title) do %>
       <p>
         <label class="form_label" for="reason"><%= _('Reason:') %></label>
         <%= select_tag :reason, options_for_select(@report_reasons, @reason), :prompt => _("Choose a reason") %>

app/views/request/_act.html.erb

@@ -44,7 +44,7 @@
 <% if AlaveteliConfiguration::enable_widgets %>
   <div class="act_link">
     <i class="act-link-icon act-link-icon--widget"></i>
-    <%= link_to _("Create a widget for this request"), new_request_widget_path(info_request) %>
+    <%= link_to _("Create a widget for this request"), new_request_widget_path(info_request.url_title) %>
   </div>
 <% end %>