From 4300c37bb51e040a07dbc84b238bc399229d3336 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 23 May 2025 09:01:51 -0400 Subject: [PATCH 01/12] dep: update dependencies - [ ] drb: 2.2.1 -> 2.2.3 - used by active_record-tenanted, geared_pagination, importmap-rails, jbuilder, kamal, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] erb: -> 5.0.1 - used by active_record-tenanted, importmap-rails, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] loofah: 2.24.0 -> 2.24.1 - used by active_record-tenanted, importmap-rails, jbuilder, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] mini_portile2: 2.8.8 -> 2.8.9 - used by active_record-tenanted, importmap-rails, jbuilder, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, sqlite3, stimulus-rails, turbo-rails - [ ] psych: 5.2.3 -> 5.2.6 - used by active_record-tenanted, importmap-rails, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] rack: 3.1.14 -> 3.1.15 - used by active_record-tenanted, importmap-rails, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] rails-dom-testing: 2.2.0 -> 2.3.0 - used by active_record-tenanted, importmap-rails, jbuilder, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] rdoc: 6.13.1 -> 6.14.0 - used by active_record-tenanted, importmap-rails, propshaft, rails, rails_structured_logging, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] zeitwerk: 2.7.2 -> 2.7.3 - used by active_record-tenanted, importmap-rails, kamal, propshaft, rails, rails_structured_logging, ruby_llm, sentry-rails, solid_cable, solid_cache, solid_queue, stimulus-rails, turbo-rails - [ ] action_text-trix: -> 2.1.15 - used by rails, rails_structured_logging - [ ] net-imap: 0.5.7 -> 0.5.8 - used by rails, rails_structured_logging - [ ] sentry-ruby: 5.23.0 -> 5.24.0 - used by sentry-rails --- Gemfile.lock | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b8dd7eebb..3965d502b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/basecamp/active_record-tenanted - revision: 612f5e8b7f77b7076a7eb21521920630161461aa + revision: 2bab09d2d6ae9791b9b3e645574b3fec4b6849f3 specs: active_record-tenanted (0.1.0) activerecord (>= 8.1.alpha) @@ -31,7 +31,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: bc7f77b32d7f3f26db654cae506c26a959852506 + revision: c1d7d56e831acf0aff5f3e27a6b384f543f358d0 branch: main specs: actioncable (8.1.0.alpha) @@ -65,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) @@ -131,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) @@ -139,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 @@ -168,8 +171,9 @@ GEM irb (~> 1.10) reline (>= 0.3.8) dotenv (3.1.8) - drb (2.2.1) + drb (2.2.3) ed25519 (1.4.0) + erb (5.0.1) erubi (1.13.1) et-orbi (1.2.11) tzinfo @@ -208,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) @@ -222,7 +226,7 @@ GEM language_server-protocol (3.17.0.4) lint_roller (1.1.0) logger (1.7.0) - loofah (2.24.0) + loofah (2.24.1) crass (~> 1.0.2) nokogiri (>= 1.12.0) mail (2.8.1) @@ -233,13 +237,13 @@ GEM marcel (1.0.4) matrix (0.4.2) mini_mime (1.1.5) - mini_portile2 (2.8.8) + mini_portile2 (2.8.9) minitest (5.25.5) msgpack (1.8.0) 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) @@ -277,7 +281,7 @@ GEM activesupport (>= 7.0.0) rack railties (>= 7.0.0) - psych (5.2.3) + psych (5.2.6) date stringio public_suffix (6.0.1) @@ -285,7 +289,7 @@ GEM nio4r (~> 2.0) raabro (1.4.0) racc (1.8.1) - rack (3.1.14) + rack (3.1.15) rack-contrib (2.5.0) rack (< 4) rack-session (2.1.1) @@ -295,7 +299,7 @@ GEM rack (>= 1.3) rackup (2.2.1) rack (>= 3) - rails-dom-testing (2.2.0) + rails-dom-testing (2.3.0) activesupport (>= 5.0.0) minitest nokogiri (>= 1.6) @@ -305,7 +309,8 @@ GEM rainbow (3.1.1) rake (13.2.1) rb_sys (0.9.106) - rdoc (6.13.1) + rdoc (6.14.0) + erb psych (>= 4.0.0) redcarpet (3.6.1) regexp_parser (2.10.0) @@ -348,19 +353,19 @@ GEM ruby-progressbar (1.13.0) 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.23.0) + sentry-ruby (~> 5.24.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) @@ -430,7 +435,7 @@ GEM websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.7.2) + zeitwerk (2.7.3) PLATFORMS arm64-darwin From c2c3a8d8ab1ac454ae3afc7144a0c229bfa1e9ae Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Wed, 14 May 2025 13:05:05 +0100 Subject: [PATCH 02/12] Don't require TLS for Beamer at the moment --- config/deploy.production.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/deploy.production.yml b/config/deploy.production.yml index 01f5fbfc8..6263ee89e 100644 --- a/config/deploy.production.yml +++ b/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 From f7759f67ef52e4ec001f2d9e50e1a468f6dd86c5 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Mon, 26 May 2025 09:43:13 -0500 Subject: [PATCH 03/12] Things got too tight --- app/assets/stylesheets/terminals.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/terminals.css b/app/assets/stylesheets/terminals.css index 0d23d000b..b709ad698 100644 --- a/app/assets/stylesheets/terminals.css +++ b/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); From d17134cb5c6cd8a67a80ec3424e7fc626ced73ce Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Tue, 27 May 2025 12:14:24 -0500 Subject: [PATCH 04/12] Detect platform and label the correct hotkey shortcut Yes, Platform Agent is overkill for just this but there isn't a one-size JS solution for updating both input placeholders and other HTML text and we're likely to need it later for other things like displaying platform-specific PWA prompts. --- Gemfile | 1 + Gemfile.lock | 4 ++++ app/controllers/application_controller.rb | 2 +- app/controllers/concerns/set_platform.rb | 12 ++++++++++ app/helpers/hotkeys_helper.rb | 19 +++++++++++++++ app/models/application_platform.rb | 29 +++++++++++++++++++++++ app/views/cards/index/_header.html.erb | 4 ++-- app/views/commands/_form.html.erb | 4 ++-- app/views/events/_filter.html.erb | 4 ++-- 9 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 app/controllers/concerns/set_platform.rb create mode 100644 app/helpers/hotkeys_helper.rb create mode 100644 app/models/application_platform.rb diff --git a/Gemfile b/Gemfile index 64a32d01a..1d8d39cd7 100644 --- a/Gemfile +++ b/Gemfile @@ -27,6 +27,7 @@ gem "rqrcode" gem "redcarpet" gem "rouge" gem "jbuilder" +gem "platform_agent" # Telemetry and logging gem "sentry-ruby" diff --git a/Gemfile.lock b/Gemfile.lock index 3965d502b..7353afc2c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -272,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) @@ -456,6 +459,7 @@ DEPENDENCIES importmap-rails jbuilder kamal + platform_agent propshaft puma (>= 5.0) rails! diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4f75453ff..20385eda0 100644 --- a/app/controllers/application_controller.rb +++ b/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 diff --git a/app/controllers/concerns/set_platform.rb b/app/controllers/concerns/set_platform.rb new file mode 100644 index 000000000..54e77d517 --- /dev/null +++ b/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 diff --git a/app/helpers/hotkeys_helper.rb b/app/helpers/hotkeys_helper.rb new file mode 100644 index 000000000..a1894104c --- /dev/null +++ b/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 diff --git a/app/models/application_platform.rb b/app/models/application_platform.rb new file mode 100644 index 000000000..c8c24a50b --- /dev/null +++ b/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 diff --git a/app/views/cards/index/_header.html.erb b/app/views/cards/index/_header.html.erb index 7ec915b4d..499e70b45 100644 --- a/app/views/cards/index/_header.html.erb +++ b/app/views/cards/index/_header.html.erb @@ -7,7 +7,7 @@
<%= 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 %>

