Merge pull request #1418 from basecamp/sort-columns

Sort columns

app/assets/images/column-left.svg

@@ -0,0 +1,52 @@
+<svg width="21" height="21" viewBox="0 0 21 21" fill="none" xmlns="http://www.w3.org/2000/svg">
+<path d="M4 9V12L0 8L4 4V7H8V9" fill="black"/>
+<g filter="url(#filter0_dd_6_4)">
+<path d="M6 5L6 2C6 1.44772 6.44772 1 7 1C7.55228 1 8 1.44772 8 2L8 5" fill="black"/>
+</g>
+<path d="M8 11V16C8 16.5523 7.55228 17 7 17C6.44772 17 6 16.5523 6 16V11" fill="black"/>
+<path d="M12 1C12.5523 1 13 1.44772 13 2V20C13 20.5523 12.5523 21 12 21C11.4477 21 11 20.5523 11 20V2C11 1.44772 11.4477 1 12 1Z" fill="black"/>
+<path d="M17 1C17.5523 1 18 1.44772 18 2V13C18 13.5523 17.5523 14 17 14C16.4477 14 16 13.5523 16 13V2C16 1.44772 16.4477 1 17 1Z" fill="black"/>
+<rect x="7" y="7" width="2" height="2" rx="1" fill="black"/>
+<path d="M4 9V12L0 8L4 4V7H8V9" fill="black"/>
+<g filter="url(#filter1_dd_6_4)">
+<path d="M6 5L6 2C6 1.44772 6.44772 1 7 1C7.55228 1 8 1.44772 8 2L8 5" fill="black"/>
+</g>
+<path d="M8 11V16C8 16.5523 7.55228 17 7 17C6.44772 17 6 16.5523 6 16V11" fill="black"/>
+<path d="M12 1C12.5523 1 13 1.44772 13 2V20C13 20.5523 12.5523 21 12 21C11.4477 21 11 20.5523 11 20V2C11 1.44772 11.4477 1 12 1Z" fill="black"/>
+<path d="M17 1C17.5523 1 18 1.44772 18 2V13C18 13.5523 17.5523 14 17 14C16.4477 14 16 13.5523 16 13V2C16 1.44772 16.4477 1 17 1Z" fill="black"/>
+<rect x="7" y="7" width="2" height="2" rx="1" fill="black"/>
+<defs>
+<filter id="filter0_dd_6_4" x="2" y="1" width="10" height="12" filterUnits="userSpaceOnUse" color-interpolation-filters="sRGB">
+<feFlood flood-opacity="0" result="BackgroundImageFix"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset dy="4"/>
+<feGaussianBlur stdDeviation="2"/>
+<feComposite in2="hardAlpha" operator="out"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.25 0"/>
+<feBlend mode="normal" in2="BackgroundImageFix" result="effect1_dropShadow_6_4"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset dy="4"/>
+<feGaussianBlur stdDeviation="2"/>
+<feComposite in2="hardAlpha" operator="out"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.25 0"/>
+<feBlend mode="normal" in2="effect1_dropShadow_6_4" result="effect2_dropShadow_6_4"/>
+<feBlend mode="normal" in="SourceGraphic" in2="effect2_dropShadow_6_4" result="shape"/>
+</filter>
+<filter id="filter1_dd_6_4" x="2" y="1" width="10" height="12" filterUnits="userSpaceOnUse" color-interpolation-filters="sRGB">
+<feFlood flood-opacity="0" result="BackgroundImageFix"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset dy="4"/>
+<feGaussianBlur stdDeviation="2"/>
+<feComposite in2="hardAlpha" operator="out"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.25 0"/>
+<feBlend mode="normal" in2="BackgroundImageFix" result="effect1_dropShadow_6_4"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset dy="4"/>
+<feGaussianBlur stdDeviation="2"/>
+<feComposite in2="hardAlpha" operator="out"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.25 0"/>
+<feBlend mode="normal" in2="effect1_dropShadow_6_4" result="effect2_dropShadow_6_4"/>
+<feBlend mode="normal" in="SourceGraphic" in2="effect2_dropShadow_6_4" result="shape"/>
+</filter>
+</defs>
+</svg>

