Merge branch 'main' into card-attachments

* main:
  Don't create mentions from drafts until those are published
  Change Active Storage account slug extension to a to_prepare block
  dep: bump actiontext-lexical for previewable attachment fix
  Add assertion for composite command line
  Remove trace
  restore the right command
  Update action text
  The prompt confirmation system wasn't really working after moving to lexical
  Parse commands where it's just a gid properly
  Move inside concerns or the override won't work
  Truncation for comment author line
  Avoid duplicated queries and, instead, bump the last ones
  Adjust screen for screen readers
  Adjust icon sizing and uploading styles

Gemfile.lock

@@ -1,6 +1,6 @@
 GIT
   remote: https://github.com/basecamp/actiontext-lexical
-  revision: 38e8d4e5781667b7d1c17eaa5cca503bbc63ee67
+  revision: 70ba099446c8a033c06c2bde9c5e70b7f983e60b
   specs:
     actiontext-lexical (0.1.0)
       rails (>= 8.0.2)
@@ -444,7 +444,7 @@ GEM
     rainbow (3.1.1)
     rake (13.3.0)
     rb_sys (0.9.106)
-    rdoc (6.14.1)
+    rdoc (6.14.2)
       erb
       psych (>= 4.0.0)
     redcarpet (3.6.1)

app/assets/stylesheets/comments.css

@@ -37,6 +37,8 @@
   }
 
   .comment__author {
+    margin-inline-end: calc(var(--comment-padding-inline) * -0.66);
+
     .comment--system & {
       h3 {
         margin-inline: auto;

app/assets/stylesheets/rich-text-content.css

@@ -217,6 +217,8 @@
   .attachment--file {
     align-items: center;
     display: inline-flex;
+    flex-wrap: wrap;
+    inline-size: auto;
 
     .attachment__caption {
       flex-direction: column;
@@ -228,12 +230,17 @@
         color: var(--color-ink);
       }
     }
+
+    .attachment__name {
+      color: var(--color-ink);
+      font-weight: bold;
+    }
   }
 
   .attachment__icon {
     aspect-ratio: 4/5;
     background-color: var(--color-canvas);
-    block-size: calc(2.5lh + 0.5ch);
+    block-size: calc(2lh + (var(--block-space) - 2px));
     border: 2px solid var(--color-ink-medium);
     border-radius: 0.3em;
     color: var(--color-ink-dark);
@@ -252,7 +259,6 @@
       block-size: 0.5lh;
       content: "";
       display: block;
-      inline-size: 100%;
       inset: 0 0 auto;
       position: absolute;
     }

app/controllers/searches_controller.rb

@@ -3,6 +3,6 @@ class SearchesController < ApplicationController
 
   def show
     @search_results = Current.user.search(@query_terms).limit(50)
-    @recent_search_queries = Current.user.search_queries.order(created_at: :desc).limit(50).uniq(&:terms)
+    @recent_search_queries = Current.user.search_queries.order(updated_at: :desc).limit(10)
   end
 end

app/javascript/controllers/terminal_controller.js

@@ -162,13 +162,15 @@ export default class extends Controller {
     this.waitingForConfirmationValue = true
   }
 
-  #showConfirmationPrompt(confirmationPrompt) {
+  async #showConfirmationPrompt(confirmationPrompt) {
     if (isMultiLineString(confirmationPrompt)) {
       this.#showOutput(confirmationPrompt)
       this.inputTarget.value = `Apply these changes? [Y/n] `
     } else {
       this.inputTarget.value = `${confirmationPrompt}? [Y/n] `
     }
+
+    await nextFrame()
   }
 
   #handleConfirmationKey(key) {
@@ -180,16 +182,17 @@ export default class extends Controller {
     }
   }
 
-  #submitWithConfirmation() {
+  async #submitWithConfirmation() {
     this.inputTarget.value = this.originalInputValue
     this.confirmationTarget.value = "confirmed"
     this.#hideOutput()
+
+    await nextFrame()
     this.#submitCommand()
   }
 
   #submitCommand() {
     this.formTarget.requestSubmit()
-    this.#reset()
   }
 
   #showOutput(markdown) {

app/models/card/mentions.rb

