Merge branch 'main' into lexical

* main:
  Don't choke when rendering commands where the card was deleted
  Update tests
  Rework system events
  test: add missing assertions
  dep: bump active_record-tenanted to fix URI support
  Remove hyphen from "Un-do"
  Prevent default when handling keyboard shortcuts
  dep: update AR::Tenanted to support database URIs
  Detect platform and label the correct hotkey shortcut
  Things got too tight
  Don't require TLS for Beamer at the moment
  dep: update dependencies

# Conflicts:
#	Gemfile
#	Gemfile.lock
#	app/assets/stylesheets/comments.css

Gemfile

@@ -29,6 +29,7 @@ gem "rouge"
 gem "jbuilder"
 gem "actiontext-lexical", github: "basecamp/actiontext-lexical"
 gem "image_processing", "~> 1.14"
+gem "platform_agent"
 
 # Telemetry and logging
 gem "sentry-ruby"

Gemfile.lock

@@ -1,13 +1,6 @@
-GIT
-  remote: https://github.com/basecamp/actiontext-lexical.git
-  revision: 19038b86b8617062adbb930990edaacc1f643e3d
-  specs:
-    actiontext-lexical (0.1.0)
-      rails (>= 8.0.2)
-
 GIT
   remote: https://github.com/basecamp/active_record-tenanted
-  revision: 173048c2af29e9a723811e552bc59e3154e82e2a
+  revision: 523947a0f0f49f66a8a62eb93e9ba1a345b3076b
   specs:
     active_record-tenanted (0.1.0)
       activerecord (>= 8.1.alpha)
@@ -25,21 +18,20 @@ GIT
 
 GIT
   remote: https://github.com/crmne/ruby_llm.git
-  revision: 87a86f70cb64e23d5b76994edf8080618f07e071
+  revision: 8412d51e869f1ee8e5229831a9a0bb5e9b8f3847
   specs:
-    ruby_llm (1.3.0rc1)
+    ruby_llm (1.2.0)
       base64
       event_stream_parser (~> 1)
-      faraday (>= 1.10.0)
+      faraday (~> 2)
-      faraday-multipart (>= 1)
+      faraday-multipart (~> 1)
-      faraday-net_http (>= 1)
+      faraday-net_http (~> 3)
-      faraday-retry (>= 1)
+      faraday-retry (~> 2)
-      marcel (~> 1.0)
       zeitwerk (~> 2)
 
 GIT
   remote: https://github.com/rails/rails.git
-  revision: bc7f77b32d7f3f26db654cae506c26a959852506
+  revision: c1d7d56e831acf0aff5f3e27a6b384f543f358d0
   branch: main
   specs:
     actioncable (8.1.0.alpha)
@@ -73,6 +65,7 @@ GIT
       rails-html-sanitizer (~> 1.6)
       useragent (~> 0.16)
     actiontext (8.1.0.alpha)
+      action_text-trix (~> 2.1.15)
       actionpack (= 8.1.0.alpha)
       activerecord (= 8.1.0.alpha)
       activestorage (= 8.1.0.alpha)
@@ -139,6 +132,8 @@ GIT
 GEM
   remote: https://rubygems.org/
   specs:
+    action_text-trix (2.1.15)
+      railties
     addressable (2.8.7)
       public_suffix (>= 2.0.2, < 7.0)
     ast (2.4.3)
@@ -147,7 +142,7 @@ GEM
     bcrypt_pbkdf (1.1.1)
     benchmark (0.4.0)
     bigdecimal (3.1.9)
-    bootsnap (1.18.4)
+    bootsnap (1.18.6)
       msgpack (~> 1.2)
     brakeman (7.0.2)
       racc
@@ -193,10 +188,6 @@ GEM
       net-http (>= 0.5.0)
     faraday-retry (2.3.1)
       faraday (~> 2.0)
-    ffi (1.17.2)
-    ffi (1.17.2-arm64-darwin)
-    ffi (1.17.2-x86_64-darwin)
-    ffi (1.17.2-x86_64-linux-gnu)
     fugit (1.11.1)
       et-orbi (~> 1, >= 1.2.11)
       raabro (~> 1.4)
@@ -205,12 +196,9 @@ GEM
       addressable (>= 2.5.0)
     globalid (1.2.1)
       activesupport (>= 6.1)
-    hashdiff (1.2.0)
+    hashdiff (1.1.2)
     i18n (1.14.7)
       concurrent-ruby (~> 1.0)
