Let owners toggle Scratch for their school (#839)

## Status

- Related to
RaspberryPiFoundation/digital-editor-issues#1375

## What's changed?

Previously members of schools could access Scratch projects if they had
the `cat_mode` feature was enabled.

This change allows members of schools to view/remix/update existing
scratch projects they have access to, regardless of the feature flag.
There is a new `scratch_enabled` flag in the database that decides if
school members can create new scratch projects.

As part of this we have:

- Re-purposed the existing update school endpoint to allow for updating
the new scratch_enabled flag
- Changed the logic in the scratch base controller to just check for
school membership instead of the feature
- Checked the `scratch_enabled` flag in the lessons controller
- Made the lessons controller safer by restricting what params are
accepted in updates
- Removed duplicated asset tests
- Made sure the Scratch controllers authorizes resources using our
standard pattern


## As part of deploy

We should deploy the frontend changes soon after this to make it easier
to toggle the Scratch feature

Author: zetter-rpf

app/controllers/api/lessons_controller.rb

@@ -6,6 +6,7 @@ class LessonsController < ApiController
 
     before_action :authorize_user, except: %i[index show]
     before_action :verify_school_class_belongs_to_school, only: :create
+    before_action :verify_can_create_scratch_projects, only: %i[create create_copy]
     load_and_authorize_resource :lesson
 
     def index
@@ -29,7 +30,7 @@ def show
     end
 
     def create
-      result = Lesson::Create.call(lesson_params:)
+      result = Lesson::Create.call(lesson_params: create_params)
 
       if result.success?
         @lesson_with_user = result[:lesson].with_user
@@ -40,7 +41,7 @@ def create
     end
 
     def create_copy
-      result = Lesson::CreateCopy.call(lesson: @lesson, lesson_params:)
+      result = Lesson::CreateCopy.call(lesson: @lesson, lesson_params: create_params)
 
       if result.success?
         @lesson_with_user = result[:lesson].with_user
@@ -53,7 +54,7 @@ def create_copy
     def update
       # TODO: Consider removing user_id from the lesson_params for update so users can update other users' lessons without changing ownership
       # OR consider dropping user_id on lessons and using teacher id/ids on the class instead
-      result = Lesson::Update.call(lesson: @lesson, lesson_params:)
+      result = Lesson::Update.call(lesson: @lesson, lesson_params: update_params)
 
       if result.success?
         @lesson_with_user = result[:lesson].with_user
@@ -77,12 +78,18 @@ def filtered_lessons_scope
     end
 
     def verify_school_class_belongs_to_school
-      return if base_params[:school_class_id].blank?
-      return if school&.classes&.pluck(:id)&.include?(base_params[:school_class_id])
+      return if create_params[:school_class_id].blank?
+      return if school&.classes&.pluck(:id)&.include?(create_params[:school_class_id])
 
       raise ParameterError, 'school_class_id does not correspond to school_id'
     end
 
+    def verify_can_create_scratch_projects
+      return unless scratch_project? && !school.scratch_enabled?
+
+      render json: { error: 'Forbidden' }, status: :forbidden
+    end
+
     def user_remixes(lessons)
       lessons.map { |lesson| user_remix(lesson) }
     end
@@ -97,11 +104,21 @@ def user_remix(lesson)
       )
     end
 
-    def lesson_params
-      base_params.merge(user_id: current_user.id)
+    def scratch_project?
+      create_params.dig(:project_attributes, :project_type) == Project::Types::CODE_EDITOR_SCRATCH
+    end
+
+    def update_params
+      params.fetch(:lesson, {}).permit(
+        :name,
+        :visibility,
+        {
+          project_attributes: [:name]
+        }
+      )
     end
 
-    def base_params
+    def create_params
       params.fetch(:lesson, {}).permit(
         :school_id,
         :school_class_id,
@@ -118,15 +135,15 @@ def base_params
             { scratch_component: {} }
           ]
         }
