From: Tom Hughes Date: Sun, 10 Jun 2018 16:02:12 +0000 (+0100) Subject: Merge branch 'master' into next X-Git-Tag: live~3010^2~6 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/d3700e6201b4b78a70bbb2941572edc985b63c2c Merge branch 'master' into next --- d3700e6201b4b78a70bbb2941572edc985b63c2c diff --cc .rubocop_todo.yml index 5846de44f,362cfcb32..da801d681 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@@ -67,7 -67,7 +67,7 @@@ Metrics/AbcSize # Offense count: 41 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 250 - Max: 247 ++ Max: 257 # Offense count: 12 # Configuration parameters: CountBlocks. diff --cc app/views/diary_entry/_diary_comment.html.erb index 8e17b0954,8565ecc67..4ac9a0f96 --- a/app/views/diary_entry/_diary_comment.html.erb +++ b/app/views/diary_entry/_diary_comment.html.erb @@@ -1,13 -1,10 +1,15 @@@
<%= user_thumbnail diary_comment.user %> -

<%= raw(t('diary_entry.diary_comment.comment_from', :link_user => (link_to h(diary_comment.user.display_name), user_path(diary_comment.user)), :comment_created_at => link_to(l(diary_comment.created_at, :format => :friendly), :anchor => "comment#{diary_comment.id}"))) %> -

<%= raw(t('.comment_from', :link_user => (link_to h(diary_comment.user.display_name), user_path(diary_comment.user)), :comment_created_at => link_to(l(diary_comment.created_at, :format => :friendly), :anchor => "comment#{diary_comment.id}"))) %>

++

<%= raw(t('.comment_from', :link_user => (link_to h(diary_comment.user.display_name), user_path(diary_comment.user)), :comment_created_at => link_to(l(diary_comment.created_at, :format => :friendly), :anchor => "comment#{diary_comment.id}"))) %> + <% if current_user and diary_comment.user.id != current_user.id %> + | <%= report_link(t(".report"), diary_comment) %> + <% end %> +

