From: Tom Hughes Date: Sun, 25 Feb 2024 09:28:48 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/4463' X-Git-Tag: live~945 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/746bfd0a4838268eaf745e85e0e27b5acf9cf64a?hp=4ed85dc8db8c6587afa719bb6771b5303117e6e1 Merge remote-tracking branch 'upstream/pull/4463' --- diff --git a/app/assets/javascripts/index/changeset.js b/app/assets/javascripts/index/changeset.js index c6e77bc71..75a1f7b4d 100644 --- a/app/assets/javascripts/index/changeset.js +++ b/app/assets/javascripts/index/changeset.js @@ -26,14 +26,14 @@ OSM.Changeset = function (map) { }); } - function updateChangeset(form, method, url, include_data) { + function updateChangeset(method, url, include_data) { var data; - $(form).find("#comment-error").prop("hidden", true); - $(form).find("input[type=submit]").prop("disabled", true); + content.find("#comment-error").prop("hidden", true); + content.find("button[data-method][data-url]").prop("disabled", true); if (include_data) { - data = { text: $(form.text).val() }; + data = { text: content.find("textarea").val() }; } else { data = {}; } @@ -47,24 +47,21 @@ OSM.Changeset = function (map) { OSM.loadSidebarContent(window.location.pathname, page.load); }, error: function (xhr) { - $(form).find("#comment-error").text(xhr.responseText); - $(form).find("#comment-error").prop("hidden", false); - $(form).find("input[type=submit]").prop("disabled", false); + content.find("button[data-method][data-url]").prop("disabled", false); + content.find("#comment-error") + .text(xhr.responseText) + .prop("hidden", false) + .get(0).scrollIntoView({ block: "nearest" }); } }); } function initialize() { - content.find("input[name=comment]").on("click", function (e) { + content.find("button[data-method][data-url]").on("click", function (e) { e.preventDefault(); var data = $(e.target).data(); - updateChangeset(e.target.form, data.method, data.url, true); - }); - - content.find(".action-button").on("click", function (e) { - e.preventDefault(); - var data = $(e.target).data(); - updateChangeset(e.target.form, data.method, data.url); + var include_data = e.target.name === "comment"; + updateChangeset(data.method, data.url, include_data); }); content.find("textarea").on("input", function (e) { diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index b5bd5adec..1f7c45db5 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -647,11 +647,6 @@ tr.turn:hover { } } - span.action-button:hover { - cursor: pointer; - text-decoration: underline; - } - .note-description { overflow: hidden; margin: 0 0 10px 10px; diff --git a/app/views/browse/changeset.html.erb b/app/views/browse/changeset.html.erb index 501712717..ac5382b14 100644 --- a/app/views/browse/changeset.html.erb +++ b/app/views/browse/changeset.html.erb @@ -18,54 +18,41 @@ <% if current_user %>
<% if @changeset.subscribers.exists?(current_user.id) %> - + <% else %> - + <% end %>
<% end %> <% if @comments.length > 0 %> -
-
-
    - <% @comments.each do |comment| %> - <% if comment.visible %> -
  • - - <%= t(".comment_by_html", - :time_ago => friendly_date_ago(comment.created_at), - :user => link_to(comment.author.display_name, user_path(comment.author))) %> - <% if current_user and current_user.moderator? %> - — <%= t("javascripts.changesets.show.hide_comment") %> - <% end %> - -
    - <%= comment.body.to_html %> -
    -
    • - <% elsif current_user and current_user.moderator? %> -
  • - - <%= t(".hidden_comment_by_html", - :time_ago => friendly_date_ago(comment.created_at), - :user => link_to(comment.author.display_name, user_path(comment.author))) %> - — <%= t("javascripts.changesets.show.unhide_comment") %> - -
    - <%= comment.body.to_html %> -
    -
    • +
      + <% @comments.each do |comment| %> + <% next unless comment.visible || current_user&.moderator? %> +
    • + + <%= t comment.visible ? ".comment_by_html" : ".hidden_comment_by_html", + :time_ago => friendly_date_ago(comment.created_at), + :user => link_to(comment.author.display_name, user_path(comment.author)) %> + <% if current_user&.moderator? %> + — + <%= tag.button t("javascripts.changesets.show.#{comment.visible ? 'hide' : 'unhide'}_comment"), + :class => "btn btn-sm small btn-link link-secondary p-0 align-baseline", + :data => { :method => "POST", + :url => comment.visible ? changeset_comment_hide_url(comment) : changeset_comment_unhide_url(comment) } %> <% end %> - <% end %> -
    - -