app/assets/images/column-right.svg

@@ -0,0 +1,28 @@
+<svg width="21" height="21" viewBox="0 0 21 21" fill="none" xmlns="http://www.w3.org/2000/svg">
+<path d="M17 9V12L21 8L17 4V7H13V9" fill="black"/>
+<g filter="url(#filter0_dd_7_17)">
+<path d="M15 5L15 2C15 1.44772 14.5523 1 14 1C13.4477 1 13 1.44772 13 2L13 5" fill="black"/>
+</g>
+<path d="M13 11V16C13 16.5523 13.4477 17 14 17C14.5523 17 15 16.5523 15 16V11" fill="black"/>
+<path d="M9 1C8.44772 1 8 1.44772 8 2V20C8 20.5523 8.44772 21 9 21C9.55228 21 10 20.5523 10 20V2C10 1.44772 9.55228 1 9 1Z" fill="black"/>
+<path d="M4 1C3.44772 1 3 1.44772 3 2V13C3 13.5523 3.44772 14 4 14C4.55228 14 5 13.5523 5 13V2C5 1.44772 4.55228 1 4 1Z" fill="black"/>
+<rect width="2" height="2" rx="1" transform="matrix(-1 0 0 1 14 7)" fill="black"/>
+<defs>
+<filter id="filter0_dd_7_17" x="9" y="1" width="10" height="12" filterUnits="userSpaceOnUse" color-interpolation-filters="sRGB">
+<feFlood flood-opacity="0" result="BackgroundImageFix"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset dy="4"/>
+<feGaussianBlur stdDeviation="2"/>
+<feComposite in2="hardAlpha" operator="out"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.25 0"/>
+<feBlend mode="normal" in2="BackgroundImageFix" result="effect1_dropShadow_7_17"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset dy="4"/>
+<feGaussianBlur stdDeviation="2"/>
+<feComposite in2="hardAlpha" operator="out"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.25 0"/>
+<feBlend mode="normal" in2="effect1_dropShadow_7_17" result="effect2_dropShadow_7_17"/>
+<feBlend mode="normal" in="SourceGraphic" in2="effect2_dropShadow_7_17" result="shape"/>
+</filter>
+</defs>
+</svg>

app/assets/stylesheets/icons.css

@@ -44,6 +44,8 @@
   .icon--close-circle { --svg: url("close-circle.svg "); }
   .icon--collection { --svg: url("collection.svg "); }
   .icon--collection-add { --svg: url("collection-add.svg "); }
+  .icon--column-left { --svg: url("column-left.svg "); }
+  .icon--column-right { --svg: url("column-right.svg "); }
   .icon--comment { --svg: url("comment.svg "); }
   .icon--copy-paste { --svg: url("copy-paste.svg "); }
   .icon--crown { --svg: url("crown.svg "); }

app/controllers/columns/left_positions_controller.rb

@@ -0,0 +1,8 @@
+class Columns::LeftPositionsController < ApplicationController
+  include ColumnScoped
+
+  def create
+    @left_column = @column.left_column
+    @column.move_left
+  end
+end

app/controllers/columns/right_positions_controller.rb

@@ -0,0 +1,8 @@
+class Columns::RightPositionsController < ApplicationController
+  include ColumnScoped
+
+  def create
+    @right_column = @column.right_column
+    @column.move_right
+  end
+end

app/controllers/concerns/column_scoped.rb

@@ -0,0 +1,12 @@
+module ColumnScoped
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :set_column
+  end
+
+  private
+    def set_column
+      @column = Column.find(params[:column_id])
+    end
+end

app/javascript/controllers/collapsible_columns_controller.js

@@ -1,5 +1,5 @@
 import { Controller } from "@hotwired/stimulus"
