Show confirmation when submitting certain commands
https://3.basecamp.com/2914079/buckets/37331921/todos/8617430347
This commit is contained in:
@@ -7,13 +7,11 @@ class CommandsController < ApplicationController
|
||||
command = parse_command(params[:command])
|
||||
|
||||
if command&.valid?
|
||||
result = command.execute
|
||||
|
||||
case result
|
||||
when Command::Result::Redirection
|
||||
redirect_to result.url
|
||||
if confirmed?(command)
|
||||
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
|
||||
@@ -30,4 +28,17 @@ class CommandsController < ApplicationController
|
||||
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
|
||||
|
||||
@@ -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,70 @@ 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)
|
||||
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()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
export const HttpStatus = {
|
||||
CONFLICT: 409,
|
||||
UNPROCESSABLE: 422
|
||||
}
|
||||
@@ -24,6 +24,10 @@ class Command < ApplicationRecord
|
||||
false
|
||||
end
|
||||
|
||||
def needs_confirmation?
|
||||
false
|
||||
end
|
||||
|
||||
private
|
||||
def redirect_to(...)
|
||||
Command::Result::Redirection.new(...)
|
||||
|
||||
@@ -43,6 +43,10 @@ class Command::Assign < Command
|
||||
true
|
||||
end
|
||||
|
||||
def needs_confirmation?
|
||||
cards.many?
|
||||
end
|
||||
|
||||
private
|
||||
def assignees
|
||||
User.where(id: assignee_ids)
|
||||
|
||||
@@ -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 ></label>
|
||||
|
||||
@@ -18,5 +19,7 @@
|
||||
turbo_permanent: true
|
||||
},
|
||||
placeholder: "Press ⌘+K to search or type commands…" %>
|
||||
|
||||
<%= hidden_field_tag "confirmed", nil, data: { terminal_target: "confirmation" } %>
|
||||
<% end %>
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
<%= tag.div class: "terminal full-width flex flex-column gap justify-end", data: {
|
||||
controller: "toggle-class terminal",
|
||||
terminal_error_class: "terminal--error",
|
||||
terminal_confirmation_class: "terminal--confirmation",
|
||||
toggle_class_toggle_class: "terminal--open" } do %>
|
||||
<%= turbo_frame_tag :recent_commands, src: commands_path, refresh: "morph" %>
|
||||
|
||||
|
||||
@@ -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_difference -> { users(:kevin).commands.count }, +1 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
|
||||
|
||||
Reference in New Issue
Block a user