diff --git a/app/controllers/api/public_projects_controller.rb b/app/controllers/api/public_projects_controller.rb new file mode 100644 index 000000000..f3eebdf68 --- /dev/null +++ b/app/controllers/api/public_projects_controller.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +module Api + class PublicProjectsController < ApiController + before_action :authorize_user + before_action :restrict_project_type, only: %i[create] + before_action :load_project, only: %i[update destroy] + before_action :restrict_to_public_projects, only: %i[update destroy] + before_action :prevent_destruction_of_public_project_with_remixes, only: %i[destroy] + + def create + authorize! :create, :public_project + result = PublicProject::Create.call(project_hash: create_params) + + if result.success? + @project = result[:project] + render 'api/projects/show', formats: [:json], status: :created + else + render json: { error: result[:error] }, status: :unprocessable_entity + end + end + + def update + authorize! :update, :public_project + result = PublicProject::Update.call(project: @project, update_hash: update_params) + + if result.success? + @project = result[:project] + render 'api/projects/show', formats: [:json] + else + render json: { error: result[:error] }, status: :unprocessable_entity + end + end + + def destroy + authorize! :update, :public_project + + if @project.destroy + head :ok + else + head :unprocessable_entity + end + end + + private + + def load_project + loader = ProjectLoader.new(params[:id], [params[:locale]]) + @project = loader.load + raise ActiveRecord::RecordNotFound if @project.blank? + end + + def create_params + params.require(:project).permit(:identifier, :locale, :project_type, :name) + end + + def update_params + params.require(:project).permit(:identifier, :name) + end + + def restrict_project_type + project_type = create_params[:project_type] + return if project_type == Project::Types::SCRATCH + + raise CanCan::AccessDenied.new("#{project_type} not yet supported", :create, :public_project) + end + + def restrict_to_public_projects + return if @project.user_id.blank? + + raise CanCan::AccessDenied.new('Cannot update non-public project', :update, :public_project) + end + + def prevent_destruction_of_public_project_with_remixes + return if @project.remixes.none? + + raise CanCan::AccessDenied.new('Cannot destroy public project with remixes', :update, :public_project) + end + end +end diff --git a/app/models/ability.rb b/app/models/ability.rb index bca1f2dfa..bd6cc7b5d 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,14 @@ 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 :create, :public_project + can :update, :public_project + can :destroy, :public_project + 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/project.rb b/app/models/project.rb index ebe956559..3b7f589c5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -7,6 +7,8 @@ module Types SCRATCH = 'scratch' end + attr_accessor :skip_identifier_generation + belongs_to :school, optional: true belongs_to :lesson, optional: true belongs_to :parent, optional: true, class_name: :Project, foreign_key: :remixed_from_id, inverse_of: :remixes @@ -20,7 +22,7 @@ module Types accepts_nested_attributes_for :components - before_validation :check_unique_not_null, on: :create + before_validation :generate_identifier, on: :create, unless: :skip_identifier_generation before_validation :create_school_project_if_needed validates :identifier, presence: true, uniqueness: { scope: :locale } @@ -81,7 +83,7 @@ def media private - def check_unique_not_null + def generate_identifier self.identifier ||= PhraseIdentifier.generate end diff --git a/app/models/public_project.rb b/app/models/public_project.rb new file mode 100644 index 000000000..ee9a8c986 --- /dev/null +++ b/app/models/public_project.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class PublicProject + include ActiveModel::Validations + + attr_reader :project + + delegate :identifier, :name, to: :project + + validates :identifier, format: { with: PhraseIdentifier::PATTERN, allow_blank: true } + validates :name, presence: true + + def initialize(project) + @project = project + end + + def valid? + valid = super + project_valid = project.valid? + errors.merge!(project.errors) + valid && project_valid + end + + def save! + raise_validation_error unless valid? + + project.save! + end + + private + + def merge_errors + project.errors.details.each do |attribute, details_array| + details_array.each_with_index do |details, index| + message = project.errors[attribute][index] + errors.add(attribute, message, **details) + end + end + end +end 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/config/routes.rb b/config/routes.rb index f48edf7f1..815a6289a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -41,6 +41,8 @@ resource :images, only: %i[show create], controller: 'projects/images' end + resources :public_projects, only: %i[create update destroy] + resource :project_errors, only: %i[create] resource :school, only: [:show], controller: 'my_school' diff --git a/lib/concepts/public_project/operations/create.rb b/lib/concepts/public_project/operations/create.rb new file mode 100644 index 000000000..419b3895d --- /dev/null +++ b/lib/concepts/public_project/operations/create.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class PublicProject + class Create + class << self + def call(project_hash:) + response = OperationResponse.new + + public_project = PublicProject.new( + Project.new(project_hash).tap do |p| + p.skip_identifier_generation = true + end + ) + public_project.save! + + response[:project] = public_project.project + response + rescue StandardError => e + Sentry.capture_exception(e) + response[:error] = "Error creating project: #{e}" + response + end + end + end +end diff --git a/lib/concepts/public_project/operations/update.rb b/lib/concepts/public_project/operations/update.rb new file mode 100644 index 000000000..b1b4d143a --- /dev/null +++ b/lib/concepts/public_project/operations/update.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class PublicProject + class Update + class << self + def call(project:, update_hash:) + response = OperationResponse.new + + project.assign_attributes(update_hash) + public_project = PublicProject.new(project) + public_project.save! + + response[:project] = public_project.project + response + rescue StandardError => e + Sentry.capture_exception(e) + response[:error] = "Error updating project: #{e}" + response + end + end + end +end diff --git a/lib/phrase_identifier.rb b/lib/phrase_identifier.rb index 8cab707d4..cb7c45916 100644 --- a/lib/phrase_identifier.rb +++ b/lib/phrase_identifier.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class PhraseIdentifier + PATTERN = /\A[a-z0-9]+(-[a-z0-9]+)*\z/ + class Error < RuntimeError end diff --git a/spec/concepts/public_project/create_spec.rb b/spec/concepts/public_project/create_spec.rb new file mode 100644 index 000000000..9d852c7d2 --- /dev/null +++ b/spec/concepts/public_project/create_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe PublicProject::Create, type: :unit do + describe '.call' do + subject(:create_project) { described_class.call(project_hash:) } + + let(:identifier) { 'foo-bar-baz' } + let(:name) { 'Foo bar baz' } + let(:project_hash) do + { + identifier:, + locale: 'en', + project_type: Project::Types::SCRATCH, + name: + } + end + + context 'with valid content' do + it 'returns success' do + expect(create_project.success?).to be(true) + end + + it 'returns project with identifier' do + new_project = create_project[:project] + expect(new_project.identifier).to eq(identifier) + end + end + + context 'when creation fails' do + let(:project_hash) { {} } + + before do + allow(Sentry).to receive(:capture_exception) + end + + it 'returns failure' do + expect(create_project.failure?).to be(true) + end + + it 'sent the exception to Sentry' do + create_project + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + end + end + + context 'when identifier is blank' do + let(:identifier) { nil } + + it 'returns failure' do + expect(create_project.failure?).to be(true) + end + + it 'returns error message' do + expect(create_project[:error]).to eq("Error creating project: Validation failed: Identifier can't be blank") + end + end + + context 'when identifier is in invalid format' do + let(:identifier) { 'FooBarBaz' } + + it 'returns failure' do + expect(create_project.failure?).to be(true) + end + + it 'returns error message' do + expect(create_project[:error]).to eq('Error creating project: Validation failed: Identifier is invalid') + end + end + + context 'when name is blank' do + let(:name) { '' } + + it 'returns failure' do + expect(create_project.failure?).to be(true) + end + + it 'returns error message' do + expect(create_project[:error]).to eq("Error creating project: Validation failed: Name can't be blank") + end + end + end +end diff --git a/spec/concepts/public_project/update_spec.rb b/spec/concepts/public_project/update_spec.rb new file mode 100644 index 000000000..6cbfa1625 --- /dev/null +++ b/spec/concepts/public_project/update_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe PublicProject::Update, type: :unit do + describe '.call' do + subject(:update_project) { described_class.call(project:, update_hash:) } + + let(:identifier) { 'foo-bar-baz' } + let(:new_identifier) { 'new-identifier' } + let(:new_name) { 'New name' } + let!(:project) { create(:project, identifier:) } + let(:update_hash) { { identifier: new_identifier, name: new_name } } + + context 'with valid content' do + it 'returns success' do + expect(update_project.success?).to be(true) + end + + it 'returns project with new identifier' do + updated_project = update_project[:project] + expect(updated_project.identifier).to eq(new_identifier) + end + + it 'returns project with new name' do + updated_project = update_project[:project] + expect(updated_project.name).to eq(new_name) + end + end + + context 'when update fails' do + before do + allow(project).to receive(:save!).and_raise('Some error') + allow(Sentry).to receive(:capture_exception) + end + + it 'returns failure' do + expect(update_project.failure?).to be(true) + end + + it 'sent the exception to Sentry' do + update_project + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + end + end + + context 'when identifier is blank' do + let(:new_identifier) { nil } + + it 'returns failure' do + expect(update_project.failure?).to be(true) + end + + it 'returns error message' do + expect(update_project[:error]).to eq("Error updating project: Validation failed: Identifier can't be blank") + end + end + + context 'when identifier is in invalid format' do + let(:new_identifier) { 'FooBarBaz' } + + it 'returns failure' do + expect(update_project.failure?).to be(true) + end + + it 'returns error message' do + expect(update_project[:error]).to eq('Error updating project: Validation failed: Identifier is invalid') + end + end + + context 'when name is blank' do + let(:new_name) { '' } + + it 'returns failure' do + expect(update_project.failure?).to be(true) + end + + it 'returns error message' do + expect(update_project[:error]).to eq("Error updating project: Validation failed: Name can't be blank") + end + end + end +end 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/public_project/creating_a_public_project_spec.rb b/spec/features/public_project/creating_a_public_project_spec.rb new file mode 100644 index 000000000..21fa47f1b --- /dev/null +++ b/spec/features/public_project/creating_a_public_project_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Creating a public project', type: :request do + let(:creator) { build(:experience_cs_admin_user) } + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let(:params) do + { + project: { + identifier: 'test-project', + locale: 'en', + project_type: Project::Types::SCRATCH, + name: 'Test Project' + } + } + end + + before do + authenticated_in_hydra_as(creator) + end + + it 'responds 201 Created' do + post('/api/public_projects', headers:, params:) + expect(response).to have_http_status(:created) + end + + it 'responds with the project JSON' do + post('/api/public_projects', headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data).to include( + { + identifier: 'test-project', + locale: 'en', + project_type: Project::Types::SCRATCH, + name: 'Test Project' + } + ) + end + + it 'does not set user_id on project even if one is supplied' do + user_id = SecureRandom.uuid + + post('/api/public_projects', headers:, params: params.merge(user_id:)) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data).to include(user_id: nil) + end + + context 'when creator is not an experience-cs admin' do + let(:creator) { build(:user) } + + it 'responds 403 Forbidden' do + post('/api/public_projects', headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + it 'responds 403 Forbidden when project type is not scratch' do + post('/api/public_projects', headers:, params: { project: { project_type: Project::Types::PYTHON } }) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 400 Bad Request when params are malformed' do + post('/api/public_projects', headers:, params: { project: {} }) + expect(response).to have_http_status(:bad_request) + end + + it 'responds 422 Unprocessable Entity when params are invalid' do + post('/api/public_projects', headers:, params: { project: { project_type: Project::Types::SCRATCH } }) + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'responds 401 Unauthorized when no token is given' do + post('/api/public_projects', params:) + expect(response).to have_http_status(:unauthorized) + end +end diff --git a/spec/features/public_project/destroying_a_public_project_spec.rb b/spec/features/public_project/destroying_a_public_project_spec.rb new file mode 100644 index 000000000..d580ba681 --- /dev/null +++ b/spec/features/public_project/destroying_a_public_project_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Destroying a public project', type: :request do + let(:destroyer) { build(:experience_cs_admin_user) } + let(:user_id) { nil } + let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id:) } + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + before do + authenticated_in_hydra_as(destroyer) + end + + it 'responds 200 OK' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:) + expect(response).to have_http_status(:success) + end + + it 'deletes the project' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:) + expect(Project).not_to exist(identifier: project.identifier) + end + + it 'responds 401 Unauthorized when no token is given' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch") + expect(response).to have_http_status(:unauthorized) + end + + context 'when destroyer is not an experience-cs admin' do + let(:destroyer) { build(:user) } + + it 'responds 403 Forbidden' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:) + expect(response).to have_http_status(:forbidden) + end + end + + context 'when project is not public' do + let(:user_id) { SecureRandom.uuid } + + it 'responds 403 Forbidden' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:) + expect(response).to have_http_status(:forbidden) + end + end + + context 'when project has one or more remixes' do + before do + project.remixes.create!(attributes_for(:project)) + end + + it 'responds 403 Forbidden' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:) + expect(response).to have_http_status(:forbidden) + end + end + + it 'responds 404 Not Found when project is not found' do + delete('/api/public_projects/another-identifier?project_type=scratch', headers:) + expect(response).to have_http_status(:not_found) + end +end diff --git a/spec/features/public_project/updating_a_public_project_spec.rb b/spec/features/public_project/updating_a_public_project_spec.rb new file mode 100644 index 000000000..3dd93d304 --- /dev/null +++ b/spec/features/public_project/updating_a_public_project_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Updating a public project', type: :request do + let(:creator) { build(:experience_cs_admin_user) } + let(:user_id) { nil } + let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id:) } + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } } + + before do + authenticated_in_hydra_as(creator) + end + + it 'responds 200 OK' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:) + expect(response).to have_http_status(:success) + end + + it 'responds with the project JSON' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data).to include(identifier: 'new-identifier', name: 'New name') + end + + it 'does not change locale on project even if one is supplied' do + locale = 'fr' + + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: params.merge(locale:)) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data).to include(locale: 'en') + end + + it 'does not change project_type on project even if one is supplied' do + project_type = Project::Types::PYTHON + + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: params.merge(project_type:)) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data).to include(project_type: Project::Types::SCRATCH) + end + + it 'does not set user_id on project even if one is supplied' do + user_id = SecureRandom.uuid + + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: params.merge(user_id:)) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data).to include(user_id: nil) + end + + context 'when creator is not an experience-cs admin' do + let(:creator) { build(:user) } + + it 'responds 403 Forbidden' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + it 'responds 400 Bad Request when params are malformed' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: {}) + expect(response).to have_http_status(:bad_request) + end + + it 'responds 401 Unauthorized when no token is given' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", params:) + expect(response).to have_http_status(:unauthorized) + end + + context 'when project is not public' do + let(:user_id) { SecureRandom.uuid } + + it 'responds 403 Forbidden' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + it 'responds 404 Not Found when project is not found' do + put('/api/public_projects/another-identifier?project_type=scratch', headers:, params:) + expect(response).to have_http_status(:not_found) + end + + it 'responds 422 Unprocessable Entity when params are invalid' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params: { project: { name: '' } }) + expect(response).to have_http_status(:unprocessable_entity) + end +end diff --git a/spec/lib/phrase_identifier_spec.rb b/spec/lib/phrase_identifier_spec.rb index 6da694d1f..f6a078542 100644 --- a/spec/lib/phrase_identifier_spec.rb +++ b/spec/lib/phrase_identifier_spec.rb @@ -4,6 +4,24 @@ require 'phrase_identifier' RSpec.describe PhraseIdentifier do + describe 'PATTERN' do + subject(:pattern) { described_class::PATTERN } + + it { is_expected.to match('abc-def-ghi') } + it { is_expected.to match('123-456-789') } + it { is_expected.to match('a2c-d5f-g8i') } + + it { is_expected.not_to match('Abc-def-ghi') } + it { is_expected.not_to match('Abc-def-GHI') } + it { is_expected.not_to match('abc--def-ghi') } + it { is_expected.not_to match('-abc-def-ghi') } + it { is_expected.not_to match('abc-def-ghi-') } + it { is_expected.not_to match('abc def-ghi') } + it { is_expected.not_to match(' abc-def-ghi') } + it { is_expected.not_to match('abc-def-ghi ') } + it { is_expected.not_to match('abc_def_ghi') } + end + describe '#generate' do subject(:generate) { described_class.generate } @@ -18,6 +36,14 @@ it { is_expected.to match phrase_regex } end + context 'when using the default words.txt file' do + it 'returns identifiers conforming to the expected pattern' do + 10.times do + expect(generate).to match(described_class::PATTERN) + end + end + end + context 'when there are no available combinations' do let(:identifier) { Faker::Verb.base } diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 39624d64e..1e8d89908 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -29,6 +29,10 @@ it { is_expected.not_to be_able_to(:update, project) } it { is_expected.not_to be_able_to(:destroy, project) } end + + it { is_expected.not_to be_able_to(:create, :public_project) } + it { is_expected.not_to be_able_to(:update, :public_project) } + it { is_expected.not_to be_able_to(:destroy, :public_project) } end context 'with a standard user' do @@ -56,6 +60,10 @@ it { is_expected.not_to be_able_to(:update, another_project) } it { is_expected.not_to be_able_to(:destroy, another_project) } end + + it { is_expected.not_to be_able_to(:create, :public_project) } + it { is_expected.not_to be_able_to(:update, :public_project) } + it { is_expected.not_to be_able_to(:destroy, :public_project) } end context 'with a teacher' do @@ -83,6 +91,10 @@ it { is_expected.not_to be_able_to(:update, another_project) } it { is_expected.not_to be_able_to(:destroy, another_project) } end + + it { is_expected.not_to be_able_to(:create, :public_project) } + it { is_expected.not_to be_able_to(:update, :public_project) } + it { is_expected.not_to be_able_to(:destroy, :public_project) } end context 'with an owner' do @@ -110,6 +122,10 @@ it { is_expected.not_to be_able_to(:update, another_project) } it { is_expected.not_to be_able_to(:destroy, another_project) } end + + it { is_expected.not_to be_able_to(:create, :public_project) } + it { is_expected.not_to be_able_to(:update, :public_project) } + it { is_expected.not_to be_able_to(:destroy, :public_project) } end context 'with a student' do @@ -137,6 +153,18 @@ it { is_expected.not_to be_able_to(:update, another_project) } it { is_expected.not_to be_able_to(:destroy, another_project) } end + + it { is_expected.not_to be_able_to(:create, :public_project) } + it { is_expected.not_to be_able_to(:update, :public_project) } + it { is_expected.not_to be_able_to(:destroy, :public_project) } + end + + context 'with an experience-cs admin' do + let(:user) { build(:experience_cs_admin_user) } + + it { is_expected.to be_able_to(:create, :public_project) } + it { is_expected.to be_able_to(:update, :public_project) } + it { is_expected.to be_able_to(:destroy, :public_project) } end # rubocop:disable RSpec/MultipleMemoizedHelpers diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6102ac091..ab56c8471 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -154,12 +154,19 @@ end end - describe 'check_unique_not_null' do - let(:saved_project) { create(:project) } - + describe 'generate_identifier' do it 'generates an identifier if nil' do - unsaved_project = build(:project, identifier: nil) - expect { unsaved_project.valid? }.to change { unsaved_project.identifier.nil? }.from(true).to(false) + project = build(:project, identifier: nil) + project.valid? + expect(project.identifier).not_to be_nil + end + + context 'when skip_identifier_generation is true' do + it 'does not generate an identifier if nil' do + project = build(:project, identifier: nil, skip_identifier_generation: true) + project.valid? + expect(project.identifier).to be_nil + end end end diff --git a/spec/models/public_project_spec.rb b/spec/models/public_project_spec.rb new file mode 100644 index 000000000..38e9a3a36 --- /dev/null +++ b/spec/models/public_project_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe PublicProject do + subject(:public_project) { described_class.new(project) } + + let(:project) { build(:project, skip_identifier_generation: true, identifier: 'valid-identifier') } + + describe 'validation' do + context 'when both project and public project are valid' do + it { is_expected.to be_valid } + + it 'has no errors' do + expect(public_project.errors).to be_empty + end + + it 'persists project on save!' do + public_project.save! + expect(project).to be_persisted + end + end + + context 'when project is not valid' do + before do + project.locale = nil + project.user_id = nil + end + + it { is_expected.not_to be_valid } + + it 'has the project error' do + public_project.valid? + expect(public_project.errors.full_messages).to include("Locale can't be blank") + end + + it 'raises validation error on save!' do + expect do + public_project.save! + end.to raise_error( + an_instance_of(ActiveModel::ValidationError) + .and(having_attributes(message: "Validation failed: Locale can't be blank")) + ) + end + + it 'does not persist project on save!' do + public_project.save! + rescue ActiveModel::ValidationError + expect(project).not_to be_persisted + end + end + + context 'when public project is not valid' do + before do + project.identifier = 'InvalidIdentifier' + end + + it { is_expected.not_to be_valid } + + it 'has the public project error' do + public_project.valid? + expect(public_project.errors.full_messages).to include('Identifier is invalid') + end + + it 'raises validation error on save!' do + expect do + public_project.save! + end.to raise_error( + an_instance_of(ActiveModel::ValidationError) + .and(having_attributes(message: 'Validation failed: Identifier is invalid')) + ) + end + + it 'does not persist project on save!' do + public_project.save! + rescue ActiveModel::ValidationError + expect(project).not_to be_persisted + end + end + + context 'when both project and public project are not valid' do + before do + project.locale = nil + project.user_id = nil + project.identifier = 'InvalidIdentifier' + end + + it { is_expected.not_to be_valid } + + it 'has the project error' do + public_project.valid? + expect(public_project.errors.full_messages).to include("Locale can't be blank") + end + + it 'has the public project error' do + public_project.valid? + expect(public_project.errors.full_messages).to include('Identifier is invalid') + end + + it 'raises validation error including both errors on save!' do + expect do + public_project.save! + end.to raise_error( + an_instance_of(ActiveModel::ValidationError) + .and(having_attributes(message: "Validation failed: Identifier is invalid, Locale can't be blank")) + ) + end + + it 'does not persist project on save!' do + public_project.save! + rescue ActiveModel::ValidationError + expect(project).not_to be_persisted + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 103e90ada..7cc065aa6 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(:user, roles: 'experience-cs-admin') + 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/public_projects/create_spec.rb b/spec/requests/public_projects/create_spec.rb new file mode 100644 index 000000000..6a9825822 --- /dev/null +++ b/spec/requests/public_projects/create_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Create public project requests' do + let(:project) { create(:project) } + let(:creator) { build(:experience_cs_admin_user) } + let(:params) { { project: { project_type: Project::Types::SCRATCH } } } + + context 'when auth is correct' do + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + context 'when creating project is successful' do + before do + authenticated_in_hydra_as(creator) + + response = OperationResponse.new + response[:project] = project + allow(PublicProject::Create).to receive(:call).and_return(response) + end + + it 'returns success' do + post('/api/public_projects', headers:, params:) + + expect(response).to have_http_status(:created) + end + end + + context 'when creating project fails' do + before do + authenticated_in_hydra_as(creator) + + response = OperationResponse.new + response[:error] = 'Error creating project' + allow(PublicProject::Create).to receive(:call).and_return(response) + end + + it 'returns error' do + post('/api/public_projects', headers:, params:) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + context 'when no token is given' do + it 'returns unauthorized' do + post('/api/public_projects', params:) + + expect(response).to have_http_status(:unauthorized) + end + end +end diff --git a/spec/requests/public_projects/destroy_spec.rb b/spec/requests/public_projects/destroy_spec.rb new file mode 100644 index 000000000..3b50e5404 --- /dev/null +++ b/spec/requests/public_projects/destroy_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Destroy public project requests' do + let(:locale) { 'fr' } + let(:project_loader) { instance_double(ProjectLoader) } + let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id: nil) } + let(:destroyer) { build(:experience_cs_admin_user) } + + context 'when auth is correct' do + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + before do + allow(ProjectLoader).to receive(:new).and_return(project_loader) + allow(project_loader).to receive(:load).and_return(project) + end + + context 'when destroying project is successful' do + before do + authenticated_in_hydra_as(destroyer) + + allow(project).to receive(:destroy).and_return(true) + end + + it 'builds ProjectLoader with identifier & locale' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:) + + expect(ProjectLoader).to have_received(:new).with(project.identifier, [locale]) + end + + it 'uses ProjectLoader#load to find the project based on identifier & locale' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:) + + expect(project_loader).to have_received(:load) + end + + it 'returns success' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:) + + expect(response).to have_http_status(:success) + end + end + + context 'when destroying project fails' do + before do + authenticated_in_hydra_as(destroyer) + + allow(project).to receive(:destroy).and_return(false) + end + + it 'returns error' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + context 'when no token is given' do + it 'returns unauthorized' do + delete("/api/public_projects/#{project.identifier}?project_type=scratch") + + expect(response).to have_http_status(:unauthorized) + end + end +end diff --git a/spec/requests/public_projects/update_spec.rb b/spec/requests/public_projects/update_spec.rb new file mode 100644 index 000000000..ef591c13f --- /dev/null +++ b/spec/requests/public_projects/update_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Update public project requests' do + let(:locale) { 'fr' } + let(:project_loader) { instance_double(ProjectLoader) } + let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id: nil) } + let(:creator) { build(:experience_cs_admin_user) } + let(:params) { { project: { identifier: 'new-identifier', name: 'New name' } } } + + context 'when auth is correct' do + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + context 'when updating project is successful' do + before do + authenticated_in_hydra_as(creator) + + allow(ProjectLoader).to receive(:new).and_return(project_loader) + allow(project_loader).to receive(:load).and_return(project) + + response = OperationResponse.new + response[:project] = project + allow(PublicProject::Update).to receive(:call).and_return(response) + end + + it 'builds ProjectLoader with identifier & locale' do + put("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:, params:) + + expect(ProjectLoader).to have_received(:new).with(project.identifier, [locale]) + end + + it 'uses ProjectLoader#load to find the project based on identifier & locale' do + put("/api/public_projects/#{project.identifier}?project_type=scratch&locale=#{locale}", headers:, params:) + + expect(project_loader).to have_received(:load) + end + + it 'returns success' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:) + + expect(response).to have_http_status(:success) + end + end + + context 'when updating project fails' do + before do + authenticated_in_hydra_as(creator) + + response = OperationResponse.new + response[:error] = 'Error updating project' + allow(PublicProject::Update).to receive(:call).and_return(response) + end + + it 'returns error' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:) + + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + context 'when no token is given' do + it 'returns unauthorized' do + put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:) + + expect(response).to have_http_status(:unauthorized) + end + end +end 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