Create feedback

app/controllers/api/feedback_controller.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module Api
+  class FeedbackController < ApiController
+    before_action :authorize_user
+    load_and_authorize_resource :feedback
+
+    def create
+      result = Feedback::Create.call(feedback_params: feedback_create_params)
+
+      if result.success?
+        @feedback = result[:feedback]
+        render :show, formats: [:json], status: :created
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
+    end
+
+    # These params are used to authorize the resource with CanCanCan. The project identifier is sent in the URL,
+    # but these params need to match the shape of the feedback object whiich is attached to the SchoolProject,
+    # not the Project.
+    def feedback_params
+      school_project = Project.find_by(identifier: base_params[:identifier])&.school_project
+      feedback_create_params.except(:identifier).merge(
+        school_project_id: school_project&.id
+      )
+    end
+
+    # These params are used to create the feedback in the Feedback::Create operation. The project_id parameter,
+    # which is automatically named by Rails based on the route structure, is renamed to identifier for readability,
+    # as it is actually the human-readable project_identifier, not the project_id.
+    def feedback_create_params
+      base_params.merge(user_id: current_user.id)
+    end
+
+    def url_params
+      permitted_params = params.permit(:project_id)
+      { identifier: permitted_params[:project_id] }
+    end
+
+    def base_params
+      params.fetch(:feedback, {}).permit(
+        :content
+      ).merge(url_params)
+    end
+  end
+end

app/models/ability.rb

