Merge pull request #464 from basecamp/fizzy-do-warnings-and-more

Fizzy do: confirmations, close cards and more

app/controllers/commands_controller.rb

@@ -1,19 +1,18 @@
 class CommandsController < ApplicationController
   def index
-    @commands = Current.user.commands.order(created_at: :desc).limit(20)
+    @commands = Current.user.commands.order(created_at: :desc).limit(20).reverse
   end
 
   def create
     command = parse_command(params[:command])
 
-    if command&.valid?
-      result = command.execute
-
-      case result
-      when Command::Result::Redirection
-        redirect_to result.url
+    if command.valid?
+      if confirmed?(command)
+        command.save!
+        result = command.execute
+        respond_with_execution_result(result)
       else
-        redirect_back_or_to root_path
+        render plain: command.title, status: :conflict
       end
     else
       head :unprocessable_entity
@@ -22,12 +21,23 @@ class CommandsController < ApplicationController
 
   private
     def parse_command(string)
-      Command::Parser.new(parsing_context).parse(string).tap do |command|
-        Current.user.commands << command
-      end
+      Command::Parser.new(parsing_context).parse(string)
     end
 
     def parsing_context
       Command::Parser::Context.new(Current.user, url: request.referrer)
     end
+
+    def confirmed?(command)
+      !command.needs_confirmation? || params[:confirmed].present?
+    end
+
+    def respond_with_execution_result(result)
+      case result
+      when Command::Result::Redirection
+        redirect_to result.url
+      else
+        redirect_back_or_to root_path
+      end
+    end
 end

app/javascript/controllers/local_time_controller.js

@@ -11,8 +11,8 @@ export default class extends Controller {
   initialize() {
     this.timeFormatter = new Intl.DateTimeFormat(undefined, { timeStyle: "short" })
     this.dateFormatter = new Intl.DateTimeFormat(undefined, { dateStyle: "long" })
-    this.shortDateFormatter = new Intl.DateTimeFormat(undefined, { month: "short", day: "numeric" })
-    this.dateTimeFormatter = new Intl.DateTimeFormat(undefined, { timeStyle: "short", dateStyle: "short" })
+    this.shortdateFormatter = new Intl.DateTimeFormat(undefined, { month: "short", day: "numeric" })
+    this.datetimeFormatter = new Intl.DateTimeFormat(undefined, { timeStyle: "short", dateStyle: "short" })
     this.agoFormatter = new AgoFormatter()
     this.daysagoFormatter = new DaysAgoFormatter()
     this.datewithweekdayFormatter = new Intl.DateTimeFormat(undefined, { weekday: "long", month: "long", day: "numeric" })
@@ -46,11 +46,11 @@ export default class extends Controller {
   }
 
   datetimeTargetConnected(target) {
-    this.#formatTime(this.dateTimeFormatter, target)
+    this.#formatTime(this.datetimeFormatter, target)
   }
 
   shortdateTargetConnected(target) {
-    this.#formatTime(this.shortDateFormatter, target)
+    this.#formatTime(this.shortdateFormatter, target)
   }
 
   agoTargetConnected(target) {
@@ -78,7 +78,7 @@ export default class extends Controller {
   #formatTime(formatter, target) {
     const dt = new Date(target.getAttribute("datetime"))
     target.innerHTML = formatter.format(dt)
-    target.title = this.dateTimeFormatter.format(dt)
+    target.title = this.datetimeFormatter.format(dt)
   }
 }
 

app/javascript/controllers/navigable_list_controller.js

@@ -0,0 +1,78 @@
+import { Controller } from "@hotwired/stimulus"
+import { nextFrame } from "helpers/timing_helpers"
+
+export default class extends Controller {
+  static targets = [ "item" ]
+  static values = { selectionAttribute: { type: String, default: "aria-current" } }
+
+  // Actions
+
+  navigate(event) {
+    if (this.itemTargets.includes(event.target)) {
+      this.#keyHandlers[event.key]?.call(this, event)
+    }
+  }
+
+  selectCurrentOrLast(event) {
+    if (this.currentItem) {
+      this.#setCurrentFrom(this.currentItem)
+    } else {
+      this.selectLast()
+    }
+  }
+
+  selectLast() {
+    this.#setCurrentFrom(this.itemTargets[this.itemTargets.length - 1])
+  }
+
+  #selectPrevious() {
+    if (this.currentItem.previousElementSibling) {
+      this.#setCurrentFrom(this.currentItem.previousElementSibling)
+    }
+  }
+
+  #selectNext() {
+    if (this.currentItem.nextElementSibling) {
+      this.#setCurrentFrom(this.currentItem.nextElementSibling)
+    }
+  }
+
+  async #setCurrentFrom(element) {
+    const selectedItem = this.itemTargets.find(item => item.contains(element))
+
+    if (selectedItem) {
+      this.#clearSelection()
+      selectedItem.setAttribute(this.selectionAttributeValue, "true")
+      this.currentItem = selectedItem
+      await nextFrame()
+      this.currentItem.focus()
+    }
+  }
+
+  #clearSelection() {
+    for (const item of this.itemTargets) {
+      item.removeAttribute(this.selectionAttributeValue)
+    }
+  }
+
+  #handleArrowKey(event, fn) {
+    if (event.shiftKey || event.metaKey || event.ctrlKey) { return }
+    fn.call()
+    event.preventDefault()
+  }
+
+  #keyHandlers = {
+    ArrowDown(event) {
+      this.#handleArrowKey(event, this.#selectNext.bind(this))
+    },
+    ArrowUp(event) {
+      this.#handleArrowKey(event, this.#selectPrevious.bind(this))
+    },
+    ArrowRight(event) {
+      this.#handleArrowKey(event, this.#selectNext.bind(this))
+    },
+    ArrowLeft(event) {
+      this.#handleArrowKey(event, this.#selectPrevious.bind(this))
+    }
+  }
+}

