diff --git a/app/controllers/commands_controller.rb b/app/controllers/commands_controller.rb index c70750bfc..1f835408f 100644 --- a/app/controllers/commands_controller.rb +++ b/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 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/javascript/controllers/navigable_list_controller.js b/app/javascript/controllers/navigable_list_controller.js new file mode 100644 index 000000000..4951517d9 --- /dev/null +++ b/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)) + } + } +} diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 177aa5a23..a4148623c 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/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() + } } diff --git a/app/javascript/helpers/http_helpers.js b/app/javascript/helpers/http_helpers.js new file mode 100644 index 000000000..1fb986f70 --- /dev/null +++ b/app/javascript/helpers/http_helpers.js @@ -0,0 +1,4 @@ +export const HttpStatus = { + CONFLICT: 409, + UNPROCESSABLE: 422 +} 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.rb b/app/models/command.rb index dc924dd82..1b38f82b9 100644 --- a/app/models/command.rb +++ b/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(...) diff --git a/app/models/command/assign.rb b/app/models/command/assign.rb index 082b54fe1..69c4a06dd 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 @@ -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 diff --git a/app/models/command/cards.rb b/app/models/command/cards.rb new file mode 100644 index 000000000..d3e47792e --- /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, :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 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/filter_cards.rb b/app/models/command/filter_cards.rb new file mode 100644 index 000000000..32d7ce426 --- /dev/null +++ b/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 diff --git a/app/models/command/parser.rb b/app/models/command/parser.rb index df5af978a..830b6c93d 100644 --- a/app/models/command/parser.rb +++ b/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 diff --git a/app/models/command/parser/context.rb b/app/models/command/parser/context.rb index 770477963..09591ce72 100644 --- a/app/models/command/parser/context.rb +++ b/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 diff --git a/app/models/filter.rb b/app/models/filter.rb index e5ce39383..772e80c8b 100644 --- a/app/models/filter.rb +++ b/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? diff --git a/app/models/filter/fields.rb b/app/models/filter/fields.rb index 9ff9764d4..8c0996649 100644 --- a/app/models/filter/fields.rb +++ b/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 diff --git a/app/models/filter/params.rb b/app/models/filter/params.rb index c6f0df13e..a7bed80bb 100644 --- a/app/models/filter/params.rb +++ b/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?)) diff --git a/app/views/commands/_command.html.erb b/app/views/commands/_command.html.erb index c10b933e3..9fcecfac9 100644 --- a/app/views/commands/_command.html.erb +++ b/app/views/commands/_command.html.erb @@ -1,7 +1,11 @@ -