Add ability to set read at on feedback

app/controllers/api/feedback_controller.rb

@@ -30,6 +30,18 @@ def create
       end
     end
 
+    def set_read
+      feedback = Feedback.find(params[:id])
+      result = Feedback::SetRead.call(feedback: feedback)
+
+      if result.success?
+        @feedback = result[:feedback]
+        render :show, formats: [:json], status: :ok
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
     private
 
     def project
@@ -68,8 +80,8 @@ def feedback_create_params
     end
 
     def url_params
-      permitted_params = params.permit(:project_id)
-      { identifier: permitted_params[:project_id] }
+      permitted_params = params.permit(:project_id, :id)
+      { identifier: permitted_params[:project_id], id: permitted_params[:id] }
     end
 
     def base_params

app/models/ability.rb

@@ -113,7 +113,7 @@ def define_school_student_abilities(user:, school:)
     can(%i[read], Lesson, school_id: school.id, visibility: 'students', school_class: { students: { student_id: user.id } })
     can(%i[read create update], Project, school_id: school.id, user_id: user.id, lesson_id: nil, remixed_from_id: visible_lesson_project_ids)
     can(%i[read show_context], Project, lesson: { school_id: school.id, visibility: 'students', school_class: { students: { student_id: user.id } } })
-    can(%i[read], Feedback, school_project: { project: { school_id: school.id, user_id: user.id, lesson_id: nil, remixed_from_id: visible_lesson_project_ids } })
+    can(%i[read set_read], Feedback, school_project: { project: { school_id: school.id, user_id: user.id, lesson_id: nil, remixed_from_id: visible_lesson_project_ids } })
     can(%i[show_finished set_finished show_status unsubmit submit], SchoolProject, project: { user_id: user.id, lesson_id: nil }, school_id: school.id)
   end
 

app/views/api/feedback/_feedback.json.jbuilder

@@ -7,5 +7,6 @@ json.call(
   :user_id,
   :content,
   :created_at,
-  :updated_at
+  :updated_at,
+  :read_at
 )

config/routes.rb

@@ -46,7 +46,9 @@
       resource :remix, only: %i[show create], controller: 'projects/remixes'
       resources :remixes, only: %i[index], controller: 'projects/remixes'
       resource :images, only: %i[show create], controller: 'projects/images'
-      resources :feedback, only: %i[index create], controller: 'feedback'
+      resources :feedback, only: %i[index create], controller: 'feedback' do
+        put :read, on: :member, to: 'feedback#set_read'
+      end
     end
 
     resource :project_errors, only: %i[create]

db/migrate/20251106170425_add_read_at_to_feedback.rb

@@ -0,0 +1,5 @@
+class AddReadAtToFeedback < ActiveRecord::Migration[7.2]
+  def change
+    add_column :feedback, :read_at, :datetime
+  end
+end

lib/concepts/feedback/set_read.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class Feedback
+  class SetRead
+    class << self
+      def call(feedback:)
+        response = OperationResponse.new
+        response[:feedback] = feedback
+        response[:feedback].read_at = Time.current
+        response[:feedback].save!
+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        response[:error] = e.message
+        response
+      end
+    end
+  end
+end

spec/concepts/feedback/set_read_at_spec.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Feedback::SetRead, type: :unit do
+  let(:feedback) { create(:feedback) }
+
+  describe '.call' do
+    context 'when set_read is successful' do
+      it 'returns a successful operation response' do
+        response = described_class.call(feedback: feedback)
+        expect(response.success?).to be(true)
+      end
+
+      it 'returns the updated feedback' do
+        response = described_class.call(feedback: feedback)
+        expect(response[:feedback]).to be_a(Feedback)
+      end
+
+      it 'returns read_at' do
+        response = described_class.call(feedback: feedback)
+        expect(response[:feedback].read_at).to be_present
+      end
+
+      it 'returns read_at as a timestamp' do
+        response = described_class.call(feedback: feedback)
+        expect(response[:feedback].read_at).to be_a(ActiveSupport::TimeWithZone)
+      end
+    end
+
+    context 'when set_read fails' do
+      before do
+        allow(feedback).to receive(:save!).and_raise(StandardError, 'Some API error')
+      end
+
+      it 'returns a failed operation response' do
+        response = described_class.call(feedback: feedback)
+        expect(response.success?).to be(false)
+      end
+
+      it 'does not persist read_at' do
+        described_class.call(feedback: feedback)
+        feedback.reload
+        expect(feedback.read_at).to be_nil
+      end
+
+      it 'includes the correct error response' do
+        response = described_class.call(feedback: feedback)
+        expect(response[:error]).to eq('Some API error')
+      end
+    end
+  end
+end

