diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index edba3986a..52ab64663 100644 --- a/app/controllers/api/projects_controller.rb +++ b/app/controllers/api/projects_controller.rb @@ -63,7 +63,7 @@ def show_context private def set_auth_cookie_for_scratch - return unless @project.project_type == Project::Types::CODE_EDITOR_SCRATCH + return unless @project.scratch_project? return unless Flipper.enabled?(:cat_mode, school) cookies[:scratch_auth] = { diff --git a/app/controllers/api/scratch/projects_controller.rb b/app/controllers/api/scratch/projects_controller.rb index bc9a1bcc0..772056ee1 100644 --- a/app/controllers/api/scratch/projects_controller.rb +++ b/app/controllers/api/scratch/projects_controller.rb @@ -14,12 +14,30 @@ def show end def create - render json: { status: 'ok', 'content-name': 'new-project-id' }, status: :ok + 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) + + 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? + + remix_origin = request.origin || request.referer + + result = Project::CreateRemix.call( + params: remix_params, + user_id: current_user.id, + original_project:, + remix_origin: + ) + + if result.success? + render json: { status: 'ok', 'content-name': result[:project].identifier }, status: :ok + else + render json: { error: result[:error] }, status: :bad_request + end end def update - scratch_content = params.permit!.slice(:meta, :targets, :monitors, :extensions) - @project.scratch_component&.content = scratch_content.to_unsafe_h + @project.scratch_component&.content = scratch_content_params @project.save! render json: { status: 'ok' }, status: :ok end @@ -27,9 +45,34 @@ def update private def ensure_create_is_a_remix - return if params[:is_remix] == '1' + return if params[:is_remix] == '1' && params[:original_id].present? + + render json: { error: I18n.t('errors.project.remixing.only_existing_allowed') }, status: :forbidden + end + + def source_project_identifier + params[:original_id] + end + + def create_params + { + identifier: source_project_identifier, + scratch_component: { content: scratch_content_params } + } + end + + def load_original_project(identifier) + project_loader = ProjectLoader.new(identifier, [params[:locale]]) + original_project = project_loader.load + + raise ActiveRecord::RecordNotFound, I18n.t('errors.project.not_found') unless original_project + raise ActiveRecord::RecordNotFound, I18n.t('errors.project.not_found') unless original_project.scratch_project? + + original_project + end - render json: { error: 'Only remixing existing projects is allowed' }, status: :forbidden + def scratch_content_params + params.slice(:meta, :targets, :monitors, :extensions).to_unsafe_h end end end diff --git a/app/models/project.rb b/app/models/project.rb index 1cfa3faab..c28650ffb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -82,6 +82,10 @@ def media images + videos + audio end + def scratch_project? + project_type == Types::CODE_EDITOR_SCRATCH + end + private def check_unique_not_null diff --git a/config/locales/en.yml b/config/locales/en.yml index 5cef4714e..ce3d9c076 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4,6 +4,7 @@ en: unauthorized: "Not authorized." csv_file_required: "A CSV file is required." project: + not_found: "Project not found" editing: delete_default_component: "Cannot delete default file" change_default_name: "Cannot amend default file name" @@ -12,6 +13,7 @@ en: remixing: invalid_params: "Invalid parameters" cannot_save: "Cannot create project remix" + only_existing_allowed: "Only remixing existing projects is allowed" validations: school: website: "must be a valid URL" diff --git a/lib/concepts/project/operations/create_remix.rb b/lib/concepts/project/operations/create_remix.rb index ef42defc3..81f70714b 100644 --- a/lib/concepts/project/operations/create_remix.rb +++ b/lib/concepts/project/operations/create_remix.rb @@ -23,7 +23,11 @@ def validate_params(response, params, user_id, original_project, remix_origin) end def remix_project(response, params, user_id, original_project, remix_origin) - response[:project] = create_remix(original_project, params, user_id, remix_origin) + response[:project] = if original_project.scratch_project? + create_scratch_remix(original_project, params, user_id, remix_origin) + else + create_remix(original_project, params, user_id, remix_origin) + end response[:project].save! response end @@ -50,11 +54,19 @@ def create_remix(original_project, params, user_id, remix_origin) remix end + def create_scratch_remix(original_project, params, user_id, remix_origin) + remix = format_project(original_project, params, user_id, remix_origin) + scratch_component = params.fetch(:scratch_component) + remix.build_scratch_component(content: scratch_component[:content]) + + remix + end + def format_project(original_project, params, user_id, remix_origin) original_project.dup.tap do |proj| proj.identifier = PhraseIdentifier.generate proj.locale = nil - proj.name = params[:name] + proj.name = params[:name] || original_project.name proj.user_id = user_id proj.remixed_from_id = original_project.id proj.remix_origin = remix_origin diff --git a/spec/concepts/project/create_remix_spec.rb b/spec/concepts/project/create_remix_spec.rb index 3978c47bc..50cc4a170 100644 --- a/spec/concepts/project/create_remix_spec.rb +++ b/spec/concepts/project/create_remix_spec.rb @@ -129,6 +129,49 @@ end end + context 'when the original project has Scratch content' do + let!(:original_project) do + create(:project, project_type: Project::Types::CODE_EDITOR_SCRATCH, locale: nil) + end + let(:original_scratch_project) do + { + meta: { semver: '3.0.0' }, + targets: ['original target'], + monitors: [], + extensions: [] + } + end + + before do + create(:scratch_component, project: original_project, content: original_scratch_project) + end + + context 'when new Scratch content is provided' do + let(:remixed_scratch_project) do + { + meta: { semver: '3.0.0' }, + targets: ['remixed target'], + monitors: [], + extensions: ['pen'] + } + end + let(:remix_params) do + { + name: 'My remixed project', + identifier: original_project.identifier, + scratch_component: { + content: remixed_scratch_project + } + } + end + + it 'uses the supplied Scratch content' do + expect(create_remix[:project].scratch_component.content.to_h) + .to eq(remixed_scratch_project.deep_stringify_keys) + end + end + end + context 'when user_id is not present' do let(:user_id) { nil } let(:params) { { project_id: original_project.identifier } } diff --git a/spec/features/scratch/creating_a_scratch_project_spec.rb b/spec/features/scratch/creating_a_scratch_project_spec.rb index a06446997..d3e17f6ce 100644 --- a/spec/features/scratch/creating_a_scratch_project_spec.rb +++ b/spec/features/scratch/creating_a_scratch_project_spec.rb @@ -5,16 +5,52 @@ RSpec.describe 'Creating a Scratch project (remixing)', type: :request do let(:school) { create(:school) } let(:teacher) { create(:teacher, school:) } - let(:cookie_headers) { { 'Cookie' => "scratch_auth=#{UserProfileMock::TOKEN}" } } - let(:params) { { original_id: 'original-project-id', project: { targets: [] }, is_remix: '1' } } + let(:headers) do + { + 'Cookie' => "scratch_auth=#{UserProfileMock::TOKEN}", + 'Origin' => 'editor.com' + } + end + let(:request_query) { { original_id: original_project.identifier, is_remix: '1' } } + let(:scratch_project) do + { + meta: { semver: '3.0.0' }, + targets: ['updated target'], + monitors: [], + extensions: ['pen'] + } + end + let(:lesson) { create(:lesson, school:, user_id: teacher.id) } + let(:original_project) do + create( + :project, + school:, + lesson:, + user_id: teacher.id, + project_type: Project::Types::CODE_EDITOR_SCRATCH, + locale: nil + ) + end before do + mock_phrase_generation('new-project-id') + create(:scratch_component, project: original_project) + Flipper.disable :cat_mode Flipper.disable_actor :cat_mode, school end + def make_request(query: request_query, request_headers: headers, request_params: scratch_project) + post( + "/api/scratch/projects?#{Rack::Utils.build_query(query)}", + params: request_params, + headers: request_headers, + as: :json + ) + end + it 'responds 401 Unauthorized when no cookie is provided' do - post '/api/scratch/projects', params: params + make_request(request_headers: {}) expect(response).to have_http_status(:unauthorized) end @@ -22,30 +58,72 @@ it 'responds 404 Not Found when cat_mode is not enabled' do authenticated_in_hydra_as(teacher) - post '/api/scratch/projects', params: params, headers: cookie_headers + make_request expect(response).to have_http_status(:not_found) end - it 'responds 403 Forbidden when not remixing' do - authenticated_in_hydra_as(teacher) - Flipper.enable_actor :cat_mode, school + context 'when authenticated and cat_mode is enabled' do + before do + authenticated_in_hydra_as(teacher) + Flipper.enable_actor :cat_mode, school + end - post '/api/scratch/projects', params: params.merge(is_remix: '0'), headers: cookie_headers + it 'responds 403 Forbidden when not remixing' do + make_request(query: request_query.merge(is_remix: '0')) - expect(response).to have_http_status(:forbidden) - end + expect(response).to have_http_status(:forbidden) + end - it 'return new project id when cat_mode is enabled and a cookie is provided' do - authenticated_in_hydra_as(teacher) - Flipper.enable_actor :cat_mode, school + it 'responds 403 Forbidden when original_id is missing' do + make_request(query: { is_remix: '1' }) + + expect(response).to have_http_status(:forbidden) + end + + it 'responds 404 Not Found when original project does not exist' do + make_request(query: { original_id: 'no-such-project', is_remix: '1' }) + + expect(response).to have_http_status(:not_found) + end + + it 'responds 404 Not Found when the original project is not a Scratch project' do + non_scratch_project = create(:project, school:, lesson:, user_id: teacher.id, locale: nil) + + make_request(query: { original_id: non_scratch_project.identifier, is_remix: '1' }) + + expect(response).to have_http_status(:not_found) + end + + it 'responds 401 Unauthorized when the user cannot access the original project' do + inaccessible_project = create(:project, project_type: Project::Types::CODE_EDITOR_SCRATCH, locale: nil) + create(:scratch_component, project: inaccessible_project) + + make_request(query: { original_id: inaccessible_project.identifier, is_remix: '1' }) + + expect(response).to have_http_status(:unauthorized) + end + + it 'responds 400 Bad Request when no Scratch content is submitted' do + make_request(request_params: {}) + + expect(response).to have_http_status(:bad_request) + end - post '/api/scratch/projects', params: params, headers: cookie_headers + it 'creates a remix, associates it to the current user, and returns the new identifier' do + expect { make_request }.to change(Project, :count).by(1) - expect(response).to have_http_status(:ok) + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq( + 'status' => 'ok', + 'content-name' => 'new-project-id' + ) - data = JSON.parse(response.body, symbolize_names: true) - expect(data[:status]).to eq('ok') - expect(data[:'content-name']).to eq('new-project-id') + remixed_project = Project.find_by!(identifier: 'new-project-id') + expect(remixed_project.user_id).to eq(teacher.id) + expect(remixed_project.remixed_from_id).to eq(original_project.id) + expect(remixed_project.lesson_id).to be_nil + expect(remixed_project.scratch_component.content.to_h).to eq(scratch_project.deep_stringify_keys) + end end end