-import { nextFrame } from "helpers/timing_helpers";
+import { nextFrame, debounce } from "helpers/timing_helpers";
 
 export default class extends Controller {
   static classes = [ "collapsed", "noTransitions" ]
@@ -8,12 +8,12 @@ export default class extends Controller {
     collection: String
   }
 
-  async connect() {
-    this.#disableTransitions()
-    this.#restoreColumns()
+  initialize() {
+    this.restoreState = debounce(this.restoreState.bind(this), 10)
+  }
 
-    await nextFrame()
-    this.#enableTransitions()
+  async connect() {
+    await this.#restoreColumnsDisablingTransitions()
   }
 
   toggle({ target }) {
@@ -27,6 +27,19 @@ export default class extends Controller {
     }
   }
 
+  async restoreState(event) {
+    await nextFrame()
+    await this.#restoreColumnsDisablingTransitions()
+  }
+
+  async #restoreColumnsDisablingTransitions() {
+    this.#disableTransitions()
+    this.#restoreColumns()
+
+    await nextFrame()
+    this.#enableTransitions()
+  }
+
   #disableTransitions() {
     this.element.classList.add(this.noTransitionsClass)
   }
@@ -79,13 +92,17 @@ export default class extends Controller {
 
   #restoreColumns() {
     this.columnTargets.forEach(column => {
-      const key = this.#localStorageKeyFor(column)
-      if (localStorage.getItem(key)) {
-        this.#expand(column)
-      }
+      this.#restoreColumn(column)
     })
   }
 
+  #restoreColumn(column) {
+    const key = this.#localStorageKeyFor(column)
+    if (localStorage.getItem(key)) {
+      this.#expand(column)
+    }
+  }
+
   #localStorageKeyFor(column) {
     return `expand-${this.collectionValue}-${column.getAttribute("id")}`
   }

app/models/column.rb

@@ -1,4 +1,6 @@
 class Column < ApplicationRecord
+  include Positioned
+
   belongs_to :collection, touch: true
   has_many :cards, dependent: :nullify
 

app/models/column/positioned.rb

@@ -0,0 +1,49 @@
+module Column::Positioned
+  extend ActiveSupport::Concern
+
+  included do
+    scope :sorted, -> { order(position: :asc) }
+
+    before_create :set_position
+  end
+
+  def move_left
+    swap_position_with left_column
+  end
+
+  def move_right
+    swap_position_with right_column
+  end
+
+  def left_column
+    collection.columns.where("position < ?", position).sorted.last
+  end
+
+  def right_column
+    collection.columns.where("position > ?", position).sorted.first
+  end
+
+  def leftmost?
+    left_column.nil?
+  end
+
+  def rightmost?
+    right_column.nil?
+  end
+
+  private
+    def set_position
+      max_position = collection.columns.maximum(:position) || 0
+      self.position = max_position + 1
+    end
+
+    def swap_position_with(other_column)
+      return if other_column.nil?
+
+      transaction do
+        old_position = self.position
+        self.update_column(:position, other_column.position)
+        other_column.update_column(:position, old_position)
+      end
+    end
+end

app/views/collections/show/_column.html.erb

@@ -4,7 +4,7 @@
   data-drag-and-strum-target="container"
   data-collapsible-columns-target="column"
   data-controller="clicker"
-  data-action="turbo:before-morph-attribute->collapsible-columns#preventToggle"
+  data-action="turbo:before-morph-attribute->collapsible-columns#preventToggle turbo:before-stream-render@document->collapsible-columns#restoreState"
   data-drag-and-drop-url="<%= columns_card_drops_column_path("__id__", column_id: column.id) %>"
 >
   <%= render "collections/show/expander", title: column.name, count: column.cards.active.count, column_id: dom_id(column) do %>

app/views/collections/show/_columns.html.erb

@@ -16,7 +16,7 @@
     <%= render "collections/show/stream", collection: collection, page: page %>
 
     <div class="card-columns__right">
-      <%= render partial: "collections/show/column", collection: collection.columns, cached: true %>
+      <%= render partial: "collections/show/column", collection: collection.columns.sorted, cached: true %>
       <%= render "collections/show/closed", collection: collection %>
 
       <%= render "collections/show/menu/columns", collection: collection %>

app/views/collections/show/menu/_column.html.erb

@@ -11,6 +11,27 @@
           <span>Edit column</span>
         </button>
       </li>