app/javascript/controllers/terminal_controller.js

@@ -1,8 +1,9 @@
 import { Controller } from "@hotwired/stimulus"
+import { HttpStatus } from "helpers/http_helpers"
 
 export default class extends Controller {
-  static targets = [ "input" ]
-  static classes = [ "error" ]
+  static targets = [ "input", "form", "confirmation" ]
+  static classes = [ "error", "confirmation" ]
 
   // Actions
 
@@ -10,17 +11,78 @@ export default class extends Controller {
     this.inputTarget.focus()
   }
 
-  handleSubmit(event) {
+  executeCommand(event) {
     if (event.detail.success) {
-      event.target.reset()
+      this.#reset()
     } else {
-      this.#handleErrorResponse(event.detail.fetchResponse.response.status)
+      const response = event.detail.fetchResponse.response
+      this.#handleErrorResponse(response)
     }
   }
 
-  #handleErrorResponse(code) {
-    if (code == 422) {
-      this.element.classList.add(this.errorClass)
+  restoreCommand(event) {
+    const target = event.target.querySelector("[data-line]") || event.target
+    if (target.dataset.line) {
+      this.#reset(target.dataset.line)
+      this.focus()
     }
   }
+
+  async #handleErrorResponse(response) {
+    const status = response.status
+    const message = await response.text()
+
+    if (status === HttpStatus.UNPROCESSABLE) {
+      this.#showError()
+    } else if (status === HttpStatus.CONFLICT) {
+      this.#requestConfirmation(message)
+    }
+  }
+
+  #reset(inputValue = "") {
+    this.formTarget.reset()
+    this.inputTarget.value = inputValue
+    this.confirmationTarget.value = ""
+
+    this.element.classList.remove(this.errorClass)
+    this.element.classList.remove(this.confirmationClass)
+  }
+
+  #showError() {
+    this.element.classList.add(this.errorClass)
+  }
+
+  async #requestConfirmation(message) {
+    const originalInputValue = this.inputTarget.value
+    this.element.classList.add(this.confirmationClass)
+    this.inputTarget.value = `${message}? [Y/n] `
+
+    try {
+      await this.#waitForConfirmation()
+      this.#submitWithConfirmation(originalInputValue)
+    } catch {
+      this.#reset(originalInputValue)
+    }
+  }
+
+  #waitForConfirmation() {
+    return new Promise((resolve, reject) => {
+      this.inputTarget.addEventListener("keydown", (event) => {
+        event.preventDefault()
+        const key = event.key.toLowerCase()
+
+        if (key === "enter" || key === "y") {
+          resolve()
+        } else {
+          reject()
+        }
+      }, { once: true })
+    })
+  }
+
+  #submitWithConfirmation(inputValue) {
+    this.inputTarget.value = inputValue
+    this.confirmationTarget.value = "confirmed"
+    this.formTarget.requestSubmit()
+  }
 }

