From 1a4541a65c6ac76d5f89892afd65c0d444295d67 Mon Sep 17 00:00:00 2001 From: Carl-Fredrik Arvidson Date: Sun, 21 Dec 2025 13:52:23 +0100 Subject: [PATCH 1/5] Setup Docker development environment with Solid Queue - Updated .dockerignore and Dockerfile.dev for better containerization - Added docker-compose.yml for local development - Configured Solid Queue in development environment - Added dedicated databases for queue and cache in MySQL config - Included necessary migration directories for multi-database setup --- .dockerignore | 60 +++++++------------------ Dockerfile.dev | 1 - config/database.mysql.yml | 8 ++++ config/environments/development.rb | 4 ++ db/cable_migrate/.keep | 0 db/cache_migrate/.keep | 0 db/queue_migrate/.keep | 0 docker-compose.yml | 70 ++++++++++++++++++++++++++++++ 8 files changed, 97 insertions(+), 46 deletions(-) create mode 100644 db/cable_migrate/.keep create mode 100644 db/cache_migrate/.keep create mode 100644 db/queue_migrate/.keep create mode 100644 docker-compose.yml diff --git a/.dockerignore b/.dockerignore index df5bbdacc5..e004b7d6ed 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,45 +1,15 @@ -# See https://docs.docker.com/engine/reference/builder/#dockerignore-file for more about ignoring files. - -# Ignore git directory. -/.git/ - -# Ignore bundler config. -/.bundle - -# Ignore documentation -/docs/ -/README.md -/CLAUDE.md -/AGENTS.md -/STYLE.md -/CONTRIBUTING.md - -# Ignore all environment files (except templates). -/.env* -!/.env*.erb - -# Ignore all default key files. -/config/master.key -/config/credentials/*.key - -# Ignore all logfiles and tempfiles. -/log/* -/tmp/* -!/log/.keep -!/tmp/.keep - -# Ignore pidfiles, but keep the directory. -/tmp/pids/* -!/tmp/pids/.keep - -# Ignore storage (uploaded files in development and any SQLite databases). -/storage/* -!/storage/.keep -/tmp/storage/* -!/tmp/storage/.keep - -# Ignore assets. -/node_modules/ -/app/assets/builds/* -!/app/assets/builds/.keep -/public/assets +.git +.bundle +log/* +tmp/* +!tmp/keep +public/assets +storage/* +!storage/keep +node_modules +.DS_Store +Dockerfile +Dockerfile.dev +docker-compose.yml +.dockerignore +vendor/bundle diff --git a/Dockerfile.dev b/Dockerfile.dev index 9c20fcda5d..268d834a33 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -19,7 +19,6 @@ RUN apt-get update -qq && \ # Install application gems COPY Gemfile Gemfile.lock .ruby-version ./ COPY lib/fizzy.rb ./lib/fizzy.rb -COPY gems ./gems/ RUN --mount=type=secret,id=GITHUB_TOKEN --mount=type=cache,id=fizzy-devbundle-${RUBY_VERSION},sharing=locked,target=/devbundle \ gem install bundler foreman && \ BUNDLE_PATH=/devbundle BUNDLE_GITHUB__COM="$(cat /run/secrets/GITHUB_TOKEN):x-oauth-basic" bundle install && \ diff --git a/config/database.mysql.yml b/config/database.mysql.yml index 2f72f6e989..62dceee526 100644 --- a/config/database.mysql.yml +++ b/config/database.mysql.yml @@ -16,6 +16,14 @@ development: <<: *default database: fizzy_development_cable migrations_paths: db/cable_migrate + queue: + <<: *default + database: fizzy_development_queue + migrations_paths: db/queue_migrate + cache: + <<: *default + database: fizzy_development_cache + migrations_paths: db/cache_migrate test: primary: diff --git a/config/environments/development.rb b/config/environments/development.rb index 6088b670d3..492f9b175a 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -95,4 +95,8 @@ # Canonical host for mailer URLs (emails always link here, not personal Tailscale URLs) config.action_mailer.default_url_options = { host: "#{config.hosts.first}:3006" } + + # Use Solid Queue for background jobs + config.active_job.queue_adapter = :solid_queue + config.solid_queue.connects_to = { database: { writing: :queue, reading: :queue } } end diff --git a/db/cable_migrate/.keep b/db/cable_migrate/.keep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/db/cache_migrate/.keep b/db/cache_migrate/.keep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/db/queue_migrate/.keep b/db/queue_migrate/.keep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000000..61fd286219 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,70 @@ +services: + db: + image: mysql:8.4 + container_name: fizzy-mysql + restart: always + environment: + MYSQL_ALLOW_EMPTY_PASSWORD: "yes" + ports: + - "3306:3306" + volumes: + - mysql_data:/var/lib/mysql + healthcheck: + test: ["CMD", "mysqladmin", "ping", "-h", "localhost"] + interval: 10s + timeout: 5s + retries: 5 + + app: + build: + context: . + dockerfile: Dockerfile.dev + secrets: + - GITHUB_TOKEN + container_name: fizzy-app + depends_on: + db: + condition: service_healthy + environment: + DATABASE_ADAPTER: mysql + MYSQL_HOST: db + MYSQL_ALLOW_EMPTY_PASSWORD: "yes" + RAILS_ENV: development + PORT: 3006 + ports: + - "3006:3006" + volumes: + - .:/rails + - bundle_cache:/usr/local/bundle + stdin_open: true + tty: true + command: + ["./bin/thrust", "./bin/rails", "server", "-p", "3006", "-b", "0.0.0.0"] + + worker: + build: + context: . + dockerfile: Dockerfile.dev + secrets: + - GITHUB_TOKEN + container_name: fizzy-worker + depends_on: + db: + condition: service_healthy + environment: + DATABASE_ADAPTER: mysql + MYSQL_HOST: db + MYSQL_ALLOW_EMPTY_PASSWORD: "yes" + RAILS_ENV: development + volumes: + - .:/rails + - bundle_cache:/usr/local/bundle + command: ["./bin/jobs"] + +volumes: + mysql_data: + bundle_cache: + +secrets: + GITHUB_TOKEN: + environment: GITHUB_TOKEN From bc2812742f71775354bfffc75256eef63715ec16 Mon Sep 17 00:00:00 2001 From: Carl-Fredrik Arvidson Date: Sun, 21 Dec 2025 14:33:11 +0100 Subject: [PATCH 2/5] Add manual card sorting feature - Add position column to cards table with migration and backfill - Implement Card::Positioner for calculating new positions and renumbering - Update drag_and_drop_controller.js to support intra-list reordering and send before/after IDs - Wire up drop controllers to handle reordering without side effects - Update board and column controllers to render cards in position order --- .../boards/columns/closeds_controller.rb | 3 +- .../boards/columns/not_nows_controller.rb | 3 +- .../boards/columns/streams_controller.rb | 3 +- app/controllers/boards/columns_controller.rb | 3 +- app/controllers/boards_controller.rb | 2 +- app/controllers/cards/closures_controller.rb | 32 ++- app/controllers/cards/not_nows_controller.rb | 8 +- app/controllers/cards/publishes_controller.rb | 20 +- app/controllers/cards/triages_controller.rb | 30 ++- .../cards/drops/closures_controller.rb | 8 +- .../columns/cards/drops/columns_controller.rb | 16 +- .../cards/drops/not_nows_controller.rb | 8 +- .../columns/cards/drops/streams_controller.rb | 18 +- .../boards/columns/closeds_controller.rb | 3 +- .../boards/columns/not_nows_controller.rb | 3 +- .../boards/columns/streams_controller.rb | 3 +- .../public/boards/columns_controller.rb | 3 +- app/controllers/public/boards_controller.rb | 3 +- .../controllers/drag_and_drop_controller.js | 225 ++++++++++++------ app/models/card.rb | 2 +- app/models/card/positioned.rb | 11 + app/models/card/positioner.rb | 107 +++++++++ .../20251221090000_add_position_to_cards.rb | 132 ++++++++++ db/schema.rb | 5 +- .../cards/drops/closures_controller_test.rb | 24 ++ .../cards/drops/columns_controller_test.rb | 30 +++ .../cards/drops/not_nows_controller_test.rb | 24 ++ .../cards/drops/streams_controller_test.rb | 27 +++ test/models/card/positioner_test.rb | 61 +++++ 29 files changed, 722 insertions(+), 95 deletions(-) create mode 100644 app/models/card/positioned.rb create mode 100644 app/models/card/positioner.rb create mode 100644 db/migrate/20251221090000_add_position_to_cards.rb create mode 100644 test/models/card/positioner_test.rb diff --git a/app/controllers/boards/columns/closeds_controller.rb b/app/controllers/boards/columns/closeds_controller.rb index 95e0c7126f..c7f8fa260b 100644 --- a/app/controllers/boards/columns/closeds_controller.rb +++ b/app/controllers/boards/columns/closeds_controller.rb @@ -2,7 +2,8 @@ class Boards::Columns::ClosedsController < ApplicationController include BoardScoped def show - set_page_and_extract_portion_from @board.cards.closed.recently_closed_first.preloaded + set_page_and_extract_portion_from \ + @board.cards.closed.ordered_by_position(Arel.sql("closures.created_at DESC, cards.id DESC")).preloaded fresh_when etag: @page.records end end diff --git a/app/controllers/boards/columns/not_nows_controller.rb b/app/controllers/boards/columns/not_nows_controller.rb index 0ff41e9ae3..79fe7ab35b 100644 --- a/app/controllers/boards/columns/not_nows_controller.rb +++ b/app/controllers/boards/columns/not_nows_controller.rb @@ -2,7 +2,8 @@ class Boards::Columns::NotNowsController < ApplicationController include BoardScoped def show - set_page_and_extract_portion_from @board.cards.postponed.latest.preloaded + set_page_and_extract_portion_from \ + @board.cards.postponed.ordered_by_position(last_active_at: :desc, id: :desc).preloaded fresh_when etag: @page.records end end diff --git a/app/controllers/boards/columns/streams_controller.rb b/app/controllers/boards/columns/streams_controller.rb index d2c7d0795e..f999a56c7a 100644 --- a/app/controllers/boards/columns/streams_controller.rb +++ b/app/controllers/boards/columns/streams_controller.rb @@ -2,7 +2,8 @@ class Boards::Columns::StreamsController < ApplicationController include BoardScoped def show - set_page_and_extract_portion_from @board.cards.awaiting_triage.latest.with_golden_first.preloaded + set_page_and_extract_portion_from \ + @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc).preloaded fresh_when etag: @page.records end end diff --git a/app/controllers/boards/columns_controller.rb b/app/controllers/boards/columns_controller.rb index 4b71e7f3e0..9ca922e5cd 100644 --- a/app/controllers/boards/columns_controller.rb +++ b/app/controllers/boards/columns_controller.rb @@ -9,7 +9,8 @@ def index end def show - set_page_and_extract_portion_from @column.cards.active.latest.with_golden_first.preloaded + set_page_and_extract_portion_from \ + @column.cards.active.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc).preloaded fresh_when etag: @page.records end diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb index 721e69b0d5..909b57f125 100644 --- a/app/controllers/boards_controller.rb +++ b/app/controllers/boards_controller.rb @@ -81,7 +81,7 @@ def show_filtered_cards end def show_columns - cards = @board.cards.awaiting_triage.latest.with_golden_first.preloaded + cards = @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc).preloaded set_page_and_extract_portion_from cards fresh_when etag: [ @board, @page.records, @user_filtering, Current.account ] end diff --git a/app/controllers/cards/closures_controller.rb b/app/controllers/cards/closures_controller.rb index c4aac26d19..37732bfdee 100644 --- a/app/controllers/cards/closures_controller.rb +++ b/app/controllers/cards/closures_controller.rb @@ -2,7 +2,13 @@ class Cards::ClosuresController < ApplicationController include CardScoped def create - @card.close + ActiveRecord::Base.transaction do + @card.close + + Card::Positioner + .new(relation: @board.cards.closed, fallback_order: Arel.sql("closures.created_at DESC, cards.id DESC")) + .reposition!(card: @card, before_number: nil, after_number: nil) + end respond_to do |format| format.turbo_stream { render_card_replacement } @@ -11,7 +17,29 @@ def create end def destroy - @card.reopen + ActiveRecord::Base.transaction do + @card.reopen + + relation = if @card.postponed? + @board.cards.postponed + elsif @card.triaged? + @card.column.cards.active + else + @board.cards.awaiting_triage + end + + if @card.active? + relation = if @card.golden? + relation.joins(:goldness) + else + relation.where.missing(:goldness) + end + end + + Card::Positioner + .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: nil, after_number: nil) + end respond_to do |format| format.turbo_stream { render_card_replacement } diff --git a/app/controllers/cards/not_nows_controller.rb b/app/controllers/cards/not_nows_controller.rb index 8eeb0e663b..9faa4c3497 100644 --- a/app/controllers/cards/not_nows_controller.rb +++ b/app/controllers/cards/not_nows_controller.rb @@ -2,7 +2,13 @@ class Cards::NotNowsController < ApplicationController include CardScoped def create - @card.postpone + ActiveRecord::Base.transaction do + @card.postpone + + Card::Positioner + .new(relation: @board.cards.postponed, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: nil, after_number: nil) + end respond_to do |format| format.turbo_stream { render_card_replacement } diff --git a/app/controllers/cards/publishes_controller.rb b/app/controllers/cards/publishes_controller.rb index 641f728cdd..06dd79a01e 100644 --- a/app/controllers/cards/publishes_controller.rb +++ b/app/controllers/cards/publishes_controller.rb @@ -2,7 +2,25 @@ class Cards::PublishesController < ApplicationController include CardScoped def create - @card.publish + ActiveRecord::Base.transaction do + @card.publish + + relation = if @card.triaged? + @card.column.cards.active + else + @board.cards.awaiting_triage + end + + relation = if @card.golden? + relation.joins(:goldness) + else + relation.where.missing(:goldness) + end + + Card::Positioner + .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: nil, after_number: nil) + end if add_another_param? redirect_to @board.cards.create!, notice: "Card added" diff --git a/app/controllers/cards/triages_controller.rb b/app/controllers/cards/triages_controller.rb index d8fc548f15..0b68e17699 100644 --- a/app/controllers/cards/triages_controller.rb +++ b/app/controllers/cards/triages_controller.rb @@ -3,7 +3,20 @@ class Cards::TriagesController < ApplicationController def create column = @card.board.columns.find(params[:column_id]) - @card.triage_into(column) + ActiveRecord::Base.transaction do + @card.triage_into(column) + + relation = column.cards.active + relation = if @card.golden? + relation.joins(:goldness) + else + relation.where.missing(:goldness) + end + + Card::Positioner + .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: nil, after_number: nil) + end respond_to do |format| format.html { redirect_to @card } @@ -12,7 +25,20 @@ def create end def destroy - @card.send_back_to_triage + ActiveRecord::Base.transaction do + @card.send_back_to_triage + + relation = @board.cards.awaiting_triage + relation = if @card.golden? + relation.joins(:goldness) + else + relation.where.missing(:goldness) + end + + Card::Positioner + .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: nil, after_number: nil) + end respond_to do |format| format.html { redirect_to @card } diff --git a/app/controllers/columns/cards/drops/closures_controller.rb b/app/controllers/columns/cards/drops/closures_controller.rb index a58e86775c..fa7211382d 100644 --- a/app/controllers/columns/cards/drops/closures_controller.rb +++ b/app/controllers/columns/cards/drops/closures_controller.rb @@ -2,6 +2,12 @@ class Columns::Cards::Drops::ClosuresController < ApplicationController include CardScoped def create - @card.close + ActiveRecord::Base.transaction do + @card.close + + Card::Positioner + .new(relation: @board.cards.closed, fallback_order: Arel.sql("closures.created_at DESC, cards.id DESC")) + .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + end end end diff --git a/app/controllers/columns/cards/drops/columns_controller.rb b/app/controllers/columns/cards/drops/columns_controller.rb index 1d2bd8ce54..d9508432cd 100644 --- a/app/controllers/columns/cards/drops/columns_controller.rb +++ b/app/controllers/columns/cards/drops/columns_controller.rb @@ -3,6 +3,20 @@ class Columns::Cards::Drops::ColumnsController < ApplicationController def create @column = @card.board.columns.find(params[:column_id]) - @card.triage_into(@column) + + ActiveRecord::Base.transaction do + @card.triage_into(@column) unless @card.active? && @card.column == @column + + relation = @column.cards.active + relation = if @card.golden? + relation.joins(:goldness) + else + relation.where.missing(:goldness) + end + + Card::Positioner + .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + end end end diff --git a/app/controllers/columns/cards/drops/not_nows_controller.rb b/app/controllers/columns/cards/drops/not_nows_controller.rb index c1b6a5fac7..11d4c16bf3 100644 --- a/app/controllers/columns/cards/drops/not_nows_controller.rb +++ b/app/controllers/columns/cards/drops/not_nows_controller.rb @@ -2,6 +2,12 @@ class Columns::Cards::Drops::NotNowsController < ApplicationController include CardScoped def create - @card.postpone + ActiveRecord::Base.transaction do + @card.postpone unless @card.postponed? + + Card::Positioner + .new(relation: @board.cards.postponed, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + end end end diff --git a/app/controllers/columns/cards/drops/streams_controller.rb b/app/controllers/columns/cards/drops/streams_controller.rb index fefdbb23f8..4f2f80932f 100644 --- a/app/controllers/columns/cards/drops/streams_controller.rb +++ b/app/controllers/columns/cards/drops/streams_controller.rb @@ -2,7 +2,21 @@ class Columns::Cards::Drops::StreamsController < ApplicationController include CardScoped def create - @card.send_back_to_triage - set_page_and_extract_portion_from @board.cards.awaiting_triage.latest.with_golden_first + ActiveRecord::Base.transaction do + @card.send_back_to_triage unless @card.awaiting_triage? + + relation = @board.cards.awaiting_triage + relation = if @card.golden? + relation.joins(:goldness) + else + relation.where.missing(:goldness) + end + + Card::Positioner + .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + end + + set_page_and_extract_portion_from @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) end end diff --git a/app/controllers/public/boards/columns/closeds_controller.rb b/app/controllers/public/boards/columns/closeds_controller.rb index 63ca9de7b2..20a082ff88 100644 --- a/app/controllers/public/boards/columns/closeds_controller.rb +++ b/app/controllers/public/boards/columns/closeds_controller.rb @@ -1,5 +1,6 @@ class Public::Boards::Columns::ClosedsController < Public::BaseController def show - set_page_and_extract_portion_from @board.cards.closed.recently_closed_first + set_page_and_extract_portion_from \ + @board.cards.closed.ordered_by_position(Arel.sql("closures.created_at DESC, cards.id DESC")) end end diff --git a/app/controllers/public/boards/columns/not_nows_controller.rb b/app/controllers/public/boards/columns/not_nows_controller.rb index 5a54df1716..fabaa2f4f7 100644 --- a/app/controllers/public/boards/columns/not_nows_controller.rb +++ b/app/controllers/public/boards/columns/not_nows_controller.rb @@ -1,5 +1,6 @@ class Public::Boards::Columns::NotNowsController < Public::BaseController def show - set_page_and_extract_portion_from @board.cards.postponed.latest + set_page_and_extract_portion_from \ + @board.cards.postponed.ordered_by_position(last_active_at: :desc, id: :desc) end end diff --git a/app/controllers/public/boards/columns/streams_controller.rb b/app/controllers/public/boards/columns/streams_controller.rb index 778817b3e1..69ba0094b4 100644 --- a/app/controllers/public/boards/columns/streams_controller.rb +++ b/app/controllers/public/boards/columns/streams_controller.rb @@ -1,5 +1,6 @@ class Public::Boards::Columns::StreamsController < Public::BaseController def show - set_page_and_extract_portion_from @board.cards.awaiting_triage.latest.with_golden_first + set_page_and_extract_portion_from \ + @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) end end diff --git a/app/controllers/public/boards/columns_controller.rb b/app/controllers/public/boards/columns_controller.rb index 5a5c23d5eb..916021c682 100644 --- a/app/controllers/public/boards/columns_controller.rb +++ b/app/controllers/public/boards/columns_controller.rb @@ -2,7 +2,8 @@ class Public::Boards::ColumnsController < Public::BaseController before_action :set_column def show - set_page_and_extract_portion_from @column.cards.active.latest.with_golden_first + set_page_and_extract_portion_from \ + @column.cards.active.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) end private diff --git a/app/controllers/public/boards_controller.rb b/app/controllers/public/boards_controller.rb index d56a1f684f..816f72aef0 100644 --- a/app/controllers/public/boards_controller.rb +++ b/app/controllers/public/boards_controller.rb @@ -1,5 +1,6 @@ class Public::BoardsController < Public::BaseController def show - set_page_and_extract_portion_from @board.cards.awaiting_triage.latest.with_golden_first + set_page_and_extract_portion_from \ + @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) end end diff --git a/app/javascript/controllers/drag_and_drop_controller.js b/app/javascript/controllers/drag_and_drop_controller.js index 5fc532ca16..9cbc49efdf 100644 --- a/app/javascript/controllers/drag_and_drop_controller.js +++ b/app/javascript/controllers/drag_and_drop_controller.js @@ -1,150 +1,231 @@ -import { Controller } from "@hotwired/stimulus" -import { post } from "@rails/request.js" -import { nextFrame } from "helpers/timing_helpers" +import { Controller } from "@hotwired/stimulus"; +import { post } from "@rails/request.js"; +import { nextFrame } from "helpers/timing_helpers"; export default class extends Controller { - static targets = [ "item", "container" ] - static classes = [ "draggedItem", "hoverContainer" ] + static targets = ["item", "container"]; + static classes = ["draggedItem", "hoverContainer"]; // Actions async dragStart(event) { - event.dataTransfer.effectAllowed = "move" - event.dataTransfer.dropEffect = "move" - event.dataTransfer.setData("37ui/move", event.target) + event.dataTransfer.effectAllowed = "move"; + event.dataTransfer.dropEffect = "move"; + event.dataTransfer.setData("37ui/move", event.target); - await nextFrame() - this.dragItem = this.#itemContaining(event.target) - this.sourceContainer = this.#containerContaining(this.dragItem) - this.originalDraggedItemCssVariable = this.#containerCssVariableFor(this.sourceContainer) - this.dragItem.classList.add(this.draggedItemClass) + await nextFrame(); + this.dragItem = this.#itemContaining(event.target); + this.sourceContainer = this.#containerContaining(this.dragItem); + this.originalDraggedItemCssVariable = this.#containerCssVariableFor( + this.sourceContainer, + ); + this.dragItem.classList.add(this.draggedItemClass); } dragOver(event) { - event.preventDefault() - if (!this.dragItem) { return } + event.preventDefault(); + if (!this.dragItem) { + return; + } + + const container = this.#containerContaining(event.target); + this.#clearContainerHoverClasses(); - const container = this.#containerContaining(event.target) - this.#clearContainerHoverClasses() + if (!container) { + return; + } - if (!container) { return } + this.#repositionDraggedItem(container, event.clientY); if (container !== this.sourceContainer) { - container.classList.add(this.hoverContainerClass) - this.#applyContainerCssVariableToDraggedItem(container) + container.classList.add(this.hoverContainerClass); + this.#applyContainerCssVariableToDraggedItem(container); } else { - this.#restoreOriginalDraggedItemCssVariable() + this.#restoreOriginalDraggedItemCssVariable(); } } async drop(event) { - const targetContainer = this.#containerContaining(event.target) + const targetContainer = this.#containerContaining(event.target); - if (!targetContainer || targetContainer === this.sourceContainer) { return } + if (!targetContainer) { + return; + } - this.wasDropped = true - this.#increaseCounter(targetContainer) - this.#decreaseCounter(this.sourceContainer) + this.wasDropped = true; + const sourceContainer = this.sourceContainer; + const movedAcrossContainers = targetContainer !== sourceContainer; - const sourceContainer = this.sourceContainer - this.#insertDraggedItem(targetContainer, this.dragItem) - await this.#submitDropRequest(this.dragItem, targetContainer) - this.#reloadSourceFrame(sourceContainer) + if (movedAcrossContainers) { + this.#increaseCounter(targetContainer); + this.#decreaseCounter(sourceContainer); + } + + this.#repositionDraggedItem(targetContainer, event.clientY); + await this.#submitDropRequest(this.dragItem, targetContainer); + + if (movedAcrossContainers) this.#reloadSourceFrame(sourceContainer); } dragEnd() { - this.dragItem.classList.remove(this.draggedItemClass) - this.#clearContainerHoverClasses() + this.dragItem.classList.remove(this.draggedItemClass); + this.#clearContainerHoverClasses(); if (!this.wasDropped) { - this.#restoreOriginalDraggedItemCssVariable() + this.#restoreOriginalDraggedItemCssVariable(); } - this.sourceContainer = null - this.dragItem = null - this.wasDropped = false - this.originalDraggedItemCssVariable = null + this.sourceContainer = null; + this.dragItem = null; + this.wasDropped = false; + this.originalDraggedItemCssVariable = null; } #itemContaining(element) { - return this.itemTargets.find(item => item.contains(element) || item === element) + return this.itemTargets.find( + (item) => item.contains(element) || item === element, + ); } #containerContaining(element) { - return this.containerTargets.find(container => container.contains(element) || container === element) + return this.containerTargets.find( + (container) => container.contains(element) || container === element, + ); } #clearContainerHoverClasses() { - this.containerTargets.forEach(container => container.classList.remove(this.hoverContainerClass)) + this.containerTargets.forEach((container) => + container.classList.remove(this.hoverContainerClass), + ); } #applyContainerCssVariableToDraggedItem(container) { - const cssVariable = this.#containerCssVariableFor(container) + const cssVariable = this.#containerCssVariableFor(container); if (cssVariable) { - this.dragItem.style.setProperty(cssVariable.name, cssVariable.value) + this.dragItem.style.setProperty(cssVariable.name, cssVariable.value); } } #restoreOriginalDraggedItemCssVariable() { if (this.originalDraggedItemCssVariable) { - const { name, value } = this.originalDraggedItemCssVariable - this.dragItem.style.setProperty(name, value) + const { name, value } = this.originalDraggedItemCssVariable; + this.dragItem.style.setProperty(name, value); } } #containerCssVariableFor(container) { - const { dragAndDropCssVariableName, dragAndDropCssVariableValue } = container.dataset + const { dragAndDropCssVariableName, dragAndDropCssVariableValue } = + container.dataset; if (dragAndDropCssVariableName && dragAndDropCssVariableValue) { - return { name: dragAndDropCssVariableName, value: dragAndDropCssVariableValue } + return { + name: dragAndDropCssVariableName, + value: dragAndDropCssVariableValue, + }; } - return null + return null; } #increaseCounter(container) { - this.#modifyCounter(container, count => count + 1) + this.#modifyCounter(container, (count) => count + 1); } #decreaseCounter(container) { - this.#modifyCounter(container, count => Math.max(0, count - 1)) + this.#modifyCounter(container, (count) => Math.max(0, count - 1)); } #modifyCounter(container, fn) { - const counterElement = container.querySelector("[data-drag-and-drop-counter]") + const counterElement = container.querySelector( + "[data-drag-and-drop-counter]", + ); if (counterElement) { - const currentValue = counterElement.textContent.trim() + const currentValue = counterElement.textContent.trim(); - if (!/^\d+$/.test(currentValue)) return + if (!/^\d+$/.test(currentValue)) return; - counterElement.textContent = fn(parseInt(currentValue)) + counterElement.textContent = fn(parseInt(currentValue)); } } - #insertDraggedItem(container, item) { - const itemContainer = container.querySelector("[data-drag-drop-item-container]") - const topItems = itemContainer.querySelectorAll("[data-drag-and-drop-top]") - const firstTopItem = topItems[0] - const lastTopItem = topItems[topItems.length - 1] - - const isTopItem = item.hasAttribute("data-drag-and-drop-top") - const referenceItem = isTopItem ? firstTopItem : lastTopItem - - if (referenceItem) { - referenceItem[isTopItem ? "before" : "after"](item) - } else { - itemContainer.prepend(item) + #repositionDraggedItem(container, clientY) { + const itemContainer = container.querySelector( + "[data-drag-drop-item-container]", + ); + if (!itemContainer) return; + + const item = this.dragItem; + const topItems = itemContainer.querySelectorAll("[data-drag-and-drop-top]"); + const firstTopItem = topItems[0]; + const lastTopItem = topItems[topItems.length - 1]; + + const isTopItem = item.hasAttribute("data-drag-and-drop-top"); + const candidates = Array.from( + itemContainer.querySelectorAll('[data-drag-and-drop-target="item"]'), + ) + .filter((candidate) => candidate !== item) + .filter( + (candidate) => + candidate.hasAttribute("data-drag-and-drop-top") === isTopItem, + ); + + const referenceItem = candidates.find((candidate) => { + const { top, height } = candidate.getBoundingClientRect(); + return clientY < top + height / 2; + }); + + if (referenceItem) return referenceItem.before(item); + + if (candidates.length > 0) + return candidates[candidates.length - 1].after(item); + + if (isTopItem) { + const firstNonTopItem = itemContainer.querySelector( + '[data-drag-and-drop-target="item"]:not([data-drag-and-drop-top])', + ); + return firstNonTopItem + ? firstNonTopItem.before(item) + : itemContainer.prepend(item); } + + return lastTopItem ? lastTopItem.after(item) : itemContainer.prepend(item); } async #submitDropRequest(item, container) { - const body = new FormData() - const id = item.dataset.id - const url = container.dataset.dragAndDropUrl.replaceAll("__id__", id) + const body = new FormData(); + const id = item.dataset.id; + const url = container.dataset.dragAndDropUrl.replaceAll("__id__", id); + + const { beforeId, afterId } = this.#neighborIdsFor(item); + if (beforeId) body.append("before_id", beforeId); + if (afterId) body.append("after_id", afterId); + + return post(url, { + body, + headers: { Accept: "text/vnd.turbo-stream.html" }, + }); + } + + #neighborIdsFor(item) { + const isTopItem = item.hasAttribute("data-drag-and-drop-top"); + + const matchesGroup = (candidate) => { + if (!candidate) return false; + if (candidate.getAttribute("data-drag-and-drop-target") !== "item") + return false; + return candidate.hasAttribute("data-drag-and-drop-top") === isTopItem; + }; + + let previous = item.previousElementSibling; + while (previous && !matchesGroup(previous)) + previous = previous.previousElementSibling; + + let next = item.nextElementSibling; + while (next && !matchesGroup(next)) next = next.nextElementSibling; - return post(url, { body, headers: { Accept: "text/vnd.turbo-stream.html" } }) + return { beforeId: next?.dataset?.id, afterId: previous?.dataset?.id }; } #reloadSourceFrame(sourceContainer) { - const frame = sourceContainer.querySelector("[data-drag-and-drop-refresh]") - if (frame) frame.reload() + const frame = sourceContainer.querySelector("[data-drag-and-drop-refresh]"); + if (frame) frame.reload(); } } diff --git a/app/models/card.rb b/app/models/card.rb index dbb45f184e..6be8eb83c3 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -1,6 +1,6 @@ class Card < ApplicationRecord include Assignable, Attachments, Broadcastable, Closeable, Colored, Entropic, Eventable, - Exportable, Golden, Mentions, Multistep, Pinnable, Postponable, Promptable, + Exportable, Golden, Mentions, Multistep, Pinnable, Positioned, Postponable, Promptable, Readable, Searchable, Stallable, Statuses, Storage::Tracked, Taggable, Triageable, Watchable belongs_to :account, default: -> { board.account } diff --git a/app/models/card/positioned.rb b/app/models/card/positioned.rb new file mode 100644 index 0000000000..de70ff9615 --- /dev/null +++ b/app/models/card/positioned.rb @@ -0,0 +1,11 @@ +module Card::Positioned + extend ActiveSupport::Concern + + included do + scope :ordered_by_position, ->(fallback_order = nil) do + relation = order(Arel.sql("cards.position IS NULL"), position: :asc) + fallback_order.present? ? relation.order(fallback_order) : relation + end + end +end + diff --git a/app/models/card/positioner.rb b/app/models/card/positioner.rb new file mode 100644 index 0000000000..7a3f68926b --- /dev/null +++ b/app/models/card/positioner.rb @@ -0,0 +1,107 @@ +class Card::Positioner + STEP = 1024 + + def initialize(relation:, fallback_order:) + @relation = relation + @fallback_order = fallback_order + end + + def reposition!(card:, before_number:, after_number:) + ensure_positions! + + before_card = resolve_neighbor(before_number) + after_card = resolve_neighbor(after_number) + + new_position = compute_position(before_card:, after_card:) + + # If we can't fit between neighbors, repack and try once more. + if new_position.nil? + renumber_all! + before_card = resolve_neighbor(before_number) + after_card = resolve_neighbor(after_number) + new_position = compute_position(before_card:, after_card:) || top_position + end + + card.update!(position: new_position) + end + + private + attr_reader :relation, :fallback_order + + def ensure_positions! + return unless relation.where(position: nil).exists? + + renumber_all!(use_fallback_order: true) + end + + def resolve_neighbor(number) + return nil unless number.present? + + card_number = Integer(number) + neighbor = relation.find_by(number: card_number) + return nil unless neighbor + + relation.where(id: neighbor.id).exists? ? neighbor : nil + rescue ArgumentError + nil + end + + def compute_position(before_card:, after_card:) + return top_position if before_card.nil? && after_card.nil? + + if before_card && after_card + before_pos = before_card.position + after_pos = after_card.position + return nil if before_pos.nil? || after_pos.nil? + + gap = before_pos - after_pos + return nil if gap <= 1 + + return after_pos + (gap / 2) + end + + return (before_card.position - STEP) if before_card + return (after_card.position + STEP) if after_card + + nil + end + + def top_position + min = relation.where.not(position: nil).minimum(:position) + min.present? ? (min - STEP) : 0 + end + + def renumber_all!(use_fallback_order: false) + order_sql = if use_fallback_order + order_sql_for(fallback_order) + else + "cards.position ASC, cards.id ASC" + end + + ranked = relation + .reorder(nil) + .reselect(Arel.sql("cards.id AS id, (ROW_NUMBER() OVER (ORDER BY #{order_sql})) * #{STEP} AS new_position")) + + sql = <<~SQL.squish + UPDATE cards + JOIN (#{ranked.to_sql}) ranked ON ranked.id = cards.id + SET cards.position = ranked.new_position + SQL + + ActiveRecord::Base.connection.execute(sql) + end + + def order_sql_for(order) + case order + when Hash + order.map do |column, direction| + direction_sql = direction.to_s.upcase + column_sql = column.to_s.include?(".") ? column.to_s : "cards.#{column}" + "#{column_sql} #{direction_sql}" + end.join(", ") + else + order.to_s + end + end +end + diff --git a/db/migrate/20251221090000_add_position_to_cards.rb b/db/migrate/20251221090000_add_position_to_cards.rb new file mode 100644 index 0000000000..7e6c2eb2a2 --- /dev/null +++ b/db/migrate/20251221090000_add_position_to_cards.rb @@ -0,0 +1,132 @@ +class AddPositionToCards < ActiveRecord::Migration[8.2] + STEP = 1024 + + def up + add_column :cards, :position, :bigint + + add_index :cards, [ :board_id, :column_id, :position ], name: "index_cards_on_board_id_and_column_id_and_position" + add_index :cards, [ :board_id, :position ], name: "index_cards_on_board_id_and_position" + + backfill_positions + end + + def down + remove_index :cards, name: "index_cards_on_board_id_and_column_id_and_position" + remove_index :cards, name: "index_cards_on_board_id_and_position" + + remove_column :cards, :position + end + + private + def backfill_positions + board_ids = select_values("SELECT DISTINCT board_id FROM cards") + + board_ids.each do |board_id| + backfill_stream_positions(board_id) + backfill_column_positions(board_id) + backfill_not_now_positions(board_id) + backfill_closed_positions(board_id) + end + end + + def backfill_stream_positions(board_id) + # Matches prior ordering: awaiting_triage.latest.with_golden_first + execute <<~SQL.squish + UPDATE cards c + JOIN ( + SELECT c2.id, + (ROW_NUMBER() OVER ( + ORDER BY (cg.id IS NULL) ASC, c2.last_active_at DESC, c2.id DESC + )) * #{STEP} AS new_position + FROM cards c2 + LEFT JOIN card_goldnesses cg ON cg.card_id = c2.id + LEFT JOIN card_not_nows cnn ON cnn.card_id = c2.id + LEFT JOIN closures cl ON cl.card_id = c2.id + WHERE c2.board_id = #{quote(board_id)} + AND c2.status = 'published' + AND c2.column_id IS NULL + AND cnn.id IS NULL + AND cl.id IS NULL + ) ranked ON ranked.id = c.id + SET c.position = ranked.new_position + WHERE c.board_id = #{quote(board_id)} + AND c.column_id IS NULL + SQL + end + + def backfill_column_positions(board_id) + column_ids = select_values <<~SQL.squish + SELECT DISTINCT column_id + FROM cards + WHERE board_id = #{quote(board_id)} AND column_id IS NOT NULL + SQL + + column_ids.each do |column_id| + # Matches prior ordering: active.latest.with_golden_first (within a column) + execute <<~SQL.squish + UPDATE cards c + JOIN ( + SELECT c2.id, + (ROW_NUMBER() OVER ( + ORDER BY (cg.id IS NULL) ASC, c2.last_active_at DESC, c2.id DESC + )) * #{STEP} AS new_position + FROM cards c2 + LEFT JOIN card_goldnesses cg ON cg.card_id = c2.id + LEFT JOIN card_not_nows cnn ON cnn.card_id = c2.id + LEFT JOIN closures cl ON cl.card_id = c2.id + WHERE c2.board_id = #{quote(board_id)} + AND c2.status = 'published' + AND c2.column_id = #{quote(column_id)} + AND cnn.id IS NULL + AND cl.id IS NULL + ) ranked ON ranked.id = c.id + SET c.position = ranked.new_position + WHERE c.board_id = #{quote(board_id)} + AND c.column_id = #{quote(column_id)} + SQL + end + end + + def backfill_not_now_positions(board_id) + # Matches prior ordering: postponed.latest + execute <<~SQL.squish + UPDATE cards c + JOIN ( + SELECT c2.id, + (ROW_NUMBER() OVER ( + ORDER BY c2.last_active_at DESC, c2.id DESC + )) * #{STEP} AS new_position + FROM cards c2 + JOIN card_not_nows cnn ON cnn.card_id = c2.id + LEFT JOIN closures cl ON cl.card_id = c2.id + WHERE c2.board_id = #{quote(board_id)} + AND c2.status = 'published' + AND cl.id IS NULL + ) ranked ON ranked.id = c.id + SET c.position = ranked.new_position + WHERE c.board_id = #{quote(board_id)} + SQL + end + + def backfill_closed_positions(board_id) + # Matches prior ordering: closed.recently_closed_first + execute <<~SQL.squish + UPDATE cards c + JOIN ( + SELECT c2.id, + (ROW_NUMBER() OVER ( + ORDER BY cl.created_at DESC, c2.id DESC + )) * #{STEP} AS new_position + FROM cards c2 + JOIN closures cl ON cl.card_id = c2.id + WHERE c2.board_id = #{quote(board_id)} + ) ranked ON ranked.id = c.id + SET c.position = ranked.new_position + WHERE c.board_id = #{quote(board_id)} + SQL + end + + def select_values(sql) + connection.select_values(sql) + end +end diff --git a/db/schema.rb b/db/schema.rb index 645cb7a96f..a87bc296ce 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_12_10_054934) do +ActiveRecord::Schema[8.2].define(version: 2025_12_21_090000) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -208,11 +208,14 @@ t.date "due_on" t.datetime "last_active_at", null: false t.bigint "number", null: false + t.bigint "position" t.string "status", default: "drafted", null: false t.string "title" t.datetime "updated_at", null: false t.index ["account_id", "last_active_at", "status"], name: "index_cards_on_account_id_and_last_active_at_and_status" t.index ["account_id", "number"], name: "index_cards_on_account_id_and_number", unique: true + t.index ["board_id", "column_id", "position"], name: "index_cards_on_board_id_and_column_id_and_position" + t.index ["board_id", "position"], name: "index_cards_on_board_id_and_position" t.index ["board_id"], name: "index_cards_on_board_id" t.index ["column_id"], name: "index_cards_on_column_id" end diff --git a/test/controllers/columns/cards/drops/closures_controller_test.rb b/test/controllers/columns/cards/drops/closures_controller_test.rb index 6a9d995e48..383d0c3760 100644 --- a/test/controllers/columns/cards/drops/closures_controller_test.rb +++ b/test/controllers/columns/cards/drops/closures_controller_test.rb @@ -13,4 +13,28 @@ class Columns::Cards::Drops::ClosuresControllerTest < ActionDispatch::Integratio assert_response :success end end + + test "reorders within done without side effects" do + card = cards(:logo) + other = cards(:layout) + + with_current_user(:kevin) do + card.close + other.close + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.closed? } do + assert_changes -> { card.reload.position }, to: 0 do + assert_no_difference -> { card.events.count } do + post columns_card_drops_closure_path(card), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end + end end diff --git a/test/controllers/columns/cards/drops/columns_controller_test.rb b/test/controllers/columns/cards/drops/columns_controller_test.rb index eeb670cbfc..fcf33f92b3 100644 --- a/test/controllers/columns/cards/drops/columns_controller_test.rb +++ b/test/controllers/columns/cards/drops/columns_controller_test.rb @@ -14,4 +14,34 @@ class Columns::Cards::Drops::ColumnsControllerTest < ActionDispatch::Integration assert_response :success end end + + test "reorders within the same column without side effects" do + board = boards(:writebook) + column = columns(:writebook_in_progress) + + card = cards(:text) + other = with_current_user(:kevin) do + board.cards.create!( + title: "Another card", + creator: users(:kevin), + status: "published", + last_active_at: 2.days.ago, + column: column + ) + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.column_id } do + assert_changes -> { card.reload.position }, to: 0 do + assert_no_difference -> { card.events.count } do + post columns_card_drops_column_path(card, column_id: column.id), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end + end end diff --git a/test/controllers/columns/cards/drops/not_nows_controller_test.rb b/test/controllers/columns/cards/drops/not_nows_controller_test.rb index 5cd19176a0..a5cb78ffc2 100644 --- a/test/controllers/columns/cards/drops/not_nows_controller_test.rb +++ b/test/controllers/columns/cards/drops/not_nows_controller_test.rb @@ -13,4 +13,28 @@ class Columns::Cards::Drops::NotNowsControllerTest < ActionDispatch::Integration assert_response :success end end + + test "reorders within not now without side effects" do + card = cards(:layout) + other = cards(:shipping) + + with_current_user(:kevin) do + card.postpone + other.postpone + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.postponed? } do + assert_changes -> { card.reload.position }, to: 0 do + assert_no_difference -> { card.events.count } do + post columns_card_drops_not_now_path(card), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end + end end diff --git a/test/controllers/columns/cards/drops/streams_controller_test.rb b/test/controllers/columns/cards/drops/streams_controller_test.rb index 5bd082e216..11e7e51452 100644 --- a/test/controllers/columns/cards/drops/streams_controller_test.rb +++ b/test/controllers/columns/cards/drops/streams_controller_test.rb @@ -13,4 +13,31 @@ class Columns::Cards::Drops::StreamsControllerTest < ActionDispatch::Integration assert_response :success end end + + test "reorders within stream without side effects" do + board = boards(:writebook) + card = cards(:buy_domain) + other = with_current_user(:kevin) do + board.cards.create!( + title: "Another stream card", + creator: users(:kevin), + status: "published", + last_active_at: 2.days.ago + ) + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.triaged? } do + assert_changes -> { card.reload.position }, to: 0 do + assert_no_difference -> { card.events.count } do + post columns_card_drops_stream_path(card), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end + end end diff --git a/test/models/card/positioner_test.rb b/test/models/card/positioner_test.rb new file mode 100644 index 0000000000..0958a866bc --- /dev/null +++ b/test/models/card/positioner_test.rb @@ -0,0 +1,61 @@ +require "test_helper" + +class Card::PositionerTest < ActiveSupport::TestCase + setup do + @column = columns(:writebook_triage) + @card_a = cards(:logo) + @card_b = cards(:layout) + @card_c = cards(:shipping) + + [ @card_a, @card_b, @card_c ].each do |card| + card.update!(column: @column, status: "published") + end + end + + test "repositions a card between two neighbors" do + @card_a.update!(position: 1024) + @card_b.update!(position: 3072) + @card_c.update!(position: 2048) + + positioner = Card::Positioner.new( + relation: @column.cards.active, + fallback_order: { last_active_at: :desc, id: :desc } + ) + + positioner.reposition!(card: @card_c, after_number: @card_a.number, before_number: @card_b.number) + + assert_equal 2048, @card_c.reload.position + end + + test "repositions a card to the top when only before neighbor is provided" do + @card_a.update!(position: 1024) + @card_b.update!(position: 2048) + + positioner = Card::Positioner.new( + relation: @column.cards.active, + fallback_order: { last_active_at: :desc, id: :desc } + ) + + positioner.reposition!(card: @card_b, after_number: nil, before_number: @card_a.number) + + assert_equal 0, @card_b.reload.position + end + + test "renumbers when there is no room between neighbors" do + @card_a.update!(position: 1, last_active_at: 3.days.ago) + @card_b.update!(position: 2, last_active_at: 2.days.ago) + @card_c.update!(position: 3, last_active_at: 1.day.ago) + + positioner = Card::Positioner.new( + relation: @column.cards.active, + fallback_order: { last_active_at: :desc, id: :desc } + ) + + positioner.reposition!(card: @card_c, after_number: @card_a.number, before_number: @card_b.number) + + assert @card_c.reload.position.between?(@card_a.reload.position, @card_b.reload.position) + assert_equal 1024, @card_a.reload.position + assert_equal 2048, @card_b.reload.position + end +end + From 51187ed4ed507c966ab1fcf303fc27c035b8d8b2 Mon Sep 17 00:00:00 2001 From: Carl-Fredrik Arvidson Date: Sun, 21 Dec 2025 14:59:02 +0100 Subject: [PATCH 3/5] Gate manual card sorting behind a new Board setting - Add manual_sorting_enabled boolean to boards table - Add a toggle for manual sorting in Board Settings (mirrors Public link style) - Update list controllers to only use position-based ordering when enabled - Update drag_and_drop_controller.js to block intra-list reordering when disabled - Update drop endpoints to ignore reorder hints unless enabled - Add unit and integration tests for the new setting and gating logic --- .../boards/columns/closeds_controller.rb | 6 ++- .../boards/columns/not_nows_controller.rb | 6 ++- .../boards/columns/streams_controller.rb | 6 ++- app/controllers/boards/columns_controller.rb | 6 ++- .../boards/manual_sortings_controller.rb | 14 +++++ app/controllers/boards_controller.rb | 6 ++- .../cards/drops/closures_controller.rb | 15 ++++-- .../columns/cards/drops/columns_controller.rb | 13 ++++- .../cards/drops/not_nows_controller.rb | 11 +++- .../columns/cards/drops/streams_controller.rb | 19 +++++-- .../boards/columns/closeds_controller.rb | 6 ++- .../boards/columns/not_nows_controller.rb | 6 ++- .../boards/columns/streams_controller.rb | 6 ++- .../public/boards/columns_controller.rb | 6 ++- app/controllers/public/boards_controller.rb | 6 ++- .../controllers/drag_and_drop_controller.js | 23 ++++++-- app/views/boards/edit.html.erb | 1 + .../boards/edit/_manual_sorting.html.erb | 37 +++++++++++++ .../manual_sortings/create.turbo_stream.erb | 3 ++ .../manual_sortings/destroy.turbo_stream.erb | 3 ++ app/views/boards/show/_closed.html.erb | 1 + app/views/boards/show/_column.html.erb | 1 + app/views/boards/show/_not_now.html.erb | 1 + app/views/boards/show/_stream.html.erb | 3 +- config/routes.rb | 1 + ...00_add_manual_sorting_enabled_to_boards.rb | 6 +++ db/schema.rb | 3 +- .../boards/manual_sortings_controller_test.rb | 53 +++++++++++++++++++ .../cards/drops/closures_controller_test.rb | 23 ++++++++ .../cards/drops/columns_controller_test.rb | 29 ++++++++++ .../cards/drops/not_nows_controller_test.rb | 23 ++++++++ .../cards/drops/streams_controller_test.rb | 26 +++++++++ test/models/board/manual_sorting_test.rb | 9 ++++ 33 files changed, 348 insertions(+), 30 deletions(-) create mode 100644 app/controllers/boards/manual_sortings_controller.rb create mode 100644 app/views/boards/edit/_manual_sorting.html.erb create mode 100644 app/views/boards/manual_sortings/create.turbo_stream.erb create mode 100644 app/views/boards/manual_sortings/destroy.turbo_stream.erb create mode 100644 db/migrate/20251221093000_add_manual_sorting_enabled_to_boards.rb create mode 100644 test/controllers/boards/manual_sortings_controller_test.rb create mode 100644 test/models/board/manual_sorting_test.rb diff --git a/app/controllers/boards/columns/closeds_controller.rb b/app/controllers/boards/columns/closeds_controller.rb index c7f8fa260b..2692f5c34f 100644 --- a/app/controllers/boards/columns/closeds_controller.rb +++ b/app/controllers/boards/columns/closeds_controller.rb @@ -2,8 +2,12 @@ class Boards::Columns::ClosedsController < ApplicationController include BoardScoped def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @board.cards.closed.ordered_by_position(Arel.sql("closures.created_at DESC, cards.id DESC")).preloaded + else + @board.cards.closed.recently_closed_first.preloaded + end + set_page_and_extract_portion_from cards fresh_when etag: @page.records end end diff --git a/app/controllers/boards/columns/not_nows_controller.rb b/app/controllers/boards/columns/not_nows_controller.rb index 79fe7ab35b..cc3f55735c 100644 --- a/app/controllers/boards/columns/not_nows_controller.rb +++ b/app/controllers/boards/columns/not_nows_controller.rb @@ -2,8 +2,12 @@ class Boards::Columns::NotNowsController < ApplicationController include BoardScoped def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @board.cards.postponed.ordered_by_position(last_active_at: :desc, id: :desc).preloaded + else + @board.cards.postponed.latest.preloaded + end + set_page_and_extract_portion_from cards fresh_when etag: @page.records end end diff --git a/app/controllers/boards/columns/streams_controller.rb b/app/controllers/boards/columns/streams_controller.rb index f999a56c7a..4d3df04630 100644 --- a/app/controllers/boards/columns/streams_controller.rb +++ b/app/controllers/boards/columns/streams_controller.rb @@ -2,8 +2,12 @@ class Boards::Columns::StreamsController < ApplicationController include BoardScoped def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc).preloaded + else + @board.cards.awaiting_triage.latest.with_golden_first.preloaded + end + set_page_and_extract_portion_from cards fresh_when etag: @page.records end end diff --git a/app/controllers/boards/columns_controller.rb b/app/controllers/boards/columns_controller.rb index 9ca922e5cd..ebe79d49d2 100644 --- a/app/controllers/boards/columns_controller.rb +++ b/app/controllers/boards/columns_controller.rb @@ -9,8 +9,12 @@ def index end def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @column.cards.active.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc).preloaded + else + @column.cards.active.latest.with_golden_first.preloaded + end + set_page_and_extract_portion_from cards fresh_when etag: @page.records end diff --git a/app/controllers/boards/manual_sortings_controller.rb b/app/controllers/boards/manual_sortings_controller.rb new file mode 100644 index 0000000000..a87d362b92 --- /dev/null +++ b/app/controllers/boards/manual_sortings_controller.rb @@ -0,0 +1,14 @@ +class Boards::ManualSortingsController < ApplicationController + include BoardScoped + + before_action :ensure_permission_to_admin_board + + def create + @board.update!(manual_sorting_enabled: true) + end + + def destroy + @board.update!(manual_sorting_enabled: false) + end +end + diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb index 909b57f125..c2a5d4cd32 100644 --- a/app/controllers/boards_controller.rb +++ b/app/controllers/boards_controller.rb @@ -81,7 +81,11 @@ def show_filtered_cards end def show_columns - cards = @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc).preloaded + cards = if @board.manual_sorting_enabled? + @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc).preloaded + else + @board.cards.awaiting_triage.latest.with_golden_first.preloaded + end set_page_and_extract_portion_from cards fresh_when etag: [ @board, @page.records, @user_filtering, Current.account ] end diff --git a/app/controllers/columns/cards/drops/closures_controller.rb b/app/controllers/columns/cards/drops/closures_controller.rb index fa7211382d..04f0233a9c 100644 --- a/app/controllers/columns/cards/drops/closures_controller.rb +++ b/app/controllers/columns/cards/drops/closures_controller.rb @@ -2,12 +2,17 @@ class Columns::Cards::Drops::ClosuresController < ApplicationController include CardScoped def create - ActiveRecord::Base.transaction do + if @card.closed? + return unless @board.manual_sorting_enabled? + else @card.close - - Card::Positioner - .new(relation: @board.cards.closed, fallback_order: Arel.sql("closures.created_at DESC, cards.id DESC")) - .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) end + + before_id = @board.manual_sorting_enabled? ? params[:before_id] : nil + after_id = @board.manual_sorting_enabled? ? params[:after_id] : nil + + Card::Positioner + .new(relation: @board.cards.closed, fallback_order: Arel.sql("closures.created_at DESC, cards.id DESC")) + .reposition!(card: @card, before_number: before_id, after_number: after_id) end end diff --git a/app/controllers/columns/cards/drops/columns_controller.rb b/app/controllers/columns/cards/drops/columns_controller.rb index d9508432cd..cf3e80f2b0 100644 --- a/app/controllers/columns/cards/drops/columns_controller.rb +++ b/app/controllers/columns/cards/drops/columns_controller.rb @@ -5,7 +5,13 @@ def create @column = @card.board.columns.find(params[:column_id]) ActiveRecord::Base.transaction do - @card.triage_into(@column) unless @card.active? && @card.column == @column + already_in_target = @card.active? && @card.column == @column + + if already_in_target + return unless @board.manual_sorting_enabled? + else + @card.triage_into(@column) + end relation = @column.cards.active relation = if @card.golden? @@ -14,9 +20,12 @@ def create relation.where.missing(:goldness) end + before_id = @board.manual_sorting_enabled? ? params[:before_id] : nil + after_id = @board.manual_sorting_enabled? ? params[:after_id] : nil + Card::Positioner .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) - .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + .reposition!(card: @card, before_number: before_id, after_number: after_id) end end end diff --git a/app/controllers/columns/cards/drops/not_nows_controller.rb b/app/controllers/columns/cards/drops/not_nows_controller.rb index 11d4c16bf3..1878d6b1c8 100644 --- a/app/controllers/columns/cards/drops/not_nows_controller.rb +++ b/app/controllers/columns/cards/drops/not_nows_controller.rb @@ -3,11 +3,18 @@ class Columns::Cards::Drops::NotNowsController < ApplicationController def create ActiveRecord::Base.transaction do - @card.postpone unless @card.postponed? + if @card.postponed? + return unless @board.manual_sorting_enabled? + else + @card.postpone + end + + before_id = @board.manual_sorting_enabled? ? params[:before_id] : nil + after_id = @board.manual_sorting_enabled? ? params[:after_id] : nil Card::Positioner .new(relation: @board.cards.postponed, fallback_order: { last_active_at: :desc, id: :desc }) - .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + .reposition!(card: @card, before_number: before_id, after_number: after_id) end end end diff --git a/app/controllers/columns/cards/drops/streams_controller.rb b/app/controllers/columns/cards/drops/streams_controller.rb index 4f2f80932f..ea3c9beeb0 100644 --- a/app/controllers/columns/cards/drops/streams_controller.rb +++ b/app/controllers/columns/cards/drops/streams_controller.rb @@ -3,7 +3,9 @@ class Columns::Cards::Drops::StreamsController < ApplicationController def create ActiveRecord::Base.transaction do - @card.send_back_to_triage unless @card.awaiting_triage? + unless @card.awaiting_triage? + @card.send_back_to_triage + end relation = @board.cards.awaiting_triage relation = if @card.golden? @@ -12,11 +14,18 @@ def create relation.where.missing(:goldness) end - Card::Positioner - .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) - .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + if @board.manual_sorting_enabled? + Card::Positioner + .new(relation: relation, fallback_order: { last_active_at: :desc, id: :desc }) + .reposition!(card: @card, before_number: params[:before_id], after_number: params[:after_id]) + end end - set_page_and_extract_portion_from @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) + cards = if @board.manual_sorting_enabled? + @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) + else + @board.cards.awaiting_triage.latest.with_golden_first + end + set_page_and_extract_portion_from cards end end diff --git a/app/controllers/public/boards/columns/closeds_controller.rb b/app/controllers/public/boards/columns/closeds_controller.rb index 20a082ff88..53d6e913a2 100644 --- a/app/controllers/public/boards/columns/closeds_controller.rb +++ b/app/controllers/public/boards/columns/closeds_controller.rb @@ -1,6 +1,10 @@ class Public::Boards::Columns::ClosedsController < Public::BaseController def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @board.cards.closed.ordered_by_position(Arel.sql("closures.created_at DESC, cards.id DESC")) + else + @board.cards.closed.recently_closed_first + end + set_page_and_extract_portion_from cards end end diff --git a/app/controllers/public/boards/columns/not_nows_controller.rb b/app/controllers/public/boards/columns/not_nows_controller.rb index fabaa2f4f7..55158a9362 100644 --- a/app/controllers/public/boards/columns/not_nows_controller.rb +++ b/app/controllers/public/boards/columns/not_nows_controller.rb @@ -1,6 +1,10 @@ class Public::Boards::Columns::NotNowsController < Public::BaseController def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @board.cards.postponed.ordered_by_position(last_active_at: :desc, id: :desc) + else + @board.cards.postponed.latest + end + set_page_and_extract_portion_from cards end end diff --git a/app/controllers/public/boards/columns/streams_controller.rb b/app/controllers/public/boards/columns/streams_controller.rb index 69ba0094b4..fe8f8badbf 100644 --- a/app/controllers/public/boards/columns/streams_controller.rb +++ b/app/controllers/public/boards/columns/streams_controller.rb @@ -1,6 +1,10 @@ class Public::Boards::Columns::StreamsController < Public::BaseController def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) + else + @board.cards.awaiting_triage.latest.with_golden_first + end + set_page_and_extract_portion_from cards end end diff --git a/app/controllers/public/boards/columns_controller.rb b/app/controllers/public/boards/columns_controller.rb index 916021c682..91b2f2b991 100644 --- a/app/controllers/public/boards/columns_controller.rb +++ b/app/controllers/public/boards/columns_controller.rb @@ -2,8 +2,12 @@ class Public::Boards::ColumnsController < Public::BaseController before_action :set_column def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @column.cards.active.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) + else + @column.cards.active.latest.with_golden_first + end + set_page_and_extract_portion_from cards end private diff --git a/app/controllers/public/boards_controller.rb b/app/controllers/public/boards_controller.rb index 816f72aef0..68b8355877 100644 --- a/app/controllers/public/boards_controller.rb +++ b/app/controllers/public/boards_controller.rb @@ -1,6 +1,10 @@ class Public::BoardsController < Public::BaseController def show - set_page_and_extract_portion_from \ + cards = if @board.manual_sorting_enabled? @board.cards.awaiting_triage.with_golden_first.ordered_by_position(last_active_at: :desc, id: :desc) + else + @board.cards.awaiting_triage.latest.with_golden_first + end + set_page_and_extract_portion_from cards end end diff --git a/app/javascript/controllers/drag_and_drop_controller.js b/app/javascript/controllers/drag_and_drop_controller.js index 9cbc49efdf..88285f2187 100644 --- a/app/javascript/controllers/drag_and_drop_controller.js +++ b/app/javascript/controllers/drag_and_drop_controller.js @@ -35,7 +35,9 @@ export default class extends Controller { return; } - this.#repositionDraggedItem(container, event.clientY); + if (container !== this.sourceContainer || this.#reorderEnabled(container)) { + this.#repositionDraggedItem(container, event.clientY); + } if (container !== this.sourceContainer) { container.classList.add(this.hoverContainerClass); @@ -52,6 +54,13 @@ export default class extends Controller { return; } + if ( + targetContainer === this.sourceContainer && + !this.#reorderEnabled(targetContainer) + ) { + return; + } + this.wasDropped = true; const sourceContainer = this.sourceContainer; const movedAcrossContainers = targetContainer !== sourceContainer; @@ -194,9 +203,11 @@ export default class extends Controller { const id = item.dataset.id; const url = container.dataset.dragAndDropUrl.replaceAll("__id__", id); - const { beforeId, afterId } = this.#neighborIdsFor(item); - if (beforeId) body.append("before_id", beforeId); - if (afterId) body.append("after_id", afterId); + if (this.#reorderEnabled(container)) { + const { beforeId, afterId } = this.#neighborIdsFor(item); + if (beforeId) body.append("before_id", beforeId); + if (afterId) body.append("after_id", afterId); + } return post(url, { body, @@ -204,6 +215,10 @@ export default class extends Controller { }); } + #reorderEnabled(container) { + return container.dataset.dragAndDropReorderEnabled === "true"; + } + #neighborIdsFor(item) { const isTopItem = item.hasAttribute("data-drag-and-drop-top"); diff --git a/app/views/boards/edit.html.erb b/app/views/boards/edit.html.erb index 36beee3c86..269a017606 100644 --- a/app/views/boards/edit.html.erb +++ b/app/views/boards/edit.html.erb @@ -32,6 +32,7 @@
<%= render "boards/edit/auto_close", board: @board %> + <%= render "boards/edit/manual_sorting", board: @board %> <%= render "boards/edit/publication", board: @board %> <%= render "boards/edit/delete", board: @board if Current.user.can_administer_board?(@board) %>
diff --git a/app/views/boards/edit/_manual_sorting.html.erb b/app/views/boards/edit/_manual_sorting.html.erb new file mode 100644 index 0000000000..96a3f68114 --- /dev/null +++ b/app/views/boards/edit/_manual_sorting.html.erb @@ -0,0 +1,37 @@ +<%= turbo_frame_tag @board, :manual_sorting do %> +
+