+ +
+ <%= comment.body.to_html %> +
+ + <% end %> + <% end %> <% unless current_user %> -

+

<%= link_to(t(".join_discussion"), login_path(:referer => request.fullpath)) %>

<% end %> @@ -79,11 +66,11 @@
- " data-changeset-id="<%= @changeset.id %>" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled="1" class="btn btn-sm btn-primary" /> +
<% else %> -

+

<%= t(".still_open") %>

<% end %> diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 7931546d4..ef8f0e371 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -33,6 +33,11 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase end end + def sign_out + visit logout_path + click_on "Logout", :match => :first + end + def within_sidebar(&block) within "#sidebar_content", &block end diff --git a/test/controllers/browse_controller_test.rb b/test/controllers/browse_controller_test.rb index 1023d76ae..2bb743636 100644 --- a/test/controllers/browse_controller_test.rb +++ b/test/controllers/browse_controller_test.rb @@ -142,20 +142,6 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest browse_check :changeset_path, changeset.id, "browse/changeset" end - def test_read_changeset_hidden_comments - changeset = create(:changeset) - create_list(:changeset_comment, 3, :changeset => changeset) - create(:changeset_comment, :visible => false, :changeset => changeset) - - browse_check :changeset_path, changeset.id, "browse/changeset" - assert_select "div.changeset-comments ul li", :count => 3 - - session_for(create(:moderator_user)) - - browse_check :changeset_path, changeset.id, "browse/changeset" - assert_select "div.changeset-comments ul li", :count => 4 - end - def test_read_changeset_element_links changeset = create(:changeset) node = create(:node, :with_history, :changeset => changeset) diff --git a/test/integration/user_changeset_comments_test.rb b/test/integration/user_changeset_comments_test.rb deleted file mode 100644 index 2b95094fe..000000000 --- a/test/integration/user_changeset_comments_test.rb +++ /dev/null @@ -1,55 +0,0 @@ -require "test_helper" - -class UserChangesetCommentsTest < ActionDispatch::IntegrationTest - # Test 'log in to comment' message for nonlogged in user - def test_log_in_message - changeset = create(:changeset, :closed) - - get "/changeset/#{changeset.id}" - assert_response :success - - assert_select "div#content" do - assert_select "div#sidebar" do - assert_select "div#sidebar_content" do - assert_select "div" do - assert_select "p.notice" do - assert_select "a[href='/login?referer=%2Fchangeset%2F#{changeset.id}']", :text => I18n.t("browse.changeset.join_discussion"), :count => 1 - end - end - end - end - end - end - - # Test if the form is shown - def test_displaying_form - user = create(:user) - changeset = create(:changeset, :closed) - - get "/login" - follow_redirect! - # We should now be at the login page - assert_response :success - assert_template "sessions/new" - # We can now login - post "/login", :params => { "username" => user.email, "password" => "test" } - assert_response :redirect - - get "/changeset/#{changeset.id}" - - assert_response :success - assert_template "browse/changeset" - - assert_select "div#content" do - assert_select "div#sidebar" do - assert_select "div#sidebar_content" do - assert_select "div" do - assert_select "form[action='#']" do - assert_select "textarea[name=text]" - end - end - end - end - end - end -end diff --git a/test/system/changeset_comments_test.rb b/test/system/changeset_comments_test.rb new file mode 100644 index 000000000..b12aab5ee --- /dev/null +++ b/test/system/changeset_comments_test.rb @@ -0,0 +1,162 @@ +require "application_system_test_case" + +class ChangesetCommentsTest < ApplicationSystemTestCase + test "open changeset has a still open notice" do + changeset = create(:changeset) + sign_in_as(create(:user)) + visit changeset_path(changeset) + + within_sidebar do + assert_no_button "Comment" + assert_text "Changeset still open" + end + end + + test "changeset has a login notice" do + changeset = create(:changeset, :closed) + visit changeset_path(changeset) + + within_sidebar do + assert_no_button "Subscribe" + assert_no_button "Comment" + assert_link "Log in to join the discussion", :href => login_path(:referer => changeset_path(changeset)) + end + end + + test "can add a comment to a changeset" do + changeset = create(:changeset, :closed) + user = create(:user) + sign_in_as(user) + visit changeset_path(changeset) + + within_sidebar do + assert_no_content "Comment from #{user.display_name}" + assert_no_content "Some newly added changeset comment" + assert_button "Comment", :disabled => true + + fill_in "text", :with => "Some newly added changeset comment" + + assert_button "Comment", :disabled => false + + click_on "Comment" + + assert_content "Comment from #{user.display_name}" + assert_content "Some newly added changeset comment" + end + end + + test "regular users can't hide comments" do + changeset = create(:changeset, :closed) + create(:changeset_comment, :changeset => changeset, :body => "Unwanted comment") + sign_in_as(create(:user)) + visit changeset_path(changeset) + + within_sidebar do + assert_text "Unwanted comment" + assert_no_button "hide" + end + end + + test "moderators can hide comments" do + changeset = create(:changeset, :closed) + create(:changeset_comment, :changeset => changeset, :body => "Unwanted comment") + + visit changeset_path(changeset) + + within_sidebar do + assert_text "Unwanted comment" + end + + sign_in_as(create(:moderator_user)) + visit changeset_path(changeset) + + within_sidebar do + assert_text "Unwanted comment" + assert_button "hide", :exact => true + assert_no_button "unhide", :exact => true + + click_on "hide", :exact => true + + assert_text "Unwanted comment" + assert_no_button "hide", :exact => true + assert_button "unhide", :exact => true + end + + sign_out + visit changeset_path(changeset) + + within_sidebar do + assert_no_text "Unwanted comment" + end + end + + test "moderators can unhide comments" do + changeset = create(:changeset, :closed) + create(:changeset_comment, :changeset => changeset, :body => "Wanted comment", :visible => false) + + visit changeset_path(changeset) + + within_sidebar do + assert_no_text "Wanted comment" + end + + sign_in_as(create(:moderator_user)) + visit changeset_path(changeset) + + within_sidebar do + assert_text "Wanted comment" + assert_no_button "hide", :exact => true + assert_button "unhide", :exact => true + + click_on "unhide", :exact => true + + assert_text "Wanted comment" + assert_button "hide", :exact => true + assert_no_button "unhide", :exact => true + end + + sign_out + visit changeset_path(changeset) + + within_sidebar do + assert_text "Wanted comment" + end + end + + test "can subscribe" do + changeset = create(:changeset, :closed) + user = create(:user) + sign_in_as(user) + visit changeset_path(changeset) + + within_sidebar do + assert_button "Subscribe" + assert_no_button "Unsubscribe" + + click_on "Subscribe" + + assert_no_button "Subscribe" + assert_button "Unsubscribe" + end + end + + test "can't subscribe when blocked" do + changeset = create(:changeset, :closed) + user = create(:user) + sign_in_as(user) + visit changeset_path(changeset) + create(:user_block, :user => user) + + within_sidebar do + assert_no_text "Your access to the API has been blocked" + assert_button "Subscribe" + assert_no_button "Unsubscribe" + + click_on "Subscribe" + + assert_text "Your access to the API has been blocked" + assert_button "Subscribe" + assert_no_button "Unsubscribe" + end + end +end