app/javascript/helpers/http_helpers.js

@@ -0,0 +1,4 @@
+export const HttpStatus = {
+  CONFLICT: 409,
+  UNPROCESSABLE: 422
+}

app/models/card/closeable.rb

@@ -42,7 +42,7 @@ module Card::Closeable
     closure&.created_at
   end
 
-  def close(user: Current.user, reason: Closure::Reason::FALLBACK_LABEL)
+  def close(user: Current.user, reason: Closure::Reason.default)
     unless closed?
       transaction do
         create_closure! user: user, reason: reason

app/models/closure/reason.rb

@@ -13,6 +13,10 @@ class Closure::Reason < ApplicationRecord
       pluck(:label).presence || [ FALLBACK_LABEL ]
     end
 
+    def default
+      labels.first
+    end
+
     def create_defaults
       DEFAULT_LABELS.each do |label|
         create! label: label

app/models/command.rb

@@ -24,6 +24,10 @@ class Command < ApplicationRecord
     false
   end
 
+  def needs_confirmation?
+    false
+  end
+
   private
     def redirect_to(...)
       Command::Result::Redirection.new(...)

app/models/command/assign.rb

@@ -1,27 +1,23 @@
 class Command::Assign < Command
-  store_accessor :data, :card_ids, :assignee_ids, :toggled_assignees_by_card
+  include Command::Cards
 
-  validates_presence_of :card_ids, :assignee_ids
+  store_accessor :data, :assignee_ids, :toggled_assignees_by_card
+
+  validates_presence_of :assignee_ids
 
   def title
-    card_description = if cards.one?
-      "card '#{cards.first.title}'"
-    else
-      "#{cards.count} cards"
-    end
-
     assignee_description = assignees.collect(&:first_name).join(", ")
 
-    "Assign #{assignee_description} to #{card_description}"
+    "Assign #{assignee_description} to #{cards_description}"
   end
 
   def execute
     toggled_assignees_by_card = {}
 
     transaction do
-      cards.each do |card|
+      cards.find_each do |card|
         toggled_assignees_by_card[card.id] = []
-        assignees.each do |assignee|
+        assignees.find_each do |assignee|
           assign(assignee, card, toggled_assignees_by_card)
         end
       end
@@ -31,27 +27,21 @@ class Command::Assign < Command
   end
 
   def undo
-    toggled_assignees_by_card.each do |card_id, assignee_ids|
-      card = user.accessible_cards.find_by_id(card_id)
-      assignees = User.where(id: assignee_ids)
+    transaction do
+      toggled_assignees_by_card.each do |card_id, assignee_ids|
+        card = user.accessible_cards.find_by_id(card_id)
+        assignees = User.where(id: assignee_ids)
 
-      undo_assignment(assignees, card)
+        undo_assignment(assignees, card)
+      end
     end
   end
 
