From: Tom Hughes <tom@compton.nu>
Date: Thu, 21 Mar 2024 16:38:18 +0000 (+0000)
Subject: Merge remote-tracking branch 'upstream/pull/4562'
X-Git-Tag: live~893
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/32ab099c3504c0a9de14ae857c0fe76990bdfc37?hp=44312c221bf79db6d0108803ba32f396befffca6

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

diff --git a/Gemfile b/Gemfile
index 0b7e25ec9..13daf8267 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,6 +2,7 @@ source "https://rubygems.org"
 
 # Require rails
 gem "rails", "~> 7.1.0"
+gem "turbo-rails"
 
 # Require json for multi_json
 gem "json"
diff --git a/Gemfile.lock b/Gemfile.lock
index 958f4579b..77cf3dc19 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -563,6 +563,10 @@ GEM
     thor (1.3.1)
     tilt (2.3.0)
     timeout (0.4.1)
+    turbo-rails (2.0.4)
+      actionpack (>= 6.0.0)
+      activejob (>= 6.0.0)
+      railties (>= 6.0.0)
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
     unicode-display_width (2.5.0)
@@ -680,6 +684,7 @@ DEPENDENCIES
   sprockets-exporters_pack
   strong_migrations
   terser
+  turbo-rails
   unicode-display_width
   validates_email_format_of (>= 1.5.1)
   vendorer
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index 0bfff869e..c71c0652d 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -69,6 +69,10 @@ window.updateLinks = function (loc, zoom, layers, object) {
 };
 
 $(document).ready(function () {
+  // NB: Turns Turbo Drive off by default. Turbo Drive must be opt-in on a per-link and per-form basis
+  // See https://turbo.hotwired.dev/reference/drive#turbo.session.drive
+  Turbo.session.drive = false;
+
   var headerWidth = 0,
       compactWidth = 0;
 
@@ -108,6 +112,7 @@ $(document).ready(function () {
     updateHeader();
 
     $(window).resize(updateHeader);
+    $(document).on("turbo:render", updateHeader);
   }, 0);
 
   $("#menu-icon").on("click", function (e) {
diff --git a/app/assets/javascripts/messages.js b/app/assets/javascripts/messages.js
index cc86da05e..eef06457b 100644
--- a/app/assets/javascripts/messages.js
+++ b/app/assets/javascripts/messages.js
@@ -1,36 +1,16 @@
 $(document).ready(function () {
-  $(".inbox-mark-unread").on("ajax:success", function (event, data) {
-    updateHtml(data);
-    updateReadState(this, false);
+  $(".messages-table .destroy-message").on("turbo:submit-end", function (event) {
+    if (event.detail.success) {
+      event.target.dataset.isDestroyed = true;
+    }
   });
 
-  $(".inbox-mark-read").on("ajax:success", function (event, data) {
-    updateHtml(data);
-    updateReadState(this, true);
+  $(".messages-table .message-summary").on("turbo:before-morph-element", function (event) {
+    if ($(event.target).find("[data-is-destroyed]").length > 0) {
+      event.preventDefault(); // NB: prevent Turbo from morhping/removing this element
+      $(event.target).fadeOut(800, "linear", function () {
+        $(this).remove();
+      });
+    }
   });
-
-  $(".inbox-destroy").on("ajax:success", function (event, data) {
-    updateHtml(data);
-
-    $(this).closest("tr").fadeOut(800, "linear", function () {
-      $(this).remove();
-    });
-  });
-
-  function updateHtml(data) {
-    $("#inboxanchor").remove();
-    $(".user-button").before(data.inboxanchor);
-
-    $("#inbox-count").replaceWith(data.inbox_count);
-    $("#outbox-count").replaceWith(data.outbox_count);
-    $("#muted-count").replaceWith(data.muted_count);
-  }
-
-  function updateReadState(target, isRead) {
-    $(target).closest("tr")
-      .toggleClass("inbox-row", isRead)
-      .toggleClass("inbox-row-unread", !isRead)
-      .find(".inbox-mark-unread").prop("hidden", !isRead).end()
-      .find(".inbox-mark-read").prop("hidden", isRead);
-  }
 });
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 2ca86fc02..d231fddde 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -60,12 +60,12 @@ class MessagesController < ApplicationController
     @message = Message.where(:recipient => current_user).or(Message.where(:sender => current_user.id)).find(params[:id])
     @message.from_user_visible = false if @message.sender == current_user
     @message.to_user_visible = false if @message.recipient == current_user
-    if @message.save && !request.xhr?
+    if @message.save
       flash[:notice] = t ".destroyed"
 
       referer = safe_referer(params[:referer]) if params[:referer]
 
-      redirect_to referer || { :action => :inbox }
+      redirect_to referer || { :action => :inbox }, :status => :see_other
     end
   rescue ActiveRecord::RecordNotFound
     @title = t "messages.no_such_message.title"
@@ -125,9 +125,9 @@ class MessagesController < ApplicationController
       notice = t ".as_read"
     end
     @message.message_read = message_read
-    if @message.save && !request.xhr?
+    if @message.save
       flash[:notice] = notice
-      redirect_to :action => :inbox
+      redirect_back_or_to inbox_messages_path, :status => :see_other
     end
   rescue ActiveRecord::RecordNotFound
     @title = t "messages.no_such_message.title"
diff --git a/app/views/layouts/_head.html.erb b/app/views/layouts/_head.html.erb
index 34046bcab..724ca5526 100644
--- a/app/views/layouts/_head.html.erb
+++ b/app/views/layouts/_head.html.erb
@@ -2,6 +2,7 @@
   <meta http-equiv="X-UA-Compatible" content="IE=edge" />
   <meta name="viewport" content="width=device-width, initial-scale=1">
   <%= javascript_include_tag "es6" unless browser.es6? %>
+  <%= javascript_include_tag "turbo", :type => "module" %>
   <%= javascript_include_tag "application" %>
   <%= javascript_include_tag "i18n/#{I18n.locale}" %>
   <%= stylesheet_link_tag "screen-#{dir}", :media => "screen" %>
diff --git a/app/views/layouts/_meta.html.erb b/app/views/layouts/_meta.html.erb
index 3517673ac..2352ad0b6 100644
--- a/app/views/layouts/_meta.html.erb
+++ b/app/views/layouts/_meta.html.erb
@@ -13,6 +13,8 @@
 <%= tag.meta :name => "msapplication-TileColor", :content => "#00a300" %>
 <%= tag.meta :name => "msapplication-TileImage", :content => image_path("mstile-144x144.png") %>
 <%= tag.meta :name => "theme-color", :content => "#ffffff" %>
+<%= turbo_refresh_method_tag :morph %>
+<%= turbo_refresh_scroll_tag :preserve %>
 <%= canonical_tag %>
 <% if Settings.key?(:publisher_url) -%>
 <%= tag.link :rel => "publisher", :href => Settings.publisher_url %>
diff --git a/app/views/messages/_inbox_count.html.erb b/app/views/messages/_inbox_count.html.erb
deleted file mode 100644
index 86bb2c474..000000000
--- a/app/views/messages/_inbox_count.html.erb
+++ /dev/null
@@ -1,7 +0,0 @@
-<h4 id="inbox-count">
-<%= t "messages.inbox.messages",
-      :new_messages => t("messages.inbox.new_messages",
-                         :count => current_user.new_messages.size),
-      :old_messages => t("messages.inbox.old_messages",
-                         :count => current_user.messages.size - current_user.new_messages.size) %>
-</h4>
diff --git a/app/views/messages/_message_summary.html.erb b/app/views/messages/_message_summary.html.erb
index cb85a62ba..81ef48402 100644
--- a/app/views/messages/_message_summary.html.erb
+++ b/app/views/messages/_message_summary.html.erb
@@ -1,13 +1,13 @@
-<tr id="inbox-<%= message.id %>" class="inbox-row<%= "-unread" unless message.message_read? %>">
-  <td><%= link_to message.sender.display_name, message.sender %></td>
-  <td><%= link_to message.title, message %></td>
+<tr id="inbox-<%= message.id %>" class="message-summary inbox-row<%= "-unread" unless message.message_read? %>">
+  <td><%= link_to message.sender.display_name, user_path(message.sender) %></td>
+  <td><%= link_to message.title, message_path(message) %></td>
   <td class="text-nowrap"><%= l message.sent_on, :format => :friendly %></td>
   <td class="text-nowrap d-flex justify-content-end gap-1">
-    <%= button_to t(".unread_button"), message_mark_path(message, :mark => "unread"), :remote => true, :class => "btn btn-sm btn-primary", :form => { :class => "inbox-mark-unread", :hidden => !message.message_read? } %>
-    <%= button_to t(".read_button"), message_mark_path(message, :mark => "read"), :remote => true, :class => "btn btn-sm btn-primary", :form => { :class => "inbox-mark-read", :hidden => message.message_read? } %>
-    <%= button_to t(".destroy_button"), message_path(message, :referer => request.fullpath), :method => :delete, :remote => true, :class => "btn btn-sm btn-danger", :form_class => "inbox-destroy" %>
+    <%= button_to t(".unread_button"), message_mark_path(message, :mark => "unread"), :class => "btn btn-sm btn-primary", :form => { :data => { :turbo => true }, :class => "inbox-mark-unread", :hidden => !message.message_read? } %>
+    <%= button_to t(".read_button"), message_mark_path(message, :mark => "read"), :class => "btn btn-sm btn-primary", :form => { :data => { :turbo => true }, :class => "inbox-mark-read", :hidden => message.message_read? } %>
+    <%= button_to t(".destroy_button"), message_path(message, :referer => request.fullpath), :method => :delete, :class => "btn btn-sm btn-danger", :form => { :data => { :turbo => true }, :class => "destroy-message" } %>
     <% if message.muted? %>
-      <%= button_to t(".unmute_button"), message_unmute_path(message), :method => :patch, :class => "btn btn-sm btn-secondary" %>
+      <%= button_to t(".unmute_button"), message_unmute_path(message), :method => :patch, :class => "btn btn-sm btn-secondary", :form => { :data => { :turbo => true } } %>
     <% end %>
   </td>
 </tr>
diff --git a/app/views/messages/_messages_table.html.erb b/app/views/messages/_messages_table.html.erb
index ce222dfab..f11fe3f62 100644
--- a/app/views/messages/_messages_table.html.erb
+++ b/app/views/messages/_messages_table.html.erb
@@ -1,4 +1,4 @@
-<table class="table table-sm align-middle">
+<table class="table table-sm align-middle messages-table">
   <thead>
     <tr>
       <% columns.each do |column| %>
diff --git a/app/views/messages/_muted_count.html.erb b/app/views/messages/_muted_count.html.erb
deleted file mode 100644
index 207973d58..000000000
--- a/app/views/messages/_muted_count.html.erb
+++ /dev/null
@@ -1,3 +0,0 @@
-<h4 id="muted-count">
-<%= t "messages.muted.messages", :count => current_user.muted_messages.size %>
-</h4>
diff --git a/app/views/messages/_outbox_count.html.erb b/app/views/messages/_outbox_count.html.erb
deleted file mode 100644
index 5b27f1d6e..000000000
--- a/app/views/messages/_outbox_count.html.erb
+++ /dev/null
@@ -1,3 +0,0 @@
-<h4 id="outbox-count">
-<%= t "messages.outbox.messages", :count => current_user.sent_messages.size %>
-</h4>
diff --git a/app/views/messages/_sent_message_summary.html.erb b/app/views/messages/_sent_message_summary.html.erb
index a9f011f5e..683d8d6ad 100644
--- a/app/views/messages/_sent_message_summary.html.erb
+++ b/app/views/messages/_sent_message_summary.html.erb
@@ -1,8 +1,8 @@
-<tr class="inbox-row">
-  <td><%= link_to message.recipient.display_name, message.recipient %></td>
-  <td><%= link_to message.title, message %></td>
+<tr id="outbox-<%= message.id %>" class="message-summary inbox-row">
+  <td><%= link_to message.recipient.display_name, user_path(message.recipient) %></td>
+  <td><%= link_to message.title, message_path(message) %></td>
   <td class="text-nowrap"><%= l message.sent_on, :format => :friendly %></td>
   <td class="text-nowrap d-flex justify-content-end gap-1">
-    <%= button_to t(".destroy_button"), message_path(message, :referer => request.fullpath), :method => :delete, :remote => true, :class => "btn btn-sm btn-danger", :form_class => "inbox-destroy" %>
+    <%= button_to t(".destroy_button"), message_path(message, :referer => request.fullpath), :method => :delete, :class => "btn btn-sm btn-danger", :form => { :data => { :turbo => true }, :class => "destroy-message" } %>
   </td>
 </tr>
diff --git a/app/views/messages/destroy.json.jbuilder b/app/views/messages/destroy.json.jbuilder
deleted file mode 100644
index 65bfd6a6b..000000000
--- a/app/views/messages/destroy.json.jbuilder
+++ /dev/null
@@ -1,4 +0,0 @@
-json.inboxanchor render(:partial => "layouts/inbox")
-json.inbox_count render(:partial => "inbox_count")
-json.outbox_count render(:partial => "outbox_count")
-json.muted_count render(:partial => "muted_count")
diff --git a/app/views/messages/inbox.html.erb b/app/views/messages/inbox.html.erb
index 611b0b1c9..835d70a74 100644
--- a/app/views/messages/inbox.html.erb
+++ b/app/views/messages/inbox.html.erb
@@ -4,7 +4,7 @@
 
 <%= render :partial => "heading", :locals => { :active_link_path => inbox_messages_path } %>
 
-<%= render :partial => "inbox_count" %>
+<h4><%= t "messages.inbox.messages", :new_messages => t(".new_messages", :count => current_user.new_messages.size), :old_messages => t(".old_messages", :count => current_user.messages.size - current_user.new_messages.size) %></h4>
 
 <% if current_user.messages.size > 0 %>
   <%= render :partial => "messages_table", :locals => { :columns => %w[from subject date], :messages => current_user.messages, :inner_partial => "message_summary" } %>
diff --git a/app/views/messages/mark.json.jbuilder b/app/views/messages/mark.json.jbuilder
deleted file mode 100644
index 65bfd6a6b..000000000
--- a/app/views/messages/mark.json.jbuilder
+++ /dev/null
@@ -1,4 +0,0 @@
-json.inboxanchor render(:partial => "layouts/inbox")
-json.inbox_count render(:partial => "inbox_count")
-json.outbox_count render(:partial => "outbox_count")
-json.muted_count render(:partial => "muted_count")
diff --git a/app/views/messages/muted.html.erb b/app/views/messages/muted.html.erb
index 8e97abc7f..40c74e915 100644
--- a/app/views/messages/muted.html.erb
+++ b/app/views/messages/muted.html.erb
@@ -4,6 +4,6 @@
 
 <%= render :partial => "heading", :locals => { :active_link_path => muted_messages_path } %>
 
-<%= render :partial => "muted_count" %>
+<h4><%= t ".messages", :count => current_user.muted_messages.size %></h4>
 
 <%= render :partial => "messages_table", :locals => { :columns => %w[from subject date], :messages => current_user.muted_messages, :inner_partial => "message_summary" } %>
diff --git a/app/views/messages/outbox.html.erb b/app/views/messages/outbox.html.erb
index 096427e8d..5cb357f6f 100644
--- a/app/views/messages/outbox.html.erb
+++ b/app/views/messages/outbox.html.erb
@@ -4,7 +4,7 @@
 
 <%= render :partial => "heading", :locals => { :active_link_path => outbox_messages_path } %>
 
-<%= render :partial => "outbox_count" %>
+<h4><%= t ".messages", :count => current_user.sent_messages.size %></h4>
 
 <% if current_user.sent_messages.size > 0 %>
   <%= render :partial => "messages_table", :locals => { :columns => %w[to subject date], :messages => current_user.sent_messages, :inner_partial => "sent_message_summary" } %>
diff --git a/config/eslint.json b/config/eslint.json
index 3b878d48a..397615d1a 100644
--- a/config/eslint.json
+++ b/config/eslint.json
@@ -13,7 +13,8 @@
     "OSM": "writable",
     "Matomo": "readonly",
     "Qs": "readonly",
-    "updateLinks": "readonly"
+    "updateLinks": "readonly",
+    "Turbo": "readonly"
   },
   "rules": {
     "accessor-pairs": "error",
diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb
index df7146ad6..40581993f 100644
--- a/test/controllers/messages_controller_test.rb
+++ b/test/controllers/messages_controller_test.rb
@@ -408,15 +408,13 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest
     assert_not Message.find(unread_message.id).message_read
 
     # Check that the marking a message read via XHR works
-    post message_mark_path(:message_id => unread_message, :mark => "read"), :xhr => true
-    assert_response :success
-    assert_template "mark"
+    post message_mark_path(:message_id => unread_message, :mark => "read")
+    assert_response :see_other
     assert Message.find(unread_message.id).message_read
 
     # Check that the marking a message unread via XHR works
-    post message_mark_path(:message_id => unread_message, :mark => "unread"), :xhr => true
-    assert_response :success
-    assert_template "mark"
+    post message_mark_path(:message_id => unread_message, :mark => "unread")
+    assert_response :see_other
     assert_not Message.find(unread_message.id).message_read
 
     # Asking to mark a message with no ID should fail
diff --git a/test/system/messages_test.rb b/test/system/messages_test.rb
index dea0d2208..b78568314 100644
--- a/test/system/messages_test.rb
+++ b/test/system/messages_test.rb
@@ -36,6 +36,7 @@ class MessagesTest < ApplicationSystemTestCase
     assert_text "1 muted message"
 
     click_on "Delete"
-    assert_text "0 muted messages"
+    refute_text "1 muted message"
+    assert_text "You have 0 new messages and 0 old messages"
   end
 end