From 2cda3fc9d6db8be270f1ded64546a5571da23209 Mon Sep 17 00:00:00 2001
From: Utku Egemen Umut <121046082+umututku03@users.noreply.github.com>
Date: Tue, 11 Jun 2024 13:42:06 -0400
Subject: [PATCH] Add validations to TextAnnotation model position attributes
 (#7112)

---
 Changelog.md                        |  1 +
 app/models/text_annotation.rb       |  8 ++++----
 doc/markus-contributors.txt         |  1 +
 spec/models/text_annotation_spec.rb | 22 +++++++++++++++++-----
 4 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/Changelog.md b/Changelog.md
index f90a3c61bb..37ef20f48b 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -19,6 +19,7 @@
 - Display error message for instructor-run tests when no test groups are runnable by instructors (#7038)
 - Ensure user params are passed as keyword arguments to database queries (#7040)
 - Added a progress bar for when a student uploads a file for submission (#7078)
+- Added validations to the `TextAnnotation` model to ensure `line_start` and `line_end` are >= 1, and `column_start` and `column_end` are >= 0. (#7081)
 
 ### 🐛 Bug fixes
 
diff --git a/app/models/text_annotation.rb b/app/models/text_annotation.rb
index 0ee4dc54ca..d0f2f0bb78 100644
--- a/app/models/text_annotation.rb
+++ b/app/models/text_annotation.rb
@@ -1,8 +1,8 @@
 class TextAnnotation < Annotation
-  validates :line_start, presence: true
-  validates :line_end, presence: true
-  validates :column_start, presence: true
-  validates :column_end, presence: true
+  validates :line_start, presence: true, numericality: { greater_than_or_equal_to: 1 }
+  validates :line_end, presence: true, numericality: { greater_than_or_equal_to: 1 }
+  validates :column_start, presence: true, numericality: { greater_than_or_equal_to: 0 }
+  validates :column_end, presence: true, numericality: { greater_than_or_equal_to: 0 }
 
   def get_data(include_creator: false)
     data = super
diff --git a/doc/markus-contributors.txt b/doc/markus-contributors.txt
index 1f57093bec..8bc074beae 100644
--- a/doc/markus-contributors.txt
+++ b/doc/markus-contributors.txt
@@ -194,6 +194,7 @@ Tianji Zhang
 Thomas Hayes
 Tudor Brindus
 Tyler Han
+Utku Egemen Umut
 Valentin Roger
 Veronica Wong
 Victoria Mui
diff --git a/spec/models/text_annotation_spec.rb b/spec/models/text_annotation_spec.rb
index 2af0ff5ac3..1990c5eafe 100644
--- a/spec/models/text_annotation_spec.rb
+++ b/spec/models/text_annotation_spec.rb
@@ -4,8 +4,25 @@
 
     it { is_expected.to validate_presence_of(:line_start) }
     it { is_expected.to validate_presence_of(:line_end) }
+    it { is_expected.to validate_presence_of(:column_start) }
+    it { is_expected.to validate_presence_of(:column_end) }
+
+    it { is_expected.to allow_value(1).for(:line_start) }
+    it { is_expected.to allow_value(1).for(:line_end) }
+    it { is_expected.to allow_value(0).for(:column_start) }
+    it { is_expected.to allow_value(0).for(:column_end) }
+
     it { is_expected.to allow_value(10).for(:line_start) }
     it { is_expected.to allow_value(10).for(:line_end) }
+    it { is_expected.to allow_value(5).for(:column_start) }
+    it { is_expected.to allow_value(5).for(:column_end) }
+
+    it { is_expected.not_to allow_value(0).for(:line_start) }
+    it { is_expected.not_to allow_value(0).for(:line_end) }
+    it { is_expected.not_to allow_value(-1).for(:line_start) }
+    it { is_expected.not_to allow_value(-1).for(:line_end) }
+    it { is_expected.not_to allow_value(-1).for(:column_start) }
+    it { is_expected.not_to allow_value(-1).for(:column_end) }
 
     include_examples 'course associations'
 
@@ -15,10 +32,5 @@
 
       it_behaves_like 'gets annotation data'
     end
-
-    # TODO: Change Model.
-    # We should not allow negative values vor lines
-    # should_not allow_value(-1).for(:line_start)
-    # should_not allow_value(-1).for(:line_end)
   end
 end