-  def undoable?
-    true
-  end
-
   private
     def assignees
       User.where(id: assignee_ids)
     end
 
-    def cards
-      user.accessible_cards.where(id: card_ids)
-    end
-
     def assign(assignee, card, toggled_assignees_by_card)
       unless card.assigned_to?(assignee)
         toggled_assignees_by_card[card.id] << assignee.id

app/models/command/cards.rb

@@ -0,0 +1,30 @@
+module Command::Cards
+  extend ActiveSupport::Concern
+
+  included do
+    store_accessor :data, :card_ids
+
+    validates_presence_of :card_ids, :cards
+  end
+
+  def undoable?
+    true
+  end
+
+  def needs_confirmation?
+    cards.many?
+  end
+
+  private
+    def cards
+      user.accessible_cards.where(id: card_ids)
+    end
+
+    def cards_description
+      if cards.one?
+        "card '#{cards.first.title}'"
+      else
+        "#{cards.count} cards"
+      end
+    end
+end

app/models/command/close.rb

@@ -0,0 +1,35 @@
+class Command::Close < Command
+  include Command::Cards
+
+  store_accessor :data, :reason, :closed_card_ids
+
+  def title
+    "Close #{cards_description}"
+  end
+
+  def execute
+    closed_card_ids = []
+
+    transaction do
+      cards.find_each do |card|
+        closed_card_ids << card.id
+        card.close(user: user, reason: reason.presence || Closure::Reason.default)
+      end
+
+      update! closed_card_ids: closed_card_ids
+    end
+  end
+
+  def undo
+    transaction do
+      closed_cards.find_each do |card|
+        card.reopen
+      end
+    end
+  end
+
+  private
+    def closed_cards
+      user.accessible_cards.where(id: closed_card_ids)
+    end
+end

app/models/command/filter_cards.rb

@@ -0,0 +1,11 @@
+class Command::FilterCards < Command
+  store_accessor :data, :card_ids, :params
+
+  def title
+    "Filter cards #{card_ids.join(", ")}"
+  end
+
+  def execute
+    redirect_to cards_path(**params.without("card_ids").merge(card_ids: card_ids))
+  end
+end

app/models/command/parser.rb

@@ -8,18 +8,28 @@ class Command::Parser
   end
 
   def parse(string)
-    command_name, *command_arguments = string.strip.split(" ")
-
-    case command_name
-    when "/assign", "/assignto"
-      Command::Assign.new(assignee_ids: assignees_from(command_arguments).collect(&:id), card_ids: cards.ids)
-    when /^@/
-      Command::GoToUser.new(user_id: assignee_from(command_name)&.id)
-    else
-      search(string)
+    parse_command(string).tap do |command|
+      command.user = user
+      command.line = string
     end
   end
 
+  private
+    def parse_command(string)
+      command_name, *command_arguments = string.strip.split(" ")
+
+      case command_name
+      when "/assign", "/assignto"
+        Command::Assign.new(assignee_ids: assignees_from(command_arguments).collect(&:id), card_ids: cards.ids)
+      when "/close"
+        Command::Close.new(card_ids: cards.ids, reason: command_arguments.join(" "))
+      when /^@/
+        Command::GoToUser.new(user_id: assignee_from(command_name)&.id)
+      else
+        parse_free_string(string)
+      end
+    end
+
   private
     def assignees_from(strings)
       Array(strings).filter_map do |string|
@@ -34,11 +44,23 @@ class Command::Parser
       User.all.find { |user| user.mentionable_handles.include?(string_without_at) }
     end
 
-    def search(string)
-      if card = user.accessible_cards.find_by_id(string)
+    def parse_free_string(string)
+      if cards = multiple_cards_from(string)
+        Command::FilterCards.new(card_ids: cards.ids, params: filter.as_params)
+      elsif card = single_card_from(string)
         Command::GoToCard.new(card_id: card.id)
       else
         Command::Search.new(query: string, params: filter.as_params)
       end
     end
