From 598a7504272718f82c85b75f7c6bb51d621e21c7 Mon Sep 17 00:00:00 2001 From: Travis Dockter Date: Sat, 31 Jan 2026 15:06:32 -0700 Subject: [PATCH 1/4] Orderable projects --- app/controllers/projects_controller.rb | 16 ++++ .../project_sortable_controller.js | 48 ++++++++++ app/models/project.rb | 21 ++++- app/services/project_reordering_service.rb | 91 +++++++++++++++++++ app/views/projects/_section.html.erb | 16 +++- config/routes.rb | 3 + 6 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 app/javascript/controllers/project_sortable_controller.js create mode 100644 app/services/project_reordering_service.rb diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 55acdd0..83e9fdf 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -69,6 +69,22 @@ def toggle_complete end end + def reorder + ids = Array(params[:order]).map(&:to_i) + section = params[:section] + + return head :unprocessable_entity if ids.empty? || section.blank? + + ProjectReorderingService.new(current_user).reorder!( + ordered_ids: ids, + section: section + ) + + head :ok + rescue ProjectReorderingService::Error => e + head :unprocessable_entity + end + private def set_project diff --git a/app/javascript/controllers/project_sortable_controller.js b/app/javascript/controllers/project_sortable_controller.js new file mode 100644 index 0000000..a460b40 --- /dev/null +++ b/app/javascript/controllers/project_sortable_controller.js @@ -0,0 +1,48 @@ +import Sortable from "stimulus-sortable" +import { patch } from "@rails/request.js" + +export default class extends Sortable { + static values = { + url: String, + section: String + } + + get options() { + const options = super.options + + return { + ...options, + fallbackOnBody: true, + delay: 200, + delayOnTouchOnly: true, + touchStartThreshold: 8, + group: { + name: 'projects', + pull: false, + put: false + } + } + } + + async onUpdate(event) { + await super.onUpdate(event) + + if (!this.hasUrlValue) return + + const ids = this.sortable.toArray() + if (ids.length === 0) return + + const body = new FormData() + ids.forEach((id) => body.append("order[]", id)) + body.append("section", this.sectionValue) + + try { + await patch(this.urlValue, { + body, + responseKind: "json" + }) + } catch (error) { + console.error("Failed to reorder projects", error) + } + } +} diff --git a/app/models/project.rb b/app/models/project.rb index b4d84d7..bff83a9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -38,11 +38,19 @@ class Project < ApplicationRecord scope :unarchived, -> { where(archived_at: nil) } scope :active, -> { unarchived.where(completed_at: nil) } scope :recently_completed, -> { unarchived.where.not(completed_at: nil).where("completed_at > ?", 30.days.ago) } - scope :ordered, -> { order(Arel.sql(SECTION_ORDER_SQL)).order(created_at: :desc) } + scope :ordered, -> { order(Arel.sql(SECTION_ORDER_SQL)).order(position: :asc) } scope :created_in_range, ->(range) { where(created_at: range) } scope :completed_in_range, ->(range) { where(completed_at: range) } + before_create :assign_position + + def self.next_position_for_user_and_section(user, section) + where(user_id: user.id, section: section, completed_at: nil, archived_at: nil) + .lock("FOR UPDATE") + .maximum(:position).to_i + 1 + end + def completed? completed_at.present? end @@ -54,4 +62,15 @@ def complete! def uncomplete! update!(completed_at: nil) end + + private + + def assign_position + return unless user + return if position.present? && position.positive? + + user.with_lock do + self.position = self.class.next_position_for_user_and_section(user, section) + end + end end diff --git a/app/services/project_reordering_service.rb b/app/services/project_reordering_service.rb new file mode 100644 index 0000000..0e0f578 --- /dev/null +++ b/app/services/project_reordering_service.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +class ProjectReorderingService + class Error < StandardError; end + class InvalidSectionError < Error; end + class InvalidIdsError < Error; end + class PartialReorderError < Error; end + + def initialize(user) + @user = user + end + + def reorder!(ordered_ids:, section:) + validate_section!(section) + validate_ordered_ids!(ordered_ids) + + section_sym = section.to_sym + + Project.transaction do + section_projects = @user.projects.active + .where(section: section_sym) + .lock("FOR UPDATE") + + section_ids = section_projects.pluck(:id) + + validate_ids_match_section!(section_ids, ordered_ids) + + reorder_projects_in_section(section_projects, ordered_ids) + end + end + + private + + def validate_section!(section) + section_sym = section.to_sym + return if Project::SECTIONS.include?(section_sym) + + raise InvalidSectionError, + "Invalid section: #{section}. Must be one of: #{Project::SECTIONS.join(', ')}" + end + + def validate_ordered_ids!(ids) + if ids.blank? + raise InvalidIdsError, "ordered_ids cannot be empty" + end + + unless ids.all? { |id| id.is_a?(Integer) && id.positive? } + raise InvalidIdsError, + "ordered_ids must contain only positive integers. Got: #{ids.inspect}" + end + + if ids.size != ids.uniq.size + raise InvalidIdsError, "ordered_ids contains duplicates" + end + end + + def validate_ids_match_section!(section_ids, ordered_ids) + section_set = Set.new(section_ids) + ordered_set = Set.new(ordered_ids) + + return if section_set == ordered_set + + missing = (section_set - ordered_set).to_a + extra = (ordered_set - section_set).to_a + + parts = [] + parts << "Missing IDs: #{missing.join(', ')}" if missing.any? + parts << "Extra IDs: #{extra.join(', ')}" if extra.any? + + raise PartialReorderError, + "ordered_ids must include all projects in section. #{parts.join('. ')}" + end + + def reorder_projects_in_section(section_projects, ordered_ids) + max_position = section_projects.maximum(:position).to_i + bump = max_position + ordered_ids.size + 1 + + section_projects.update_all([ "position = position + ?", bump ]) + + when_clauses = ordered_ids.map { "WHEN ? THEN ?" }.join(" ") + params = ordered_ids.each_with_index.flat_map { |id, idx| [ id, idx + 1 ] } + + update_sql = ActiveRecord::Base.sanitize_sql_array([ + "position = CASE id #{when_clauses} END, updated_at = ?", + *params, + Time.current + ]) + + section_projects.where(id: ordered_ids).update_all(update_sql) + end +end diff --git a/app/views/projects/_section.html.erb b/app/views/projects/_section.html.erb index 737ba62..857cc4b 100644 --- a/app/views/projects/_section.html.erb +++ b/app/views/projects/_section.html.erb @@ -10,14 +10,26 @@ + - + <% projects.each do |project| %> - + +
Name Overview
+
+ + + +
+ 		<%= link_to project.name, project_path(project), class: "font-medium text-slate-900 hover:text-blue-600 transition" %> diff --git a/config/routes.rb b/config/routes.rb index c70a45d..9b36241 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,6 +22,9 @@ end end resources :projects, only: %i[index new create edit update show] do + collection do + patch :reorder + end member do patch :toggle_complete end From f5b737cebbf43cae0254ac6bff43c1d8a4f4b9bf Mon Sep 17 00:00:00 2001 From: Travis Dockter Date: Sat, 31 Jan 2026 15:19:12 -0700 Subject: [PATCH 2/4] Add backfill for existing project position --- ...260131221532_backfill_project_positions.rb | 28 +++++++++++++++++++ db/schema.rb | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20260131221532_backfill_project_positions.rb diff --git a/db/migrate/20260131221532_backfill_project_positions.rb b/db/migrate/20260131221532_backfill_project_positions.rb new file mode 100644 index 0000000..6db0507 --- /dev/null +++ b/db/migrate/20260131221532_backfill_project_positions.rb @@ -0,0 +1,28 @@ +class BackfillProjectPositions < ActiveRecord::Migration[8.0] + def up + # Assign positions to existing projects based on their created_at order + # within each user/section combination. + # + # The previous ordering was created_at DESC (newest first), so we assign + # position 1 to the newest project, position 2 to the second newest, etc. + # This preserves the existing display order when switching to position ASC. + execute <<-SQL + UPDATE projects + SET position = ( + SELECT COUNT(*) + 1 + FROM projects AS p2 + WHERE p2.user_id = projects.user_id + AND p2.section = projects.section + AND p2.archived_at IS NULL + AND p2.completed_at IS NULL + AND p2.created_at > projects.created_at + ) + WHERE archived_at IS NULL AND completed_at IS NULL + SQL + end + + def down + # Reset all positions to 0 + execute "UPDATE projects SET position = 0" + end +end diff --git a/db/schema.rb b/db/schema.rb index 8a4f5b5..243f3be 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.1].define(version: 2026_01_30_230011) do +ActiveRecord::Schema[8.1].define(version: 2026_01_31_221532) do create_table "active_storage_attachments", force: :cascade do |t| t.bigint "blob_id", null: false t.datetime "created_at", null: false From 89b7111416ed3a7c8800efefb0af55be44957a26 Mon Sep 17 00:00:00 2001 From: Travis Dockter Date: Sat, 31 Jan 2026 15:45:21 -0700 Subject: [PATCH 3/4] Add tests --- .../project_reordering_service_test.rb | 289 ++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 test/services/project_reordering_service_test.rb diff --git a/test/services/project_reordering_service_test.rb b/test/services/project_reordering_service_test.rb new file mode 100644 index 0000000..56099eb --- /dev/null +++ b/test/services/project_reordering_service_test.rb @@ -0,0 +1,289 @@ +require "test_helper" + +class ProjectReorderingServiceTest < ActiveSupport::TestCase + def setup + @user = users(:one) + # Clean up fixture projects to avoid position conflicts + @user.projects.destroy_all + @service = ProjectReorderingService.new(@user) + end + + test "reorders projects within a section" do + project1 = @user.projects.create!(name: "First", section: :active, position: 1) + project2 = @user.projects.create!(name: "Second", section: :active, position: 2) + project3 = @user.projects.create!(name: "Third", section: :active, position: 3) + + # Reverse the order + @service.reorder!( + ordered_ids: [ project3.id, project2.id, project1.id ], + section: :active + ) + + assert_equal 1, project3.reload.position + assert_equal 2, project2.reload.position + assert_equal 3, project1.reload.position + end + + test "accepts section as string or symbol" do + project1 = @user.projects.create!(name: "First", section: :active, position: 1) + project2 = @user.projects.create!(name: "Second", section: :active, position: 2) + + # Test with string + @service.reorder!( + ordered_ids: [ project2.id, project1.id ], + section: "active" + ) + + assert_equal 1, project2.reload.position + assert_equal 2, project1.reload.position + + # Test with symbol + @service.reorder!( + ordered_ids: [ project1.id, project2.id ], + section: :active + ) + + assert_equal 1, project1.reload.position + assert_equal 2, project2.reload.position + end + + test "raises error if section is invalid" do + project = @user.projects.create!(name: "Project", section: :active) + + error = assert_raises(ProjectReorderingService::InvalidSectionError) do + @service.reorder!( + ordered_ids: [ project.id ], + section: :invalid_section + ) + end + + assert_match(/Invalid section/, error.message) + assert_match(/invalid_section/, error.message) + end + + test "raises error if ordered_ids is empty" do + error = assert_raises(ProjectReorderingService::InvalidIdsError) do + @service.reorder!( + ordered_ids: [], + section: :active + ) + end + + assert_match(/cannot be empty/, error.message) + end + + test "raises error if ordered_ids contains non-integers" do + error = assert_raises(ProjectReorderingService::InvalidIdsError) do + @service.reorder!( + ordered_ids: [ 1, "2", 3 ], + section: :active + ) + end + + assert_match(/must contain only positive integers/, error.message) + end + + test "raises error if ordered_ids contains zero or negative numbers" do + error = assert_raises(ProjectReorderingService::InvalidIdsError) do + @service.reorder!( + ordered_ids: [ 1, 0, -1 ], + section: :active + ) + end + + assert_match(/must contain only positive integers/, error.message) + end + + test "raises error if ordered_ids contains duplicates" do + error = assert_raises(ProjectReorderingService::InvalidIdsError) do + @service.reorder!( + ordered_ids: [ 1, 2, 2, 3 ], + section: :active + ) + end + + assert_match(/contains duplicates/, error.message) + end + + test "raises error if ordered_ids doesn't include all section projects" do + project1 = @user.projects.create!(name: "First", section: :active, position: 1) + project2 = @user.projects.create!(name: "Second", section: :active, position: 2) + project3 = @user.projects.create!(name: "Third", section: :active, position: 3) + + # Only include 2 of 3 projects + error = assert_raises(ProjectReorderingService::PartialReorderError) do + @service.reorder!( + ordered_ids: [ project1.id, project2.id ], + section: :active + ) + end + + assert_match(/must include all projects in section/, error.message) + assert_match(/Missing IDs: #{project3.id}/, error.message) + end + + test "raises error if ordered_ids includes projects from different section" do + active_project = @user.projects.create!(name: "Active", section: :active, position: 1) + this_month_project = @user.projects.create!(name: "This Month", section: :this_month, position: 1) + + error = assert_raises(ProjectReorderingService::PartialReorderError) do + @service.reorder!( + ordered_ids: [ active_project.id, this_month_project.id ], + section: :active + ) + end + + assert_match(/must include all projects in section/, error.message) + assert_match(/Extra IDs: #{this_month_project.id}/, error.message) + end + + test "raises error if project doesn't belong to user" do + other_user = users(:two) + other_user.projects.destroy_all + other_project = other_user.projects.create!(name: "Other", section: :active, position: 1) + + # The service validates that ordered_ids match the section's projects, + # so it will raise PartialReorderError because the other user's project + # won't be in the current user's section + error = assert_raises(ProjectReorderingService::PartialReorderError) do + @service.reorder!( + ordered_ids: [ other_project.id ], + section: :active + ) + end + + assert_match(/Extra IDs: #{other_project.id}/, error.message) + end + + test "doesn't affect projects in other sections" do + active1 = @user.projects.create!(name: "Active 1", section: :active, position: 1) + active2 = @user.projects.create!(name: "Active 2", section: :active, position: 2) + this_month1 = @user.projects.create!(name: "This Month 1", section: :this_month, position: 1) + + @service.reorder!( + ordered_ids: [ active2.id, active1.id ], + section: :active + ) + + assert_equal 1, active2.reload.position + assert_equal 2, active1.reload.position + assert_equal 1, this_month1.reload.position # Unchanged + end + + test "doesn't affect other users' projects" do + other_user = users(:two) + other_user.projects.destroy_all + @user.projects.create!(name: "User 1 Project", section: :active, position: 1) + other_project = other_user.projects.create!(name: "User 2 Project", section: :active, position: 1) + + @service.reorder!( + ordered_ids: [ @user.projects.first.id ], + section: :active + ) + + assert_equal 1, other_project.reload.position # Unchanged + end + + test "doesn't affect completed projects" do + active1 = @user.projects.create!(name: "Active 1", section: :active, position: 1) + active2 = @user.projects.create!(name: "Active 2", section: :active, position: 2) + completed = @user.projects.create!(name: "Completed", section: :active, position: 3, completed_at: 1.day.ago) + + @service.reorder!( + ordered_ids: [ active2.id, active1.id ], + section: :active + ) + + assert_equal 1, active2.reload.position + assert_equal 2, active1.reload.position + assert_equal 3, completed.reload.position # Unchanged + end + + test "doesn't affect archived projects" do + active1 = @user.projects.create!(name: "Active 1", section: :active, position: 1) + active2 = @user.projects.create!(name: "Active 2", section: :active, position: 2) + archived = @user.projects.create!(name: "Archived", section: :active, position: 3, archived_at: 1.day.ago) + + @service.reorder!( + ordered_ids: [ active2.id, active1.id ], + section: :active + ) + + assert_equal 1, active2.reload.position + assert_equal 2, active1.reload.position + assert_equal 3, archived.reload.position # Unchanged + end + + test "handles reordering large lists efficiently" do + # Create 50 projects + projects = 50.times.map do |i| + @user.projects.create!(name: "Project #{i}", section: :active, position: i + 1) + end + + # Reverse the entire list + reversed_ids = projects.map(&:id).reverse + + @service.reorder!( + ordered_ids: reversed_ids, + section: :active + ) + + # Verify all positions are correct + projects.each_with_index do |project, index| + expected_position = 50 - index + assert_equal expected_position, project.reload.position + end + end + + test "updates updated_at timestamp" do + project1 = @user.projects.create!(name: "First", section: :active, position: 1) + project2 = @user.projects.create!(name: "Second", section: :active, position: 2) + + original_updated_at = project1.updated_at + + # Wait a tiny bit to ensure timestamp changes + sleep 0.01 + + @service.reorder!( + ordered_ids: [ project2.id, project1.id ], + section: :active + ) + + assert project1.reload.updated_at > original_updated_at + end + + test "is atomic - rolls back on error" do + project1 = @user.projects.create!(name: "First", section: :active, position: 1) + project2 = @user.projects.create!(name: "Second", section: :active, position: 2) + + # Try to reorder with invalid ID (should fail validation and not change positions) + assert_raises(ProjectReorderingService::PartialReorderError) do + @service.reorder!( + ordered_ids: [ project1.id, 99999 ], + section: :active + ) + end + + # Verify positions unchanged + assert_equal 1, project1.reload.position + assert_equal 2, project2.reload.position + end + + test "works with all project sections" do + Project::SECTIONS.each do |section| + project1 = @user.projects.create!(name: "First", section: section, position: 1) + project2 = @user.projects.create!(name: "Second", section: section, position: 2) + + @service.reorder!( + ordered_ids: [ project2.id, project1.id ], + section: section + ) + + assert_equal 1, project2.reload.position + assert_equal 2, project1.reload.position + + # Clean up for next iteration + @user.projects.destroy_all + end + end +end From 9d7acb64c9273b0729214f53bbbcf6d966768594 Mon Sep 17 00:00:00 2001 From: Travis Dockter Date: Sat, 31 Jan 2026 15:53:41 -0700 Subject: [PATCH 4/4] Add more tests --- test/controllers/projects_controller_test.rb | 169 +++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/test/controllers/projects_controller_test.rb b/test/controllers/projects_controller_test.rb index c5b0c55..1900241 100644 --- a/test/controllers/projects_controller_test.rb +++ b/test/controllers/projects_controller_test.rb @@ -69,4 +69,173 @@ class ProjectsControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_nil @project.reload.completed_at end + + # Reorder tests + + test "reorder returns 200 on valid reorder" do + user = users(:one) + user.projects.destroy_all + + project1 = user.projects.create!(name: "First", section: :active, position: 1) + project2 = user.projects.create!(name: "Second", section: :active, position: 2) + + patch reorder_projects_url, params: { + order: [ project2.id, project1.id ], + section: "active" + } + + assert_response :ok + end + + test "reorder updates positions correctly" do + user = users(:one) + user.projects.destroy_all + + project1 = user.projects.create!(name: "First", section: :active, position: 1) + project2 = user.projects.create!(name: "Second", section: :active, position: 2) + project3 = user.projects.create!(name: "Third", section: :active, position: 3) + + patch reorder_projects_url, params: { + order: [ project3.id, project1.id, project2.id ], + section: "active" + } + + assert_response :ok + assert_equal 1, project3.reload.position + assert_equal 2, project1.reload.position + assert_equal 3, project2.reload.position + end + + test "reorder returns 422 when order is empty" do + patch reorder_projects_url, params: { + order: [], + section: "active" + } + + assert_response :unprocessable_entity + end + + test "reorder returns 422 when order is missing" do + patch reorder_projects_url, params: { + section: "active" + } + + assert_response :unprocessable_entity + end + + test "reorder returns 422 when section is blank" do + user = users(:one) + user.projects.destroy_all + project = user.projects.create!(name: "Project", section: :active, position: 1) + + patch reorder_projects_url, params: { + order: [ project.id ], + section: "" + } + + assert_response :unprocessable_entity + end + + test "reorder returns 422 when section is missing" do + user = users(:one) + user.projects.destroy_all + project = user.projects.create!(name: "Project", section: :active, position: 1) + + patch reorder_projects_url, params: { + order: [ project.id ] + } + + assert_response :unprocessable_entity + end + + test "reorder returns 422 on invalid section" do + user = users(:one) + user.projects.destroy_all + project = user.projects.create!(name: "Project", section: :active, position: 1) + + patch reorder_projects_url, params: { + order: [ project.id ], + section: "invalid_section" + } + + assert_response :unprocessable_entity + end + + test "reorder returns 422 when ids don't match section" do + user = users(:one) + user.projects.destroy_all + + active_project = user.projects.create!(name: "Active", section: :active, position: 1) + other_project = user.projects.create!(name: "Other", section: :this_month, position: 1) + + patch reorder_projects_url, params: { + order: [ active_project.id, other_project.id ], + section: "active" + } + + assert_response :unprocessable_entity + end + + test "reorder requires authentication" do + sign_out + + patch reorder_projects_url, params: { + order: [ 1, 2 ], + section: "active" + } + + assert_redirected_to new_session_url + end + + test "reorder does not affect other users projects" do + user = users(:one) + other_user = users(:two) + user.projects.destroy_all + other_user.projects.destroy_all + + project1 = user.projects.create!(name: "User1 First", section: :active, position: 1) + project2 = user.projects.create!(name: "User1 Second", section: :active, position: 2) + other_project = other_user.projects.create!(name: "User2 Project", section: :active, position: 1) + + patch reorder_projects_url, params: { + order: [ project2.id, project1.id ], + section: "active" + } + + assert_response :ok + assert_equal 1, other_project.reload.position # Unchanged + end + + test "reorder does not affect other sections" do + user = users(:one) + user.projects.destroy_all + + active1 = user.projects.create!(name: "Active 1", section: :active, position: 1) + active2 = user.projects.create!(name: "Active 2", section: :active, position: 2) + this_month = user.projects.create!(name: "This Month", section: :this_month, position: 1) + + patch reorder_projects_url, params: { + order: [ active2.id, active1.id ], + section: "active" + } + + assert_response :ok + assert_equal 1, active2.reload.position + assert_equal 2, active1.reload.position + assert_equal 1, this_month.reload.position # Unchanged + end + + test "reorder cannot reorder other users projects" do + other_user = users(:two) + other_user.projects.destroy_all + other_project = other_user.projects.create!(name: "Their Project", section: :active, position: 1) + + patch reorder_projects_url, params: { + order: [ other_project.id ], + section: "active" + } + + assert_response :unprocessable_entity + assert_equal 1, other_project.reload.position # Unchanged + end end