From: Tom Hughes <tom@compton.nu>
Date: Fri, 16 Aug 2024 16:24:50 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/5088'
X-Git-Tag: live~363
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/9b2db63a957a325d16e3989f08c483bd88f1993e?hp=80c43f51c2bd97f0bf92026fbb273615e8f9f015

Merge remote-tracking branch 'upstream/pull/5088'
---

diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss
index 321b0bcb9..d551462b2 100644
--- a/app/assets/stylesheets/common.scss
+++ b/app/assets/stylesheets/common.scss
@@ -904,6 +904,12 @@ div.secondary-actions {
   }
 }
 
+/* Rules for block pages */
+
+#block_list .username {
+  max-width: 20em;
+}
+
 /* Rules for tabs inside secondary background sections */
 
 .bg-body-secondary .nav-tabs {
diff --git a/app/views/user_blocks/_block.html.erb b/app/views/user_blocks/_block.html.erb
index a18d1dbdb..5444bdd5b 100644
--- a/app/views/user_blocks/_block.html.erb
+++ b/app/views/user_blocks/_block.html.erb
@@ -1,9 +1,9 @@
 <tr>
   <% if show_user_name %>
-  <td><%= link_to block.user.display_name, block.user %></td>
+  <td><%= link_to block.user.display_name, block.user, :class => "username d-inline-block text-truncate text-wrap" %></td>
   <% end %>
   <% if show_creator_name %>
-  <td><%= link_to block.creator.display_name, block.creator %></td>
+  <td><%= link_to block.creator.display_name, block.creator, :class => "username d-inline-block text-truncate text-wrap" %></td>
   <% end %>
   <td><%= h truncate(block.reason) %></td>
   <td><%= h block_status(block) %></td>
diff --git a/app/views/user_blocks/_navigation.html.erb b/app/views/user_blocks/_navigation.html.erb
index f53f5cf28..f4f9b04f8 100644
--- a/app/views/user_blocks/_navigation.html.erb
+++ b/app/views/user_blocks/_navigation.html.erb
@@ -38,7 +38,7 @@
     <li class="nav-item">
       <%= link_to t(".block", :id => @user_block.id),
                   user_block_path(@user_block),
-                  :class => ["nav-link", { :active => action_name == "show" }] %>
+                  :class => "nav-link active" %>
     </li>
   <% end %>
 </ul>
diff --git a/app/views/user_blocks/edit.html.erb b/app/views/user_blocks/edit.html.erb
index 88441a15d..2d8956ffc 100644
--- a/app/views/user_blocks/edit.html.erb
+++ b/app/views/user_blocks/edit.html.erb
@@ -1,27 +1,33 @@
 <% @title = t ".title", :name => @user_block.user.display_name %>
+
+<% content_for :heading_class, "pb-0" %>
 <% content_for :heading do %>
   <h1><%= t(".heading_html", :name => link_to(@user_block.user.display_name, @user_block.user)) %></h1>
-  <nav class='secondary-actions'>
-    <ul class='clearfix'>
-      <li><%= link_to t(".show"), @user_block %></li>
-      <li><%= link_to t(".back"), user_blocks_path %></li>
-    </ul>
-  </nav>
+  <%= render :partial => "navigation" %>
 <% end %>
 
 <%= bootstrap_form_for(@user_block) do |f| %>
   <%= f.richtext_field :reason, :cols => 80, :rows => 20, :format => @user_block.reason_format %>
 
-  <%= f.form_group do %>
-    <%= label_tag "user_block_period", t(".period"), :class => "form-label" %>
-    <%= select_tag("user_block_period",
-                   options_for_select(UserBlock::PERIODS.collect { |h| [block_duration_in_words(h.hours), h.to_s] },
-                                      UserBlock::PERIODS.min_by { |h| (params[:user_block_period].to_i - h).abs }),
-                   :class => "form-select") %>
-  <% end %>
+  <% if @user_block.active? %>
+    <%= f.form_group do %>
+      <%= label_tag "user_block_period", t(".period"), :class => "form-label" %>
+      <%= select_tag "user_block_period",
+                     options_for_select(UserBlock::PERIODS.collect { |h| [block_duration_in_words(h.hours), h.to_s] },
+                                        UserBlock::PERIODS.min_by { |h| (params[:user_block_period].to_i - h).abs }),
+                     :class => "form-select" %>
+    <% end %>
+
+    <%= f.form_group :needs_view do %>
+      <%= f.check_box :needs_view %>
+    <% end %>
+  <% else %>
+    <div class="alert alert-info">
+      <%= t "user_blocks.update.inactive_block_cannot_be_reactivated" %>
+    </div>
 
-  <%= f.form_group :needs_view do %>
-    <%= f.check_box :needs_view %>
+    <%= hidden_field_tag "user_block_period", 0 %>
+    <%= f.hidden_field :needs_view %>
   <% end %>
 
   <%= f.primary %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index c4cae4b22..a97eca9c0 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2941,8 +2941,6 @@ en:
       title: "Editing block on %{name}"
       heading_html: "Editing block on %{name}"
       period: "How long, starting now, the user will be blocked from the API for."
-      show: "View this block"
-      back: "View all blocks"
     filter:
       block_period: "The blocking period must be one of the values selectable in the drop-down list."
     create:
diff --git a/test/system/user_blocks_test.rb b/test/system/user_blocks_test.rb
index 6e4f5b471..89f98d759 100644
--- a/test/system/user_blocks_test.rb
+++ b/test/system/user_blocks_test.rb
@@ -68,4 +68,34 @@ class UserBlocksSystemTest < ApplicationSystemTestCase
     assert_unchecked_field "Are you sure you wish to revoke 2 active blocks?"
     assert_button "Revoke!"
   end
+
+  test "duration controls are present for active blocks" do
+    creator_user = create(:moderator_user)
+    block = create(:user_block, :creator => creator_user, :reason => "Testing editing active blocks", :ends_at => Time.now.utc + 2.days)
+    sign_in_as(creator_user)
+
+    visit edit_user_block_path(block)
+    assert_field "Reason", :with => "Testing editing active blocks"
+    assert_select "user_block_period", :selected => "2 days"
+    assert_unchecked_field "Needs view"
+
+    fill_in "Reason", :with => "Editing active blocks works"
+    click_on "Update block"
+    assert_text(/Reason for block:\s+Editing active blocks works/)
+  end
+
+  test "duration controls are removed for inactive blocks" do
+    creator_user = create(:moderator_user)
+    block = create(:user_block, :expired, :creator => creator_user, :reason => "Testing editing expired blocks")
+    sign_in_as(creator_user)
+
+    visit edit_user_block_path(block)
+    assert_field "Reason", :with => "Testing editing expired blocks"
+    assert_no_select "user_block_period"
+    assert_no_field "Needs view"
+
+    fill_in "Reason", :with => "Editing expired blocks works"
+    click_on "Update block"
+    assert_text(/Reason for block:\s+Editing expired blocks works/)
+  end
 end