-    image_processing (1.14.0)
-      mini_magick (>= 4.9.5, < 6)
-      ruby-vips (>= 2.0.17, < 3)
     importmap-rails (2.1.0)
       actionpack (>= 6.0.0)
       activesupport (>= 6.0.0)
@@ -224,7 +212,7 @@ GEM
       actionview (>= 5.0.0)
       activesupport (>= 5.0.0)
     json (2.11.3)
-    kamal (2.6.0)
+    kamal (2.6.1)
       activesupport (>= 7.0)
       base64 (~> 0.2)
       bcrypt_pbkdf (~> 1.0)
@@ -248,9 +236,6 @@ GEM
       net-smtp
     marcel (1.0.4)
     matrix (0.4.2)
-    mini_magick (5.2.0)
-      benchmark
-      logger
     mini_mime (1.1.5)
     mini_portile2 (2.8.9)
     minitest (5.25.5)
@@ -258,7 +243,7 @@ GEM
     multipart-post (2.4.1)
     net-http (0.6.0)
       uri
-    net-imap (0.5.7)
+    net-imap (0.5.8)
       date
       net-protocol
     net-pop (0.1.2)
@@ -287,6 +272,9 @@ GEM
     parser (3.3.8.0)
       ast (~> 2.4.1)
       racc
+    platform_agent (1.0.1)
+      activesupport (>= 5.2.0)
+      useragent (~> 0.16.3)
     pp (0.6.2)
       prettyprint
     prettyprint (0.2.0)
@@ -366,24 +354,21 @@ GEM
       rubocop-performance (>= 1.24)
       rubocop-rails (>= 2.30)
     ruby-progressbar (1.13.0)
-    ruby-vips (2.2.3)
-      ffi (~> 1.12)
-      logger
     rubyzip (2.4.1)
     securerandom (0.4.1)
-    selenium-webdriver (4.31.0)
+    selenium-webdriver (4.32.0)
       base64 (~> 0.2)
       logger (~> 1.4)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 3.0)
       websocket (~> 1.0)
-    sentry-rails (5.23.0)
+    sentry-rails (5.24.0)
       railties (>= 5.0)
-      sentry-ruby (~> 5.23.0)
+      sentry-ruby (~> 5.24.0)
-    sentry-ruby (5.23.0)
+    sentry-ruby (5.24.0)
       bigdecimal
       concurrent-ruby (~> 1.0, >= 1.0.2)
-    solid_cable (3.0.7)
+    solid_cable (3.0.8)
       actioncable (>= 7.2)
       activejob (>= 7.2)
       activerecord (>= 7.2)
@@ -463,7 +448,6 @@ PLATFORMS
   x86_64-mingw32
 
 DEPENDENCIES
-  actiontext-lexical!
   active_record-tenanted!
   bcrypt (~> 3.1.7)
   bootsnap
@@ -472,10 +456,10 @@ DEPENDENCIES
   capybara
   debug
   geared_pagination (~> 1.2)
-  image_processing (~> 1.14)
   importmap-rails
   jbuilder
   kamal
+  platform_agent
   propshaft
   puma (>= 5.0)
   rails!

app/assets/stylesheets/comments.css