@@ -0,0 +1,11 @@
+module Card::Mentions
+  extend ActiveSupport::Concern
+
+  included do
+    include ::Mentions
+
+    def mentionable?
+      published?
+    end
+  end
+end

app/models/command/ai/parser.rb

@@ -30,7 +30,7 @@ class Command::Ai::Parser
         commands.unshift Command::VisitUrl.new(user: user, url: query_context.url, context: resolved_context)
       end
 
-      Command::Composite.new(title: query, commands: commands, user: user, line: normalized_query, context: resolved_context)
+      Command::Composite.new(title: query, commands: commands, user: user, line: query, context: resolved_context)
     end
 
     def commands_from_query(normalized_query, context)

app/models/command/parser.rb

@@ -37,9 +37,7 @@ class Command::Parser
       when /^#/
         Command::FilterByTag.new(tag_title: tag_title_from(string), params: filter.as_params)
       when /^@/
-        Command::GoToUser.new(user_id: context.find_user(command_name)&.id)
-      when "/user"
-        Command::GoToUser.new(user_id: context.find_user(combined_arguments)&.id)
+        Command::GoToUser.new(user_id: context.find_user(string)&.id)
       when "/assign", "/assignto"
         Command::Assign.new(assignee_ids: assignees_from(command_arguments).collect(&:id), card_ids: cards.ids)
       when "/clear"
@@ -64,11 +62,22 @@ class Command::Parser
         Command::VisitUrl.new(url: command_arguments.first)
       when "/tag"
         Command::Tag.new(tag_title: tag_title_from(combined_arguments), card_ids: cards.ids)
+      when /^gid:\/\//
+        parse_gid command_name
       else
         parse_free_string(string)
       end
     end
 
+    def parse_gid(command_name)
+      case record = GlobalID::Locator.locate(command_name)
+      when Tag
+        Command::FilterByTag.new(tag_title: record.title, params: filter.as_params)
+      when User
+        Command::GoToUser.new(user_id: record.id)
+      end
+    end
+
     def assignees_from(strings)
       Array(strings).filter_map do |string|
         context.find_user(string)

app/models/comment/mentions.rb

@@ -0,0 +1,11 @@
+module Comment::Mentions
+  extend ActiveSupport::Concern
+
+  included do
+    include ::Mentions
+
+    def mentionable?
+      card.published?
+    end
+  end
+end

app/models/concerns/mentions.rb

@@ -4,7 +4,7 @@ module Mentions
   included do
     has_many :mentions, as: :source, dependent: :destroy
     has_many :mentionees, through: :mentions
-    after_save_commit :create_mentions_later, if: :mentionable_content_changed?
+    after_save_commit :create_mentions_later, if: :should_create_mentions?
   end
 
   def create_mentions(mentioner: Current.user)
@@ -48,6 +48,10 @@ module Mentions
       self.class.reflect_on_all_associations(:has_one).filter { it.klass == ActionText::RichText }
     end
 
+    def should_create_mentions?
+      mentionable? && mentionable_content_changed?
+    end
+
     def mentionable_content_changed?
       rich_text_associations.any? { send(it.name)&.body_previously_changed? }
     end
@@ -55,4 +59,9 @@ module Mentions
     def create_mentions_later
       Mention::CreateJob.perform_later(self, mentioner: Current.user)
     end
+
+    # Template method
+    def mentionable?
+      true
+    end
 end

app/models/tag/attachable.rb

@@ -3,9 +3,9 @@ module Tag::Attachable
 
   included do
     include ActionText::Attachable
-  end
 
-  def attachable_plain_text_representation(...)
-    "##{title}"
+    def attachable_plain_text_representation(...)
+      "##{title}"
+    end
   end
 end

app/models/user/attachable.rb

@@ -3,9 +3,9 @@ module User::Attachable
 
   included do
     include ActionText::Attachable
-  end
 
-  def attachable_plain_text_representation(...)
-    "@#{first_name.downcase}"
+    def attachable_plain_text_representation(...)
+      "@#{first_name.downcase}"
+    end
   end
 end

app/models/user/searcher.rb

@@ -10,6 +10,8 @@ module User::Searcher
   end
 
   def remember_search(terms)
