Auto-creating project when lesson created (#342)

Co-authored-by: Dan Halson <[email protected]>

Author: loiswells97

Gemfile.lock

@@ -498,6 +498,7 @@ GEM
 PLATFORMS
   aarch64-linux
   arm64-darwin-22
+  arm64-darwin-23
   x86_64-linux
 
 DEPENDENCIES

app/controllers/api/lessons_controller.rb

@@ -72,24 +72,25 @@ def verify_school_class_belongs_to_school
     end
 
     def lesson_params
-      if school_owner?
-        # A school owner must specify who the lesson user is.
-        base_params
-      else
-        # A school teacher may only create lessons they own.
-        base_params.merge(user_id: current_user.id)
-      end
+      base_params.merge(user_id: current_user.id)
     end
 
     def base_params
       params.fetch(:lesson, {}).permit(
         :school_id,
         :school_class_id,
-        :user_id,
         :name,
         :description,
         :visibility,
-        :due_date
+        :due_date,
+        {
+          project_attributes: [
+            :name,
+            :project_type,
+            :locale,
+            { components: %i[id name extension content index default] }
+          ]
+        }
       )
     end
 

app/models/lesson.rb

@@ -5,7 +5,8 @@ class Lesson < ApplicationRecord
   belongs_to :school_class, optional: true
   belongs_to :parent, optional: true, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :copies
   has_many :copies, dependent: :nullify, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :parent
-  has_many :projects, dependent: :nullify
+  has_one :project, dependent: :nullify
+  accepts_nested_attributes_for :project
 
   before_validation :assign_school_from_school_class
   before_destroy -> { throw :abort }

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

@@ -18,4 +18,12 @@ json.call(
   :updated_at
 )
 
+if lesson.project
+  json.project(
+    lesson.project,
+    :identifier,
+    :project_type
+  )
+end
+
 json.user_name(user&.name)

lib/concepts/lesson/operations/create.rb

@@ -5,15 +5,30 @@ class Create
     class << self
       def call(lesson_params:)
         response = OperationResponse.new
-        response[:lesson] = Lesson.new(lesson_params)
+        response[:lesson] = build_lesson(lesson_params)
         response[:lesson].save!
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
-        errors = response[:lesson].errors.full_messages.join(',')
-        response[:error] = "Error creating lesson: #{errors}"
+        if response[:lesson].nil?
+          response[:error] = "Error creating lesson #{e}"
+        else
+          errors = response[:lesson].errors.full_messages.join(',')
+          response[:error] = "Error creating lesson: #{errors}"
+        end
         response
       end
+
+      private
+
+      def build_lesson(lesson_hash)
+        new_lesson = Lesson.new(lesson_hash.except(:project_attributes))
+        project_params = lesson_hash[:project_attributes].merge({ user_id: lesson_hash[:user_id],
+                                                                  school_id: lesson_hash[:school_id],
+                                                                  lesson_id: new_lesson.id })
+        new_lesson.project = Project.new(project_params)
+        new_lesson
+      end
     end
   end
 end

lib/concepts/project/operations/create.rb

@@ -10,7 +10,7 @@ def call(project_hash:)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
-        response[:error] = 'Error creating project'
+        response[:error] = "Error creating project: #{e}"
         response
       end
 

lib/tasks/classroom_management.rake

@@ -27,7 +27,8 @@ namespace :classroom_management do
         exit
       end
 
-      Project.where(school_id:).destroy_all
+      lesson_id = Lesson.where(school_id:).pluck(:id)
+      Project.where(lesson_id:).destroy_all
       # The `before_destroy` prevents us using destroy, but as we've removed projects already we can safely delete
       Lesson.where(school_id:).delete_all
       SchoolClass.where(school_id:).destroy_all

spec/concepts/lesson/create_spec.rb

@@ -6,7 +6,17 @@
   let(:teacher_id) { SecureRandom.uuid }
 
   let(:lesson_params) do
-    { name: 'Test Lesson', user_id: teacher_id }
+    {
+      name: 'Test Lesson',
+      user_id: teacher_id,
+      project_attributes: {
+        name: 'Hello world project',
+        project_type: 'python',
+        components: [
+          { name: 'main.py', extension: 'py', content: 'print("Hello, world!")' }
+        ]
+      }
+    }
   end
 
   it 'returns a successful operation response' do
@@ -28,13 +38,71 @@
     expect(response[:lesson].name).to eq('Test Lesson')
   end
 
+  it 'creates a project for the lesson' do
+    expect { described_class.call(lesson_params:) }.to change(Project, :count).by(1)
+  end
+
+  it 'associates the project to the lesson' do
+    response = described_class.call(lesson_params:)
+    expect(response[:lesson].project).to be_a(Project)
+  end
+
   it 'assigns the user_id' do
     response = described_class.call(lesson_params:)
     expect(response[:lesson].user_id).to eq(teacher_id)
   end
 
-  context 'when creation fails' do
-    let(:lesson_params) { {} }
+  context 'when lesson creation fails' do
+    let(:lesson_params) do
+      {
+        project_attributes: {
+          name: 'Hello world project',
+          project_type: 'python',
+          components: [
+            { name: 'main.py', extension: 'py', content: 'print("Hello, world!")' }
+          ]
+        }
+      }
+    end
+
+    before do
+      allow(Sentry).to receive(:capture_exception)
+    end
+
+    it 'does not create a lesson' do
+      expect { described_class.call(lesson_params:) }.not_to change(Lesson, :count)
+    end
+
+    it 'does not create a project' do
+      expect { described_class.call(lesson_params:) }.not_to change(Project, :count)
+    end
+
+    it 'returns a failed operation response' do
+      response = described_class.call(lesson_params:)
+      expect(response.failure?).to be(true)
+    end
+
+    it 'returns the error message in the operation response' do
+      response = described_class.call(lesson_params:)
+      expect(response[:error]).to match(/Error creating lesson/)
+    end
+
+    it 'sent the exception to Sentry' do
+      described_class.call(lesson_params:)
+      expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
+    end
+  end
+
+  context 'when project creation fails' do
+    let(:lesson_params) do
+      {
+        name: 'Test Lesson',
+        user_id: teacher_id,
+        project_attributes: {
+          invalid_attribute: 'blah blah blah'
+        }
+      }
+    end
 
     before do
       allow(Sentry).to receive(:capture_exception)
@@ -44,6 +112,10 @@
       expect { described_class.call(lesson_params:) }.not_to change(Lesson, :count)
     end
 
+    it 'does not create a project' do
+      expect { described_class.call(lesson_params:) }.not_to change(Project, :count)
+    end
+
     it 'returns a failed operation response' do
       response = described_class.call(lesson_params:)
       expect(response.failure?).to be(true)

spec/concepts/project/create_spec.rb

@@ -54,7 +54,7 @@
       end
 
       it 'returns error message' do
-        expect(create_project[:error]).to eq('Error creating project')
+        expect(create_project[:error]).to eq('Error creating project: Some error')
       end
 
       it 'sent the exception to Sentry' do

spec/factories/lesson.rb

@@ -6,5 +6,6 @@
     sequence(:name) { |n| "Lesson #{n}" }
     description { 'Description' }
     visibility { 'private' }
+    project { create(:project) }
   end
 end

spec/features/lesson/creating_a_lesson_spec.rb

@@ -16,7 +16,14 @@
   let(:params) do
     {
       lesson: {
-        name: 'Test Lesson'
+        name: 'Test Lesson',
+        project_attributes: {
+          name: 'Hello world project',
+          project_type: 'python',
+          components: [
+            { name: 'main.py', extension: 'py', content: 'print("Hello, world!")' }
+          ]
+        }
       }
     }
   end
@@ -59,7 +66,13 @@
         lesson: {
           name: 'Test Lesson',
           school_id: school.id,
-          user_id: teacher.id
+          project_attributes: {
+            name: 'Hello world project',
+            project_type: 'python',
+            components: [
+              { name: 'main.py', extension: 'py', content: 'print("Hello, world!")' }
+            ]
+          }
         }
       }
     end
