Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions app/controllers/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +72 to +83
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new ProjectsController#reorder behavior isn’t covered by tests. Add an integration test (similar to existing toggle_complete tests) to assert: 200 on valid reorder, 422 on missing/invalid params, and that positions actually change only for the current user/section.

Copilot uses AI. Check for mistakes.
rescue ProjectReorderingService::Error => e
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rescue ProjectReorderingService::Error => e assigns e but never uses it, which will be flagged by RuboCop (and also drops useful debugging context). Either remove the exception variable (rescue ProjectReorderingService::Error) or log e.message similarly to TodosController#reorder.

Suggested change
rescue ProjectReorderingService::Error => e
rescue ProjectReorderingService::Error => e
Rails.logger.error("Project reordering failed: #{e.message}")

Copilot uses AI. Check for mistakes.
head :unprocessable_entity
end

private

def set_project
Expand Down
48 changes: 48 additions & 0 deletions app/javascript/controllers/project_sortable_controller.js
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
21 changes: 20 additions & 1 deletion app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
91 changes: 91 additions & 0 deletions app/services/project_reordering_service.rb
Original file line number Diff line number Diff line change
@@ -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
Comment on lines +13 to +29
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProjectReorderingService introduces non-trivial validation and update logic but has no test coverage. Add a test/services/project_reordering_service_test.rb modeled after test/services/todo_reordering_service_test.rb to cover successful reorder, invalid section, invalid/duplicate IDs, missing/extra IDs, and ensuring other sections/users aren’t affected.

Copilot uses AI. Check for mistakes.
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
16 changes: 14 additions & 2 deletions app/views/projects/_section.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,26 @@
<table class="w-full">
<thead>
<tr class="border-b border-slate-200 text-left text-sm font-medium text-slate-500">
<th class="pb-2 w-8"></th>
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new drag-handle column header is an empty <th>, which is not accessible for screen readers. Add a visually-hidden label (e.g., “Reorder”) or appropriate ARIA so the column has a meaningful header.

Suggested change
<th class="pb-2 w-8"></th>
<th class="pb-2 w-8" scope="col" aria-label="Reorder"></th>

Copilot uses AI. Check for mistakes.
<th class="pb-2 pr-4">Name</th>
<th class="hidden sm:table-cell pb-2 pr-4">Description</th>
<th class="pb-2 text-right">Overview</th>
</tr>
</thead>
<tbody class="divide-y divide-slate-100">
<tbody class="divide-y divide-slate-100"
data-controller="project-sortable"
data-project-sortable-url-value="<%= reorder_projects_path %>"
data-project-sortable-section-value="<%= section %>"
data-project-sortable-handle-value=".project-drag-handle">
<% projects.each do |project| %>
<tr class="group">
<tr class="group" data-id="<%= project.id %>">
<td class="py-3 w-8">
<div class="project-drag-handle cursor-grab active:cursor-grabbing text-slate-300 hover:text-slate-500 transition">
<svg class="h-5 w-5" viewBox="0 0 20 20" fill="currentColor">
<path d="M7 2a2 2 0 1 0 .001 4.001A2 2 0 0 0 7 2zm0 6a2 2 0 1 0 .001 4.001A2 2 0 0 0 7 8zm0 6a2 2 0 1 0 .001 4.001A2 2 0 0 0 7 14zm6-8a2 2 0 1 0-.001-4.001A2 2 0 0 0 13 6zm0 2a2 2 0 1 0 .001 4.001A2 2 0 0 0 13 8zm0 6a2 2 0 1 0 .001 4.001A2 2 0 0 0 13 14z" />
</svg>
</div>
Comment on lines +27 to +31
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The drag handle is a plain <div>/<svg> with no accessible name; unlike the todo drag handle elsewhere, this one is missing an aria-label and the SVG lacks aria-hidden. Add an accessible label (or use a <button type="button">) and mark the decorative SVG as aria-hidden to avoid noisy announcements.

Copilot uses AI. Check for mistakes.
</td>
<td class="py-3 pr-4">
<%= link_to project.name, project_path(project),
class: "font-medium text-slate-900 hover:text-blue-600 transition" %>
Expand Down
3 changes: 3 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions db/migrate/20260131221532_backfill_project_positions.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading