Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions app/controllers/api/lessons/batch_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class BatchController < ApiController

before_action :authorize_user
before_action :verify_school_class_belongs_to_school
before_action :verify_can_create_scratch_projects
before_action :authorize_lesson_projects!
before_action :authorize_source_projects!

Expand All @@ -33,15 +32,6 @@ def verify_school_class_belongs_to_school
params[:lesson_projects].each { |lesson_params| verify_lesson_school_class!(lesson_params) }
end

def verify_can_create_scratch_projects
return unless lesson_projects?

batch_lessons_params.each_index do |index|
verify_lesson_scratch!(batch_lessons_params[index], source_project: source_project_for(index))
break if performed?
end
end

def batch_lessons_params
@batch_lessons_params ||= params[:lesson_projects].map { |lesson_params| create_batch_params(lesson_params) }
end
Expand Down
5 changes: 0 additions & 5 deletions app/controllers/api/lessons_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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
Expand Down Expand Up @@ -86,10 +85,6 @@ def verify_school_class_belongs_to_school
verify_lesson_school_class!(create_params)
end

def verify_can_create_scratch_projects
verify_lesson_scratch!(create_params, source_project:)
end

def user_remixes(lessons)
lessons.map { |lesson| user_remix(lesson) }
end
Expand Down
13 changes: 0 additions & 13 deletions app/controllers/concerns/lesson_creation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,6 @@ def verify_lesson_school_class!(lesson_params)
raise ParameterError, 'school_class_id does not correspond to school_id'
end

def verify_lesson_scratch!(lesson_params, source_project: nil)
return unless scratch_project?(lesson_params) || source_project&.scratch_project?

school = School.find_by(id: lesson_params[:school_id])
return if school&.scratch_enabled?

render json: { error: 'Forbidden' }, status: :forbidden
end

def scratch_project?(lesson_params)
lesson_params.dig(:project_attributes, :project_type) == Project::Types::CODE_EDITOR_SCRATCH
end

def find_source_project!(identifier, locale)
return nil if identifier.blank?

Expand Down
29 changes: 2 additions & 27 deletions spec/features/lesson/creating_a_batch_of_lessons_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
end

let(:teacher) { create(:teacher, school:) }
let(:school) { create(:school, scratch_enabled:) }
let(:scratch_enabled) { true }
let(:school) { create(:school) }
let(:batch_path) { '/api/lessons/batch' }
let(:lesson_projects) { lesson_project_params }

Expand Down Expand Up @@ -176,7 +175,7 @@
end

context 'when the user does not belong to the school' do
let(:other_school) { create(:school, scratch_enabled: true) }
let(:other_school) { create(:school) }
let(:lesson_project_params) do
[
{
Expand All @@ -201,18 +200,6 @@
end
end

context 'when the school does not have Scratch enabled' do
let(:scratch_enabled) { false }

it 'returns forbidden' do
expect(response).to have_http_status(:forbidden)
end

it 'does not create any lessons' do
expect(Lesson.count).to eq(0)
end
end

context 'when there lesson projects is an empty array' do
let(:lesson_project_params) { [] }

Expand Down Expand Up @@ -317,17 +304,5 @@
expect(Lesson.count).to eq(0)
end
end

context 'when a source_project_identifier points at a scratch project and the school does not have Scratch enabled' do
let(:scratch_enabled) { false }

it 'responds 403 Forbidden' do
expect(response).to have_http_status(:forbidden)
end

it 'does not create any lessons' do
expect(Lesson.count).to eq(0)
end
end
end
end
37 changes: 1 addition & 36 deletions spec/features/lesson/creating_a_lesson_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,7 @@
}
end

it 'creates a lesson with a scratch component when school has Scratch enabled' do
school.update!(scratch_enabled: true)
it 'creates a lesson with a scratch component' do
post('/api/lessons', headers:, params:)
expect(response).to have_http_status(:created)

Expand All @@ -239,12 +238,6 @@
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

# #create resolves `lesson[:source_project_identifier]` through
Expand Down Expand Up @@ -278,7 +271,6 @@

context 'when the source project is a shared Experience CS project' do
before do
school.update!(scratch_enabled: true)
post('/api/lessons', headers:, params:)
end

Expand Down Expand Up @@ -312,32 +304,5 @@
expect(lesson_project.origin).to eq(source_project.origin)
end
end

context 'when the school does not have Scratch enabled' do
# The request carries no project_attributes[:project_type], so the scratch gate has to
# look at the source project to see that a Scratch project is about to be created.
let(:params) do
{
lesson: {
name: 'Test Lesson',
school_id: school.id,
source_project_identifier: source_project.identifier,
project_attributes: {
name: 'My digital canvas',
locale: 'en'
}
}
}
end

before do
school.update!(scratch_enabled: false)
end

it 'responds 403 Forbidden when only source_project_identifier points at a scratch project' do
post('/api/lessons', headers:, params:)
expect(response).to have_http_status(:forbidden)
end
end
end
end
Loading