@@ -76,13 +89,6 @@
       expect(response).to have_http_status(:created)
     end
 
-    it 'sets the lesson user to the specified user for school-owner users' do
-      post('/api/lessons', headers:, params:)
-      data = JSON.parse(response.body, symbolize_names: true)
-
-      expect(data[:user_id]).to eq(teacher.id)
-    end
-
     it 'sets the lesson user to the current user for school-teacher users' do
       authenticated_in_hydra_as(teacher)
       new_params = { lesson: params[:lesson].merge(user_id: 'ignored') }
@@ -122,16 +128,17 @@
           name: 'Test Lesson',
           school_id: school.id,
           school_class_id: school_class.id,
-          user_id: teacher.id
+          project_attributes: {
+            name: 'Hello world project',
+            project_type: 'python',
+            components: [
+              { name: 'main.py', extension: 'py', content: 'print("Hello, world!")' }
+            ]
+          }
         }
       }
     end
 
-    it 'responds 201 Created' do
-      post('/api/lessons', headers:, params:)
-      expect(response).to have_http_status(:created)
-    end
-
     it 'responds 201 Created when the user is the school-teacher for the class' do
       authenticated_in_hydra_as(teacher)
       school_class.update!(teacher_id: teacher.id)

spec/features/lesson/updating_a_lesson_spec.rb