+
+      <li class="popup__item">
+        <%= button_to column_left_position_path(column),
+              class: "popup__btn btn",
+              form_class: "display-contents",
+              disabled: column.leftmost? do %>
+          <%= icon_tag "column-left" %>
+          <span>Move to the left</span>
+        <% end %>
+      </li>
+
+      <li class="popup__item">
+        <%= button_to column_right_position_path(column),
+              class: "popup__btn btn",
+              form_class: "display-contents",
+              disabled: column.rightmost? do %>
+          <%= icon_tag "column-right" %>
+          <span>Move to the right</span>
+        <% end %>
+      </li>
+
       <li class="popup__item">
         <%= button_to collection_column_path(column.collection, column),
               method: :delete,

app/views/columns/left_positions/create.turbo_stream.erb

@@ -0,0 +1,4 @@
+<% if @left_column %>
+  <%= turbo_stream.remove(dom_id(@column)) %>
+  <%= turbo_stream.before(@left_column, partial: "collections/show/column", locals: { column: @column }) %>
+<% end %>

app/views/columns/right_positions/create.turbo_stream.erb

@@ -0,0 +1,4 @@
+<% if @right_column %>
+  <%= turbo_stream.remove(dom_id(@column)) %>
+  <%= turbo_stream.after(@right_column, partial: "collections/show/column", locals: { column: @column }) %>
+<% end %>

config/routes.rb

@@ -35,6 +35,11 @@ Rails.application.routes.draw do
     end
   end
 
+  resources :columns, only: [] do
+    resource :left_position, module: :columns
+    resource :right_position, module: :columns
+  end
+
   namespace :columns do
     resources :cards do
       scope module: "cards" do

db/migrate/20251029142418_add_position_to_columns.rb

@@ -0,0 +1,9 @@
+class AddPositionToColumns < ActiveRecord::Migration[8.2]
+  def change
+    add_column :columns, :position, :integer, default: 0
+
+    execute "UPDATE columns SET position = 0"
+
+    change_column_null :columns, :position, false
+  end
+end

db/migrate/20251029161222_add_index_on_collection_and_position_to_columns.rb

@@ -0,0 +1,5 @@
+class AddIndexOnCollectionAndPositionToColumns < ActiveRecord::Migration[8.2]
+  def change
+    add_index :columns, [ :collection_id, :position ]
+  end
+end

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_10_29_104115) do
+ActiveRecord::Schema[8.2].define(version: 2025_10_29_161222) do
   create_table "accesses", force: :cascade do |t|
     t.datetime "accessed_at"
     t.integer "collection_id", null: false
@@ -191,7 +191,9 @@ ActiveRecord::Schema[8.2].define(version: 2025_10_29_104115) do
     t.string "color", null: false
     t.datetime "created_at", null: false
     t.string "name", null: false
+    t.integer "position", default: 0, null: false
     t.datetime "updated_at", null: false
+    t.index ["collection_id", "position"], name: "index_columns_on_collection_id_and_position"
     t.index ["collection_id"], name: "index_columns_on_collection_id"
   end
 

db/schema_cache.yml

@@ -562,6 +562,16 @@ columns:
   - *5
   - *6
   - *10
+  - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
+    auto_increment:
+    name: position
+    cast_type: *3
+    sql_type_metadata: *4
+    'null': false
+    default: 0
+    default_function:
+    collation:
+    comment:
   - *9
   comments:
   - *22
@@ -1919,6 +1929,23 @@ indexes:
     nulls_not_distinct:
     comment:
     valid: true
+  - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
+    table: columns
+    name: index_columns_on_collection_id_and_position
+    unique: false
+    columns:
+    - collection_id
+    - position
+    lengths: {}
+    orders: {}
+    opclasses: {}
+    where:
+    type:
+    using:
+    include:
+    nulls_not_distinct:
+    comment:
+    valid: true
   comments:
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
     table: comments
@@ -2818,4 +2845,4 @@ indexes:
     nulls_not_distinct:
     comment:
     valid: true
-version: 20251029104115
+version: 20251029161222

script/migrations/20251029-populate-column-positions.rb

