]> git.openstreetmap.org Git - rails.git/commitdiff
Merge remote-tracking branch 'upstream/pull/4463'
authorTom Hughes <tom@compton.nu>
Sun, 25 Feb 2024 09:28:48 +0000 (09:28 +0000)
committerTom Hughes <tom@compton.nu>
Sun, 25 Feb 2024 09:28:48 +0000 (09:28 +0000)
app/assets/javascripts/index/changeset.js
app/assets/stylesheets/common.scss
app/views/browse/changeset.html.erb
test/application_system_test_case.rb
test/controllers/browse_controller_test.rb
test/integration/user_changeset_comments_test.rb [deleted file]
test/system/changeset_comments_test.rb [new file with mode: 0644]

index c6e77bc71f2f9af2f8a69d194e3975598251f5a2..75a1f7b4dfa770463867cf7fafafbaf05babb1e3 100644 (file)
@@ -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) {
index b5bd5adec641cf06afe707334796584904aae21c..1f7c45db504ab41f3fdb64efb96055937f5e29ee 100644 (file)
@@ -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;
index 50171271763f3bd0b2bd125b078ef734716d6212..ac5382b14ee550fda6ba7d21ea9990d1515da26f 100644 (file)
     <% if current_user %>
       <div class="col-auto">
         <% if @changeset.subscribers.exists?(current_user.id) %>
-          <button class="action-button btn btn-sm btn-primary" name="unsubscribe" data-method="POST" data-url="<%= changeset_unsubscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.unsubscribe") %></button>
+          <button class="btn btn-sm btn-primary" name="unsubscribe" data-method="POST" data-url="<%= changeset_unsubscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.unsubscribe") %></button>
         <% else %>
-          <button class="action-button btn btn-sm btn-primary" name="subscribe" data-method="POST" data-url="<%= changeset_subscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.subscribe") %></button>
+          <button class="btn btn-sm btn-primary" name="subscribe" data-method="POST" data-url="<%= changeset_subscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.subscribe") %></button>
         <% end %>
       </div>
     <% end %>
   </div>
 
   <% if @comments.length > 0 %>
-    <div class='changeset-comments'>
-      <form action="#">
-        <ul class="list-unstyled">
-          <% @comments.each do |comment| %>
-            <% if comment.visible %>
-              <li id="c<%= comment.id %>">
-                <small class='text-muted'>
-                  <%= 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? %>
-                    — <span class="action-button" data-comment-id="<%= comment.id %>" data-method="POST" data-url="<%= changeset_comment_hide_url(comment.id) %>"><%= t("javascripts.changesets.show.hide_comment") %></span>
-                  <% end %>
-                </small>
-                <div class="mx-2">
-                  <%= comment.body.to_html %>
-                </div>
-              </li>
-            <% elsif current_user and current_user.moderator? %>
-              <li id="c<%= comment.id %>">
-                <small class='text-muted'>
-                  <%= t(".hidden_comment_by_html",
-                        :time_ago => friendly_date_ago(comment.created_at),
-                        :user => link_to(comment.author.display_name, user_path(comment.author))) %>
-                  — <span class="action-button text-muted" data-comment-id="<%= comment.id %>" data-method="POST" data-url="<%= changeset_comment_unhide_url(comment.id) %>"><%= t("javascripts.changesets.show.unhide_comment") %></span>
-                 </small>
-                <div class="mx-2">
-                  <%= comment.body.to_html %>
-                </div>
-              </li>
+    <ul class="list-unstyled">
+      <% @comments.each do |comment| %>
+        <% next unless comment.visible || current_user&.moderator? %>
+        <li id="c<%= comment.id %>">
+          <small class='text-muted'>
+            <%= 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 %>
-        </ul>
-      </form>
-    </div>
+          </small>
+          <div class="mx-2">
+            <%= comment.body.to_html %>
+          </div>
+        </li>
+      <% end %>
+    </ul>
   <% end %>
 
   <% unless current_user %>
-    <p class="notice">
+    <p>
       <%= link_to(t(".join_discussion"), login_path(:referer => request.fullpath)) %>
     </p>
   <% end %>
         <div id="comment-error" class="alert alert-danger p-2 mb-3" hidden>
         </div>
         <div>
-          <input type="submit" name="comment" value="<%= t("javascripts.changesets.show.comment") %>" data-changeset-id="<%= @changeset.id %>" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled="1" class="btn btn-sm btn-primary" />
+          <button name="comment" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled class="btn btn-sm btn-primary"><%= t("javascripts.changesets.show.comment") %></button>
         </div>
       </form>
     <% else %>
-      <p class="notice">
+      <p>
         <%= t(".still_open") %>
       </p>
     <% end %>
index 7931546d403028e755d1aaa6af6abb89a0e76f77..ef8f0e371feb924e35651e227fe47dbaf279f572 100644 (file)
@@ -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
index 1023d76ae58713b521e6d1353c3dee11d630b213..2bb743636ce9d5f5845d0922ddf8a65c5eecf06a 100644 (file)
@@ -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 (file)
index 2b95094..0000000
+++ /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 (file)
index 0000000..b12aab5
--- /dev/null
@@ -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