chore: Files feedback_message without NULL state

Column type string should not have a null state to avoid logical
distinction between NULL and '' case.

User defined tests are created without feedback_message but
render a possible feedback_message. Removing the NULL case allows
new records render the empty feedback_message.

An edgecase remains when a test for assessment with a feedback_message
is transformed to a user defined test. The feedback message will
be displayed.

Resolves #3014

Author: arkirchner

app/models/code_ocean/file.rb

@@ -16,6 +16,7 @@ class File < ApplicationRecord
 
     after_initialize :set_default_values
     before_validation :clear_weight, unless: :teacher_defined_assessment?
+    before_validation :clear_feedback_message, unless: :teacher_defined_assessment?
     before_validation :hash_content, if: :content_present?
     before_validation :set_ancestor_values, if: :incomplete_descendent?
 
@@ -49,7 +50,6 @@ class File < ApplicationRecord
     default_scope { order(path: :asc, name: :asc) }
 
     validates :feedback_message, if: :teacher_defined_assessment?, presence: true
-    validates :feedback_message, absence: true, unless: :teacher_defined_assessment?
     validates :hashed_content, if: :content_present?, presence: true
     validates :hidden, inclusion: [true, false]
     validates :hidden_feedback, inclusion: [true, false]
@@ -153,6 +153,11 @@ def set_default_values
     end
     private :set_default_values
 
+    def clear_feedback_message
+      self.feedback_message = ''
+    end
+    private :clear_feedback_message
+
     def visible?
       !hidden
     end

app/services/proforma_service/convert_task_to_exercise.rb

@@ -89,7 +89,6 @@ def model_solution_files
         model_solution.files.map do |task_file|
           codeocean_file_from_task_file(task_file, model_solution).tap do |file|
             file.role ||= 'reference_implementation'
-            file.feedback_message = nil
           end
         end
       end
@@ -99,7 +98,6 @@ def task_files
       @task.files.reject {|file| file.id == 'ms-placeholder-file' }.map do |task_file|
         codeocean_file_from_task_file(task_file).tap do |file|
           file.role ||= 'regular_file'
-          file.feedback_message = nil
         end
       end
     end

db/migrate/20250729085843_remove_null_from_files_feedback_message.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class RemoveNullFromFilesFeedbackMessage < ActiveRecord::Migration[8.0]
+  def up
+    execute <<~SQL
+      UPDATE files SET feedback_message = '' WHERE feedback_message IS NULL;
+    SQL
+
+    change_column_default :files, :feedback_message, ''
+    change_column_null :files, :feedback_message, false
+  end
+
+  def down
+    change_column_null :files, :feedback_message, true
+    change_column_default :files, :feedback_message, nil
+  end
+end

db/schema.rb

@@ -3,15 +3,21 @@
 require 'rails_helper'
 
 RSpec.describe CodeOcean::File do
-  let(:file) { described_class.create.tap {|file| file.update(content: nil, hidden: nil, read_only: nil) } }
+  let(:file) do
+    described_class.new.tap do |file|
+      file.assign_attributes(content: nil, hidden: nil, read_only: nil)
+      file.validate
+    end
+  end
 
   it 'validates the presence of a file type' do
     expect(file.errors[:file_type]).to be_present
   end
 
   it 'validates the presence of the hidden flag' do
     expect(file.errors[:hidden]).to be_present
-    file.update(hidden: false)
+    file.hidden = false
+    file.validate
     expect(file.errors[:hidden]).to be_blank
   end
 
@@ -21,19 +27,23 @@
 
   it 'validates the presence of the read-only flag' do
     expect(file.errors[:read_only]).to be_present
-    file.update(read_only: false)
+    file.read_only = false
+    file.validate
     expect(file.errors[:read_only]).to be_blank
   end
 
   context 'with a teacher-defined test' do
-    before { file.update(role: 'teacher_defined_test') }
+    before do
+      file.role = 'teacher_defined_test'
+      file.validate
+    end
 
     it 'validates the presence of a feedback message' do
       expect(file.errors[:feedback_message]).to be_present
     end
 
     it 'validates the numericality of a weight' do
-      file.update(weight: 'heavy')
+      file.weight = 'heavy'
       expect(file.errors[:weight]).to be_present
     end
 
@@ -43,16 +53,21 @@
   end
 
   context 'with another file type' do
-    before { file.update(role: 'regular_file') }
+    before do
+      file.role = 'regular_file'
+    end
 
-    it 'validates the absence of a feedback message' do
-      file.update(feedback_message: 'Your solution is not correct yet.')
-      expect(file.errors[:feedback_message]).to be_present
+    it 'removes the feedback message' do
+      file.feedback_message = 'Your solution is not correct yet.'
+
+      expect { file.validate }
+        .to change(file, :feedback_message).to('')
     end
 
     it 'validates the absence of a weight' do
       allow(file).to receive(:clear_weight)
-      file.update(weight: 1)
+      file.weight = 1
+      file.validate
       expect(file.errors[:weight]).to be_present
     end
   end