+
+    def multiple_cards_from(string)
+      if tokens = string.split(/[\s,]+/).filter { it =~ /\A\d+\z/ }.presence
+        user.accessible_cards.where(id: tokens).presence if tokens.many?
+      end
+    end
+
+    def single_card_from(string)
+      user.accessible_cards.find_by_id(string)
+    end
 end

app/models/command/parser/context.rb

@@ -11,6 +11,8 @@ class Command::Parser::Context
       user.accessible_cards.where id: params[:id]
     elsif controller == "cards" && action == "index"
       filter.cards
+    else
+      Card.none
     end
   end
 

app/models/filter.rb

@@ -19,6 +19,7 @@ class Filter < ApplicationRecord
   def cards
     @cards ||= begin
       result = creator.accessible_cards.indexed_by(indexed_by)
+      result = result.where(id: card_ids) if card_ids.present? && !indexed_by.closed?
       result = result.open unless indexed_by.closed?
       result = result.by_engagement_status(engagement_status) if engagement_status.present?
       result = result.unassigned if assignment_status.unassigned?

app/models/filter/fields.rb

@@ -16,7 +16,7 @@ module Filter::Fields
   end
 
   included do
-    store_accessor :fields, :assignment_status, :indexed_by, :terms, :engagement_status
+    store_accessor :fields, :assignment_status, :indexed_by, :terms, :engagement_status, :card_ids
 
     def assignment_status
       super.to_s.inquiry

app/models/filter/params.rb

@@ -5,6 +5,7 @@ module Filter::Params
     :assignment_status,
     :indexed_by,
     :engagement_status,
+    card_ids: [],
     assignee_ids: [],
     creator_ids: [],
     collection_ids: [],
@@ -40,8 +41,9 @@ module Filter::Params
       params[:assignment_status] = assignment_status
       params[:terms]             = terms
       params[:tag_ids]           = tags.ids
-      params[:collection_ids]        = collections.ids
+      params[:collection_ids]    = collections.ids
       params[:stage_ids]         = stages.ids
+      params[:card_ids]          = card_ids
       params[:assignee_ids]      = assignees.ids
       params[:creator_ids]       = creators.ids
     end.compact_blank.reject(&method(:default_value?))

app/views/commands/_command.html.erb

@@ -1,7 +1,11 @@
-<li class="min-width full-width flex gap-half terminal__item justify-start">
-  <button class="btn btn--plain overflow-ellipsis terminal__command flex-item-grow justify-start"><%= command.title %></button>
+<%= tag.li class: "min-width full-width flex gap-half terminal__item justify-start", tabindex: 0, data: {
+      action: "keydown.enter->terminal#restoreCommand:prevent keydown.enter->toggle-class#remove:prevent",
+      navigable_list_target: "item" } do %>
+  <%= button_tag command.title, type: "button", class: "btn btn--plain overflow-ellipsis terminal__command flex-item-grow justify-start",
+        data: { action: "toggle-class#remove terminal#restoreCommand", line: command.line } %>
 
   <% if command.undoable? %>
-    <%= button_to "Undo", command_undo_path(command), class: "btn btn--plain terminal__button flex-item-justify-end", data: { turbo_confirm: "Are you sure you want to undo '#{command.title}'?", turbo_frame: "_top" } %>
+    <%= button_to "Undo", command_undo_path(command), class: "btn btn--plain terminal__button flex-item-justify-end",
+          data: { turbo_confirm: "Are you sure you want to undo '#{command.title}'?", turbo_frame: "_top" } %>
   <% end %>
-</li>
+<% end %>

app/views/commands/_form.html.erb

@@ -3,7 +3,8 @@
       class: [ "flex align-center gap-half" ],
       data: {
         controller: "form",
-        action: "turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus turbo:submit-end->terminal#handleSubmit"
+        terminal_target: "form",
+        action: "turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus turbo:submit-end->terminal#executeCommand"
       } do %>
   <label class="terminal__label txt-nowrap" for="fizzy_do">Fizzy do &gt;</label>
 