@@ -68,6 +68,7 @@
     end
 
     it 'responds 200 OK when assigning the lesson to a school class' do
+      authenticated_in_hydra_as(teacher)
       school_class = create(:school_class, school:, teacher_id: teacher.id)
 
       new_params = { lesson: params[:lesson].merge(school_class_id: school_class.id) }
@@ -105,6 +106,10 @@
     let(:school_class) { create(:school_class, teacher_id: teacher.id, school:) }
     let!(:lesson) { create(:lesson, school_class:, name: 'Test Lesson', visibility: 'students', user_id: teacher.id) }
 
+    before do
+      authenticated_in_hydra_as(teacher)
+    end
+
     it 'responds 200 OK when the user is a school-owner' do
       put("/api/lessons/#{lesson.id}", headers:, params:)
       expect(response).to have_http_status(:ok)
@@ -122,12 +127,5 @@
       expect(response).to have_http_status(:unprocessable_entity)
     end
     # rubocop:enable RSpec/ExampleLength
-
-    it 'responds 422 Unprocessable Entity when trying to re-assign the lesson to a different user' do
-      new_params = { lesson: params[:lesson].merge(user_id: owner.id) }
-      put("/api/lessons/#{lesson.id}", headers:, params: new_params)
-
-      expect(response).to have_http_status(:unprocessable_entity)
-    end
   end
 end

spec/lib/classroom_management_spec.rb

@@ -16,7 +16,6 @@
       create(:teacher_role, user_id: creator_id, school:)
       create(:school_class, school_id: school.id, teacher_id: creator_id)
       create(:lesson, school_id: school.id, user_id: creator_id)
-      create(:project, school_id: school.id, user_id: creator_id)
     end
 
     # rubocop:disable RSpec/ExampleLength, RSpec/MultipleExpectations

spec/models/lesson_spec.rb

@@ -33,10 +33,10 @@
       expect(lesson.copies.size).to eq(2)
     end
 
-    it 'has many projects' do
+    it 'has one project' do
       user_id = SecureRandom.uuid
-      lesson = create(:lesson, user_id:, projects: [build(:project, user_id:)])
-      expect(lesson.projects.size).to eq(1)
+      lesson = create(:lesson, user_id:, project: build(:project, user_id:))
+      expect(lesson.project).to be_a(Project)
     end
   end