Add validations to TextAnnotation model position attributes (#7112)

MarkUsProject · Jun 11, 2024 · 2cda3fc · 2cda3fc
1 parent 9a4e9a4
commit 2cda3fc
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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