@@ -14,9 +15,11 @@
         class: "terminal__input input fill-transparent unpad",
         data: {
           terminal_target: "input",
-          action: "keydown.up->toggle-class#add:prevent keydown.down->toggle-class#remove:prevent",
+          action: "keydown.up->toggle-class#add:prevent keydown.up->navigable-list#selectCurrentOrLast",
           turbo_permanent: true
         },
         placeholder: "Press ⌘+K to search or type commands…" %>
+
+  <%=  hidden_field_tag "confirmed", nil, data: { terminal_target: "confirmation" } %>
 <% end %>
 

app/views/commands/_terminal.html.erb

@@ -1,7 +1,9 @@
 <%= tag.div class: "terminal full-width flex flex-column gap justify-end", data: {
-      controller: "toggle-class terminal",
+      controller: "toggle-class terminal navigable-list",
       terminal_error_class: "terminal--error",
-      toggle_class_toggle_class: "terminal--open" } do %>
+      terminal_confirmation_class: "terminal--confirmation",
+      toggle_class_toggle_class: "terminal--open",
+      action: "keydown->navigable-list#navigate keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast" } do %>
   <%= turbo_frame_tag :recent_commands, src: commands_path, refresh: "morph" %>
 
   <%= render "commands/form" %>

db/migrate/20250507095113_add_line_to_commands.rb

@@ -0,0 +1,5 @@
+class AddLineToCommands < ActiveRecord::Migration[8.1]
+  def change
+    add_column :commands, :line, :text
+  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_05_05_123008) do
+ActiveRecord::Schema[8.1].define(version: 2025_05_07_095113) do
   create_table "accesses", force: :cascade do |t|
     t.integer "collection_id", null: false
     t.datetime "created_at", null: false
@@ -162,6 +162,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_05_05_123008) do
   create_table "commands", force: :cascade do |t|
     t.datetime "created_at", null: false
     t.json "data", default: {}
+    t.text "line"
     t.string "type"
     t.datetime "updated_at", null: false
     t.integer "user_id", null: false

db/schema_cache.yml

@@ -553,6 +553,16 @@ columns:
     collation:
     comment:
   - *6
+  - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
+    auto_increment:
+    name: line
+    cast_type: *14
+    sql_type_metadata: *15
+    'null': true
+    default:
+    default_function:
+    collation:
+    comment:
   - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
     auto_increment:
     name: type
@@ -2051,4 +2061,4 @@ indexes:
     comment:
     valid: true
   workflows: []
-version: 20250505123008
+version: 20250507095113

test/controllers/commands_controller_test.rb

@@ -15,12 +15,20 @@ class CommandsControllerTest < ActionDispatch::IntegrationTest
 
   test "command that triggers a redirect back" do
     assert_difference -> { users(:kevin).commands.count }, +1 do
-      post commands_path, params: { command: "/assign @kevin" }, headers: { "HTTP_REFERER" => cards_path }
+      post commands_path, params: { command: "/assign @kevin", confirmed: "confirmed" }, headers: { "HTTP_REFERER" => cards_path }
     end
 
     assert_redirected_to cards_path
   end
 
+  test "commands requiring confirmation return a 409 conflict response" do
+    assert_no_difference -> { users(:kevin).commands.count } do
+      post commands_path, params: { command: "/assign @kevin" }, headers: { "HTTP_REFERER" => cards_path }
+    end
+
+    assert_response :conflict
+  end
+
   test "get a 422 on errors" do
     post commands_path, params: { command: "/assign @some_missing_user" }, headers: { "HTTP_REFERER" => cards_path }
     assert_response :unprocessable_entity

test/models/command/assign_test.rb

