Skip to content
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
21 changes: 15 additions & 6 deletions app/controllers/reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ class ReportsController < ApplicationController
before_action :check_database_writable, :only => [:new, :create]

def new
if required_new_report_params_present?
@report = Report.new
@report.issue = Issue.find_or_initialize_by(create_new_report_params)
else
head :bad_request
end
return head :bad_request unless required_new_report_params_present?
return redirect_to root_url, :warning => t(".invalid_report_params") unless new_report_params_valid?

@report = Report.new
@report.issue = Issue.find_or_initialize_by(create_new_report_params)
end

def create
Expand Down Expand Up @@ -46,6 +45,16 @@ def required_new_report_params_present?
create_new_report_params["reportable_id"].present? && create_new_report_params["reportable_type"].present?
end

def new_report_params_valid?
id = create_new_report_params["reportable_id"]
type = create_new_report_params["reportable_type"]

(type == "DiaryComment" && DiaryComment.exists?(id)) ||
Comment thread
tomhughes marked this conversation as resolved.
(type == "DiaryEntry" && DiaryEntry.exists?(id)) ||
(type == "Note" && Note.exists?(id)) ||
(type == "User" && User.exists?(id))
end

def create_new_report_params
params.permit(:reportable_id, :reportable_type)
end
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1709,6 +1709,7 @@ en:
personal_label: This note contains personal data
abusive_label: This note is abusive
other_label: Other
invalid_report_params: The item you are trying to report could not be found
create:
successful_report: Your report has been registered successfully
provide_details: Please provide the required details
Expand Down
15 changes: 15 additions & 0 deletions test/controllers/reports_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,21 @@ def test_new_missing_parameters
assert_response :bad_request
end

def test_new_invalid_parameters
target_user = create(:user)
session_for(target_user)

# Invalid id
get new_report_path(:reportable_id => target_user.id + 1, :reportable_type => "User")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is just incrementing the user ID a safe way of guaranteeing an invalid user? I guess it probably is so long as users are created in monotonic order.

Also we should probably test the other item types?

assert_redirected_to root_path
assert_match(/The item you are trying to report could not be found/, flash[:warning])

# Invalid type
get new_report_path(:reportable_id => target_user.id, :reportable_type => "Users")
assert_redirected_to root_path
assert_match(/The item you are trying to report could not be found/, flash[:warning])
end

def test_new_report_without_login
target_user = create(:user)
get new_report_path(:reportable_id => target_user.id, :reportable_type => "User")
Expand Down