Manual sorting

+
Turn on Manual sorting to enable drag-and-drop ordering within each list on this board.
+
+ + <% if board.manual_sorting_enabled? %> +
+ <%= form_with url: board_manual_sorting_path(board), method: :delete, class: "flex-inline center justify-between gap", data: { controller: "form" } do |form| %> + + + <% end %> +
+ <% else %> +
+ <%= form_with url: board_manual_sorting_path(board), method: :post, class: "flex-inline center justify-between gap", data: { controller: "form" } do |form| %> + + + <% end %> +
+ <% end %> +<% end %> + diff --git a/app/views/boards/manual_sortings/create.turbo_stream.erb b/app/views/boards/manual_sortings/create.turbo_stream.erb new file mode 100644 index 0000000000..46693c9424 --- /dev/null +++ b/app/views/boards/manual_sortings/create.turbo_stream.erb @@ -0,0 +1,3 @@ +<%= turbo_stream.replace([ @board, :manual_sorting ], partial: "boards/edit/manual_sorting", locals:{ board: @board }) %> +<%= turbo_stream_flash(notice: "Saved") %> + diff --git a/app/views/boards/manual_sortings/destroy.turbo_stream.erb b/app/views/boards/manual_sortings/destroy.turbo_stream.erb new file mode 100644 index 0000000000..46693c9424 --- /dev/null +++ b/app/views/boards/manual_sortings/destroy.turbo_stream.erb @@ -0,0 +1,3 @@ +<%= turbo_stream.replace([ @board, :manual_sorting ], partial: "boards/edit/manual_sorting", locals:{ board: @board }) %> +<%= turbo_stream_flash(notice: "Saved") %> + diff --git a/app/views/boards/show/_closed.html.erb b/app/views/boards/show/_closed.html.erb index 5d2820399b..754b35c0eb 100644 --- a/app/views/boards/show/_closed.html.erb +++ b/app/views/boards/show/_closed.html.erb @@ -2,6 +2,7 @@ data: { drag_and_strum_target: "container", collapsible_columns_target: "column", + drag_and_drop_reorder_enabled: board.manual_sorting_enabled?, action: "focus->collapsible-columns#focusOnColumn" } do %>
diff --git a/app/views/boards/show/_column.html.erb b/app/views/boards/show/_column.html.erb index b5bba33399..e636909ad7 100644 --- a/app/views/boards/show/_column.html.erb +++ b/app/views/boards/show/_column.html.erb @@ -2,6 +2,7 @@ class: "cards--doing", style: "--card-color: #{column.color};", data: { drag_and_strum_target: "container", + drag_and_drop_reorder_enabled: column.board.manual_sorting_enabled?, collapsible_columns_target: "column", controller: "clicker", action: "turbo:before-stream-render@document->collapsible-columns#restoreState focus->collapsible-columns#focusOnColumn" diff --git a/app/views/boards/show/_not_now.html.erb b/app/views/boards/show/_not_now.html.erb index 66cd99519e..0e9c15e960 100644 --- a/app/views/boards/show/_not_now.html.erb +++ b/app/views/boards/show/_not_now.html.erb @@ -2,6 +2,7 @@ data: { collapsible_columns_target: "column", drag_and_strum_target: "container", + drag_and_drop_reorder_enabled: board.manual_sorting_enabled?, action: "focus->collapsible-columns#focusOnColumn" } do %>
diff --git a/app/views/boards/show/_stream.html.erb b/app/views/boards/show/_stream.html.erb index d4753eba59..7ed3891155 100644 --- a/app/views/boards/show/_stream.html.erb +++ b/app/views/boards/show/_stream.html.erb @@ -1,4 +1,5 @@ -<%= column_tag id: "the-stream", name: "Maybe?", drop_url: columns_card_drops_stream_path("__id__"), collapsed: false, selected: "true", class: "cards--considering" do %> +<%= column_tag id: "the-stream", name: "Maybe?", drop_url: columns_card_drops_stream_path("__id__"), collapsed: false, selected: "true", class: "cards--considering", + data: { drag_and_drop_reorder_enabled: board.manual_sorting_enabled? } do %>

Maybe?

diff --git a/config/routes.rb b/config/routes.rb index d84fa06e38..4e6104e66d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,6 +27,7 @@ resource :subscriptions resource :involvement resource :publication + resource :manual_sorting, only: %i[ create destroy ] resource :entropy namespace :columns do diff --git a/db/migrate/20251221093000_add_manual_sorting_enabled_to_boards.rb b/db/migrate/20251221093000_add_manual_sorting_enabled_to_boards.rb new file mode 100644 index 0000000000..b6710e8b47 --- /dev/null +++ b/db/migrate/20251221093000_add_manual_sorting_enabled_to_boards.rb @@ -0,0 +1,6 @@ +class AddManualSortingEnabledToBoards < ActiveRecord::Migration[8.2] + def change + add_column :boards, :manual_sorting_enabled, :boolean, default: false, null: false + end +end + diff --git a/db/schema.rb b/db/schema.rb index a87bc296ce..6c622a2212 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_12_21_090000) do +ActiveRecord::Schema[8.2].define(version: 2025_12_21_093000) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -147,6 +147,7 @@ t.boolean "all_access", default: false, null: false t.datetime "created_at", null: false t.uuid "creator_id", null: false + t.boolean "manual_sorting_enabled", default: false, null: false t.string "name", null: false t.datetime "updated_at", null: false t.index ["account_id"], name: "index_boards_on_account_id" diff --git a/test/controllers/boards/manual_sortings_controller_test.rb b/test/controllers/boards/manual_sortings_controller_test.rb new file mode 100644 index 0000000000..8f91d2b773 --- /dev/null +++ b/test/controllers/boards/manual_sortings_controller_test.rb @@ -0,0 +1,53 @@ +require "test_helper" + +class Boards::ManualSortingsControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :kevin + @board = boards(:writebook) + end + + test "enable manual sorting" do + assert_not @board.manual_sorting_enabled? + + assert_changes -> { @board.reload.manual_sorting_enabled? }, from: false, to: true do + post board_manual_sorting_path(@board, format: :turbo_stream) + end + + assert_turbo_stream action: :replace, target: dom_id(@board, :manual_sorting) + end + + test "disable manual sorting" do + @board.update!(manual_sorting_enabled: true) + assert @board.manual_sorting_enabled? + + assert_changes -> { @board.reload.manual_sorting_enabled? }, from: true, to: false do + delete board_manual_sorting_path(@board, format: :turbo_stream) + end + + assert_turbo_stream action: :replace, target: dom_id(@board, :manual_sorting) + end + + test "enable requires board admin permission" do + logout_and_sign_in_as :jz + + assert_not @board.manual_sorting_enabled? + + post board_manual_sorting_path(@board, format: :turbo_stream) + + assert_response :forbidden + assert_not @board.reload.manual_sorting_enabled? + end + + test "disable requires board admin permission" do + logout_and_sign_in_as :jz + + @board.update!(manual_sorting_enabled: true) + assert @board.manual_sorting_enabled? + + delete board_manual_sorting_path(@board, format: :turbo_stream) + + assert_response :forbidden + assert @board.reload.manual_sorting_enabled? + end +end + diff --git a/test/controllers/columns/cards/drops/closures_controller_test.rb b/test/controllers/columns/cards/drops/closures_controller_test.rb index 383d0c3760..ad9f89475d 100644 --- a/test/controllers/columns/cards/drops/closures_controller_test.rb +++ b/test/controllers/columns/cards/drops/closures_controller_test.rb @@ -17,6 +17,7 @@ class Columns::Cards::Drops::ClosuresControllerTest < ActionDispatch::Integratio test "reorders within done without side effects" do card = cards(:logo) other = cards(:layout) + card.board.update!(manual_sorting_enabled: true) with_current_user(:kevin) do card.close @@ -37,4 +38,26 @@ class Columns::Cards::Drops::ClosuresControllerTest < ActionDispatch::Integratio end end end + + test "does not reorder within done when manual sorting is disabled" do + card = cards(:logo) + other = cards(:layout) + + with_current_user(:kevin) do + card.close + other.close + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.closed? } do + assert_no_changes -> { card.reload.position } do + post columns_card_drops_closure_path(card), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end end diff --git a/test/controllers/columns/cards/drops/columns_controller_test.rb b/test/controllers/columns/cards/drops/columns_controller_test.rb index fcf33f92b3..ef63628af9 100644 --- a/test/controllers/columns/cards/drops/columns_controller_test.rb +++ b/test/controllers/columns/cards/drops/columns_controller_test.rb @@ -17,6 +17,7 @@ class Columns::Cards::Drops::ColumnsControllerTest < ActionDispatch::Integration test "reorders within the same column without side effects" do board = boards(:writebook) + board.update!(manual_sorting_enabled: true) column = columns(:writebook_in_progress) card = cards(:text) @@ -44,4 +45,32 @@ class Columns::Cards::Drops::ColumnsControllerTest < ActionDispatch::Integration end end end + + test "does not reorder within the same column when manual sorting is disabled" do + board = boards(:writebook) + column = columns(:writebook_in_progress) + + card = cards(:text) + other = with_current_user(:kevin) do + board.cards.create!( + title: "Another card", + creator: users(:kevin), + status: "published", + last_active_at: 2.days.ago, + column: column + ) + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.column_id } do + assert_no_changes -> { card.reload.position } do + post columns_card_drops_column_path(card, column_id: column.id), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end end diff --git a/test/controllers/columns/cards/drops/not_nows_controller_test.rb b/test/controllers/columns/cards/drops/not_nows_controller_test.rb index a5cb78ffc2..956035e188 100644 --- a/test/controllers/columns/cards/drops/not_nows_controller_test.rb +++ b/test/controllers/columns/cards/drops/not_nows_controller_test.rb @@ -17,6 +17,7 @@ class Columns::Cards::Drops::NotNowsControllerTest < ActionDispatch::Integration test "reorders within not now without side effects" do card = cards(:layout) other = cards(:shipping) + card.board.update!(manual_sorting_enabled: true) with_current_user(:kevin) do card.postpone @@ -37,4 +38,26 @@ class Columns::Cards::Drops::NotNowsControllerTest < ActionDispatch::Integration end end end + + test "does not reorder within not now when manual sorting is disabled" do + card = cards(:layout) + other = cards(:shipping) + + with_current_user(:kevin) do + card.postpone + other.postpone + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.postponed? } do + assert_no_changes -> { card.reload.position } do + post columns_card_drops_not_now_path(card), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end end diff --git a/test/controllers/columns/cards/drops/streams_controller_test.rb b/test/controllers/columns/cards/drops/streams_controller_test.rb index 11e7e51452..7670064d4c 100644 --- a/test/controllers/columns/cards/drops/streams_controller_test.rb +++ b/test/controllers/columns/cards/drops/streams_controller_test.rb @@ -16,6 +16,7 @@ class Columns::Cards::Drops::StreamsControllerTest < ActionDispatch::Integration test "reorders within stream without side effects" do board = boards(:writebook) + board.update!(manual_sorting_enabled: true) card = cards(:buy_domain) other = with_current_user(:kevin) do board.cards.create!( @@ -40,4 +41,29 @@ class Columns::Cards::Drops::StreamsControllerTest < ActionDispatch::Integration end end end + + test "does not reorder within stream when manual sorting is disabled" do + board = boards(:writebook) + card = cards(:buy_domain) + other = with_current_user(:kevin) do + board.cards.create!( + title: "Another stream card", + creator: users(:kevin), + status: "published", + last_active_at: 2.days.ago + ) + end + + other.update!(position: 1024) + card.update!(position: 2048) + + assert_no_changes -> { card.reload.triaged? } do + assert_no_changes -> { card.reload.position } do + post columns_card_drops_stream_path(card), + params: { before_id: other.number }, + as: :turbo_stream + assert_response :success + end + end + end end diff --git a/test/models/board/manual_sorting_test.rb b/test/models/board/manual_sorting_test.rb new file mode 100644 index 0000000000..bd28f0fa89 --- /dev/null +++ b/test/models/board/manual_sorting_test.rb @@ -0,0 +1,9 @@ +require "test_helper" + +class Board::ManualSortingTest < ActiveSupport::TestCase + test "manual sorting is disabled by default" do + board = boards(:writebook) + assert_equal false, board.manual_sorting_enabled? + end +end + From 604212b1dcac694024c2634d14d9047630181429 Mon Sep 17 00:00:00 2001 From: Carl-Fredrik Arvidson Date: Sun, 21 Dec 2025 15:04:49 +0100 Subject: [PATCH 4/5] Use ON/OFF labels for manual sorting toggle Replace the lock/move icons in Board Settings with larger, vertically centered ON/OFF labels. --- app/views/boards/edit/_manual_sorting.html.erb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/boards/edit/_manual_sorting.html.erb b/app/views/boards/edit/_manual_sorting.html.erb index 96a3f68114..1b84a28e20 100644 --- a/app/views/boards/edit/_manual_sorting.html.erb +++ b/app/views/boards/edit/_manual_sorting.html.erb @@ -8,26 +8,26 @@
<%= form_with url: board_manual_sorting_path(board), method: :delete, class: "flex-inline center justify-between gap", data: { controller: "form" } do |form| %> - + <% end %>
<% else %>
<%= form_with url: board_manual_sorting_path(board), method: :post, class: "flex-inline center justify-between gap", data: { controller: "form" } do |form| %> - + <% end %> From aa8b97725693926a44acd1c50769d3de8944a32e Mon Sep 17 00:00:00 2001 From: Carl-Fredrik Arvidson Date: Sat, 3 Jan 2026 21:48:24 +0100 Subject: [PATCH 5/5] Fix the toggle --- app/views/boards/edit/_manual_sorting.html.erb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/views/boards/edit/_manual_sorting.html.erb b/app/views/boards/edit/_manual_sorting.html.erb index 1b84a28e20..c3acd07b17 100644 --- a/app/views/boards/edit/_manual_sorting.html.erb +++ b/app/views/boards/edit/_manual_sorting.html.erb @@ -8,14 +8,14 @@
<%= form_with url: board_manual_sorting_path(board), method: :delete, class: "flex-inline center justify-between gap", data: { controller: "form" } do |form| %> - + <% end %>
<% else %> @@ -34,4 +34,3 @@
<% end %> <% end %> -