-    search_queries.create(terms: terms) if search_queries.last&.terms != terms
+    search_queries.find_or_create_by(terms: terms).tap do |search_query|
+      search_query.touch unless search_query.previously_new_record?
+    end
   end
 end

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

@@ -8,7 +8,7 @@
 
       <div class="comment__content flex flex-column flex-item-grow full-width">
         <div class="comment__author flex align-center gap-half">
-          <h3 class="font-weight-normal txt-normal flex-item-justify-start">
+          <h3 class="font-weight-normal txt-normal flex-item-justify-start min-width overflow-ellipsis">
             <strong>
               <%= link_to comment.creator.name, comment.creator, class: "txt-ink btn btn--plain fill-transparent", data: { turbo_frame: "_top" } %>
             </strong>

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

@@ -7,9 +7,10 @@
   <%= form_with model: card, url: collection_card_path(card.collection, card), data: { controller: "form" } do |form| %>
     <label class="btn input--file">
       <%= icon_tag "picture-add" %>
-      <span class="for-screen-reader">Add background image</span>
+      <span class="for-screen-reader"></span>
       <%= form.file_field :image, class: "input",
             accept: "image/png, image/jpeg, image/jpg, image/webp",
+            aria: { label: "Add background image" },
             data: { action: "upload-preview#previewImage form#submit", upload_preview_target: "input" } %>
     </label>
   <% end %>

config/initializers/tenanting/active_storage.rb

@@ -14,6 +14,6 @@ module ActiveStorageControllerExtensions
   end
 end
 
-Rails.application.config.after_initialize do
+Rails.application.config.to_prepare do
   ActiveStorage::BaseController.include ActiveStorageControllerExtensions
 end

db/migrate/20250703193928_add_index_to_search_queries.rb

@@ -0,0 +1,5 @@
+class AddIndexToSearchQueries < ActiveRecord::Migration[8.1]
+  def change
+    add_index :search_queries, %w[ user_id updated_at ], unique: true
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.1].define(version: 2025_07_02_211937) do
+ActiveRecord::Schema[8.1].define(version: 2025_07_03_193928) do
   create_table "accesses", force: :cascade do |t|
     t.datetime "accessed_at"
     t.integer "collection_id", null: false
@@ -315,6 +315,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_07_02_211937) do
     t.datetime "updated_at", null: false
     t.integer "user_id", null: false
     t.index ["user_id", "terms"], name: "index_search_queries_on_user_id_and_terms"
+    t.index ["user_id", "updated_at"], name: "index_search_queries_on_user_id_and_updated_at", unique: true
     t.index ["user_id"], name: "index_search_queries_on_user_id"
   end
 

db/schema_cache.yml

@@ -2312,6 +2312,23 @@ indexes:
     nulls_not_distinct:
     comment:
     valid: true
+  - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
+    table: search_queries
+    name: index_search_queries_on_user_id_and_updated_at
+    unique: true
+    columns:
+    - user_id
+    - updated_at
+    lengths: {}
+    orders: {}
+    opclasses: {}
+    where:
+    type:
+    using:
+    include:
+    nulls_not_distinct:
+    comment:
+    valid: true
   search_results: []
   sessions:
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
@@ -2499,4 +2516,4 @@ indexes:
     comment:
     valid: true
   workflows: []
-version: 20250702211937
+version: 20250703193928

script/remove_duplicated_search_queries.rb

@@ -0,0 +1,19 @@
+#!/usr/bin/env ruby
+
+require_relative "../config/environment"
+
+ApplicationRecord.with_each_tenant do |tenant|
+  User.find_each do |user|
+    search_queries = Set.new
+    to_delete = []
+    user.search_queries.find_each do |search_query|
+      if search_queries.include?(search_query.terms)
+        to_delete << search_query
+      end
+
+      search_queries << search_query.terms
+    end
+
+    to_delete.each(&:destroy)
+  end
+end

test/models/command/ai/parser_test.rb

@@ -4,10 +4,11 @@ class Command::Ai::ParserTest < ActionDispatch::IntegrationTest
   include CommandTestHelper, VcrTestHelper
 
   test "parse command strings into a composite command containing the individual commands" do