spec/features/feedback/setting_read_at_spec.rb

@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Set read_at on feedback requests', type: :request do
+  let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+  let(:school) { create(:school) }
+  let(:student) { create(:student, school:) }
+  let(:teacher) { create(:teacher, school:) }
+  let(:school_class) { create(:school_class, teacher_ids: [teacher.id], school:) }
+  let(:class_student) { create(:class_student, school_class:, student_id: student.id) }
+  let(:lesson) { create(:lesson, school:, school_class:, user_id: teacher.id, visibility: 'students') }
+  let(:teacher_project) { create(:project, user_id: teacher.id, school:, lesson:) }
+  let(:student_project) { create(:project, user_id: class_student.student_id, school:, parent: teacher_project) }
+  let!(:feedback) { create(:feedback, school_project: student_project.school_project, user_id: teacher.id, content: 'Excellent work!') }
+  let(:feedback_json) do
+    {
+      id: feedback.id,
+      school_project_id: feedback.school_project_id,
+      user_id: feedback.user_id,
+      content: feedback.content,
+      created_at: feedback.created_at,
+      updated_at: feedback.updated_at,
+      read_at: feedback.read_at
+    }.to_json
+  end
+
+  context 'when logged in as the class teacher' do
+    before do
+      authenticated_in_hydra_as(teacher)
+      put("/api/projects/#{student_project.identifier}/feedback/#{feedback.id}/read", headers: headers)
+      feedback.reload
+    end
+
+    it 'returns forbidden response' do
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it 'does not set the read_at on feedback' do
+      expect(feedback.read_at).to be_nil
+    end
+  end
+
+  context 'when logged in as the student' do
+    before do
+      authenticated_in_hydra_as(student)
+    end
+
+    context 'when feedback exists' do
+      before do
+        put("/api/projects/#{student_project.identifier}/feedback/#{feedback.id}/read", headers: headers)
+        feedback.reload
+      end
+
+      it 'returns ok response' do
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'returns the feedback json' do
+        expect(response.body).to eq(feedback_json)
+      end
+
+      it 'does set the read_at on feedback' do
+        expect(feedback.read_at).to be_present
+      end
+
+      it 'sets read_at to be a time' do
+        expect(feedback.read_at).to be_a(ActiveSupport::TimeWithZone)
+      end
+
+      it 'sets read_at to a recent time' do
+        expect(feedback.read_at).to be_within(5.seconds).of(Time.current)
+      end
+    end
+
+    context 'when feedback does not exist' do
+      before do
+        put("/api/projects/#{student_project.identifier}/feedback/does-not-exist/read", headers: headers)
+        feedback.reload
+      end
+
+      it 'returns not found response' do
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+  end
+end

spec/models/ability_spec.rb

@@ -309,6 +309,7 @@
         it { is_expected.to be_able_to(:read, remixed_project) }
         it { is_expected.not_to be_able_to(:create, feedback) }
         it { is_expected.to be_able_to(:read, feedback) }
+        it { is_expected.to be_able_to(:set_read, feedback) }
         it { is_expected.to be_able_to(:create, remixed_project) }
         it { is_expected.to be_able_to(:update, remixed_project) }
         it { is_expected.not_to be_able_to(:destroy, remixed_project) }
@@ -338,6 +339,7 @@
         it { is_expected.not_to be_able_to(:read, remixed_project) }
         it { is_expected.not_to be_able_to(:create, feedback) }
         it { is_expected.not_to be_able_to(:read, feedback) }
+        it { is_expected.not_to be_able_to(:set_read, feedback) }
         it { is_expected.not_to be_able_to(:create, remixed_project) }
         it { is_expected.not_to be_able_to(:update, remixed_project) }
         it { is_expected.not_to be_able_to(:destroy, remixed_project) }
@@ -354,6 +356,7 @@
         it { is_expected.to be_able_to(:read, remixed_project) }
         it { is_expected.to be_able_to(:create, feedback) }
         it { is_expected.to be_able_to(:read, feedback) }
+        it { is_expected.not_to be_able_to(:set_read, feedback) }
         it { is_expected.not_to be_able_to(:create, remixed_project) }
         it { is_expected.not_to be_able_to(:update, remixed_project) }
         it { is_expected.not_to be_able_to(:destroy, remixed_project) }
@@ -370,6 +373,7 @@
         it { is_expected.to be_able_to(:read, original_project) }
         it { is_expected.to be_able_to(:create, feedback) }
         it { is_expected.to be_able_to(:read, feedback) }
+        it { is_expected.not_to be_able_to(:set_read, feedback) }
         it { is_expected.not_to be_able_to(:create, original_project) }
         it { is_expected.to be_able_to(:update, original_project) }