-      )
+      ).merge(user_id: current_user.id)
     end
 
     def school_owner?
       school && current_user.school_owner?(school)
     end
 
     def school
-      @school ||= @lesson&.school || School.find_by(id: base_params[:school_id]) || SchoolClass.find_by(id: params[:school_class_id])&.school
+      @school ||= @lesson&.school || School.find_by(id: create_params[:school_id]) || SchoolClass.find_by(id: params[:school_class_id])&.school
     end
   end
 end

app/controllers/api/schools_controller.rb

@@ -16,7 +16,7 @@ def show
     end
 
     def create
-      result = School::Create.call(school_params:, creator_id: current_user.id, token: current_user.token)
+      result = School::Create.call(school_params: create_params, creator_id: current_user.id, token: current_user.token)
 
       if result.success?
         @school = result[:school]
@@ -31,7 +31,7 @@ def create
 
     def update
       school = School.find(params[:id])
-      result = School::Update.call(school:, school_params:)
+      result = School::Update.call(school:, school_params: update_params)
 
       if result.success?
         @school = result[:school]
@@ -76,7 +76,7 @@ def import
 
     private
 
-    def school_params
+    def create_params
       params.expect(
         school: %i[name
                    website
@@ -99,5 +99,11 @@ def school_params
                    user_origin]
       )
     end
+
+    def update_params
+      params.expect(
+        school: %i[scratch_enabled]
+      )
+    end
   end
 end

app/controllers/api/scratch/assets_controller.rb

@@ -5,12 +5,11 @@ module Scratch
     class AssetsController < ScratchController
       include ActiveStorage::SetCurrent
 
-      skip_before_action :authorize_user, only: [:show]
       prepend_before_action :load_project_from_header, only: %i[show create]
+      authorize_resource :project_from_header
 
       def show
         filename_with_extension = "#{params[:id]}.#{params[:format]}"
