diff --git a/Gemfile b/Gemfile index 6c98af75b..f36c26e60 100644 --- a/Gemfile +++ b/Gemfile @@ -134,7 +134,6 @@ group :test do gem 'capybara' gem 'capybara-email' gem 'cucumber-rails', '~> 1.8', require: false - gem 'database_cleaner' gem 'json_matchers' gem 'selenium-webdriver' gem 'shoulda-matchers' diff --git a/Gemfile.lock b/Gemfile.lock index e536f5d83..69c3ef9c5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -236,7 +236,6 @@ GEM railties (>= 4.2, < 7) cucumber-tag_expressions (1.1.1) cucumber-wire (0.0.1) - database_cleaner (1.8.5) debug_inspector (1.0.0) deface (1.6.1) nokogiri (>= 1.6) diff --git a/app/models/concerns/assignable.rb b/app/models/concerns/assignable.rb index 471444ad6..c51665693 100644 --- a/app/models/concerns/assignable.rb +++ b/app/models/concerns/assignable.rb @@ -12,7 +12,7 @@ module Assignable UserAssignment.create( user: created_by, assignable: self, - user_role: UserRole.find_by(name: 'Owner') + user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) ) UserAssignments::GenerateUserAssignmentsJob.perform_later(self, created_by) diff --git a/app/models/project.rb b/app/models/project.rb index 7a1abd5a8..3bc1788eb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -367,11 +367,11 @@ class Project < ApplicationRecord end def assign_project_ownership - UserAssignment.create( + UserAssignment.create!( user: created_by, assignable: self, assigned: :manually, # we set this to manually since was the user action to create the project - user_role: UserRole.find_by(name: 'Owner') + user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) ) end diff --git a/app/models/project_member.rb b/app/models/project_member.rb index 0a43305f9..3ea66ea51 100644 --- a/app/models/project_member.rb +++ b/app/models/project_member.rb @@ -130,6 +130,6 @@ class ProjectMember def project_owners @project_owners ||= @project.user_assignments .includes(:user_role) - .where(user_roles: { name: 'Owner' }) + .where(user_roles: { name: I18n.t('user_roles.predefined.owner') }) end end diff --git a/app/models/user_role.rb b/app/models/user_role.rb index b73ce5568..3661e1c92 100644 --- a/app/models/user_role.rb +++ b/app/models/user_role.rb @@ -84,7 +84,7 @@ class UserRole < ApplicationRecord end def owner? - name == 'Owner' + name == I18n.t('user_roles.predefined.owner') end private diff --git a/app/models/user_team.rb b/app/models/user_team.rb index cc337961a..71d0caa3f 100644 --- a/app/models/user_team.rb +++ b/app/models/user_team.rb @@ -9,8 +9,8 @@ class UserTeam < ApplicationRecord belongs_to :assigned_by, foreign_key: 'assigned_by_id', class_name: 'User', optional: true belongs_to :team, inverse_of: :user_teams - before_destroy :destroy_associations after_create :assign_user_to_visible_projects + before_destroy :destroy_associations def role_str I18n.t("user_teams.enums.role.#{role}") diff --git a/features/support/env.rb b/features/support/env.rb index e5ecdae3a..7c17c7dc4 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -54,31 +54,6 @@ Capybara.javascript_driver = :chrome # ActionController::Base.allow_rescue = false -# Remove/comment out the lines below if your app doesn't have a database. -# For some databases (like MongoDB and CouchDB) you may need to use :truncation instead. -begin - require 'database_cleaner' - require 'database_cleaner/cucumber' - DatabaseCleaner.strategy = :truncation -rescue NameError - raise "You need to add database_cleaner to your Gemfile (in the :test group) if you wish to use it." -end - -# You may also want to configure DatabaseCleaner to use different strategies for certain features and scenarios. -# See the DatabaseCleaner documentation for details. Example: -# -# Before('@no-txn,@selenium,@culerity,@celerity,@javascript') do -# # { except: [:widgets] } may not do what you expect here -# # as Cucumber::Rails::Database.javascript_strategy overrides -# # this setting. -# DatabaseCleaner.strategy = :truncation -# end -# -# Before('not @no-txn', 'not @selenium', 'not @culerity', 'not @celerity', 'not @javascript') do -# DatabaseCleaner.strategy = :transaction -# end -# - # Possible values are :truncation and :transaction # The :transaction strategy is faster, but might give you threading problems. # See https://github.com/cucumber/cucumber-rails/blob/master/features/choose_javascript_database_strategy.feature diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 1d95a3395..01f7ed283 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -4,65 +4,11 @@ require 'rails_helper' describe ProjectsController, type: :controller do login_user - render_views - PROJECTS_CNT = 26 - time = Time.new(2015, 8, 1, 14, 35, 0) - let!(:user) { User.first } - let!(:team) { create :team, created_by: user } - let!(:user_team) { create :user_team, team: team, user: user } - let!(:owner_role) { create :owner_role } - before do - @projects_overview = ProjectsOverviewService.new(team, user, nil, params) - end - - let!(:project_1) do - create :project, name: 'test project D', visibility: 1, team: team, - archived: false, created_at: time.advance(hours: 2), - created_by: user - end - let!(:project_2) do - create :project, name: 'test project B', visibility: 1, team: team, - archived: true, created_at: time, created_by: user - end - let!(:project_3) do - create :project, name: 'test project C', visibility: 1, team: team, - archived: false, created_at: time.advance(hours: 3), - created_by: user - end - let!(:project_4) do - create :project, name: 'test project A', visibility: 1, team: team, - archived: true, created_at: time.advance(hours: 1), - created_by: user - end - let!(:project_5) do - create :project, name: 'test project E', visibility: 1, team: team, - archived: true, created_at: time.advance(hours: 5), - created_by: user - end - let!(:project_6) do - create :project, name: 'test project F', visibility: 0, team: team, - archived: false, created_at: time.advance(hours: 4), - created_by: user - end - (7..PROJECTS_CNT).each do |i| - let!("project_#{i}") do - create :project, name: "test project #{(64 + i).chr}", - visibility: 1, - team: team, archived: i % 2, - created_at: time.advance(hours: 6, minutes: i), - created_by: user - end - end - - - - - before do - PROJECTS_CNT.times do |i| - create_user_assignment(public_send("project_#{i+1}"), owner_role, user) - end - end + include_context 'reference_project_structure', { + projects: 3, + skip_my_module: true + } describe '#index' do let(:params) { { team: team.id, sort: 'atoz' } } @@ -107,7 +53,7 @@ describe ProjectsController, type: :controller do describe '#edit' do context 'in JSON format' do - let(:params) { { id: project_1.id } } + let(:params) { { id: projects.first.id } } it 'returns success response' do get :edit, params: params, format: :json @@ -121,9 +67,9 @@ describe ProjectsController, type: :controller do context 'in HTML format' do let(:action) { put :update, params: params } let(:params) do - { id: project_1.id, - project: { name: project_1.name, team_id: project_1.team.id, - visibility: project_1.visibility } } + { id: projects.first.id, + project: { name: projects.first.name, team_id: projects.first.team.id, + visibility: projects.first.visibility } } end it 'returns redirect response' do @@ -133,7 +79,7 @@ describe ProjectsController, type: :controller do end it 'calls create activity service (change_project_visibility)' do - params[:project][:visibility] = 'hidden' + params[:project][:visibility] = 'visible' expect(Activities::CreateActivityService).to receive(:call) .with(hash_including(activity_type: :change_project_visibility)) action @@ -147,7 +93,7 @@ describe ProjectsController, type: :controller do end it 'calls create activity service (restore_project)' do - project_1.update(archived: true) + projects.first.update(archived: true) params[:project][:archived] = false expect(Activities::CreateActivityService).to receive(:call) .with(hash_including(activity_type: :restore_project)) @@ -172,7 +118,7 @@ describe ProjectsController, type: :controller do describe '#show' do context 'in HTML format' do let(:params) do - { id: project_1.id, sort: 'old', + { id: projects.first.id, sort: 'old', project: { name: 'test project A1', team_id: team.id, visibility: 'visible' } } end @@ -188,7 +134,7 @@ describe ProjectsController, type: :controller do describe '#notifications' do context 'in JSON format' do let(:params) do - { id: project_1.id, + { id: projects.first.id, project: { name: 'test project A1', team_id: team.id, visibility: 'visible' } } end @@ -204,7 +150,7 @@ describe ProjectsController, type: :controller do describe '#experiment_archive' do context 'in HTML format' do let(:params) do - { id: project_1.id, + { id: projects.first.id, view_mode: :archived, project: { name: 'test project A1', team_id: team.id, visibility: 'visible' } } diff --git a/spec/permissions/controllers/project_comments_controller_spec.rb b/spec/permissions/controllers/project_comments_controller_spec.rb new file mode 100644 index 000000000..272c48380 --- /dev/null +++ b/spec/permissions/controllers/project_comments_controller_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ProjectCommentsController, type: :controller do + include PermissionExtends + + it_behaves_like "a controller with authentication", { + index: { project_id: 1, id: 1 }, + create: { project_id: 1, id: 1 }, + update: { project_id: 1, id: 1 }, + destroy: { project_id: 1, id: 1 } + }, [] + + login_user + + describe 'permissions checking' do + include_context 'reference_project_structure', { + team_role: :normal_user, + project_comment: true, + skip_my_module: true + } + + it_behaves_like "a controller action with permissions checking", :get, :index do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::READ] } + let(:action_params) { { project_id: project.id } } + end + + it_behaves_like "a controller action with permissions checking", :post, :create do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::COMMENTS_CREATE] } + let(:action_params) { { project_id: project.id, comment: { message: 'Test' } } } + end + + it_behaves_like "a controller action with permissions checking", :put, :update do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::COMMENTS_MANAGE] } + let(:action_params) { { project_id: project.id, id: project_comment.id, comment: { message: 'Test1' } } } + end + + it_behaves_like "a controller action with permissions checking", :post, :destroy do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::COMMENTS_MANAGE] } + let(:action_params) { { project_id: project.id, id: project_comment.id } } + end + end +end diff --git a/spec/permissions/controllers/projects_controller_spec.rb b/spec/permissions/controllers/projects_controller_spec.rb new file mode 100644 index 000000000..5ccc23676 --- /dev/null +++ b/spec/permissions/controllers/projects_controller_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe ProjectsController, type: :controller do + include PermissionExtends + + it_behaves_like "a controller with authentication", { + sidebar: { id: 1 }, + edit: { id: 1 }, + update: { id: 1 }, + show: { id: 1 }, + experiments_cards: { id: 1 }, + notifications: { id: 1 }, + view_type: { id: 1 } + }, [:current_folder] + + login_user + + describe 'permissions checking' do + include_context 'reference_project_structure', { + team_role: :normal_user, + projects: 5, + skip_my_module: true + } + + it_behaves_like "a controller action with permissions checking", :get, :sidebar do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::READ] } + let(:action_params) { { id: project.id } } + end + + it_behaves_like "a controller action with permissions checking", :put, :update do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::MANAGE] } + let(:action_params) { { id: project.id, project: { name: 'Test' } } } + end + + it_behaves_like "a controller action with permissions checking", :post, :archive_group do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::MANAGE] } + let(:action_params) { { projects_ids: [project.id] } } + let(:custom_response_status) { :unprocessable_entity } + end + + it_behaves_like "a controller action with permissions checking", :post, :restore_group do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::MANAGE] } + let(:action_params) { { projects_ids: [project.id] } } + let(:custom_response_status) { :unprocessable_entity } + end + + it_behaves_like "a controller action with permissions checking", :get, :show do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::READ] } + let(:action_params) { { id: project.id } } + end + + it_behaves_like "a controller action with permissions checking", :get, :experiments_cards do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::READ] } + let(:action_params) { { id: project.id } } + end + + it_behaves_like "a controller action with permissions checking", :get, :notifications do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::READ] } + let(:action_params) { { id: project.id } } + end + + it_behaves_like "a controller action with permissions checking", :put, :view_type do + let(:testable) { project } + let(:permissions) { [ProjectPermissions::READ] } + let(:action_params) { { id: project.id } } + end + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index f310a803e..f81907521 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,7 +1,6 @@ # This file is copied to spec/ when you run 'rails generate rspec:install' require 'spec_helper' require 'shoulda-matchers' -require 'database_cleaner' require 'devise' require_relative 'support/controller_macros' ENV['RAILS_ENV'] = 'test' @@ -40,50 +39,10 @@ RSpec.configure do |config| # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures config.fixture_path = "#{::Rails.root}/spec/fixtures" - # If you're not using ActiveRecord, or you'd prefer not to run each of your - # examples within a transaction, remove the following line or assign false - # instead of true. - # http://www.virtuouscode.com/2012/08/31/configuring-database_cleaner-with-rails-rspec-capybara-and-selenium/ - config.use_transactional_fixtures = false - config.before(:suite) do - DatabaseCleaner.clean_with(:truncation) - end + config.use_transactional_fixtures = true - config.before(:each) do - DatabaseCleaner.strategy = :transaction - end + Delayed::Worker.delay_jobs = false - config.before(:each, js: true) do - DatabaseCleaner.strategy = :truncation - end - - config.before(:each) do - DatabaseCleaner.start - end - - config.after(:each) do - DatabaseCleaner.clean - end - - config.before(:all) do - DatabaseCleaner.strategy = :transaction - end - - config.before(:all) do - DatabaseCleaner.start - end - - config.after(:all) do - DatabaseCleaner.clean - end - - config.around(:each, type: :background_job) do |example| - run_background_jobs_immediately do - example.run - end - end - - config.include BackgroundJobs # RSpec Rails can automatically mix in different behaviours to your tests # based on their file location, for example enabling you to call `get` and # `post` in specs under `spec/controllers`. @@ -111,6 +70,7 @@ RSpec.configure do |config| end # Devise config.include Devise::Test::ControllerHelpers, type: :controller + config.include Devise::Test::IntegrationHelpers, type: :system config.include ApiHelper, type: :controller config.include ApiHelper, type: :request config.extend ControllerMacros, type: :controller diff --git a/spec/services/notifications/handle_system_notification_in_communication_channel_service_spec.rb b/spec/services/notifications/handle_system_notification_in_communication_channel_service_spec.rb index 5d4450ae3..d9786201b 100644 --- a/spec/services/notifications/handle_system_notification_in_communication_channel_service_spec.rb +++ b/spec/services/notifications/handle_system_notification_in_communication_channel_service_spec.rb @@ -12,14 +12,6 @@ describe Notifications::HandleSystemNotificationInCommunicationChannelService do Notifications::HandleSystemNotificationInCommunicationChannelService.call(system_notification) end - before do - Delayed::Worker.delay_jobs = false - end - - after do - Delayed::Worker.delay_jobs = true - end - context 'when user has enabled notifications' do it 'calls AppMailer' do allow_any_instance_of(User).to receive(:system_message_email_notification).and_return(true) diff --git a/spec/services/notifications/sync_system_notifications_service_spec.rb b/spec/services/notifications/sync_system_notifications_service_spec.rb index a2a2784d9..3ea110d1f 100644 --- a/spec/services/notifications/sync_system_notifications_service_spec.rb +++ b/spec/services/notifications/sync_system_notifications_service_spec.rb @@ -84,11 +84,9 @@ describe Notifications::SyncSystemNotificationsService do end it 'calls service to notify users about notification' do - Delayed::Worker.delay_jobs = false expect(Notifications::PushToCommunicationChannelService).to receive(:call).exactly(10) service_call - Delayed::Worker.delay_jobs = true end end @@ -113,11 +111,9 @@ describe Notifications::SyncSystemNotificationsService do end it 'does not call service to notify users about notification' do - Delayed::Worker.delay_jobs = false expect(Notifications::PushToCommunicationChannelService).to_not receive(:call) service_call - Delayed::Worker.delay_jobs = true end end end diff --git a/spec/support/background_job.rb b/spec/support/background_job.rb deleted file mode 100644 index 7cd13c2f7..000000000 --- a/spec/support/background_job.rb +++ /dev/null @@ -1,8 +0,0 @@ -module BackgroundJobs - def run_background_jobs_immediately - delay_jobs = Delayed::Worker.delay_jobs - Delayed::Worker.delay_jobs = false - yield - Delayed::Worker.delay_jobs = delay_jobs - end -end diff --git a/spec/support/reference_project_structure_context.rb b/spec/support/reference_project_structure_context.rb index c575945ef..063b23a48 100644 --- a/spec/support/reference_project_structure_context.rb +++ b/spec/support/reference_project_structure_context.rb @@ -13,15 +13,20 @@ # } RSpec.shared_context 'reference_project_structure' do |config| - config ||= {} - let(:user) { subject.current_user } - let(:role) { create (config[:role] || :owner_role) } unless config[:skip_role] + let!(:user) { subject.current_user } + let!(:role) { create (config[:role] || :owner_role) } let!(:team) { config[:team] || (create :team, created_by: user) } - let!(:user_team) { create :user_team, :admin, user: user, team: team } - let(:project) { create :project, team: team } - let(:experiment) { create :experiment, project: project } - let(:my_module) { create :my_module, experiment: experiment } unless config[:skip_my_module] + let!(:user_team) { create :user_team, config[:team_role] || :admin, user: user, team: team } + + let!(:project) { create :project, team: team, created_by: user } + let!(:projects) { create_list :project, config[:projects], team: team, created_by: user } if config[:projects] + + let!(:experiment) { create :experiment, project: project } unless config[:skip_experiment] + let!(:experiments) { create_list :experiment, config[:experiments], project: project } if config[:experiments] + + let!(:my_module) { create :my_module, experiment: experiment } unless config[:skip_my_module] + let!(:my_modules) { create_list :my_module, config[:my_modules], experiment: experiment } if config[:my_modules] let(:tag) { create :tag, project: project} if config[:tag] let(:tags) { create_list :tag, config[:tags], project: project} if config[:tags] diff --git a/spec/support/shared_examples/controller_action_with_permission_checking.rb b/spec/support/shared_examples/controller_action_with_permission_checking.rb new file mode 100644 index 000000000..ff06c73aa --- /dev/null +++ b/spec/support/shared_examples/controller_action_with_permission_checking.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.shared_examples 'a controller action with permissions checking' do |verb, action| + + describe 'controller action' do + context 'user without permissions' do + it "returns forbidden response for action :#{action}" do + testable_role = testable.user_assignments.find_by(user: user ).user_role + testable_role.update_column(:permissions, testable_role.permissions - permissions) + send(verb, action, params: defined?(action_params) ? action_params : {}) + expect(response).to have_http_status(defined?(custom_response_status) ? custom_response_status : :forbidden) + end + end + end +end diff --git a/spec/support/shared_examples/controller_with_authentication.rb b/spec/support/shared_examples/controller_with_authentication.rb new file mode 100644 index 000000000..50da97791 --- /dev/null +++ b/spec/support/shared_examples/controller_with_authentication.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.shared_examples 'a controller with authentication' do |actions_with_params, actions_to_skip| + let (:protected_actions) { described_class.instance_methods(false) - (actions_to_skip || []) } + + describe 'controller actions' do + context 'unauthenticated user' do + it 'returns forbidden response for all actions' do + sign_out :user + actions_with_params ||= {} + protected_actions.each do |action| + params = actions_with_params[action] + get action, params: params + expect(response).to have_http_status(:forbidden).or redirect_to('/users/sign_in') + end + end + end + end +end