<%= filter_title filter %> @@ -38,7 +38,7 @@ <%= render "cards/index/collections_filter", filter: filter %>

- Press ⌘j anytime to open this, esc to close. + Press <%= hotkey_label(["ctrl", "J"]) -%> anytime to open this, esc to close.

<% end %>

diff --git a/app/views/commands/_form.html.erb b/app/views/commands/_form.html.erb index d10a919d8..c01705fe7 100644 --- a/app/views/commands/_form.html.erb +++ b/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 keydown.ctrl+k@document->terminal#focus keydown.enter->terminal#executeCommand keydown.esc->terminal#hideMenus turbo:submit-end->terminal#handleCommandResponse" } do %> @@ -17,7 +17,7 @@ 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" } %> diff --git a/app/views/events/_filter.html.erb b/app/views/events/_filter.html.erb index 9f46f826f..38f13feb0 100644 --- a/app/views/events/_filter.html.erb +++ b/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 %> <%= filter_selected_collections_label(filter) %> @@ -35,7 +35,7 @@ <% end %>

- Press ⌘j anytime to open this, esc to close. + Press <%= hotkey_label(["ctrl", "K"]) -%> anytime to open this, esc to close.

<% end %> From 5f63bc68097884b70a8c89a2584e4c74d7b38f44 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Tue, 27 May 2025 14:18:39 -0400 Subject: [PATCH 05/12] dep: update AR::Tenanted to support database URIs ref: https://37s.fizzy.37signals.com/collections/7/cards/649 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 3965d502b..2236d60f5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/basecamp/active_record-tenanted - revision: 2bab09d2d6ae9791b9b3e645574b3fec4b6849f3 + revision: b3e75fd8c2d76c268b7aa0e662d068eb6e0e11bd specs: active_record-tenanted (0.1.0) activerecord (>= 8.1.alpha) From 880e3d98b2884a80fd719556ba35ff372a4598b6 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Wed, 28 May 2025 08:18:49 +0100 Subject: [PATCH 06/12] Prevent default when handling keyboard shortcuts Without this, any default action would still happen. With Chrome on Linux, that causes browser features to be triggered with our CTRL+J and CTRL+K actions. --- app/javascript/controllers/hotkey_controller.js | 1 + app/views/commands/_form.html.erb | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/controllers/hotkey_controller.js b/app/javascript/controllers/hotkey_controller.js index 7f6e5f3bf..2106155f3 100644 --- a/app/javascript/controllers/hotkey_controller.js +++ b/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() } } diff --git a/app/views/commands/_form.html.erb b/app/views/commands/_form.html.erb index c01705fe7..68e49081e 100644 --- a/app/views/commands/_form.html.erb +++ b/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.ctrl+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 %> @@ -22,4 +22,3 @@ <%= hidden_field_tag "confirmed", nil, data: { terminal_target: "confirmation" } %> <% end %> - From 50e2085e8b4148830ee2399e60031ad05360d85b Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Wed, 28 May 2025 13:58:26 +0100 Subject: [PATCH 07/12] Remove hyphen from "Un-do" --- app/views/cards/container/_closure.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/cards/container/_closure.html.erb b/app/views/cards/container/_closure.html.erb index 462bdfffd..24931b77c 100644 --- a/app/views/cards/container/_closure.html.erb +++ b/app/views/cards/container/_closure.html.erb @@ -3,7 +3,7 @@
Completed by <%= card.closed_by.name %> on <%= local_datetime_tag(card.closed_at, style: :shortdate) %>. <%= button_to card_closure_path(card), method: :delete, class: "btn btn--plain borderless fill-transparent" do %> - Un-do + Undo <% end %>
<% else %> From 8e873693bb6320d337375e35fb98850ae8c29317 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Wed, 28 May 2025 11:12:16 -0400 Subject: [PATCH 08/12] dep: bump active_record-tenanted to fix URI support --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 0874e8170..5b9940aad 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/basecamp/active_record-tenanted - revision: b3e75fd8c2d76c268b7aa0e662d068eb6e0e11bd + revision: 523947a0f0f49f66a8a62eb93e9ba1a345b3076b specs: active_record-tenanted (0.1.0) activerecord (>= 8.1.alpha) From 12622851812d41f6818cf9f905f745cce37196fd Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Wed, 28 May 2025 12:13:40 -0400 Subject: [PATCH 09/12] test: add missing assertions --- test/models/command/close_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/models/command/close_test.rb b/test/models/command/close_test.rb index 138e37aa4..83ea770e1 100644 --- a/test/models/command/close_test.rb +++ b/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 From c6c9e0d5fc0bf404f97bf65c1486530eae3e5cec Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 28 May 2025 15:26:41 -0500 Subject: [PATCH 10/12] Rework system events - Restyle system events so they don't look like comments and aren't attributed to the system user - Ensure the event creator is included in the description so it's clear who did it. - Walk back system comment replacement. Yes, it potentially avoids embarassing people when they change their mind, it's also losing data. If two intentional assigments happen in proximity, we've been only showing the latest one,. --- app/assets/stylesheets/comments.css | 30 +++++++++++++++- app/models/card/eventable/system_commenter.rb | 34 +++---------------- app/views/cards/comments/_comment.html.erb | 2 +- 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/app/assets/stylesheets/comments.css b/app/assets/stylesheets/comments.css index 8f9b26172..2a5e47960 100644 --- a/app/assets/stylesheets/comments.css +++ b/app/assets/stylesheets/comments.css @@ -37,6 +37,10 @@ } .comment__author { + .comment--system & strong { + display: none; + } + .btn { font-weight: inherit; } @@ -49,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; + } + p { &:first-child { margin-block-start: 0; @@ -95,8 +107,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 */ @@ -104,6 +121,17 @@ 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; + } } .comment__input { diff --git a/app/models/card/eventable/system_commenter.rb b/app/models/card/eventable/system_commenter.rb index 2bd4e88fd..73247d2a3 100644 --- a/app/models/card/eventable/system_commenter.rb +++ b/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 - comment.update! body: comment_body - else - card.comments.create! creator: User.system, body: comment_body, created_at: event.created_at - end + card.comments.create! creator: User.system, body: comment_body, created_at: event.created_at 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 diff --git a/app/views/cards/comments/_comment.html.erb b/app/views/cards/comments/_comment.html.erb index 9c41c81d6..800c44bcd 100644 --- a/app/views/cards/comments/_comment.html.erb +++ b/app/views/cards/comments/_comment.html.erb @@ -1,6 +1,6 @@ <% cache comment do %> <%= turbo_frame_tag comment do %> -
+
" data-creator-id="<%= comment.creator_id %>" data-created-by-current-user-target="creation"> From d1f01182f2d75c7014221f565c92939a858f1855 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 28 May 2025 15:32:00 -0500 Subject: [PATCH 11/12] Update tests --- .../card/eventable/system_commenter_test.rb | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/test/models/card/eventable/system_commenter_test.rb b/test/models/card/eventable/system_commenter_test.rb index 64eb05f0d..67678d32e 100644 --- a/test/models/card/eventable/system_commenter_test.rb +++ b/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)) From 3eeba253ad2972771b95682e88b0774665550b33 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 29 May 2025 00:39:06 +0200 Subject: [PATCH 12/12] Don't choke when rendering commands where the card was deleted --- app/models/command/go_to_card.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/command/go_to_card.rb b/app/models/command/go_to_card.rb index 0943413f3..a330d733b 100644 --- a/app/models/command/go_to_card.rb +++ b/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