@@ -0,0 +1,20 @@
+#!/usr/bin/env ruby
+
+require_relative "../../config/environment"
+
+ApplicationRecord.with_each_tenant do |tenant|
+  puts "Processing tenant: #{tenant}"
+
+  Collection.find_each do |collection|
+    puts "  Processing collection: #{collection.name} (ID: #{collection.id})"
+
+    columns = collection.columns.order(:id)
+
+    columns.each_with_index do |column, index|
+      column.update_column(:position, index)
+      puts "    Set position #{index} for column '#{column.name}' (ID: #{column.id})"
+    end
+  end
+end
+
+puts "Migration completed!"

test/controllers/columns/left_positions_controller_test.rb

@@ -0,0 +1,23 @@
+require "test_helper"
+
+class Columns::LeftPositionsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in_as :kevin
+  end
+
+  test "move column left" do
+    collection = collections(:writebook)
+    columns = collection.columns.sorted.to_a
+
+    column_a = columns[0]
+    column_b = columns[1]
+    original_position_a = column_a.position
+    original_position_b = column_b.position
+
+    post column_left_position_path(column_b), as: :turbo_stream
+    assert_response :success
+
+    assert_equal original_position_b, column_a.reload.position
+    assert_equal original_position_a, column_b.reload.position
+  end
+end

test/controllers/columns/right_positions_controller_test.rb

@@ -0,0 +1,23 @@
+require "test_helper"
+
+class Columns::RightPositionsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in_as :kevin
+  end
+
+  test "move column right" do
+    collection = collections(:writebook)
+    columns = collection.columns.sorted.to_a
+
+    column_a = columns[0]
+    column_b = columns[1]
+    original_position_a = column_a.position
+    original_position_b = column_b.position
+
+    post column_right_position_path(column_a), as: :turbo_stream
+    assert_response :success
+
+    assert_equal original_position_b, column_a.reload.position
+    assert_equal original_position_a, column_b.reload.position
+  end
+end

test/fixtures/columns.yml

@@ -3,18 +3,22 @@ writebook_triage:
   name: Triage
   color: "#000000"
   collection: writebook
+  position: 0
 
 writebook_in_progress:
   name: In progress
   color: "#3b3633"
   collection: writebook
+  position: 1
 
 writebook_on_hold:
   name: On Hold
   color: "#67695e"
   collection: writebook
+  position: 2
 
 writebook_review:
   name: Review
   color: "#eb7a32"
-  collection: writebook
+  collection: writebook
+  position: 3

test/models/column/positioned_test.rb

@@ -0,0 +1,62 @@
+require "test_helper"
+
+class Column::PositionedTest < ActiveSupport::TestCase
+  test "auto position new columns" do
+    collection = collections(:writebook)
+    max_position = collection.columns.maximum(:position)
+
+    new_column = collection.columns.create!(name: "New Column", color: "#000000")
+
+    assert_equal max_position + 1, new_column.position
+  end
+
+  test "move column to the left" do
+    collection = collections(:writebook)
+    columns = collection.columns.sorted.to_a
+
+    column_a = columns[0]
+    column_b = columns[1]
+    original_position_a = column_a.position
+    original_position_b = column_b.position
+
+    column_b.move_left
+
+    assert_equal original_position_b, column_a.reload.position
+    assert_equal original_position_a, column_b.reload.position
+  end
+
+  test "move left when already at leftmost position" do
+    collection = collections(:writebook)
+    leftmost_column = collection.columns.sorted.first
+    original_position = leftmost_column.position
+
+    leftmost_column.move_left
+
+    assert_equal original_position, leftmost_column.reload.position
+  end
+
+  test "move column to the right" do
+    collection = collections(:writebook)
+    columns = collection.columns.sorted.to_a
+
+    column_a = columns[0]
+    column_b = columns[1]
+    original_position_a = column_a.position
+    original_position_b = column_b.position
+
+    column_a.move_right
+
+    assert_equal original_position_b, column_a.reload.position
+    assert_equal original_position_a, column_b.reload.position
+  end
+
+  test "move right when already at rightmost position" do
+    collection = collections(:writebook)
+    rightmost_column = collection.columns.sorted.last
+    original_position = rightmost_column.position
+
+    rightmost_column.move_right
+
+    assert_equal original_position, rightmost_column.reload.position
+  end
+end