From dd850813cb3ddd3c50c003d5b84941bfc51948ef Mon Sep 17 00:00:00 2001
From: Trevor Vallender <trevor@tsvallender.co.uk>
Date: Fri, 21 Jun 2024 11:22:25 +0100
Subject: [PATCH] Prevent moving features beyond section bounds

---
 app/models/character_sheet_feature.rb       |  6 +-----
 app/models/character_sheet_section.rb       |  8 ++++++++
 test/fixtures/character_sheet_features.yml  | 12 ++++++++++++
 test/fixtures/character_sheet_sections.yml  |  4 ++++
 test/fixtures/stats.yml                     | 10 ++++++++++
 test/models/character_sheet_feature_test.rb |  3 ---
 test/models/character_sheet_section_test.rb | 10 ++++++++++
 todo.md                                     |  5 -----
 8 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/app/models/character_sheet_feature.rb b/app/models/character_sheet_feature.rb
index 6c89e41..eaacd6e 100644
--- a/app/models/character_sheet_feature.rb
+++ b/app/models/character_sheet_feature.rb
@@ -17,11 +17,7 @@ class CharacterSheetFeature < ApplicationRecord
 
   def move(to:)
     return if to == order_index
-
-    unless character_sheet_section.character_sheet_features.exists?(order_index: to)
-      update!(order_index: to)
-      return
-    end
+    return if to < character_sheet_section.lowest_order_index || to > character_sheet_section.highest_order_index
 
     if to < order_index
       character_sheet_section.character_sheet_features.where(order_index: to...order_index)
diff --git a/app/models/character_sheet_section.rb b/app/models/character_sheet_section.rb
index 930c025..2a6798a 100644
--- a/app/models/character_sheet_section.rb
+++ b/app/models/character_sheet_section.rb
@@ -17,4 +17,12 @@ class CharacterSheetSection < ApplicationRecord
                    length: { maximum: 255 }
 
   scope :top_level, -> { where.missing(:parent_section) }
+
+  def lowest_order_index
+    character_sheet_features.minimum(:order_index) || 1
+  end
+
+  def highest_order_index
+    character_sheet_features.maximum(:order_index) || 1
+  end
 end
diff --git a/test/fixtures/character_sheet_features.yml b/test/fixtures/character_sheet_features.yml
index b55c6d0..b4b1d6c 100644
--- a/test/fixtures/character_sheet_features.yml
+++ b/test/fixtures/character_sheet_features.yml
@@ -7,6 +7,18 @@ stats-strength:
   featurable: strength (Stat)
   order_index: 1
 
+stats-dexterity:
+  <<: *DEFAULTS
+  character_sheet_section: stats
+  featurable: dexterity (Stat)
+  order_index: 2
+
+stats-constitution:
+  <<: *DEFAULTS
+  character_sheet_section: stats
+  featurable: constitution (Stat)
+  order_index: 3
+
 text_field:
   <<: *DEFAULTS
   character_sheet_section: info
diff --git a/test/fixtures/character_sheet_sections.yml b/test/fixtures/character_sheet_sections.yml
index 435831a..a782305 100644
--- a/test/fixtures/character_sheet_sections.yml
+++ b/test/fixtures/character_sheet_sections.yml
@@ -10,3 +10,7 @@ subsection:
 info:
   name: Info
   character: nardren
+
+empty:
+  name: Empty
+  character: nardren
diff --git a/test/fixtures/stats.yml b/test/fixtures/stats.yml
index 6761014..1e1165e 100644
--- a/test/fixtures/stats.yml
+++ b/test/fixtures/stats.yml
@@ -2,3 +2,13 @@ strength:
   name: Strength
   value: 10
   roll_command: d20+self
+
+dexterity:
+  name: Dexterity
+  value: 10
+  roll_command: d20+self
+
+constitution:
+  name: Constitution
+  value: 10
+  roll_command: d20+self
diff --git a/test/models/character_sheet_feature_test.rb b/test/models/character_sheet_feature_test.rb
index 48bdfc5..0a5fe6e 100644
--- a/test/models/character_sheet_feature_test.rb
+++ b/test/models/character_sheet_feature_test.rb
@@ -3,7 +3,4 @@
 require "test_helper"
 
 class CharacterSheetFeatureTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
 end
diff --git a/test/models/character_sheet_section_test.rb b/test/models/character_sheet_section_test.rb
index 98a83e8..bb8c2aa 100644
--- a/test/models/character_sheet_section_test.rb
+++ b/test/models/character_sheet_section_test.rb
@@ -6,4 +6,14 @@ class CharacterSheetSectionTest < ActiveSupport::TestCase
   test "name must exist" do
     assert_must_exist(character_sheet_sections(:stats), "name")
   end
+
+  test "returns correct order index" do
+    assert_equal 3, character_sheet_sections(:stats).highest_order_index
+    assert_equal 1, character_sheet_sections(:stats).lowest_order_index
+  end
+
+  test "an empty section returns 1 for lowest/highest order index" do
+    assert_equal 1, character_sheet_sections(:empty).lowest_order_index
+    assert_equal 1, character_sheet_sections(:empty).highest_order_index
+  end
 end
diff --git a/todo.md b/todo.md
index e5e5e36..fe0af6a 100644
--- a/todo.md
+++ b/todo.md
@@ -1,11 +1,6 @@
 - request invite
-- don't move down/up top/bottom features
 - auto save text fields
   - indicate save status on titlebar
-- easily edit text fields without entering edit mode
-- add roll command to counters (e.g. luck)
-- test for rolls controller & system tests
-- Show individual dice on dice roll
 - Sheets
   - Lists
   - Weapons/spells