-        authorize! :show, @project_from_header
 
         scratch_asset = ScratchAsset.find_visible_to_project(
           project: @project_from_header,
@@ -23,8 +22,6 @@ def show
       end
 
       def create
-        authorize! :show, @project_from_header
-
         filename_with_extension = "#{params[:id]}.#{params[:format]}"
         scratch_asset = ScratchAsset.find_or_initialize_by(
           project: @project_from_header,

app/controllers/api/scratch/projects_controller.rb

@@ -5,20 +5,19 @@ module Scratch
     class ProjectsController < ScratchController
       include RemixSelection
 
-      skip_before_action :authorize_user, only: [:show]
-      skip_before_action :check_scratch_feature, only: [:show]
-      before_action :load_project, only: %i[show update]
+      before_action :load_project, except: %i[create]
+      authorize_resource :project, except: %i[create]
 
       before_action :ensure_create_is_a_remix, only: %i[create]
+      before_action :load_original_project, only: %i[create]
+      authorize_resource :original_project, only: %i[create]
 
       def show
         render json: @project.scratch_component.content_with_stage_first
       end
 
       def create
-        original_project = load_original_project(source_project_identifier)
-        return render json: { error: I18n.t('errors.admin.unauthorized') }, status: :unauthorized unless current_ability.can?(:show, original_project)
-
+        original_project = @original_project
         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?
 
@@ -74,8 +73,8 @@ def create_params
         }
       end
 
-      def load_original_project(identifier)
-        Project.find_by!(identifier:, project_type: Project::Types::CODE_EDITOR_SCRATCH)
+      def load_original_project
+        @original_project = Project.find_by!(identifier: source_project_identifier, project_type: Project::Types::CODE_EDITOR_SCRATCH)
       end
 
       def scratch_content_params
@@ -100,6 +99,10 @@ def move_pending_scratch_upload_to_remix(pending_upload, remix_project)
       rescue ActiveRecord::RecordNotUnique
         pending_upload.destroy!
       end
+
+      def load_project
+        @project = Project.find_by!(identifier: params[:id], project_type: Project::Types::CODE_EDITOR_SCRATCH)
+      end
     end
   end
 end

app/controllers/api/scratch/scratch_controller.rb

@@ -4,20 +4,13 @@ module Api
   module Scratch
     class ScratchController < ApiController
       before_action :authorize_user
-      before_action :check_scratch_feature
+      before_action :only_allow_schools_to_use_scratch
 
-      def check_scratch_feature
-        return if current_user.nil?
-
-        school = current_user&.schools&.first
-        return if Flipper.enabled?(:cat_mode, school)
+      def only_allow_schools_to_use_scratch
+        return true if current_user.schools.any?
 
         raise ActiveRecord::RecordNotFound, 'Not Found'
       end
-
-      def load_project
-        @project = Project.find_by!(identifier: params[:id], project_type: Project::Types::CODE_EDITOR_SCRATCH)
-      end
     end
   end
 end

app/views/api/schools/_school.json.jbuilder

@@ -17,7 +17,8 @@ json.call(
   :country_code,
   :verified_at,
   :created_at,
-  :updated_at
+  :updated_at,
+  :scratch_enabled
 )
 
 include_roles = local_assigns.fetch(:roles, false)

db/migrate/20260528141937_add_scratch_enabled_to_school.rb

@@ -0,0 +1,5 @@
+class AddScratchEnabledToSchool < ActiveRecord::Migration[8.1]
+  def change
+      add_column :schools, :scratch_enabled, :boolean, default: false, null: false
+  end
+end

db/schema.rb

@@ -94,6 +94,7 @@
       new_params = { lesson: params[:lesson].merge(user_id: 'ignored') }
 
       post('/api/lessons', headers:, params: new_params)
+      expect(response).to have_http_status(:created)
       data = JSON.parse(response.body, symbolize_names: true)
 
       expect(data[:user_id]).to eq(teacher.id)
@@ -186,4 +187,44 @@
       expect(response).to have_http_status(:unprocessable_content)
     end
   end
+
+  describe 'working with Scratch projects' do
+    let(:params) do
+      {
+        lesson: {
+          name: 'Test Lesson',
+          school_id: school.id,
+          project_attributes: {
+            name: 'Hello Scratch project',
+            project_type: Project::Types::CODE_EDITOR_SCRATCH,
+            scratch_component: {
+              content: {
+                example_data: 'true'
+              }
+            }
+          }
+        }
+      }
+    end
+
+    it 'creates a lesson with a scratch component when school has Scratch enabled' do
+      school.update!(scratch_enabled: true)
+      post('/api/lessons', headers:, params:)
+      expect(response).to have_http_status(:created)
+
+      data = JSON.parse(response.body, symbolize_names: true)
+
+      lesson_id = data[:id]
+
+      project = Lesson.find(lesson_id).project
+      expect(project.project_type).to eq(Project::Types::CODE_EDITOR_SCRATCH)
+      expect(project.scratch_component.content).to eq({ 'example_data' => 'true' })
+    end
+
+    it 'returns forbidden when school does not have Scratch enabled' do
+      school.update!(scratch_enabled: false)
+      post('/api/lessons', headers:, params:)
+      expect(response).to have_http_status(:forbidden)
+    end
+  end
 end

spec/features/lesson/creating_a_lesson_spec.rb

@@ -117,15 +117,17 @@
       expect(response).to have_http_status(:ok)
     end
 
-    it 'responds 422 Unprocessable Entity when trying to re-assign the lesson to a different class' do
+    it 'does not allow re-assigning the lesson to a different class' do
       school = create(:school, id: SecureRandom.uuid)
       teacher = create(:teacher, school:)
       school_class = create(:school_class, school:, teacher_ids: [teacher.id])
 
       new_params = { lesson: params[:lesson].merge(school_class_id: school_class.id) }
       put("/api/lessons/#{lesson.id}", headers:, params: new_params)
+      expect(response).to have_http_status(:ok)
 
-      expect(response).to have_http_status(:unprocessable_content)
+      lesson.reload
+      expect(lesson.school_class_id).not_to eq(school_class.id)
     end
   end
 end