trial either checking for owner role or if project has associations

Author: Scott Adams

app/controllers/api/lessons_controller.rb

@@ -42,7 +42,7 @@ def create_copy
     end
 
     def update
-      result = Lesson::Update.call(lesson: @lesson, lesson_params:)
+      result = Lesson::Update.call(lesson: @lesson, lesson_params:, current_user:)
 
       if result.success?
         @lesson_with_user = result[:lesson].with_user

app/models/project.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class Project < ApplicationRecord
+  attr_accessor :current_user
+
   belongs_to :school, optional: true
   belongs_to :lesson, optional: true
   belongs_to :parent, optional: true, class_name: :Project, foreign_key: :remixed_from_id, inverse_of: :remixes
@@ -17,7 +19,7 @@ class Project < ApplicationRecord
   validate :identifier_cannot_be_taken_by_another_user
   validates :locale, presence: true, unless: :user_id
   validate :user_has_a_role_within_the_school
-  validate :user_is_a_member_or_the_owner_of_the_lesson
+  validate :user_is_a_member_or_the_owner_of_the_lesson_or_school
 
   scope :internal_projects, -> { where(user_id: nil) }
 
@@ -79,9 +81,14 @@ def user_has_a_role_within_the_school
     errors.add(:user, msg)
   end
 
-  def user_is_a_member_or_the_owner_of_the_lesson
+  def user_is_a_member_or_the_owner_of_the_lesson_or_school
     return if !lesson || user_id == lesson.user_id || lesson.school_class&.members&.exists?(student_id: user_id)
 
+    # either we explicitly check the user is an owner of the school
+    return if current_user&.school_owner?(lesson.school)
+    # or we bypass if the lesson is associated with a school but not a school class
+    return if lesson.school && !lesson.school_class && !lesson.school_class&.members&.any?
+
     errors.add(:user, "'#{user_id}' is not the owner or a member of the lesson '#{lesson_id}'")
   end
 end

lib/concepts/lesson/operations/update.rb

@@ -3,14 +3,14 @@
 class Lesson
   class Update
     class << self
-      def call(lesson:, lesson_params:)
+      def call(lesson:, lesson_params:, current_user:)
         response = OperationResponse.new
         response[:lesson] = lesson
         response[:lesson].assign_attributes(lesson_params)
         response[:lesson].save!
         if lesson_params[:name].present?
-          rename_lesson_project(lesson: response[:lesson], name: lesson_params[:name])
-          rename_lesson_remixes(lesson: response[:lesson], name: lesson_params[:name])
+          rename_lesson_project(current_user:, lesson: response[:lesson], name: lesson_params[:name])
+          rename_lesson_remixes(current_user:, lesson: response[:lesson], name: lesson_params[:name])
         end
         response
       rescue StandardError => e
@@ -20,20 +20,20 @@ def call(lesson:, lesson_params:)
         response
       end
 
-      def rename_lesson_project(lesson:, name:)
+      def rename_lesson_project(current_user:, lesson:, name:)
         return unless lesson.project
 
+        lesson.project.current_user = current_user
         lesson.project.assign_attributes(name:)
-        # TODO: determine school owner mechanism for project model validation rather than skipping validation
-        lesson.project.save!(validate: false)
+        lesson.project.save!
       end
 
-      def rename_lesson_remixes(lesson:, name:)
+      def rename_lesson_remixes(current_user:, lesson:, name:)
         lesson_remixes = Project.where(remixed_from_id: lesson.project.id)
         lesson_remixes.each do |remix|
+          remix.current_user = current_user
           remix.assign_attributes(name:)
-          # TODO: determine school owner mechanism for project model validation rather than skipping validation
-          remix.save!(validate: false)
+          remix.save!
         end
       end
     end

spec/concepts/lesson/update_spec.rb

@@ -8,33 +8,38 @@
   let(:student) { create(:student, school:) }
   let(:lesson) { create(:lesson, name: 'Test Lesson', user_id: teacher.id) }
   let!(:student_project) { create(:project, remixed_from_id: lesson.project.id, user_id: student.id) }
+  let(:current_user) { authenticated_user }
 
   let(:lesson_params) do
     { name: 'New Name' }
   end
 
+  before do
+    authenticated_in_hydra_as(teacher)
+  end
+
   it 'returns a successful operation response' do
-    response = described_class.call(lesson:, lesson_params:)
+    response = described_class.call(lesson:, lesson_params:, current_user:)
     expect(response.success?).to be(true)
   end
 
   it 'updates the lesson' do
-    response = described_class.call(lesson:, lesson_params:)
+    response = described_class.call(lesson:, lesson_params:, current_user:)
     expect(response[:lesson].name).to eq('New Name')
   end
 
   it 'updates the project name' do
-    described_class.call(lesson:, lesson_params:)
+    described_class.call(lesson:, lesson_params:, current_user:)
     expect(lesson.project.name).to eq('New Name')
   end
 
   it 'updates the student project name' do
-    described_class.call(lesson:, lesson_params:)
+    described_class.call(lesson:, lesson_params:, current_user:)
     expect(student_project.reload.name).to eq('New Name')
   end
 
   it 'returns the lesson in the operation response' do
-    response = described_class.call(lesson:, lesson_params:)
+    response = described_class.call(lesson:, lesson_params:, current_user:)
     expect(response[:lesson]).to be_a(Lesson)
   end
 
@@ -46,22 +51,22 @@
     end
 
     it 'does not update the lesson' do
-      response = described_class.call(lesson:, lesson_params:)
+      response = described_class.call(lesson:, lesson_params:, current_user:)
       expect(response[:lesson].reload.name).to eq('Test Lesson')
     end
 
     it 'returns a failed operation response' do
-      response = described_class.call(lesson:, lesson_params:)
+      response = described_class.call(lesson:, lesson_params:, current_user:)
       expect(response.failure?).to be(true)
     end
 
     it 'returns the error message in the operation response' do
-      response = described_class.call(lesson:, lesson_params:)
+      response = described_class.call(lesson:, lesson_params:, current_user:)
       expect(response[:error]).to match(/Error updating lesson/)
     end
 
     it 'sent the exception to Sentry' do
-      described_class.call(lesson:, lesson_params:)
+      described_class.call(lesson:, lesson_params:, current_user:)
       expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
     end
   end