Use a student's remix for Scratch asset uploads

Author: abcampo-iry

app/controllers/api/scratch/assets_controller.rb

@@ -14,7 +14,7 @@ def show
         authorize! :show, @project_from_header
 
         scratch_asset = ScratchAsset.find_visible_to_project(
-          project: @project_from_header,
+          project: readable_scratch_asset_project,
           filename: filename_with_extension
         )
         raise ActiveRecord::RecordNotFound, 'Not Found' unless scratch_asset
@@ -23,11 +23,12 @@ def show
       end
 
       def create
-        authorize! :update, @project_from_header
+        project = writable_scratch_asset_project
+        return if performed?
 
         filename_with_extension = "#{params[:id]}.#{params[:format]}"
         scratch_asset = ScratchAsset.find_or_initialize_by(
-          project: @project_from_header,
+          project:,
           filename: filename_with_extension
         )
 
@@ -37,7 +38,7 @@ def create
             scratch_asset.save!
           rescue ActiveRecord::RecordNotUnique
             logger.info("Scratch asset already created during concurrent upload: #{filename_with_extension}")
-            ScratchAsset.find_by!(project: @project_from_header, filename: filename_with_extension)
+            ScratchAsset.find_by!(project:, filename: filename_with_extension)
           end
         end
 

app/controllers/api/scratch/projects_controller.rb

@@ -21,6 +21,17 @@ def create
         remix_params = create_params
         return render json: { error: I18n.t('errors.project.remixing.invalid_params') }, status: :bad_request if remix_params.dig(:scratch_component, :content).blank?
 
+        existing_remix = remix_for_user(original_project, current_user)
+        if existing_remix
+          authorize! :update, existing_remix
+
+          scratch_component = existing_remix.scratch_component || existing_remix.build_scratch_component
+          scratch_component.content = scratch_content_params
+          existing_remix.save!
+
+          return render json: { status: 'ok', 'content-name': existing_remix.identifier }, status: :ok
+        end
+
         remix_origin = request.origin || request.referer
 
         result = Project::CreateRemix.call(

app/controllers/api/scratch/scratch_controller.rb

@@ -3,6 +3,8 @@
 module Api
   module Scratch
     class ScratchController < ApiController
+      include RemixSelection
+
       before_action :authorize_user
       before_action :check_scratch_feature
 
@@ -31,6 +33,52 @@ def load_project_from_header
           project_type: Project::Types::CODE_EDITOR_SCRATCH
         )
       end
+
+      def readable_scratch_asset_project
+        return @project_from_header if can?(:update, @project_from_header)
+
+        remix_for_user(@project_from_header, current_user) || @project_from_header
+      end
+
+      def writable_scratch_asset_project
+        return @project_from_header if can?(:update, @project_from_header)
+
+        authorize! :show, @project_from_header
+
+        remix = remix_for_user(@project_from_header, current_user)
+        if remix
+          authorize! :update, remix
+          return remix
+        end
+
+        create_scratch_remix_from(@project_from_header)
+      end
+
+      def create_scratch_remix_from(project)
+        result = Project::CreateRemix.call(
+          params: {
+            identifier: project.identifier,
+            scratch_component: { content: scratch_component_content_for(project) }
+          },
+          user_id: current_user.id,
+          original_project: project,
+          remix_origin: request.origin || request.referer || request.base_url
+        )
+
+        return result[:project] if result.success?
+
+        render json: { error: result[:error] }, status: :bad_request
+        nil
+      end
+
+      def scratch_component_content_for(project)
+        project.scratch_component&.content&.deep_dup || {
+          meta: {},
+          targets: [],
+          monitors: [],
+          extensions: []
+        }
+      end
     end
   end
 end

spec/features/scratch/creating_a_scratch_asset_spec.rb

@@ -14,8 +14,7 @@
   let(:project_headers) { auth_headers.merge('X-Project-ID' => project.identifier) }
 
   before do
-    Flipper.disable :cat_mode
-    Flipper.disable_actor :cat_mode, school
+    Flipper.enable_actor :cat_mode, school
   end
 
   it 'responds 401 Unauthorized when no Authorization header is provided' do
@@ -26,6 +25,8 @@
 
   it 'responds 404 Not Found when cat_mode is not enabled' do
     authenticated_in_hydra_as(teacher)
+    Flipper.disable :cat_mode
+    Flipper.disable_actor :cat_mode, school
 
     post '/api/scratch/assets/example.svg', headers: project_headers
 
@@ -34,7 +35,6 @@
 
   it 'creates an asset when cat_mode is enabled and the required headers are provided' do
     authenticated_in_hydra_as(teacher)
-    Flipper.enable_actor :cat_mode, school
 
     post '/api/scratch/assets/example.svg', headers: project_headers
 

spec/features/scratch/creating_a_scratch_project_spec.rb

@@ -36,8 +36,7 @@
     mock_phrase_generation('new-project-id')
     create(:scratch_component, project: original_project)
 
-    Flipper.disable :cat_mode
-    Flipper.disable_actor :cat_mode, school
+    Flipper.enable_actor :cat_mode, school
   end
 
   def make_request(query: request_query, request_headers: headers, request_params: scratch_project)
@@ -57,6 +56,8 @@ def make_request(query: request_query, request_headers: headers, request_params:
 
   it 'responds 404 Not Found when cat_mode is not enabled' do
     authenticated_in_hydra_as(teacher)
+    Flipper.disable :cat_mode
+    Flipper.disable_actor :cat_mode, school
 
     make_request
 
@@ -66,7 +67,6 @@ def make_request(query: request_query, request_headers: headers, request_params:
   context 'when authenticated and cat_mode is enabled' do
     before do
       authenticated_in_hydra_as(teacher)
-      Flipper.enable_actor :cat_mode, school
     end
 
     it 'responds 403 Forbidden when not remixing' do
@@ -125,5 +125,31 @@ def make_request(query: request_query, request_headers: headers, request_params:
       expect(remixed_project.lesson_id).to be_nil
       expect(remixed_project.scratch_component.content.to_h).to eq(scratch_project.deep_stringify_keys)
     end
+
+    it 'updates the existing remix instead of creating a duplicate' do
+      student = create(:student, school:)
+      school_class = create(:school_class, school:, teacher_ids: [teacher.id])
+      create(:class_student, school_class:, student_id: student.id)
+      original_project.update!(lesson: create(:lesson, school:, school_class:, user_id: teacher.id, visibility: 'students'))
+      existing_remix = create(
+        :project,
+        school:,
+        user_id: student.id,
+        remixed_from_id: original_project.id,
+        project_type: Project::Types::CODE_EDITOR_SCRATCH,
+        locale: nil
+      )
+      create(:scratch_component, project: existing_remix, content: { targets: ['old target'], monitors: [], extensions: [], meta: {} })
+      authenticated_in_hydra_as(student)
+
+      expect { make_request }.not_to change(Project, :count)
+
+      expect(response).to have_http_status(:ok)
+      expect(response.parsed_body).to eq(
+        'status' => 'ok',
+        'content-name' => existing_remix.identifier
+      )
+      expect(existing_remix.reload.scratch_component.content.to_h).to eq(scratch_project.deep_stringify_keys)
+    end
   end
 end