@@ -37,8 +37,10 @@
   }
 
   .comment__author {
-    margin-inline-end: calc(var(--comment-padding-inline) * -0.5);
+    .comment--system & strong {
-  
+      display: none;
+    }
+
     .btn {
       font-weight: inherit;
     }
@@ -51,9 +53,17 @@
     .comment--mine_ & {
       margin-inline: calc(var(--comment-padding-inline) * -0.75);
     }
+
+    .comment--system & {
+      display: none;
+    }
   }
 
   .comment__body {
+    .comment--system & {
+      text-align: center;
+    }
+
     * {
       &:first-child {
         margin-block-start: 0;
@@ -99,8 +109,13 @@
     }
   }
 
-  .comment__event {
+  .comment--system {
+    --comment-padding-block: calc(var(--block-space) * 0.66);
+
+    line-height: 1.2;
     max-inline-size: var(--comment-max);
+    text-align: center;
+
 
     &::before {
       /* Make up space for lack of avatar */
@@ -108,5 +123,16 @@
       display: flex;
       inline-size: calc(var(--comment-padding-inline) * 0.75);
     }
+
+    .comment__author {
+      a {
+        margin: 0 auto;
+      }
+    }
+
+    .comment__body {
+      margin-block-start: calc(var(--block-space-half) * -0.33);
+      text-align: center !important;
+    }
   }
 }

app/assets/stylesheets/terminals.css

@@ -7,7 +7,7 @@
     font-family: var(--font-mono);
     font-size: var(--text-normal);
     inset: auto 0 0 0;
-    padding: var(--block-space) calc(var(--tray-size) + var(--inline-space-double));
+    padding: var(--block-space) calc(var(--tray-size) + calc(var(--inline-space) * 3));
     position: fixed;
     z-index: var(--z-terminal);
 

app/controllers/application_controller.rb

@@ -1,5 +1,5 @@
 class ApplicationController < ActionController::Base
-  include Authentication, CurrentTimezone
+  include Authentication, CurrentTimezone, SetPlatform
 
   stale_when_importmap_changes
   allow_browser versions: :modern

app/controllers/concerns/set_platform.rb

@@ -0,0 +1,12 @@
+module SetPlatform
+  extend ActiveSupport::Concern
+
+  included do
+    helper_method :platform
+  end
+
+  private
+    def platform
+      @platform ||= ApplicationPlatform.new(request.user_agent)
+    end
+end

app/helpers/hotkeys_helper.rb

@@ -0,0 +1,19 @@
+module HotkeysHelper
+  def hotkey_label(hotkey)
+    hotkey.split(",").first if hotkey
+    case hotkey
+    when Array
+      hotkey.map { |key|
+        if key == "ctrl" && platform.mac?
+          "⌘"
+        elsif key == "enter"
+          platform.mac? ? "return" : "enter"
+        else
+          key
+        end
+      }.join("+")
+    else
+      hotkey.split(",").first if hotkey
+    end
+  end
+end

app/javascript/controllers/hotkey_controller.js

@@ -3,6 +3,7 @@ import { Controller } from "@hotwired/stimulus"
 export default class extends Controller {
   click(event) {
     if (this.#isClickable && !this.#shouldIgnore(event)) {
+      event.preventDefault()
       this.element.click()
     }
   }

app/models/application_platform.rb

@@ -0,0 +1,29 @@
+class ApplicationPlatform < PlatformAgent
+  def ios?
+    match? /iPhone|iPad/
+  end
+
+  def android?
+    match? /Android/
+  end
+
+  def mac?
+    match? /Macintosh/
+  end
+
+  def chrome?
+    user_agent.browser.match? /Chrome/
+  end
+
+  def safari?
+    user_agent.browser.match? /Safari/
+  end
+
+  def mobile?
+    ios? || android?
+  end
+
+  def desktop?
+    !mobile?
+  end
+end

app/models/card/eventable/system_commenter.rb

@@ -8,46 +8,22 @@ class Card::Eventable::SystemCommenter
   def comment
     return unless comment_body.present?
 
-    if comment = find_replaceable_system_comment
+    card.comments.create! creator: User.system, body: comment_body, created_at: event.created_at
-      comment.update! body: comment_body
-    else
-      card.comments.create! creator: User.system, body: comment_body, created_at: event.created_at
-    end
   end
 
   private
-    RELATED_EVENTS = [
-      %w[ card_assigned card_unassigned ]
-    ]
-
     def comment_body
       case event.action
       when "card_assigned"
-        "Assigned to #{event.assignees.pluck(:name).to_sentence}."
+        "#{event.creator.name} assigned this to #{event.assignees.pluck(:name).to_sentence}."
       when "card_unassigned"
-        "Unassigned from #{event.assignees.pluck(:name).to_sentence}."
+        "#{event.creator.name} unassigned from #{event.assignees.pluck(:name).to_sentence}."
       when "card_staged"
         "#{event.creator.name} moved this to '#{event.stage_name}'."
       when "card_closed"
-        "Closed by #{ event.creator.name }"
+        "Closed as “#{ card.closure.reason }” by #{ event.creator.name }"
       when "card_title_changed"
-        "#{event.creator.name} changed title from '#{event.particulars.dig('particulars', 'old_title')}' to '#{event.particulars.dig('particulars', 'new_title')}'."
+        "#{event.creator.name} changed the title from '#{event.particulars.dig('particulars', 'old_title')}' to '#{event.particulars.dig('particulars', 'new_title')}'."
       end
     end
-
-    def find_replaceable_system_comment
-      previous_comment, previous_event = card.comments.last, card.events[-2]
-      if previous_comment&.creator&.system? && replaceable_event?(previous_event, event)
-        previous_comment
-      end
-    end
-
-    def replaceable_event?(candidate_event, replacement_event)
-      candidate_event && replacement_event &&
-        (candidate_event.action == replacement_event.action || related_events?(candidate_event, replacement_event))
-    end
-
-    def related_events?(candidate_event, replacement_event)
-      RELATED_EVENTS.find { |related_actions| related_actions.include?(candidate_event.action) && related_actions.include?(replacement_event.action) }
-    end
 end

app/models/command/go_to_card.rb

@@ -4,7 +4,7 @@ class Command::GoToCard < Command
   validates_presence_of :card
 
   def title
-    "Visit card '#{card.title}'"
+    "Visit card '#{card&.title}'"
   end
 
   def execute
@@ -13,6 +13,6 @@ class Command::GoToCard < Command
 
   private
     def card
-      user.accessible_cards.find(card_id)
+      user.accessible_cards.find_by_id(card_id)
     end
 end

app/views/cards/comments/_comment.html.erb

@@ -1,6 +1,6 @@
 <% cache comment do %>
   <%= turbo_frame_tag comment do %>
-    <div class="comment flex align-start full-width" data-creator-id="<%= comment.creator_id %>" data-created-by-current-user-target="creation">
+    <div class="comment flex align-start full-width <%= "comment--system" if comment.creator.system? %>" data-creator-id="<%= comment.creator_id %>" data-created-by-current-user-target="creation">
       <figure class="comment__avatar flex-item-no-shrink" aria-hidden="true">
         <%= avatar_tag comment.creator, hidden_for_screen_reader: true %>
       </figure>

app/views/cards/container/_closure.html.erb

@@ -3,7 +3,7 @@
     <div class="flex gap-half margin-block-start-double margin-block-end">
       <span class="card-perma__closure-message">Completed by <%= card.closed_by.name %> on <%= local_datetime_tag(card.closed_at, style: :shortdate) %>.</span>
       <%= button_to card_closure_path(card), method: :delete, class: "btn btn--plain borderless fill-transparent" do %>
-        <span class="pad-inline-end">Un-do</span>
+        <span class="pad-inline-end">Undo</span>
       <% end %>
     </div>
   <% else %>

app/views/cards/index/_header.html.erb

@@ -7,7 +7,7 @@
     <div class="card-filter--collection flex-inline flex-column align-center center position-relative" data-controller="dialog" data-action="click@document->dialog#closeOnClickOutside">
       <%= tag.button class: "txt-align-center input input--select borderless txt-medium",
           data: {
-            action: "click->dialog#open:stop keydown.meta+j@document->hotkey#click",
+            action: "click->dialog#open:stop keydown.meta+j@document->hotkey#click keydown.ctrl+j@document->hotkey#click",
             controller: "hotkey" } do %>
         <h1 class="txt-large overflow-ellipsis margin-none">
           <span><%= filter_title filter %></span>
@@ -38,7 +38,7 @@
           <%= render "cards/index/collections_filter", filter: filter %>
         </menu>
         <p class="txt-small txt-nowrap txt-align-center margin-block-half full-width">
-          Press <kbd class="kbd">⌘j</kbd> anytime to open this, <kbd class="kbd">esc</kbd> to close.
+          Press <kbd class="kbd"><%= hotkey_label(["ctrl", "J"]) -%></kbd> anytime to open this, <kbd class="kbd">esc</kbd> to close.
         </p>
       <% end %>
     </div>

app/views/commands/_form.html.erb

@@ -4,7 +4,7 @@
       data: {
         controller: "form",
         terminal_target: "form",
-        action: "turbo:submit-start->terminal#commandSubmitted turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus keydown.enter->terminal#executeCommand keydown.esc->terminal#hideMenus turbo:submit-end->terminal#handleCommandResponse"
+        action: "turbo:submit-start->terminal#commandSubmitted turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus:prevent keydown.ctrl+k@document->terminal#focus:prevent keydown.enter->terminal#executeCommand keydown.esc->terminal#hideMenus turbo:submit-end->terminal#handleCommandResponse"
       } do %>
   <label class="terminal__label txt-nowrap" for="fizzy_do">Fizzy do &gt;</label>
 
@@ -17,9 +17,8 @@
           terminal_target: "input",
           action: "keydown.up->toggle-class#add:prevent keydown.up->navigable-list#selectCurrentOrLast terminal#hideError"
         },
-        placeholder: "Press ⌘+K to search or type commands…",
+        placeholder: "Press #{ hotkey_label(["ctrl", "K"]) } to search or type commands…",
         spellcheck: "false" %>
 
   <%=  hidden_field_tag "confirmed", nil, data: { terminal_target: "confirmation" } %>
 <% end %>
-

app/views/events/_filter.html.erb

@@ -2,7 +2,7 @@
 		data-controller="dialog" data-action="keydown.esc->dialog#close click@document->dialog#closeOnClickOutside">
 	<%= tag.button class:"events__filter-btn input input--select flex-inline center min-width max-width txt-small",
 			data: {
-				action: "click->dialog#open:stop keydown.meta+j@document->hotkey#click",
+				action: "click->dialog#open:stop keydown.meta+j@document->hotkey#click keydown.ctrl+j@document->hotkey#click",
 				controller: "hotkey" } do %>
 		<span class="overflow-ellipsis center">
 			<%= filter_selected_collections_label(filter) %>
@@ -35,7 +35,7 @@
 		<% end %>
 
 		<p class="txt-small txt-nowrap txt-align-center margin-block-half full-width">
-			Press <kbd class="kbd">⌘j</kbd> anytime to open this, <kbd class="kbd">esc</kbd> to close.
+			Press <kbd class="kbd"><%= hotkey_label(["ctrl", "K"]) -%></kbd> anytime to open this, <kbd class="kbd">esc</kbd> to close.
 		</p>
 	<% end %>
 </div>

config/deploy.production.yml

@@ -33,7 +33,7 @@ accessories:
   beamer-primary:
     <<: *beamer-accessory
     service: beamer-primary
-    cmd: beamer primary --remove-after=1h --dir=/storage
+    cmd: beamer primary --remove-after=1h --dir=/storage --disable-tls
     port: 5000:80
     roles:
       - web

test/models/card/eventable/system_commenter_test.rb

@@ -7,7 +7,7 @@ class Card::Eventable::SystemCommenterTest < ActiveSupport::TestCase
   end
 
   test "card_assigned" do
-    assert_system_comment "Assigned to Kevin" do
+    assert_system_comment "David assigned this to Kevin." do
       @card.toggle_assignment users(:kevin)
     end
   end
@@ -16,7 +16,7 @@ class Card::Eventable::SystemCommenterTest < ActiveSupport::TestCase
     @card.toggle_assignment users(:kevin)
     @card.comments.destroy_all # To skip deduplication logic
 
-    assert_system_comment "Unassigned from Kevin" do
+    assert_system_comment "David unassigned from Kevin" do
       @card.toggle_assignment users(:kevin)
     end
   end
@@ -28,35 +28,17 @@ class Card::Eventable::SystemCommenterTest < ActiveSupport::TestCase
   end
 
   test "card_closed" do
-    assert_system_comment "Closed by David" do
+    assert_system_comment "Closed as “Done” by David" do
       @card.close
     end
   end
 
   test "card_title_changed" do
-    assert_system_comment "David changed title from 'The text is too small' to 'Make text larger'" do
+    assert_system_comment "David changed the title from 'The text is too small' to 'Make text larger'." do
       @card.update! title: "Make text larger"
     end
   end
 
-  test "don't duplicate comments for the same kind of events" do
-    @card.change_stage_to workflow_stages(:qa_in_progress)
-
-    assert_no_difference -> { @card.comments.count } do
-      @card.reload.change_stage_to workflow_stages(:qa_on_hold)
-      assert_match /David moved this to 'On hold'/i, @card.comments.last.body.to_plain_text
-    end
-  end
-
-  test "don't duplicate comments for related events" do
-    @card.toggle_assignment users(:kevin)
-
-    assert_no_difference -> { @card.comments.count } do
-      @card.reload.toggle_assignment users(:kevin)
-      assert_match /Unassigned from Kevin/i, @card.comments.last.body.to_plain_text
-    end
-  end
-
   test "don't notify on system comments" do
     @card.watch_by(users(:david))
 

test/models/command/close_test.rb

@@ -32,7 +32,7 @@ class Command::CloseTest < ActionDispatch::IntegrationTest
 
     execute_command "/close", context_url: collection_cards_url(@card.collection)
 
-    cards.each { it.reload.closed? }
+    assert cards.map(&:reload).all?(&:closed?)
   end
 
   test "undo close" do
@@ -42,10 +42,10 @@ class Command::CloseTest < ActionDispatch::IntegrationTest
     command = parse_command "/close", context_url: collection_cards_url(@card.collection)
     command.execute
 
-    cards.each { it.reload.closed? }
+    assert cards.map(&:reload).all?(&:closed?)
 
     command.undo
 
-    cards.each { it.reload.open? }
+    assert cards.map(&:reload).all?(&:open?)
   end
 end