-    result = parse_command "assign @kevin and close"
+    command = parse_command "assign @kevin and close"
 
-    assert_instance_of Command::Composite, result
-    commands = result.commands
+    assert_equal "assign @kevin and close", command.line
+    assert_instance_of Command::Composite, command
+    commands = command.commands
 
     assert_instance_of Command::Assign, commands.first
     assert_instance_of Command::Close, commands.last

test/models/concerns/mentions_test.rb

@@ -5,20 +5,75 @@ class MentionsTest < ActiveSupport::TestCase
     Current.session = sessions(:david)
   end
 
-  test "create mentions from plain text mentions" do
-    assert_difference -> { Mention.count }, +1 do
+  test "don't create mentions when creating or updating drafts" do
+    assert_no_difference -> { Mention.count } do
       perform_enqueued_jobs only: Mention::CreateJob do
-        collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, @david?"
+        card = collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, @david?"
+        card.update description: "Any thoughts here @jz"
       end
     end
   end
 
-  test "create mentions from rich text mentions" do
-    assert_difference -> { Mention.count }, +1 do
-      perform_enqueued_jobs only: Mention::CreateJob do
+  test "create mentions from plain text mentions when publishing cards" do
+    perform_enqueued_jobs only: Mention::CreateJob do
+      card = assert_no_difference -> { Mention.count } do
+        collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, @david?"
+      end
+
+      assert_difference -> { Mention.count }, +1 do
+        card.published!
+      end
+    end
+  end
+
+  test "create mentions from rich text mentions when publishing cards" do
+    perform_enqueued_jobs only: Mention::CreateJob do
+      card = assert_no_difference -> { Mention.count } do
         attachment = ActionText::Attachment.from_attachable(users(:david))
         collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, #{attachment.to_html}?"
       end
+
+      assert_difference -> { Mention.count }, +1 do
+        card.published!
+      end
+    end
+  end
+
+  test "don't create repeated mentions when updating cards" do
+    perform_enqueued_jobs only: Mention::CreateJob do
+      card = collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, @david?"
+
+      assert_difference -> { Mention.count }, +1 do
+        card.published!
+      end
+
+      assert_no_difference -> { Mention.count } do
+        card.update description: "Any thoughts here @david"
+      end
+
+      assert_difference -> { Mention.count }, +1 do
+        card.update description: "Any thoughts here @jz"
+      end
+    end
+  end
+
+  test "create mentions from plain text mentions when posting comments" do
+    perform_enqueued_jobs only: Mention::CreateJob do
+      card = collections(:writebook).cards.create title: "Cleanup", description: "Some initial content", status: :published
+
+      assert_difference -> { Mention.count }, +1 do
+        card.comments.create!(body: "Great work on this @david!")
+      end
+    end
+  end
+
+  test "don't create mentions from comments when belonging to unpublished cards" do
+    perform_enqueued_jobs only: Mention::CreateJob do
+      card = collections(:writebook).cards.create title: "Cleanup", description: "Some initial content"
+
+      assert_no_difference -> { Mention.count } do
+        card.comments.create!(body: "Great work on this @david!")
+      end
     end
   end
 
@@ -37,6 +92,7 @@ class MentionsTest < ActiveSupport::TestCase
   test "mentionees are added as watchers of the card" do
     perform_enqueued_jobs only: Mention::CreateJob do
       card = collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup @kevin?"
+      card.published!
       assert card.watchers.include?(users(:kevin))
     end
   end

test/models/user/searcher_test.rb

@@ -13,11 +13,16 @@ class User::SearcherTest < ActiveSupport::TestCase
     assert_equal "broken", @user.search_queries.last.terms
   end
 
-  test "don't duplicate repeated searches" do
-    @user.remember_search("broken")
+  test "don't duplicate repeated searches but touch the existing match" do
+    search_result = @user.remember_search("broken")
+    original_updated_at = search_result.updated_at
+
+    travel_to 1.day.from_now
 
     assert_no_difference -> { @user.search_queries.count }, +1 do
       @user.remember_search("broken")
     end
+
+    assert search_result.reload.updated_at > original_updated_at
   end
 end