From c6c9e0d5fc0bf404f97bf65c1486530eae3e5cec Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 28 May 2025 15:26:41 -0500 Subject: [PATCH] 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">