diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bb65024c03..9fcdc26004 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index 1c1fdedfac..0c02a76e80 100644 --- a/app/controllers/attachments_controller.rb +++ b/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 diff --git a/app/controllers/citations_controller.rb b/app/controllers/citations_controller.rb index 0034ebca64..5f26fe1ac2 100644 --- a/app/controllers/citations_controller.rb +++ b/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 diff --git a/app/controllers/followups_controller.rb b/app/controllers/followups_controller.rb index 0e33ba77be..11cd2439bb 100644 --- a/app/controllers/followups_controller.rb +++ b/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 diff --git a/app/controllers/refusal_advice_controller.rb b/app/controllers/refusal_advice_controller.rb index 84b190f2a7..a8b99c6a14 100644 --- a/app/controllers/refusal_advice_controller.rb +++ b/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( diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 9913f6f721..30e590bf5b 100644 --- a/app/controllers/reports_controller.rb +++ b/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 diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index d486b621d1..a5abeec4ef 100644 --- a/app/controllers/request_controller.rb +++ b/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 diff --git a/app/controllers/request_game_controller.rb b/app/controllers/request_game_controller.rb index 3f171493ef..12bda32f83 100644 --- a/app/controllers/request_game_controller.rb +++ b/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 diff --git a/app/controllers/widget_votes_controller.rb b/app/controllers/widget_votes_controller.rb index d5338ddc0c..7e097b4df0 100644 --- a/app/controllers/widget_votes_controller.rb +++ b/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 diff --git a/app/controllers/widgets_controller.rb b/app/controllers/widgets_controller.rb index 4484734993..1a57827989 100644 --- a/app/controllers/widgets_controller.rb +++ b/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 diff --git a/app/helpers/info_request_helper.rb b/app/helpers/info_request_helper.rb index 135b65b865..a39c74a113 100644 --- a/app/helpers/info_request_helper.rb +++ b/app/helpers/info_request_helper.rb @@ -112,9 +112,12 @@ def status_text_waiting_response_very_overdue(info_request, _opts = {}) str += ' ' str += _('You can complain 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? diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb index 54004ffc83..4c0f860745 100755 --- a/app/helpers/link_to_helper.rb +++ b/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 diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index d614721967..b464a88d61 100644 --- a/app/mailers/request_mailer.rb +++ b/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 diff --git a/app/models/alaveteli_pro/activity_list/overdue.rb b/app/models/alaveteli_pro/activity_list/overdue.rb index 011c15cf7e..c70d9220a4 100644 --- a/app/models/alaveteli_pro/activity_list/overdue.rb +++ b/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 diff --git a/app/models/alaveteli_pro/activity_list/very_overdue.rb b/app/models/alaveteli_pro/activity_list/very_overdue.rb index 3b78bd5db5..a862c187fe 100644 --- a/app/models/alaveteli_pro/activity_list/very_overdue.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index ee15eee72e..f3ce6c37af 100644 --- a/app/models/user.rb +++ b/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? diff --git a/app/views/alaveteli_pro/info_requests/_after_actions.html.erb b/app/views/alaveteli_pro/info_requests/_after_actions.html.erb index d9b8306345..e2af845947 100644 --- a/app/views/alaveteli_pro/info_requests/_after_actions.html.erb +++ b/app/views/alaveteli_pro/info_requests/_after_actions.html.erb @@ -8,14 +8,14 @@