From dc28f4c9c52b42c26970d2ae75bfeac772fe5e4f Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 10:46:39 +0200 Subject: [PATCH] Add command to close cards https://3.basecamp.com/2914079/buckets/37331921/todos/8620236700 --- .../controllers/local_time_controller.js | 10 ++-- app/models/card/closeable.rb | 2 +- app/models/closure/reason.rb | 4 ++ app/models/command/assign.rb | 30 +++-------- app/models/command/cards.rb | 30 +++++++++++ app/models/command/close.rb | 35 +++++++++++++ app/models/command/parser.rb | 2 + test/models/command/assign_test.rb | 2 +- test/models/command/close_test.rb | 51 +++++++++++++++++++ 9 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 app/models/command/cards.rb create mode 100644 app/models/command/close.rb create mode 100644 test/models/command/close_test.rb diff --git a/app/javascript/controllers/local_time_controller.js b/app/javascript/controllers/local_time_controller.js index 7fe903c85..e539d9eb5 100644 --- a/app/javascript/controllers/local_time_controller.js +++ b/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) } } diff --git a/app/models/card/closeable.rb b/app/models/card/closeable.rb index b599b6a03..e0d15831b 100644 --- a/app/models/card/closeable.rb +++ b/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 diff --git a/app/models/closure/reason.rb b/app/models/closure/reason.rb index 1472d09d1..9dca74fc3 100644 --- a/app/models/closure/reason.rb +++ b/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 diff --git a/app/models/command/assign.rb b/app/models/command/assign.rb index 439f01f12..c149b936c 100644 --- a/app/models/command/assign.rb +++ b/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 @@ -39,23 +35,11 @@ class Command::Assign < Command end end - def undoable? - true - end - - def needs_confirmation? - cards.many? - 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 diff --git a/app/models/command/cards.rb b/app/models/command/cards.rb new file mode 100644 index 000000000..8ba9e4c74 --- /dev/null +++ b/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 + 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 diff --git a/app/models/command/close.rb b/app/models/command/close.rb new file mode 100644 index 000000000..ac1051c07 --- /dev/null +++ b/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 diff --git a/app/models/command/parser.rb b/app/models/command/parser.rb index df5af978a..c56032d91 100644 --- a/app/models/command/parser.rb +++ b/app/models/command/parser.rb @@ -13,6 +13,8 @@ class Command::Parser 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 diff --git a/test/models/command/assign_test.rb b/test/models/command/assign_test.rb index c551da602..705dad10e 100644 --- a/test/models/command/assign_test.rb +++ b/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| diff --git a/test/models/command/close_test.rb b/test/models/command/close_test.rb new file mode 100644 index 000000000..abdb821b3 --- /dev/null +++ b/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