@@ -17,7 +17,7 @@ class Command::AssignTest < ActionDispatch::IntegrationTest
     assert_includes @card.assignees, users(:kevin)
   end
 
-  test "assigns card on cards' index page" do
+  test "assigns cards on cards' index page" do
     execute_command "/assign @kevin @david", context_url: collection_cards_url(@card.collection)
 
     cards(:logo, :text, :layout).each do |card|

test/models/command/close_test.rb

@@ -0,0 +1,51 @@
+require "test_helper"
+
+class Command::CloseTest < ActionDispatch::IntegrationTest
+  include CommandTestHelper
+
+  setup do
+    Current.session = sessions(:david)
+    @card = cards(:text)
+  end
+
+  test "closes card on perma" do
+    assert_changes -> { @card.reload.closed? }, from: false, to: true do
+      execute_command "/close", context_url: collection_card_url(@card.collection, @card)
+    end
+
+    assert_equal Closure::Reason.default, @card.closure.reason
+    assert_equal users(:david), @card.closed_by
+  end
+
+  test "closes card on perma with reason" do
+    assert_changes -> { @card.reload.closed? }, from: false, to: true do
+      execute_command "/close Not now", context_url: collection_card_url(@card.collection, @card)
+    end
+
+    assert_equal users(:david), @card.closed_by
+    assert_equal "Not now", @card.closure.reason
+  end
+
+  test "closes cards on cards' index page" do
+    cards_to_check = cards(:logo, :text, :layout)
+    cards_to_check.each(&:reopen)
+
+    execute_command "/close", context_url: collection_cards_url(@card.collection)
+
+    cards_to_check.each { it.reload.closed? }
+  end
+
+  test "undo closing" do
+    cards_to_check = cards(:logo, :text, :layout)
+    cards_to_check.each(&:reopen)
+
+    command = parse_command "/close", context_url: collection_cards_url(@card.collection)
+    command.execute
+
+    cards_to_check.each { it.reload.closed? }
+
+    command.undo
+
+    cards_to_check.each { it.reload.open? }
+  end
+end

test/models/command/filter_cards_test.rb

@@ -0,0 +1,21 @@
+require "test_helper"
+
+class Command::FilterCardsTest < ActionDispatch::IntegrationTest
+  include CommandTestHelper
+
+  setup do
+    @card_ids = cards(:logo, :layout).collect(&:id)
+  end
+
+  test "redirects to the cards index filtering by cards" do
+    result = execute_command "#{@card_ids.join(" ")}"
+
+    assert_equal cards_path(indexed_by: "newest", card_ids: @card_ids), result.url
+  end
+
+  test "respects existing filters" do
+    result = execute_command "#{@card_ids.join(",")}", context_url: "http://37signals.fizzy.localhost:3006/cards?collection_ids%5B%5D=#{collections(:writebook).id}"
+
+    assert_equal cards_path(indexed_by: "newest", collection_ids: [ collections(:writebook).id ], card_ids: @card_ids), result.url
+  end
+end

test/models/command/parser_test.rb

@@ -1 +1,11 @@
+require "test_helper"
+
 # The parser is tested through the tests of specific commands. See +Command::AssignTests+, etc.
+class Command::ParserTest < ActionDispatch::IntegrationTest
+  include CommandTestHelper
+
+  test "the parsed command contains the raw line" do
+    result = parse_command "assign @kevin"
+    assert_equal "assign @kevin", result.line
+  end
+end

test/models/filter_test.rb

@@ -31,6 +31,9 @@ class FilterTest < ActiveSupport::TestCase
 
     filter = users(:david).filters.new indexed_by: "closed"
     assert_equal [ cards(:shipping) ], filter.cards
+
+    filter = users(:david).filters.new card_ids: [ cards(:logo, :layout).collect(&:id) ]
+    assert_equal [ cards(:logo), cards(:layout) ], filter.cards
   end
 
   test "can't see cards in collections that aren't accessible" do