+
<%= diary_comment.body.to_html %>
- <%= if_administrator(:span) do %> - <%= link_to t('diary_entry.diary_comment.hide_link'), hide_diary_comment_path(:display_name => diary_comment.diary_entry.user.display_name, :id => diary_comment.diary_entry.id, :comment => diary_comment.id), :method => :post, :data=> { :confirm => t('diary_entry.diary_comment.confirm') } %> + <% if current_user && current_user.administrator? %> + + <%= link_to t('.hide_link'), hide_diary_comment_path(:display_name => diary_comment.diary_entry.user.display_name, :id => diary_comment.diary_entry.id, :comment => diary_comment.id), :method => :post, :data=> { :confirm => t('.confirm') } %> + <% end %>
diff --cc app/views/diary_entry/_diary_entry.html.erb index ce311c168,848221a28..930f20e5e --- a/app/views/diary_entry/_diary_entry.html.erb +++ b/app/views/diary_entry/_diary_entry.html.erb @@@ -22,23 -22,17 +22,25 @@@ diff --cc app/views/user/view.html.erb index f797c4ba4,3a09165b9..0acb3f014 --- a/app/views/user/view.html.erb +++ b/app/views/user/view.html.erb @@@ -102,11 -102,6 +102,11 @@@ <% end %> - <% if current_user and @this_user.id != current_user.id %> ++ <% if current_user and @user.id != current_user.id %> +
  • - <%= report_link(t(".report"), @this_user) %> ++ <%= report_link(t(".report"), @user) %> +
    • + <% end %> <% end %> diff --cc test/models/issue_test.rb index 8804b6b28,000000000..257212953 mode 100644,000000..100644 --- a/test/models/issue_test.rb +++ b/test/models/issue_test.rb @@@ -1,58 -1,0 +1,58 @@@ +require "test_helper" + +class IssueTest < ActiveSupport::TestCase + def test_assigned_role + issue = create(:issue) + + assert issue.valid? + issue.assigned_role = "bogus" - assert !issue.valid? ++ assert_not issue.valid? + end + + def test_reported_user + note = create(:note_comment, :author => create(:user)).note + anonymous_note = create(:note_comment, :author => nil).note + user = create(:user) + create(:language, :code => "en") + diary_entry = create(:diary_entry) + issue = Issue.new + + issue.reportable = user + issue.save! + assert_equal issue.reported_user, user + + issue.reportable = note + issue.save! + assert_equal issue.reported_user, note.author + + issue.reportable = anonymous_note + issue.save! + assert_nil issue.reported_user + + issue.reportable = diary_entry + issue.save! + assert_equal issue.reported_user, diary_entry.user + end + + def test_default_assigned_role + create(:language, :code => "en") + diary_entry = create(:diary_entry) + note = create(:note_with_comments) + + issue = Issue.new + issue.reportable = diary_entry + issue.save! + assert_equal "administrator", issue.assigned_role + + issue = Issue.new + issue.reportable = note + issue.save! + assert_equal "moderator", issue.assigned_role + + # check the callback doesn't override an explicitly set role + issue.assigned_role = "administrator" + issue.save! + issue.reload + assert_equal "administrator", issue.assigned_role + end +end diff --cc test/models/report_test.rb index 810035c9d,000000000..b65753b82 mode 100644,000000..100644 --- a/test/models/report_test.rb +++ b/test/models/report_test.rb @@@ -1,35 -1,0 +1,35 @@@ +require "test_helper" + +class ReportTest < ActiveSupport::TestCase + def test_issue_required + report = create(:report) + + assert report.valid? + report.issue = nil - assert !report.valid? ++ assert_not report.valid? + end + + def test_user_required + report = create(:report) + + assert report.valid? + report.user = nil - assert !report.valid? ++ assert_not report.valid? + end + + def test_details_required + report = create(:report) + + assert report.valid? + report.details = "" - assert !report.valid? ++ assert_not report.valid? + end + + def test_category_required + report = create(:report) + + assert report.valid? + report.category = "" - assert !report.valid? ++ assert_not report.valid? + end +end diff --cc test/system/issues_test.rb index 5e60ee0f0,000000000..b77908f07 mode 100644,000000..100644 --- a/test/system/issues_test.rb +++ b/test/system/issues_test.rb @@@ -1,116 -1,0 +1,116 @@@ +require "application_system_test_case" + +class IssuesTest < ApplicationSystemTestCase + include IssuesHelper + + def test_view_issues_not_logged_in + visit issues_path + assert page.has_content?(I18n.t("user.login.title")) + end + + def test_view_issues_normal_user + sign_in_as(create(:user)) + + visit issues_path + assert page.has_content?(I18n.t("application.require_moderator_or_admin.not_a_moderator_or_admin")) + end + + def test_view_no_issues + sign_in_as(create(:moderator_user)) + + visit issues_path + assert page.has_content?(I18n.t("issues.index.issues_not_found")) + end + + def test_view_issues + sign_in_as(create(:moderator_user)) + issues = create_list(:issue, 3, :assigned_role => "moderator") + + visit issues_path + assert page.has_content?(issues.first.reported_user.display_name) + end + + def test_view_issues_with_no_reported_user + sign_in_as(create(:moderator_user)) + anonymous_note = create(:note_with_comments) + issue = create(:issue, :reportable => anonymous_note) + + visit issues_path + assert page.has_content?(reportable_title(anonymous_note)) + + visit issue_path(issue) + assert page.has_content?(reportable_title(anonymous_note)) + end + + def test_search_issues_by_user + good_user = create(:user) + bad_user = create(:user) + create(:issue, :reportable => bad_user, :reported_user => bad_user, :assigned_role => "administrator") + + sign_in_as(create(:administrator_user)) + + # No issues against the user + visit issues_path + fill_in "search_by_user", :with => good_user.display_name + click_on "Search" + assert page.has_content?(I18n.t("issues.index.issues_not_found")) + + # User doesn't exist + visit issues_path + fill_in "search_by_user", :with => "Nonexistant User" + click_on "Search" + assert page.has_content?(I18n.t("issues.index.user_not_found")) + + # Find Issue against bad_user + visit issues_path + fill_in "search_by_user", :with => bad_user.display_name + click_on "Search" - assert !page.has_content?(I18n.t("issues.index.issues_not_found")) ++ assert_not page.has_content?(I18n.t("issues.index.issues_not_found")) + end + + def test_commenting + issue = create(:issue) + sign_in_as(create(:moderator_user)) + + visit issue_path(issue) + + fill_in :issue_comment_body, :with => "test comment" + click_on "Submit" + assert page.has_content?(I18n.t("issue_comments.create.comment_created")) + assert page.has_content?("test comment") + + issue.reload + assert_equal issue.comments.first.body, "test comment" + end + + def test_reassign_issue + issue = create(:issue) + assert_equal "administrator", issue.assigned_role + sign_in_as(create(:administrator_user)) + + visit issue_path(issue) + + fill_in :issue_comment_body, :with => "reassigning to moderators" + check :reassign + click_on "Submit" + + issue.reload + assert_equal "moderator", issue.assigned_role + end + + def test_issue_index_with_multiple_roles + user1 = create(:user) + user2 = create(:user) + issue1 = create(:issue, :reportable => user1, :assigned_role => "administrator") + issue2 = create(:issue, :reportable => user2, :assigned_role => "moderator") + + user = create(:administrator_user) + create(:user_role, :user => user, :role => "moderator") + sign_in_as(user) + + visit issues_path + + assert page.has_link?(I18n.t("issues.index.reports_count", :count => issue1.reports_count), :href => issue_path(issue1)) + assert page.has_link?(I18n.t("issues.index.reports_count", :count => issue2.reports_count), :href => issue_path(issue2)) + end +end diff --cc test/system/report_diary_comment_test.rb index 05b76f695,000000000..852bcfa65 mode 100644,000000..100644 --- a/test/system/report_diary_comment_test.rb +++ b/test/system/report_diary_comment_test.rb @@@ -1,35 -1,0 +1,35 @@@ +require "application_system_test_case" + +class ReportDiaryCommentTest < ApplicationSystemTestCase + def setup + create(:language, :code => "en") + @diary_entry = create(:diary_entry) + @comment = create(:diary_comment, :diary_entry => @diary_entry) + end + + def test_no_link_when_not_logged_in + visit diary_entry_path(@diary_entry.user.display_name, @diary_entry) + assert page.has_content?(@comment.body) + - assert !page.has_content?(I18n.t("diary_entry.diary_comment.report")) ++ assert_not page.has_content?(I18n.t("diary_entry.diary_comment.report")) + end + + def test_it_works + sign_in_as(create(:user)) + visit diary_entry_path(@diary_entry.user.display_name, @diary_entry) + assert page.has_content? @diary_entry.title + + click_on I18n.t("diary_entry.diary_comment.report") + assert page.has_content? "Report" + assert page.has_content? I18n.t("reports.new.disclaimer.intro") + + choose I18n.t("reports.new.categories.diary_comment.spam") + fill_in "report_details", :with => "This comment is spam" + click_on "Create Report" + + assert page.has_content? "Your report has been registered sucessfully" + + assert_equal 1, Issue.count + assert Issue.last.reportable == @comment + end +end diff --cc test/system/report_diary_entry_test.rb index 86d301e3c,000000000..15752777c mode 100644,000000..100644 --- a/test/system/report_diary_entry_test.rb +++ b/test/system/report_diary_entry_test.rb @@@ -1,58 -1,0 +1,58 @@@ +require "application_system_test_case" + +class ReportDiaryEntryTest < ApplicationSystemTestCase + def setup + create(:language, :code => "en") + @diary_entry = create(:diary_entry) + end + + def test_no_link_when_not_logged_in + visit diary_entry_path(@diary_entry.user.display_name, @diary_entry) + assert page.has_content?(@diary_entry.title) + - assert !page.has_content?(I18n.t("diary_entry.diary_entry.report")) ++ assert_not page.has_content?(I18n.t("diary_entry.diary_entry.report")) + end + + def test_it_works + sign_in_as(create(:user)) + visit diary_entry_path(@diary_entry.user.display_name, @diary_entry) + assert page.has_content? @diary_entry.title + + click_on I18n.t("diary_entry.diary_entry.report") + assert page.has_content? "Report" + assert page.has_content? I18n.t("reports.new.disclaimer.intro") + + choose I18n.t("reports.new.categories.diary_entry.spam") + fill_in "report_details", :with => "This is advertising" + click_on "Create Report" + + assert page.has_content? "Your report has been registered sucessfully" + end + + def test_it_reopens_issue + issue = create(:issue, :reportable => @diary_entry) + issue.resolve! + + sign_in_as(create(:user)) + visit diary_entry_path(@diary_entry.user.display_name, @diary_entry) + assert page.has_content? @diary_entry.title + + click_on I18n.t("diary_entry.diary_entry.report") + assert page.has_content? "Report" + assert page.has_content? I18n.t("reports.new.disclaimer.intro") + + choose I18n.t("reports.new.categories.diary_entry.spam") + fill_in "report_details", :with => "This is advertising" + click_on "Create Report" + + issue.reload - assert !issue.resolved? ++ assert_not issue.resolved? + assert issue.open? + end + + def test_missing_report_params + sign_in_as(create(:user)) + visit new_report_path + assert page.has_content? I18n.t("reports.new.missing_params") + end +end diff --cc test/system/report_note_test.rb index f88588834,000000000..759306e08 mode 100644,000000..100644 --- a/test/system/report_note_test.rb +++ b/test/system/report_note_test.rb @@@ -1,49 -1,0 +1,49 @@@ +require "application_system_test_case" + +class ReportNoteTest < ApplicationSystemTestCase + def test_no_link_when_not_logged_in + note = create(:note_with_comments) + visit browse_note_path(note) + assert page.has_content?(note.comments.first.body) + - assert !page.has_content?(I18n.t("browse.note.report")) ++ assert_not page.has_content?(I18n.t("browse.note.report")) + end + + def test_can_report_anonymous_notes + note = create(:note_with_comments) + sign_in_as(create(:user)) + visit browse_note_path(note) + + click_on I18n.t("browse.note.report") + assert page.has_content? "Report" + assert page.has_content? I18n.t("reports.new.disclaimer.intro") + + choose I18n.t("reports.new.categories.note.spam") + fill_in "report_details", :with => "This is spam" + click_on "Create Report" + + assert page.has_content? "Your report has been registered sucessfully" + + assert_equal 1, Issue.count + assert Issue.last.reportable == note + end + + def test_can_report_notes_with_author + note = create(:note_comment, :author => create(:user)).note + sign_in_as(create(:user)) + visit browse_note_path(note) + + click_on I18n.t("browse.note.report") + assert page.has_content? "Report" + assert page.has_content? I18n.t("reports.new.disclaimer.intro") + + choose I18n.t("reports.new.categories.note.spam") + fill_in "report_details", :with => "This is spam" + click_on "Create Report" + + assert page.has_content? "Your report has been registered sucessfully" + + assert_equal 1, Issue.count + assert Issue.last.reportable == note + end +end diff --cc test/system/report_user_test.rb index 0331e3ad7,000000000..8bb7d9dd7 mode 100644,000000..100644 --- a/test/system/report_user_test.rb +++ b/test/system/report_user_test.rb @@@ -1,30 -1,0 +1,30 @@@ +require "application_system_test_case" + +class ReportUserTest < ApplicationSystemTestCase + def test_no_link_when_not_logged_in + note = create(:note_with_comments) + visit browse_note_path(note) + assert page.has_content?(note.comments.first.body) + - assert !page.has_content?(I18n.t("user.view.report")) ++ assert_not page.has_content?(I18n.t("user.view.report")) + end + + def test_can_report_user + user = create(:user) + sign_in_as(create(:user)) + visit user_path(user.display_name) + + click_on I18n.t("user.view.report") + assert page.has_content? "Report" + assert page.has_content? I18n.t("reports.new.disclaimer.intro") + + choose I18n.t("reports.new.categories.user.vandal") + fill_in "report_details", :with => "This user is a vandal" + click_on "Create Report" + + assert page.has_content? "Your report has been registered sucessfully" + + assert_equal 1, Issue.count + assert Issue.last.reportable == user + end +end