neiam
/waxx
waxx
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages
⭐
Log in to mark this repository.
Credo: board show handlers share a board-card lookup and lose a nesting level
cc331cf · Gabe Morell · 2026-11-04 20:36
Files changed
modified
lib/waxx_web/live/board_live/show.ex
+131
−131
@@ -362,84 +362,72 @@ defmodule WaxxWeb.BoardLive.Show do
| 362 | 362 | # drag doesn't need two round-trips. |
| 363 | 363 | def handle_event("place_card", %{"id" => id} = params, socket) do |
| 364 | 364 | guard_edit(socket, fn -> |
| 365 | − card = Kanban.get_card(id) | |
| 365 | + card = get_board_card(socket, id) | |
| 366 | 366 | actor = socket.assigns.current_scope.user |
| 367 | 367 | target_stage_id = params["stage-id"] |
| 368 | 368 | target_subboard_id = blank_to_nil(params["subboard-id"]) |
| 369 | 369 | target_index = parse_index(params["index"]) |
| 370 | 370 | |
| 371 | − cond do | |
| 372 | − is_nil(card) or card.board_id != socket.assigns.board.id -> | |
| 373 | − {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 374 | − | |
| 375 | − true -> | |
| 376 | − # Always run the stage move first (it enforces transitions). If | |
| 377 | − # the stage isn't changing, this falls through to reorder. Then | |
| 378 | − # set the subboard if it changed; the second write is a no-op | |
| 379 | − # when source/target match. | |
| 380 | − case Kanban.move_card(card, target_stage_id, target_index, actor: actor) do | |
| 381 | − {:ok, moved} -> | |
| 382 | − if moved.subboard_id != target_subboard_id do | |
| 383 | − target_sb = | |
| 384 | − Enum.find(socket.assigns.board.subboards, &(&1.id == target_subboard_id)) | |
| 385 | − | |
| 386 | − Kanban.set_card_subboard(moved, target_sb, actor: actor) | |
| 387 | − end | |
| 388 | − | |
| 389 | − {:noreply, socket} | |
| 371 | + if is_nil(card) do | |
| 372 | + {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 373 | + else | |
| 374 | + # Always run the stage move first (it enforces transitions). If | |
| 375 | + # the stage isn't changing, this falls through to reorder. Then | |
| 376 | + # set the subboard if it changed; the second write is a no-op | |
| 377 | + # when source/target match. | |
| 378 | + case Kanban.move_card(card, target_stage_id, target_index, actor: actor) do | |
| 379 | + {:ok, moved} -> | |
| 380 | + maybe_move_to_subboard(socket, moved, target_subboard_id, actor) | |
| 381 | + {:noreply, socket} | |
| 390 | 382 | |
| 391 | − {:error, :invalid_transition} -> | |
| 392 | − {:noreply, put_flash(socket, :error, "That move isn't allowed by the workflow.")} | |
| 383 | + {:error, :invalid_transition} -> | |
| 384 | + {:noreply, put_flash(socket, :error, "That move isn't allowed by the workflow.")} | |
| 393 | 385 | |
| 394 | − {:error, _} -> | |
| 395 | − {:noreply, put_flash(socket, :error, "Couldn't move the card.")} | |
| 396 | − end | |
| 386 | + {:error, _} -> | |
| 387 | + {:noreply, put_flash(socket, :error, "Couldn't move the card.")} | |
| 388 | + end | |
| 397 | 389 | end |
| 398 | 390 | end) |
| 399 | 391 | end |
| 400 | 392 | |
| 401 | 393 | def handle_event("move_card", %{"id" => id, "stage-id" => stage_id} = params, socket) do |
| 402 | 394 | guard_edit(socket, fn -> |
| 403 | − card = Kanban.get_card(id) | |
| 395 | + card = get_board_card(socket, id) | |
| 404 | 396 | target_index = parse_index(params["index"]) |
| 405 | 397 | |
| 406 | − cond do | |
| 407 | − is_nil(card) or card.board_id != socket.assigns.board.id -> | |
| 408 | − {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 409 | − | |
| 410 | − true -> | |
| 411 | − case Kanban.move_card(card, stage_id, target_index, | |
| 412 | − actor: socket.assigns.current_scope.user | |
| 413 | − ) do | |
| 414 | − {:ok, _} -> | |
| 415 | − # Broadcast refreshes both :cards and :expanded_card. | |
| 416 | − {:noreply, socket} | |
| 398 | + if is_nil(card) do | |
| 399 | + {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 400 | + else | |
| 401 | + case Kanban.move_card(card, stage_id, target_index, | |
| 402 | + actor: socket.assigns.current_scope.user | |
| 403 | + ) do | |
| 404 | + {:ok, _} -> | |
| 405 | + # Broadcast refreshes both :cards and :expanded_card. | |
| 406 | + {:noreply, socket} | |
| 417 | 407 | |
| 418 | − {:error, :invalid_transition} -> | |
| 419 | − {:noreply, put_flash(socket, :error, "That move isn't allowed by the workflow.")} | |
| 408 | + {:error, :invalid_transition} -> | |
| 409 | + {:noreply, put_flash(socket, :error, "That move isn't allowed by the workflow.")} | |
| 420 | 410 | |
| 421 | − {:error, :invalid_stage} -> | |
| 422 | − {:noreply, put_flash(socket, :error, "Unknown stage.")} | |
| 423 | − end | |
| 411 | + {:error, :invalid_stage} -> | |
| 412 | + {:noreply, put_flash(socket, :error, "Unknown stage.")} | |
| 413 | + end | |
| 424 | 414 | end |
| 425 | 415 | end) |
| 426 | 416 | end |
| 427 | 417 | |
| 428 | 418 | def handle_event("reorder_card", %{"id" => id, "index" => idx}, socket) do |
| 429 | 419 | guard_edit(socket, fn -> |
| 430 | − card = Kanban.get_card(id) | |
| 420 | + card = get_board_card(socket, id) | |
| 431 | 421 | |
| 432 | − cond do | |
| 433 | − is_nil(card) or card.board_id != socket.assigns.board.id -> | |
| 434 | − {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 435 | − | |
| 436 | − true -> | |
| 437 | − case Kanban.reorder_card(card, parse_index(idx) || 0, | |
| 438 | − actor: socket.assigns.current_scope.user | |
| 439 | − ) do | |
| 440 | − {:ok, _} -> {:noreply, socket} | |
| 441 | − _ -> {:noreply, put_flash(socket, :error, "Couldn't reorder card.")} | |
| 442 | − end | |
| 422 | + if is_nil(card) do | |
| 423 | + {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 424 | + else | |
| 425 | + case Kanban.reorder_card(card, parse_index(idx) || 0, | |
| 426 | + actor: socket.assigns.current_scope.user | |
| 427 | + ) do | |
| 428 | + {:ok, _} -> {:noreply, socket} | |
| 429 | + _ -> {:noreply, put_flash(socket, :error, "Couldn't reorder card.")} | |
| 430 | + end | |
| 443 | 431 | end |
| 444 | 432 | end) |
| 445 | 433 | end |
@@ -463,71 +451,38 @@ defmodule WaxxWeb.BoardLive.Show do
| 463 | 451 | # `name="value"`, so the new value always arrives at params["value"]. |
| 464 | 452 | def handle_event("save_field", %{"card-id" => card_id, "field-id" => field_id} = params, socket) do |
| 465 | 453 | guard_edit(socket, fn -> |
| 466 | − card = Kanban.get_card(card_id) | |
| 454 | + card = get_board_card(socket, card_id) | |
| 467 | 455 | field_id_int = field_id |
| 468 | 456 | field = Enum.find(socket.assigns.board.fields, &(&1.id == field_id_int)) |
| 469 | 457 | value = params["value"] || "" |
| 470 | 458 | |
| 471 | 459 | cond do |
| 472 | − is_nil(card) or card.board_id != socket.assigns.board.id -> | |
| 460 | + is_nil(card) -> | |
| 473 | 461 | {:noreply, put_flash(socket, :error, "Card not found.")} |
| 474 | 462 | |
| 475 | 463 | is_nil(field) -> |
| 476 | 464 | {:noreply, put_flash(socket, :error, "Unknown field.")} |
| 477 | 465 | |
| 478 | 466 | true -> |
| 479 | − case Kanban.set_card_field_value(card, field, value, | |
| 480 | − actor: socket.assigns.current_scope.user | |
| 481 | − ) do | |
| 482 | − {:ok, _} -> | |
| 483 | − {:noreply, socket} | |
| 484 | − | |
| 485 | − {:error, :invalid_option} -> | |
| 486 | − {:noreply, put_flash(socket, :error, "Pick one of the listed options.")} | |
| 487 | − | |
| 488 | − {:error, :invalid_value} -> | |
| 489 | − {:noreply, put_flash(socket, :error, "That value isn't valid for this field.")} | |
| 490 | − | |
| 491 | − {:error, _} -> | |
| 492 | − {:noreply, put_flash(socket, :error, "Couldn't save field.")} | |
| 493 | − end | |
| 467 | + save_field_value(socket, card, field, value) | |
| 494 | 468 | end |
| 495 | 469 | end) |
| 496 | 470 | end |
| 497 | 471 | |
| 498 | 472 | def handle_event("add_note", %{"id" => card_id} = params, socket) do |
| 499 | 473 | guard_edit(socket, fn -> |
| 500 | − card = Kanban.get_card(card_id) | |
| 474 | + card = get_board_card(socket, card_id) | |
| 501 | 475 | user = socket.assigns.current_scope.user |
| 502 | 476 | |
| 503 | − cond do | |
| 504 | − is_nil(card) or card.board_id != socket.assigns.board.id -> | |
| 505 | − {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 506 | − | |
| 507 | − true -> | |
| 508 | − attrs = %{ | |
| 509 | − "body" => params["body"], | |
| 510 | − "kind" => params["kind"] | |
| 511 | − } | |
| 512 | − | |
| 513 | − # Optional stage override. Must belong to this card's board — | |
| 514 | − # the LiveView form only ever offers stages from `@board.stages`, | |
| 515 | − # but defence-in-depth. | |
| 516 | − attrs = | |
| 517 | − case params["board_stage_id"] do | |
| 518 | − s when is_binary(s) and s != "" -> | |
| 519 | − if Enum.any?(socket.assigns.board.stages, &(&1.id == s)), | |
| 520 | − do: Map.put(attrs, "board_stage_id", s), | |
| 521 | − else: attrs | |
| 522 | − | |
| 523 | − _ -> | |
| 524 | − attrs | |
| 525 | − end | |
| 477 | + if is_nil(card) do | |
| 478 | + {:noreply, put_flash(socket, :error, "Card not found.")} | |
| 479 | + else | |
| 480 | + attrs = note_attrs(socket, params) | |
| 526 | 481 | |
| 527 | − case Kanban.add_card_note(card, user, attrs) do | |
| 528 | − {:ok, _} -> {:noreply, socket} | |
| 529 | − {:error, _} -> {:noreply, put_flash(socket, :error, "Couldn't add the note.")} | |
| 530 | − end | |
| 482 | + case Kanban.add_card_note(card, user, attrs) do | |
| 483 | + {:ok, _} -> {:noreply, socket} | |
| 484 | + {:error, _} -> {:noreply, put_flash(socket, :error, "Couldn't add the note.")} | |
| 485 | + end | |
| 531 | 486 | end |
| 532 | 487 | end) |
| 533 | 488 | end |
@@ -612,20 +567,13 @@ defmodule WaxxWeb.BoardLive.Show do
| 612 | 567 | |
| 613 | 568 | def handle_event("toggle_label", %{"card-id" => card_id, "label-id" => label_id}, socket) do |
| 614 | 569 | guard_edit(socket, fn -> |
| 615 | − card = Kanban.get_card(card_id) | |
| 570 | + card = get_board_card(socket, card_id) | |
| 616 | 571 | label_id_int = label_id |
| 617 | 572 | |
| 618 | 573 | label = |
| 619 | 574 | Enum.find(socket.assigns.board.labels, &(&1.id == label_id_int)) |
| 620 | 575 | |
| 621 | − # Only allow applying a label that's in scope for the card's subboard; | |
| 622 | − # a label already on the card stays toggleable so it can be removed. | |
| 623 | − in_scope? = | |
| 624 | − card && label && | |
| 625 | − (Kanban.BoardLabel.applies_to_subboard?(label, card.subboard_id) or | |
| 626 | − Enum.any?(card.labels, &(&1.id == label.id))) | |
| 627 | − | |
| 628 | − if card && card.board_id == socket.assigns.board.id && in_scope? do | |
| 576 | + if card && label && label_in_scope?(card, label) do | |
| 629 | 577 | case Kanban.toggle_card_label(card, label, actor: socket.assigns.current_scope.user) do |
| 630 | 578 | {:ok, _} -> |
| 631 | 579 | {:noreply, socket} |
@@ -644,35 +592,87 @@ defmodule WaxxWeb.BoardLive.Show do
| 644 | 592 | |
| 645 | 593 | def handle_event("toggle_assignee", %{"card-id" => card_id, "user-id" => user_id}, socket) do |
| 646 | 594 | guard_edit(socket, fn -> |
| 647 | − card = Kanban.get_card(card_id) | |
| 648 | 595 | user_id_int = user_id |
| 649 | 596 | |
| 650 | − if card && card.board_id == socket.assigns.board.id do | |
| 651 | − already? = Enum.any?(card.assignees, &(&1.id == user_id_int)) | |
| 652 | − | |
| 653 | − target_user = | |
| 654 | − Enum.find(assignable_users(socket.assigns.board), &(&1.id == user_id_int)) | |
| 655 | − | |
| 656 | − if target_user do | |
| 657 | − if already? do | |
| 658 | − {_, _} = | |
| 659 | − Kanban.unassign_user(card, target_user, actor: socket.assigns.current_scope.user) | |
| 660 | − else | |
| 661 | − {:ok, _} = | |
| 662 | − Kanban.assign_user(card, target_user, actor: socket.assigns.current_scope.user) | |
| 663 | − end | |
| 664 | − | |
| 665 | − # Broadcast refreshes :cards and :expanded_card. | |
| 666 | − {:noreply, socket} | |
| 667 | − else | |
| 668 | − {:noreply, socket} | |
| 669 | − end | |
| 670 | − else | |
| 671 | − {:noreply, socket} | |
| 597 | + with %{} = card <- get_board_card(socket, card_id), | |
| 598 | + %{} = target_user <- | |
| 599 | + Enum.find(assignable_users(socket.assigns.board), &(&1.id == user_id_int)) do | |
| 600 | + toggle_assignee(card, target_user, socket.assigns.current_scope.user) | |
| 672 | 601 | end |
| 602 | + | |
| 603 | + # Broadcast refreshes :cards and :expanded_card. | |
| 604 | + {:noreply, socket} | |
| 673 | 605 | end) |
| 674 | 606 | end |
| 675 | 607 | |
| 608 | + # The card with this id, but only if it's on this board. | |
| 609 | + defp get_board_card(socket, id) do | |
| 610 | + board_id = socket.assigns.board.id | |
| 611 | + | |
| 612 | + case Kanban.get_card(id) do | |
| 613 | + %{board_id: ^board_id} = card -> card | |
| 614 | + _ -> nil | |
| 615 | + end | |
| 616 | + end | |
| 617 | + | |
| 618 | + defp maybe_move_to_subboard(_socket, %{subboard_id: subboard_id}, subboard_id, _actor), do: :ok | |
| 619 | + | |
| 620 | + defp maybe_move_to_subboard(socket, moved, target_subboard_id, actor) do | |
| 621 | + target_sb = Enum.find(socket.assigns.board.subboards, &(&1.id == target_subboard_id)) | |
| 622 | + Kanban.set_card_subboard(moved, target_sb, actor: actor) | |
| 623 | + end | |
| 624 | + | |
| 625 | + defp save_field_value(socket, card, field, value) do | |
| 626 | + case Kanban.set_card_field_value(card, field, value, actor: socket.assigns.current_scope.user) do | |
| 627 | + {:ok, _} -> | |
| 628 | + {:noreply, socket} | |
| 629 | + | |
| 630 | + {:error, :invalid_option} -> | |
| 631 | + {:noreply, put_flash(socket, :error, "Pick one of the listed options.")} | |
| 632 | + | |
| 633 | + {:error, :invalid_value} -> | |
| 634 | + {:noreply, put_flash(socket, :error, "That value isn't valid for this field.")} | |
| 635 | + | |
| 636 | + {:error, _} -> | |
| 637 | + {:noreply, put_flash(socket, :error, "Couldn't save field.")} | |
| 638 | + end | |
| 639 | + end | |
| 640 | + | |
| 641 | + # Optional stage override. Must belong to this card's board — the | |
| 642 | + # LiveView form only ever offers stages from `@board.stages`, but | |
| 643 | + # defence-in-depth. | |
| 644 | + defp note_attrs(socket, params) do | |
| 645 | + attrs = %{ | |
| 646 | + "body" => params["body"], | |
| 647 | + "kind" => params["kind"] | |
| 648 | + } | |
| 649 | + | |
| 650 | + case params["board_stage_id"] do | |
| 651 | + s when is_binary(s) and s != "" -> | |
| 652 | + if Enum.any?(socket.assigns.board.stages, &(&1.id == s)), | |
| 653 | + do: Map.put(attrs, "board_stage_id", s), | |
| 654 | + else: attrs | |
| 655 | + | |
| 656 | + _ -> | |
| 657 | + attrs | |
| 658 | + end | |
| 659 | + end | |
| 660 | + | |
| 661 | + # Only allow applying a label that's in scope for the card's subboard; | |
| 662 | + # a label already on the card stays toggleable so it can be removed. | |
| 663 | + defp label_in_scope?(card, label) do | |
| 664 | + Kanban.BoardLabel.applies_to_subboard?(label, card.subboard_id) or | |
| 665 | + Enum.any?(card.labels, &(&1.id == label.id)) | |
| 666 | + end | |
| 667 | + | |
| 668 | + defp toggle_assignee(card, target_user, actor) do | |
| 669 | + if Enum.any?(card.assignees, &(&1.id == target_user.id)) do | |
| 670 | + {_, _} = Kanban.unassign_user(card, target_user, actor: actor) | |
| 671 | + else | |
| 672 | + {:ok, _} = Kanban.assign_user(card, target_user, actor: actor) | |
| 673 | + end | |
| 674 | + end | |
| 675 | + | |
| 676 | 676 | defp guard_edit(socket, fun) do |
| 677 | 677 | if Kanban.can_edit?(socket.assigns.role) do |
| 678 | 678 | fun.() |
Parents: b7432b9