@@ -88,8 +88,15 @@ def define_school_teacher_abilities(user:, school:)
       school_teacher_can_manage_project?(user:, school:, project:)
     end
     can(%i[read update show_context], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
-    can(%i[read], Project,
-        remixed_from_id: Project.where(school_id: school.id, remixed_from_id: nil, lesson_id: Lesson.where(school_class_id: ClassTeacher.where(teacher_id: user.id).select(:school_class_id))).pluck(:id))
+    teacher_project_ids = Project.where(
+      school_id: school.id,
+      remixed_from_id: nil,
+      lesson_id: Lesson.where(
+        school_class_id: ClassTeacher.where(teacher_id: user.id).select(:school_class_id)
+      )
+    ).pluck(:id)
+    can(%i[read], Project, remixed_from_id: teacher_project_ids)
+    can(%i[create], Feedback, school_project: { project: { remixed_from_id: teacher_project_ids } })
   end
 
   def define_school_student_abilities(user:, school:)

app/models/feedback.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+class Feedback < ApplicationRecord
+  belongs_to :school_project
+  validates :content, presence: true
+  validates :user_id, presence: true
+  validate :user_has_the_school_owner_or_school_teacher_role_for_the_school
+  validate :parent_project_belongs_to_lesson
+  validate :parent_project_belongs_to_school_class
+  validate :user_is_the_class_teacher_for_the_school_project
+
+  has_paper_trail(
+    meta: {
+      meta_school_project_id: ->(f) { f.school_project&.id },
+      meta_school_id: ->(f) { f.school_project&.school_id }
+    }
+  )
+
+  private
+
+  def user_has_the_school_owner_or_school_teacher_role_for_the_school
+    school = school_project&.school
+    return unless user_id_changed? && errors.blank? && school
+
+    user = User.new(id: user_id)
+    return if user.school_owner?(school)
+    return if user.school_teacher?(school)
+
+    msg = "'#{user_id}' does not have the 'school-owner' or 'school-teacher' role for organisation '#{school.id}'"
+    errors.add(:user, msg)
+  end
+
+  def parent_project_belongs_to_lesson
+    parent_project = school_project&.project&.parent
+    return if parent_project&.lesson_id.present?
+
+    msg = "Parent project '#{parent_project&.id}' does not belong to a 'lesson'"
+    errors.add(:user, msg)
+  end
+
+  def parent_project_belongs_to_school_class
+    parent_project = school_project&.project&.parent
+    return if parent_project&.lesson&.school_class_id.present?
+
+    msg = "Parent project '#{parent_project&.id}' does not belong to a 'school-class'"
+    errors.add(:user, msg)
+  end
+
+  def user_is_the_class_teacher_for_the_school_project
+    return if !school_project || school_class&.teacher_ids&.include?(user_id)
+
+    errors.add(:user, "'#{user_id}' is not the 'school-teacher' for school_project '#{school_project.id}'")
+  end
+
+  def school_class
+    school_project&.project&.parent&.lesson&.school_class
+  end
+end

app/models/school_project.rb

@@ -3,4 +3,5 @@
 class SchoolProject < ApplicationRecord
   belongs_to :school
   belongs_to :project
+  has_many :feedback, dependent: :destroy
 end

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

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+json.call(
+  feedback,
+  :id,
+  :school_project_id,
+  :user_id,
+  :content,
+  :created_at,
+  :updated_at
+)

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

@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+json.partial! 'feedback', feedback: @feedback

config/initializers/inflections.rb

@@ -12,6 +12,10 @@
 #   inflect.uncountable %w( fish sheep )
 # end
 
+ActiveSupport::Inflector.inflections(:en) do |inflect|
+  inflect.uncountable %w[feedback]
+end
+
 # These inflection rules are supported but not enabled by default:
 ActiveSupport::Inflector.inflections(:en) do |inflect|
   inflect.acronym 'SSO'

config/routes.rb

@@ -41,6 +41,7 @@
       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'
+      resource :feedback, only: %i[create], controller: 'feedback'
     end
 
     resource :project_errors, only: %i[create]

db/migrate/20251008161139_create_feedback.rb

@@ -0,0 +1,10 @@
+class CreateFeedback < ActiveRecord::Migration[7.2]
+  def change
+    create_table :feedback, id: :uuid do |t|
+      t.references :school_project, foreign_key: true, type: :uuid
+      t.text :content
+      t.uuid :user_id, null: false
+      t.timestamps
+    end
+  end
+end

db/migrate/20251021162845_add_meta_school_project_id_to_versions.rb

@@ -0,0 +1,5 @@
+class AddMetaSchoolProjectIdToVersions < ActiveRecord::Migration[7.2]
+  def change
+    add_column :versions, :meta_school_project_id, :string
+  end
+end

lib/concepts/feedback/create.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class Feedback
+  class Create
+    class << self
+      def call(feedback_params:)
+        response = OperationResponse.new
+        response[:feedback] = build_feedback(feedback_params)
+        response[:feedback].save!
+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        if response[:feedback].present? && response[:feedback].errors.any?
+          errors = response[:feedback]&.errors&.full_messages&.join(',')
+          response[:error] = "Error creating feedback: #{errors}"
+        else
+          response[:error] = "Error creating feedback: #{e.message}"
+        end
+        response
+      end
+
+      private
+
+      def build_feedback(feedback_hash)
+        project = Project.find_by(identifier: feedback_hash[:identifier])
+        school_project = project&.school_project
+
+        # replace identifier with school_project_id
+        feedback_hash[:school_project_id] = school_project&.id
+        feedback_hash.delete(:identifier)
+        Feedback.new(feedback_hash)
+      end
+    end
+  end
+end

spec/concepts/feedback/create_spec.rb

@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Feedback::Create, type: :unit do
+  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) }
+  let(:teacher_project) { create(:project, user_id: teacher.id, school:, lesson:) }
+  let(:student_project) { create(:project, user_id: class_student.student_id, school:, lesson:, parent: teacher_project) }
+
+  let(:feedback_params) do
+    {
+      content: 'Great job!',
+      user_id: teacher.id,
+      identifier: student_project.identifier
+    }
+  end
+
+  context 'when a teacher' do
+    before do
+      allow(User).to receive(:from_userinfo).with(ids: teacher.id).and_return([teacher])
+    end
+
+    it 'returns a successful operation response' do
+      response = described_class.call(feedback_params:)
+      expect(response.success?).to be(true)
+    end
+
+    it 'creates a piece of feedback' do
+      expect { described_class.call(feedback_params:) }.to change(Feedback, :count).by(1)
+    end
+
+    it 'returns the feedback in the operation response' do
+      response = described_class.call(feedback_params:)
+      expect(response[:feedback]).to be_a(Feedback)
+    end
+
+    it 'assigns the content' do
+      response = described_class.call(feedback_params:)
+      expect(response[:feedback].content).to eq('Great job!')
+    end
+
+    it 'assigns the user_id' do
+      response = described_class.call(feedback_params:)
+      expect(response[:feedback].user_id).to eq(teacher.id)
+    end
+
+    it 'assigns the school_project_id' do
+      response = described_class.call(feedback_params:)
+      expect(response[:feedback].school_project_id).to eq(student_project.school_project.id)
+    end
+  end
+
+  context 'when feedback creation fails' do
+    let(:rogue_project) { create(:project, user_id: student.id) }
+    let(:feedback_params) do
+      {
+        content: nil,
+        user_id: teacher.id,
+        identifier: rogue_project.identifier
+      }
+    end
+
+    before do
+      allow(Sentry).to receive(:capture_exception)
+    end
+
+    it 'does not create feedback' do
+      expect { described_class.call(feedback_params:) }.not_to change(Feedback, :count)
+    end
+
+    it 'returns a failed operation response' do
+      response = described_class.call(feedback_params:)
+      expect(response.failure?).to be(true)
+    end
+
+    it 'returns the error message in the operation response' do
+      response = described_class.call(feedback_params:)
+      expect(response[:error]).to match(/Error creating feedback/)
+    end
+
+    it 'raises school project not found error when no school project' do
+      response = described_class.call(feedback_params:)
+      expect(response[:error]).to match(/School project must exist/)
+    end
+
+    it 'sent the exception to Sentry' do
+      described_class.call(feedback_params:)
+      expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
+    end
+  end
+end

spec/factories/feedback.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :feedback do
+    content { Faker::Lorem.paragraph }
+    user_id { nil }
+    school_project { nil }
+
+    after(:build) do |feedback, _evaluator|
+      school = create(:school)
+      teacher = create(:teacher, school: school)
+      student = create(:student, school: school)
+      school_class = create(:school_class, school: school, teacher_ids: [teacher.id])
+      create(:class_student, school_class: school_class, student_id: student.id)
+      parent_project = create(:project, user_id: teacher.id, school: school, lesson: create(:lesson, school_class: school_class, user_id: teacher.id))
+      student_project = create(:project, parent: parent_project, user_id: student.id)
+
+      feedback.user_id ||= teacher.id
+      feedback.school_project ||= create(:school_project, school: school, project: student_project)
+    end
+  end
+end