diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index f44710faf..2ac99ca99 100644 --- a/app/controllers/api/projects_controller.rb +++ b/app/controllers/api/projects_controller.rb @@ -27,7 +27,7 @@ def show end def create - result = Project::Create.call(project_hash: project_params) + result = Project::Create.call(project_hash: project_params, current_user:) if result.success? @project = result[:project] @@ -80,8 +80,8 @@ def load_projects end def project_params - if school_owner? - # A school owner must specify who the project user is. + if school_owner? || current_user&.experience_cs_admin? + # A school owner or an Experience CS admin must specify who the project user is. base_params else # A school teacher may only create projects they own. diff --git a/app/graphql/mutations/create_project.rb b/app/graphql/mutations/create_project.rb index c7135168a..07c98c3f7 100644 --- a/app/graphql/mutations/create_project.rb +++ b/app/graphql/mutations/create_project.rb @@ -13,7 +13,7 @@ def resolve(**input) components: input[:components]&.map(&:to_h) ) - response = Project::Create.call(project_hash:) + response = Project::Create.call(project_hash:, current_user: context[:current_user]) raise GraphQL::ExecutionError, response[:error] unless response.success? { project: response[:project] } diff --git a/app/models/ability.rb b/app/models/ability.rb index bca1f2dfa..a7b65202e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -14,6 +14,8 @@ def initialize(user) define_school_teacher_abilities(user:, school:) if user.school_teacher?(school) define_school_owner_abilities(school:) if user.school_owner?(school) end + + define_experience_cs_admin_abilities(user) end private @@ -100,6 +102,12 @@ def define_school_student_abilities(user:, school:) can(%i[show_finished set_finished], SchoolProject, project: { user_id: user.id, lesson_id: nil }, school_id: school.id) end + def define_experience_cs_admin_abilities(user) + return unless user&.experience_cs_admin? + + can %i[read create update destroy], Project, user_id: nil + end + def school_teacher_can_manage_lesson?(user:, school:, lesson:) is_my_lesson = lesson.school_id == school.id && lesson.user_id == user.id is_my_class = lesson.school_class&.teacher_ids&.include?(user.id) diff --git a/app/models/user.rb b/app/models/user.rb index a7d90a6be..386ab2615 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,7 +51,15 @@ def student? end def admin? - (roles&.to_s&.split(',')&.map(&:strip) || []).include?('editor-admin') + parsed_roles.include?('editor-admin') + end + + def experience_cs_admin? + parsed_roles.include?('experience-cs-admin') + end + + def parsed_roles + roles&.to_s&.split(',')&.map(&:strip) || [] end def ==(other) diff --git a/lib/concepts/project/operations/create.rb b/lib/concepts/project/operations/create.rb index 625cdba85..a2e942889 100644 --- a/lib/concepts/project/operations/create.rb +++ b/lib/concepts/project/operations/create.rb @@ -3,9 +3,9 @@ class Project class Create class << self - def call(project_hash:) + def call(project_hash:, current_user:) response = OperationResponse.new - response[:project] = build_project(project_hash) + response[:project] = build_project(project_hash, current_user) response[:project].save! response rescue StandardError => e @@ -16,9 +16,9 @@ def call(project_hash:) private - def build_project(project_hash) - identifier = PhraseIdentifier.generate - new_project = Project.new(project_hash.except(:components).merge(identifier:)) + def build_project(project_hash, current_user) + project_hash[:identifier] = PhraseIdentifier.generate unless current_user&.experience_cs_admin? + new_project = Project.new(project_hash.except(:components)) new_project.components.build(project_hash[:components]) new_project end diff --git a/spec/concepts/project/create_spec.rb b/spec/concepts/project/create_spec.rb index 2af9426f1..519b7d788 100644 --- a/spec/concepts/project/create_spec.rb +++ b/spec/concepts/project/create_spec.rb @@ -3,9 +3,10 @@ require 'rails_helper' RSpec.describe Project::Create, type: :unit do - subject(:create_project) { described_class.call(project_hash:) } + subject(:create_project) { described_class.call(project_hash:, current_user:) } - let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } + let(:current_user) { create(:user) } + let(:user_id) { current_user.id } before do mock_phrase_generation @@ -16,7 +17,7 @@ let(:project_hash) { ActionController::Parameters.new({}).merge(user_id:) } context 'with valid content' do - subject(:create_project_with_content) { described_class.call(project_hash:) } + subject(:create_project_with_content) { described_class.call(project_hash:, current_user:) } let(:project_hash) do { diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 473db5ec7..d7470f672 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -11,6 +11,10 @@ roles { 'editor-admin' } end + factory :experience_cs_admin_user do + roles { 'experience-cs-admin' } + end + factory :student do email { nil } username { Faker::Internet.username } diff --git a/spec/features/project/creating_a_project_spec.rb b/spec/features/project/creating_a_project_spec.rb index bbcd47836..c8b671deb 100644 --- a/spec/features/project/creating_a_project_spec.rb +++ b/spec/features/project/creating_a_project_spec.rb @@ -3,16 +3,11 @@ require 'rails_helper' RSpec.describe 'Creating a project', type: :request do - before do - authenticated_in_hydra_as(teacher) - mock_phrase_generation - end - + let(:generated_identifier) { 'word1-word2-word3' } let(:headers) { { Authorization: UserProfileMock::TOKEN } } let(:teacher) { create(:teacher, school:) } let(:school) { create(:school) } let(:owner) { create(:owner, school:) } - let(:params) do { project: { @@ -24,11 +19,24 @@ } end + before do + authenticated_in_hydra_as(teacher) + mock_phrase_generation(generated_identifier) + end + it 'responds 201 Created' do post('/api/projects', headers:, params:) expect(response).to have_http_status(:created) end + it 'generates an identifier for the project even if another identifier is specified' do + params_with_identifier = { project: { identifier: 'test-identifier', components: [] } } + post('/api/projects', headers:, params: params_with_identifier) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:identifier]).to eq(generated_identifier) + end + it 'responds with the project JSON' do post('/api/projects', headers:, params:) data = JSON.parse(response.body, symbolize_names: true) @@ -200,4 +208,64 @@ expect(response).to have_http_status(:forbidden) end end + + context 'when an Experience CS admin creates a starter Scratch project' do + let(:experience_cs_admin) { create(:experience_cs_admin_user) } + let(:params) do + { + project: { + identifier: 'test-project', + name: 'Test Project', + locale: 'fr', + project_type: Project::Types::SCRATCH, + user_id: nil, + components: [] + } + } + end + + before do + authenticated_in_hydra_as(experience_cs_admin) + end + + it 'responds 201 Created' do + post('/api/projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'sets the project identifier to the specified (not the generated) value' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:identifier]).to eq('test-project') + end + + it 'sets the project name to the specified value' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('Test Project') + end + + it 'sets the project locale to the specified value' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:locale]).to eq('fr') + end + + it 'sets the project type to the specified value' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:project_type]).to eq(Project::Types::SCRATCH) + end + + it 'sets the project user_id to the specified value (i.e. nil to represent a public project)' do + post('/api/projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:user_id]).to be_nil + end + end end diff --git a/spec/features/project/updating_a_project_spec.rb b/spec/features/project/updating_a_project_spec.rb index 41d325ce5..b0f31c34a 100644 --- a/spec/features/project/updating_a_project_spec.rb +++ b/spec/features/project/updating_a_project_spec.rb @@ -10,7 +10,9 @@ end let(:headers) { { Authorization: UserProfileMock::TOKEN } } - let!(:project) { create(:project, name: 'Test Project', user_id: owner.id) } + let(:project_type) { Project::Types::PYTHON } + let(:user_id) { owner.id } + let!(:project) { create(:project, name: 'Test Project', user_id:, locale: 'en', project_type:) } let(:owner) { create(:owner, school:) } let(:school) { create(:school) } @@ -53,4 +55,27 @@ put("/api/projects/#{project.id}", params:) expect(response).to have_http_status(:unauthorized) end + + context 'when an Experience CS admin creates a starter Scratch project' do + let(:experience_cs_admin) { create(:experience_cs_admin_user) } + let(:user_id) { nil } + let(:project_type) { Project::Types::SCRATCH } + let(:params) { { project: { name: 'Test Project' } } } + + before do + authenticated_in_hydra_as(experience_cs_admin) + end + + it 'responds 200 OK' do + put("/api/projects/#{project.identifier}?project_type=scratch", headers:, params:) + expect(response).to have_http_status(:success) + end + + it 'sets the project name to the specified value' do + put("/api/projects/#{project.identifier}?project_type=scratch", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:name]).to eq('Test Project') + end + end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 39624d64e..e3d959581 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -139,6 +139,32 @@ end end + context 'with an experience-cs admin' do + let(:user) { build(:experience_cs_admin_user, id: user_id) } + let(:another_project) { build(:project) } + + context 'with a starter project' do + it { is_expected.to be_able_to(:read, starter_project) } + it { is_expected.to be_able_to(:create, starter_project) } + it { is_expected.to be_able_to(:update, starter_project) } + it { is_expected.to be_able_to(:destroy, starter_project) } + end + + context 'with own project' do + it { is_expected.to be_able_to(:read, project) } + it { is_expected.to be_able_to(:create, project) } + it { is_expected.to be_able_to(:update, project) } + it { is_expected.to be_able_to(:destroy, project) } + end + + context 'with another user\'s project' do + it { is_expected.not_to be_able_to(:read, another_project) } + it { is_expected.not_to be_able_to(:create, another_project) } + it { is_expected.not_to be_able_to(:update, another_project) } + it { is_expected.not_to be_able_to(:destroy, another_project) } + end + end + # rubocop:disable RSpec/MultipleMemoizedHelpers context "with a teacher's project where the lesson is visible to students" do let(:user) { create(:user) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 103e90ada..ce42c44b3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -277,6 +277,28 @@ end end + describe '#parsed_roles' do + it 'returns array of role names when roles is set to comma-separated string' do + user = build(:user, roles: 'role-1,role-2') + expect(user.parsed_roles).to eq(%w[role-1 role-2]) + end + + it 'strips leading & trailing spaces from role names' do + user = build(:user, roles: ' role-1 , role-2 ') + expect(user.parsed_roles).to eq(%w[role-1 role-2]) + end + + it 'returns empty array when roles is set to empty string' do + user = build(:user, roles: '') + expect(user.parsed_roles).to eq([]) + end + + it 'returns empty array when roles is set to nil' do + user = build(:user, roles: nil) + expect(user.parsed_roles).to eq([]) + end + end + describe '#admin?' do it 'returns true if the user has the editor-admin role in Hydra' do user = build(:user, roles: 'editor-admin') @@ -287,15 +309,17 @@ user = build(:user, roles: 'another-editor-admin') expect(user).not_to be_admin end + end - it 'returns false if roles are empty in Hydra' do - user = build(:user, roles: '') - expect(user).not_to be_admin + describe '#experience_cs_admin?' do + it 'returns true if the user has the experience-cs-admin role in Hydra' do + user = build(:experience_cs_admin_user) + expect(user).to be_experience_cs_admin end - it 'returns false if roles are nil in Hydra' do - user = build(:user, roles: nil) - expect(user).not_to be_admin + it 'returns false if the user does not have the experience-cs-admin role in Hydra' do + user = build(:user, roles: 'another-admin') + expect(user).not_to be_experience_cs_admin end end diff --git a/spec/requests/projects/destroy_spec.rb b/spec/requests/projects/destroy_spec.rb index f1d54066d..bf139f029 100644 --- a/spec/requests/projects/destroy_spec.rb +++ b/spec/requests/projects/destroy_spec.rb @@ -36,6 +36,29 @@ expect(response).to have_http_status(:forbidden) end end + + context 'when an Experience CS admin destroys a starter Scratch project' do + let(:project) do + create( + :project, { + project_type: Project::Types::SCRATCH, + user_id: nil, + locale: 'en' + } + ) + end + let(:experience_cs_admin) { create(:experience_cs_admin_user) } + + before do + authenticated_in_hydra_as(experience_cs_admin) + end + + it 'deletes the project' do + expect do + delete("/api/projects/#{project.identifier}?project_type=scratch", headers:) + end.to change(Project.unscoped, :count).by(-1) + end + end end context 'when no token is given' do diff --git a/spec/support/user_profile_mock.rb b/spec/support/user_profile_mock.rb index 4f56e353a..c694d158c 100644 --- a/spec/support/user_profile_mock.rb +++ b/spec/support/user_profile_mock.rb @@ -31,7 +31,8 @@ def user_to_hash(user, user_type, id_field = :id) id_field => user_type ? "#{user_type}:#{user.id}" : user.id, name: user.name, email: user.email, - username: user.username + username: user.username, + roles: user.roles } end