From f68d724202a87274437817f6242e6f40ba3881bc Mon Sep 17 00:00:00 2001 From: artoscinote <85488244+artoscinote@users.noreply.github.com> Date: Thu, 30 Sep 2021 11:32:11 +0200 Subject: [PATCH] Rework experiment permissions [SCI-6054] (#3538) * Rework experiment permissions [SCI-6054] --- Gemfile | 1 + Gemfile.lock | 6 ++ .../experiments_controller.rb | 2 +- app/controllers/api/v1/tasks_controller.rb | 2 +- app/controllers/experiments_controller.rb | 29 ++++--- app/controllers/my_modules_controller.rb | 6 +- .../project_comments_controller.rb | 6 +- app/controllers/result_comments_controller.rb | 2 +- app/controllers/step_comments_controller.rb | 15 +++- app/helpers/comment_helper.rb | 4 +- .../generate_user_assignments_job.rb | 5 +- app/models/concerns/assignable.rb | 10 ++- app/models/experiment.rb | 13 ++- app/models/my_module.rb | 10 ++- app/models/project.rb | 4 +- app/models/user.rb | 4 +- app/models/user_role.rb | 84 ++---------------- app/permissions/experiment.rb | 47 +++++----- app/permissions/my_module.rb | 48 +++++++---- app/permissions/project.rb | 18 +++- app/permissions/result.rb | 6 +- .../experiments/move_to_project_service.rb | 7 +- app/services/experiments_overview_service.rb | 3 +- app/services/report_actions/report_content.rb | 2 +- app/services/templates_service.rb | 3 +- .../_experiment_actions_dropdown.html.erb | 2 +- .../results/partials/_asset_text.html.erb | 2 +- .../shared/sidebar/_experiments.html.erb | 2 +- .../extends/permission_extends.rb | 85 +++++++++++++++++++ features/support/env.rb | 25 ++++++ .../experiments_controller_spec.rb | 4 +- .../my_modules_controller_spec.rb | 6 +- .../projects_controller_spec.rb | 2 +- spec/controllers/assets_controller_spec.rb | 2 +- spec/controllers/canvas_controller_spec.rb | 16 ++-- .../my_module_comments_controller_spec.rb | 2 +- .../my_module_repositories_controller_spec.rb | 8 +- .../controllers/my_modules_controller_spec.rb | 10 +-- spec/controllers/reports_controller_spec.rb | 2 +- spec/controllers/teams_controller_spec.rb | 1 - spec/factories/experiments.rb | 2 +- spec/factories/my_modules.rb | 4 + spec/factories/user_roles.rb | 38 +-------- .../activities/dispatch_webhooks_job_spec.rb | 2 +- spec/jobs/activities/send_webhook_job_spec.rb | 4 +- .../generate_user_assignments_job_spec.rb | 23 +++-- .../propagate_assignment_job_spec.rb | 8 +- spec/lib/tasks/my_modules_spec.rb | 2 +- spec/models/experiment_member_spec.rb | 3 +- spec/models/my_module_member_spec.rb | 5 +- spec/models/project_member_spec.rb | 13 +-- spec/models/project_spec.rb | 2 +- .../controllers/canvas_controller_spec.rb | 2 +- spec/permissions/result_permission_spec.rb | 5 +- spec/rails_helper.rb | 41 ++++++++- .../requests/api/v1/assets_controller_spec.rb | 2 +- .../api/v1/checklist_items_controller_spec.rb | 2 +- .../api/v1/checklists_controller_spec.rb | 2 +- ...riment_user_assignments_controller_spec.rb | 2 +- .../api/v1/experiments_controller_spec.rb | 2 +- ...roject_user_assignments_controller_spec.rb | 2 +- .../api/v1/projects_controller_spec.rb | 2 +- .../api/v1/results_controller_spec.rb | 2 +- spec/requests/api/v1/steps_controller_spec.rb | 2 +- .../requests/api/v1/tables_controller_spec.rb | 2 +- .../task_user_assignments_controller_spec.rb | 4 +- spec/requests/api/v1/tasks_controller_spec.rb | 2 +- .../move_to_project_service_spec.rb | 9 +- .../experiments_overview_service_spec.rb | 2 +- .../smart_annotations/html_preview_spec.rb | 2 +- .../smart_annotations/permission_eval_spec.rb | 4 +- .../smart_annotations/tag_to_html_spec.rb | 2 +- .../smart_annotations/tag_to_text_spec.rb | 2 +- .../smart_annotations/text_preview_spec.rb | 2 +- .../reference_project_structure_context.rb | 16 ++-- ...troller_action_with_permission_checking.rb | 2 +- 76 files changed, 421 insertions(+), 306 deletions(-) diff --git a/Gemfile b/Gemfile index f36c26e60..6c98af75b 100644 --- a/Gemfile +++ b/Gemfile @@ -134,6 +134,7 @@ 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 69c3ef9c5..eb4f88504 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -236,6 +236,12 @@ GEM railties (>= 4.2, < 7) cucumber-tag_expressions (1.1.1) cucumber-wire (0.0.1) + database_cleaner (2.0.1) + database_cleaner-active_record (~> 2.0.0) + database_cleaner-active_record (2.0.1) + activerecord (>= 5.a) + database_cleaner-core (~> 2.0.0) + database_cleaner-core (2.0.1) debug_inspector (1.0.0) deface (1.6.1) nokogiri (>= 1.6) diff --git a/app/controllers/access_permissions/experiments_controller.rb b/app/controllers/access_permissions/experiments_controller.rb index 2f9daa937..fb8af1948 100644 --- a/app/controllers/access_permissions/experiments_controller.rb +++ b/app/controllers/access_permissions/experiments_controller.rb @@ -50,7 +50,7 @@ module AccessPermissions end def check_manage_permissions - render_403 unless can_manage_experiment_access?(@experiment) + render_403 unless can_manage_experiment_users?(@experiment) end def check_read_permissions diff --git a/app/controllers/api/v1/tasks_controller.rb b/app/controllers/api/v1/tasks_controller.rb index d29179ac4..26a26f621 100644 --- a/app/controllers/api/v1/tasks_controller.rb +++ b/app/controllers/api/v1/tasks_controller.rb @@ -36,7 +36,7 @@ module Api def create raise PermissionError.new(MyModule, :create) unless can_manage_experiment?(@experiment) - my_module = @experiment.my_modules.create!(task_params_create) + my_module = @experiment.my_modules.create!(task_params_create.merge(created_by: current_user)) render jsonapi: my_module, serializer: TaskSerializer, rte_rendering: render_rte?, diff --git a/app/controllers/experiments_controller.rb b/app/controllers/experiments_controller.rb index a937836cf..e99b7a07b 100644 --- a/app/controllers/experiments_controller.rb +++ b/app/controllers/experiments_controller.rb @@ -9,9 +9,11 @@ class ExperimentsController < ApplicationController before_action :load_project, only: %i(new create archive_group restore_group) before_action :load_experiment, except: %i(new create archive_group restore_group) - before_action :check_view_permissions, except: %i(edit archive clone move new create archive_group restore_group) + before_action :check_read_permissions, except: %i(edit archive clone move new create archive_group restore_group) + before_action :check_canvas_read_permissions, only: %i(canvas) before_action :check_create_permissions, only: %i(new create) before_action :check_manage_permissions, only: %i(edit) + before_action :check_update_permissions, only: %i(update) before_action :check_archive_permissions, only: :archive before_action :check_clone_permissions, only: %i(clone_modal clone) before_action :check_move_permissions, only: %i(move_modal move) @@ -82,12 +84,6 @@ class ExperimentsController < ApplicationController end def update - render_403 && return unless if experiment_params[:archived] == 'false' - can_restore_experiment?(@experiment) - else - can_manage_experiment?(@experiment) - end - old_text = @experiment.description @experiment.assign_attributes(experiment_params) @experiment.last_modified_by = current_user @@ -221,7 +217,7 @@ class ExperimentsController < ApplicationController # GET: move_modal_experiment_path(id) def move_modal - @projects = @experiment.moveable_projects(current_user) + @projects = @experiment.movable_projects(current_user) respond_to do |format| format.json do render json: { @@ -309,8 +305,13 @@ class ExperimentsController < ApplicationController params.require(:experiment).require(:project_id) end - def check_view_permissions - render_403 unless can_read_experiment?(@experiment) + def check_read_permissions + render_403 unless can_read_experiment?(@experiment) || + @experiment.archived? && can_read_archived_experiment?(@experiment) + end + + def check_canvas_read_permissions + render 403 unless can_read_experiment_canvas?(@experiment) end def check_create_permissions @@ -321,6 +322,14 @@ class ExperimentsController < ApplicationController render_403 unless can_manage_experiment?(@experiment) end + def check_update_permissions + if experiment_params[:archived] == 'false' + render_403 unless can_restore_experiment?(@experiment) + else + render_403 unless can_manage_experiment?(@experiment) + end + end + def check_archive_permissions render_403 unless can_archive_experiment?(@experiment) end diff --git a/app/controllers/my_modules_controller.rb b/app/controllers/my_modules_controller.rb index 97df0194c..85f0577a4 100644 --- a/app/controllers/my_modules_controller.rb +++ b/app/controllers/my_modules_controller.rb @@ -9,7 +9,7 @@ class MyModulesController < ApplicationController before_action :load_vars, except: %i(restore_group) before_action :check_archive_permissions, only: %i(update) before_action :check_manage_permissions, only: %i(description due_date update_description update_protocol_description) - before_action :check_view_permissions, except: %i(update update_description update_protocol_description restore_group) + before_action :check_read_permissions, except: %i(update update_description update_protocol_description restore_group) before_action :check_update_state_permissions, only: :update_state before_action :set_inline_name_editing, only: %i(protocols results activities archive) before_action :load_experiment_my_modules, only: %i(protocols results activities archive) @@ -327,8 +327,8 @@ class MyModulesController < ApplicationController return render_403 if my_module_params[:archived] == 'true' && !can_archive_my_module?(@my_module) end - def check_view_permissions - render_403 unless can_read_protocol_in_module?(@my_module.protocol) + def check_read_permissions + render_403 unless can_read_my_module?(@my_module) end def check_update_state_permissions diff --git a/app/controllers/project_comments_controller.rb b/app/controllers/project_comments_controller.rb index 7be3f3b77..4de02cdea 100644 --- a/app/controllers/project_comments_controller.rb +++ b/app/controllers/project_comments_controller.rb @@ -41,7 +41,7 @@ class ProjectCommentsController < ApplicationController def load_vars @last_comment_id = params[:from].to_i @per_page = Constants::COMMENTS_SEARCH_LIMIT - @project = Project.find_by_id(params[:project_id]) + @project = current_team.projects.find_by(id: params[:project_id]) render_404 unless @project end @@ -55,9 +55,9 @@ class ProjectCommentsController < ApplicationController end def check_manage_permissions - @comment = ProjectComment.find_by_id(params[:id]) + @comment = @project.project_comments.find_by(id: params[:id]) render_403 unless @comment.present? && - can_manage_project_comments?(@project) + can_manage_project_comment?(@comment) end def comment_params diff --git a/app/controllers/result_comments_controller.rb b/app/controllers/result_comments_controller.rb index e0d81d10f..a4ca7b14c 100644 --- a/app/controllers/result_comments_controller.rb +++ b/app/controllers/result_comments_controller.rb @@ -57,7 +57,7 @@ class ResultCommentsController < ApplicationController end def check_manage_permissions - @comment = ResultComment.find_by_id(params[:id]) + @comment = @result.result_comments.find_by(id: params[:id]) render_403 unless @comment.present? && can_manage_result_comment?(@comment) end diff --git a/app/controllers/step_comments_controller.rb b/app/controllers/step_comments_controller.rb index 405dd141e..fbecb8e16 100644 --- a/app/controllers/step_comments_controller.rb +++ b/app/controllers/step_comments_controller.rb @@ -10,7 +10,8 @@ class StepCommentsController < ApplicationController before_action :check_view_permissions, only: [:index] before_action :check_add_permissions, only: [:create] - before_action :check_manage_permissions, only: %i(update destroy) + before_action :check_update_permissions, only: %i(update) + before_action :check_destroy_permissions, only: %i(destroy) def index comments = @step.last_comments(@last_comment_id, @per_page) @@ -56,10 +57,16 @@ class StepCommentsController < ApplicationController render_403 unless can_create_my_module_comments?(@protocol.my_module) end - def check_manage_permissions - @comment = StepComment.find_by_id(params[:id]) + def check_destroy_permissions + @comment = @step.step_comments.find_by(id: params[:id]) render_403 unless @comment.present? && - can_manage_comment_in_module?(@comment.becomes(Comment)) + can_delete_comment_in_my_module_steps?(@comment) + end + + def check_update_permissions + @comment = @step.step_comments.find_by(id: params[:id]) + render_403 unless @comment.present? && + can_update_comment_in_my_module_steps?(@comment) end def comment_params diff --git a/app/helpers/comment_helper.rb b/app/helpers/comment_helper.rb index 0dd6f23fc..ad562acad 100644 --- a/app/helpers/comment_helper.rb +++ b/app/helpers/comment_helper.rb @@ -76,11 +76,11 @@ module CommentHelper when 'TaskComment' can_manage_my_module_comment?(comment) when 'StepComment' - can_manage_comment_in_module?(comment.becomes(Comment)) + can_manage_my_module_step_comment?(comment) when 'ResultComment' can_manage_result_comment?(comment.becomes(Comment)) when 'ProjectComment' - can_manage_comment_in_project?(comment) + can_manage_project_comment?(comment) else false end diff --git a/app/jobs/user_assignments/generate_user_assignments_job.rb b/app/jobs/user_assignments/generate_user_assignments_job.rb index 249426d5b..37c4b72ca 100644 --- a/app/jobs/user_assignments/generate_user_assignments_job.rb +++ b/app/jobs/user_assignments/generate_user_assignments_job.rb @@ -7,9 +7,10 @@ module UserAssignments def perform(object, assigned_by) @assigned_by = assigned_by ActiveRecord::Base.transaction do - if object.is_a? Experiment + case object + when Experiment assign_users_to_experiment(object) - elsif object.is_a? MyModule + when MyModule assign_users_to_my_module(object) end end diff --git a/app/models/concerns/assignable.rb b/app/models/concerns/assignable.rb index c51665693..24f3b2589 100644 --- a/app/models/concerns/assignable.rb +++ b/app/models/concerns/assignable.rb @@ -4,12 +4,20 @@ module Assignable extend ActiveSupport::Concern included do + include Canaid::Helpers::PermissionsHelper + has_many :user_assignments, as: :assignable, dependent: :destroy default_scope { includes(user_assignments: :user_role) } + scope :readable_by_user, lambda { |user| + joins(user_assignments: :user_role) + .where(user_assignments: { user: user }) + .where('? = ANY(user_roles.permissions)', "::#{self.class.to_s.split('::').first}Permissions".constantize::READ) + } + after_create_commit do - UserAssignment.create( + UserAssignment.create!( user: created_by, assignable: self, user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) diff --git a/app/models/experiment.rb b/app/models/experiment.rb index e28dc726d..0c27fdd70 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -237,14 +237,11 @@ class Experiment < ApplicationRecord # Projects to which this experiment can be moved (inside the same # team and not archived), all users assigned on experiment.project has # to be assigned on such project - def moveable_projects(current_user) - projects = projects_with_role_above_user(current_user) - - projects = projects.each_with_object([]) do |p, arr| - arr << p if (project.users - p.users).empty? - arr + def movable_projects(current_user) + current_user.projects.where.not(id: project_id).where(archived: false, team: project.team).select do |p| + can_create_project_experiments?(current_user, p) && + project.users == p.users end - projects - [project] end def permission_parent @@ -274,7 +271,7 @@ class Experiment < ApplicationRecord cloned_pairs = {} ids_map = {} to_add.each do |m| - original = MyModule.find_by_id(to_clone.fetch(m[:id], nil)) if to_clone.present? + original = MyModule.find_by(id: to_clone.fetch(m[:id], nil)) if to_clone.present? if original.present? my_module = original.deep_clone(current_user) cloned_pairs[my_module] = original diff --git a/app/models/my_module.rb b/app/models/my_module.rb index e8863e77f..acbf1f131 100644 --- a/app/models/my_module.rb +++ b/app/models/my_module.rb @@ -364,7 +364,15 @@ class MyModule < ApplicationRecord def deep_clone_to_experiment(current_user, experiment_dest) # Copy the module - clone = MyModule.new(name: name, experiment: experiment_dest, description: description, x: x, y: y) + clone = MyModule.new( + name: name, + experiment: experiment_dest, + description: description, + x: x, + y: y, + created_by: created_by + ) + # set new position if cloning in the same experiment clone.attributes = get_new_position if clone.experiment == experiment diff --git a/app/models/project.rb b/app/models/project.rb index 3bc1788eb..171cd9e32 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -209,7 +209,7 @@ class Project < ApplicationRecord user_assignments.find_by_user_id(user)&.user_role.name end - def sorted_experiments(sort_by = :new, archived = false) + def sorted_experiments(user, sort_by = :new, archived = false) sort = case sort_by when 'old' then { created_at: :asc } when 'atoz' then { name: :asc } @@ -218,7 +218,7 @@ class Project < ApplicationRecord when 'archived_old' then { archived_on: :asc } else { created_at: :desc } end - experiments.is_archived(archived).order(sort) + experiments.readable_by_user(user).is_archived(archived).order(sort) end def archived_experiments diff --git a/app/models/user.rb b/app/models/user.rb index d761e8eb0..43b7460ea 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -63,9 +63,9 @@ class User < ApplicationRecord has_many :teams, through: :user_teams has_many :user_assignments, dependent: :destroy has_many :user_projects, inverse_of: :user - has_many :projects, through: :user_projects + has_many :projects, through: :user_assignments, source: :assignable, source_type: 'Project' has_many :user_my_modules, inverse_of: :user - has_many :my_modules, through: :user_my_modules + has_many :my_modules, through: :user_assignments, source: :assignable, source_type: 'MyModule' has_many :comments, inverse_of: :user has_many :activities, inverse_of: :owner, foreign_key: 'owner_id' has_many :results, inverse_of: :user diff --git a/app/models/user_role.rb b/app/models/user_role.rb index bd71722f0..6361f523a 100644 --- a/app/models/user_role.rb +++ b/app/models/user_role.rb @@ -11,16 +11,14 @@ class UserRole < ApplicationRecord validates :created_by, presence: true, unless: :predefined? validates :last_modified_by, presence: true, unless: :predefined? - belongs_to :created_by, foreign_key: 'created_by_id', class_name: 'User', optional: true - belongs_to :last_modified_by, foreign_key: 'last_modified_by_id', class_name: 'User', optional: true + belongs_to :created_by, class_name: 'User', optional: true + belongs_to :last_modified_by, class_name: 'User', optional: true has_many :user_assignments, dependent: :destroy def self.owner_role new( name: I18n.t('user_roles.predefined.owner'), - permissions: ProjectPermissions.constants.map { |const| ProjectPermissions.const_get(const) } + - ExperimentPermissions.constants.map { |const| ExperimentPermissions.const_get(const) } + - MyModulePermissions.constants.map { |const| MyModulePermissions.const_get(const) }, + permissions: PredefinedRoles::OWNER_PERMISSIONS, predefined: true ) end @@ -28,39 +26,7 @@ class UserRole < ApplicationRecord def self.normal_user_role new( name: I18n.t('user_roles.predefined.normal_user'), - permissions: - [ - ProjectPermissions::READ, - ProjectPermissions::READ_ARCHIVED, - ProjectPermissions::ACTIVITIES_READ, - ProjectPermissions::USERS_READ, - ProjectPermissions::COMMENTS_READ, - ProjectPermissions::COMMENTS_CREATE, - ProjectPermissions::EXPERIMENTS_CREATE, - ExperimentPermissions::READ, - ExperimentPermissions::MANAGE, - ExperimentPermissions::TASKS_MANAGE, - MyModulePermissions::READ, - MyModulePermissions::MANAGE, - MyModulePermissions::RESULTS_MANAGE, - MyModulePermissions::PROTOCOL_MANAGE, - MyModulePermissions::STEPS_MANAGE, - MyModulePermissions::TAGS_MANAGE, - MyModulePermissions::COMMENTS_CREATE, - MyModulePermissions::COMMENTS_MANAGE, - MyModulePermissions::COMMENTS_MANAGE_OWN, - MyModulePermissions::COMPLETE, - MyModulePermissions::UPDATE_STATUS, - MyModulePermissions::STEPS_COMPLETE, - MyModulePermissions::STEPS_UNCOMPLETE, - MyModulePermissions::STEPS_CHECKLIST_CHECK, - MyModulePermissions::STEPS_CHECKLIST_UNCHECK, - MyModulePermissions::STEPS_COMMENTS_CREATE, - MyModulePermissions::STEPS_COMMENTS_DELETE_OWN, - MyModulePermissions::STEPS_COMMENT_UPDATE_OWN, - MyModulePermissions::REPOSITORY_ROWS_ASSIGN, - MyModulePermissions::REPOSITORY_ROWS_MANAGE - ], + permissions: PredefinedRoles::NORMAL_USER_PERMISSIONS, predefined: true ) end @@ -68,33 +34,7 @@ class UserRole < ApplicationRecord def self.technician_role new( name: I18n.t('user_roles.predefined.technician'), - permissions: - [ - ProjectPermissions::READ, - ProjectPermissions::READ_ARCHIVED, - ProjectPermissions::ACTIVITIES_READ, - ProjectPermissions::USERS_READ, - ProjectPermissions::COMMENTS_READ, - ProjectPermissions::COMMENTS_CREATE, - ExperimentPermissions::READ, - ExperimentPermissions::READ_ARCHIVED, - ExperimentPermissions::ACTIVITIES_READ, - ExperimentPermissions::USERS_READ, - MyModulePermissions::READ, - MyModulePermissions::COMMENTS_CREATE, - MyModulePermissions::COMMENTS_MANAGE_OWN, - MyModulePermissions::COMPLETE, - MyModulePermissions::UPDATE_STATUS, - MyModulePermissions::STEPS_COMPLETE, - MyModulePermissions::STEPS_UNCOMPLETE, - MyModulePermissions::STEPS_CHECKLIST_CHECK, - MyModulePermissions::STEPS_CHECKLIST_UNCHECK, - MyModulePermissions::STEPS_COMMENTS_CREATE, - MyModulePermissions::STEPS_COMMENTS_DELETE_OWN, - MyModulePermissions::STEPS_COMMENT_UPDATE_OWN, - MyModulePermissions::REPOSITORY_ROWS_ASSIGN, - MyModulePermissions::REPOSITORY_ROWS_MANAGE - ], + permissions: PredefinedRoles::TECHNICIAN_PERMISSIONS, predefined: true ) end @@ -102,19 +42,7 @@ class UserRole < ApplicationRecord def self.viewer_role new( name: I18n.t('user_roles.predefined.viewer'), - permissions: - [ - ProjectPermissions::READ, - ProjectPermissions::READ_ARCHIVED, - ProjectPermissions::ACTIVITIES_READ, - ProjectPermissions::USERS_READ, - ProjectPermissions::COMMENTS_READ, - ExperimentPermissions::READ, - ExperimentPermissions::READ_ARCHIVED, - ExperimentPermissions::ACTIVITIES_READ, - ExperimentPermissions::USERS_READ, - MyModulePermissions::READ - ], + permissions: PredefinedRoles::VIEWER_PERMISSIONS, predefined: true ) end diff --git a/app/permissions/experiment.rb b/app/permissions/experiment.rb index 81dd5c1df..091af9aee 100644 --- a/app/permissions/experiment.rb +++ b/app/permissions/experiment.rb @@ -1,6 +1,8 @@ Canaid::Permissions.register_for(Experiment) do # Experiment and its project must be active for all the specified permissions %i(manage_experiment + manage_experiment_tasks + manage_experiment_users archive_experiment clone_experiment move_experiment @@ -12,18 +14,6 @@ Canaid::Permissions.register_for(Experiment) do end end - # experiment: read (read archive) - # canvas: read - # module: read (read users, read comments, read archive) - # result: read (read comments) - can :read_experiment do |user, experiment| - experiment.permission_granted?(user, ExperimentPermissions::READ) - end - - can :read_users_of_experiment do |user, project| - project.permission_granted?(user, ExperimentPermissions::USERS_READ) - end - # experiment: create/update/delete # canvas: update # module: create, copy, reposition, create/update/delete connection, @@ -42,19 +32,38 @@ Canaid::Permissions.register_for(Experiment) do end end - # experiment: manage access policies - can :manage_experiment_access do |user, experiment| + can :read_experiment do |user, experiment| + experiment.permission_granted?(user, ExperimentPermissions::READ) + end + + can :read_archived_experiment do |user, experiment| + experiment.permission_granted?(user, ExperimentPermissions::READ_ARCHIVED) + end + + can :read_experiment_canvas do |user, experiment| + experiment.permission_granted?(user, ExperimentPermissions::READ_CANVAS) + end + + can :read_experiment_activities do |user, experiment| + experiment.permission_granted?(user, ExperimentPermissions::ACTIVITIES_READ) + end + + can :read_experiment_users do |user, experiment| + experiment.permission_granted?(user, ExperimentPermissions::USERS_READ) + end + + can :manage_experiment_users do |user, experiment| experiment.permission_granted?(user, ExperimentPermissions::USERS_MANAGE) end - # experiment: archive + can :manage_experiment_tasks do |user, experiment| + experiment.permission_granted?(user, ExperimentPermissions::TASKS_MANAGE) + end + can :archive_experiment do |user, experiment| experiment.permission_granted?(user, ExperimentPermissions::MANAGE) end - # NOTE: Must not be dependent on canaid parmision for which we check if it's - # active - # experiment: restore can :restore_experiment do |user, experiment| project = experiment.project experiment.permission_granted?(user, ExperimentPermissions::MANAGE) && @@ -62,12 +71,10 @@ Canaid::Permissions.register_for(Experiment) do project.active? end - # experiment: copy can :clone_experiment do |user, experiment| experiment.permission_granted?(user, ExperimentPermissions::MANAGE) end - # experiment: move can :move_experiment do |user, experiment| experiment.permission_granted?(user, ExperimentPermissions::MANAGE) end diff --git a/app/permissions/my_module.rb b/app/permissions/my_module.rb index 06f0da0c8..9931b8032 100644 --- a/app/permissions/my_module.rb +++ b/app/permissions/my_module.rb @@ -107,22 +107,6 @@ Canaid::Permissions.register_for(MyModule) do my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_CREATE) end - can :delete_comments_in_my_module_steps do |user, my_module| - my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_DELETE) - end - - can :delete_own_comments_in_my_module_steps do |user, my_module| - my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_DELETE_OWN) - end - - can :update_comments_in_my_module_steps do |user, my_module| - my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_UPDATE) - end - - can :update_own_comments_in_my_module_steps do |user, my_module| - my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENT_UPDATE_OWN) - end - can :manage_my_module_users do |user, my_module| my_module.permission_granted?(user, MyModulePermissions::MANAGE) end @@ -146,7 +130,7 @@ Canaid::Permissions.register_for(TaskComment) do %i(manage_my_module_comment) .each do |perm| can perm do |_, comment| - my_module = ::PermissionsUtil.get_comment_module(comment) + my_module = comment.my_module my_module.active? && my_module.experiment.active? && my_module.experiment.project.active? @@ -154,8 +138,36 @@ Canaid::Permissions.register_for(TaskComment) do end can :manage_my_module_comment do |user, comment| - my_module = ::PermissionsUtil.get_comment_module(comment) + my_module = comment.my_module my_module.permission_granted?(user, MyModulePermissions::COMMENTS_MANAGE) || ((comment.user == user) && my_module.permission_granted?(user, MyModulePermissions::COMMENTS_MANAGE_OWN)) end end + +Canaid::Permissions.register_for(StepComment) do + # Module, its experiment and its project must be active for all the specified + # permissions + %i(delete_comment_in_my_module_steps + update_comment_in_my_module_steps) + .each do |perm| + can perm do |_, comment| + my_module = comment.step.my_module + my_module.active? && + !my_module.archived_branch? && + my_module.experiment.active? && + my_module.experiment.project.active? + end + end + + can :delete_comment_in_my_module_steps do |user, comment| + my_module = comment.step.my_module + my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_DELETE) || + ((comment.user == user) && my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_DELETE_OWN)) + end + + can :update_comment_in_my_module_steps do |user, comment| + my_module = comment.step.my_module + my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_UPDATE) || + ((comment.user == user) && my_module.permission_granted?(user, MyModulePermissions::STEPS_COMMENTS_UPDATE_OWN)) + end +end diff --git a/app/permissions/project.rb b/app/permissions/project.rb index 9ec1cda28..29c705a0e 100644 --- a/app/permissions/project.rb +++ b/app/permissions/project.rb @@ -38,6 +38,14 @@ Canaid::Permissions.register_for(Project) do end end + can :read_project_users do |user, project| + project.permission_granted?(user, ProjectPermissions::USERS_READ) + end + + can :read_project_activities do |user, project| + project.permission_granted?(user, ProjectPermissions::ACTIVITIES_READ) + end + can :manage_project_users do |user, project| project.permission_granted?(user, ProjectPermissions::USERS_MANAGE) end @@ -58,10 +66,6 @@ Canaid::Permissions.register_for(Project) do project.permission_granted?(user, ProjectPermissions::COMMENTS_CREATE) end - can :manage_project_comments do |user, project| - project.permission_granted?(user, ProjectPermissions::COMMENTS_MANAGE) - end - can :manage_project_tags do |user, project| project.permission_granted?(user, ProjectPermissions::MANAGE) end @@ -70,3 +74,9 @@ Canaid::Permissions.register_for(Project) do project.permission_granted?(user, ProjectPermissions::TASKS_MANAGE) end end + +Canaid::Permissions.register_for(ProjectComment) do + can :manage_project_comment do |user, comment| + comment.project.permission_granted?(user, ProjectPermissions::COMMENTS_MANAGE) + end +end diff --git a/app/permissions/result.rb b/app/permissions/result.rb index b684545db..7730a5bce 100644 --- a/app/permissions/result.rb +++ b/app/permissions/result.rb @@ -7,6 +7,7 @@ Canaid::Permissions.register_for(Result) do can :manage_result do |user, result| !result.archived? && + !result.my_module.archived_branch? && result.unlocked?(result) && result.my_module.permission_granted?(user, MyModulePermissions::RESULTS_MANAGE) end @@ -24,8 +25,7 @@ Canaid::Permissions.register_for(ResultComment) do %i(manage_result_comment) .each do |perm| can perm do |_, comment| - my_module = ::PermissionsUtil.get_comment_module(comment) - !my_module.archived_branch? + !comment.result.my_module.archived_branch? end end @@ -33,7 +33,7 @@ Canaid::Permissions.register_for(ResultComment) do # result: update/delete comment # step: update/delete comment can :manage_result_comment do |user, comment| - my_module = ::PermissionsUtil.get_comment_module(comment) + my_module = comment.result.my_module (comment.user == user && my_module.permission_granted?(user, MyModulePermissions::RESULTS_COMMENTS_MANAGE_OWN)) || my_module.permission_granted?(user, MyModulePermissions::RESULTS_COMMENTS_MANAGE) end diff --git a/app/services/experiments/move_to_project_service.rb b/app/services/experiments/move_to_project_service.rb index b35a1c01c..9a8688224 100644 --- a/app/services/experiments/move_to_project_service.rb +++ b/app/services/experiments/move_to_project_service.rb @@ -62,12 +62,11 @@ module Experiments return false end - if !@exp.moveable_projects(@user).include?(@project) + if @exp.movable_projects(@user).include?(@project) + true + else @errors[:target_project_not_valid] = ['Experiment cannot be moved to this project'] - false - else - true end end diff --git a/app/services/experiments_overview_service.rb b/app/services/experiments_overview_service.rb index b27e1fd61..8639445b9 100644 --- a/app/services/experiments_overview_service.rb +++ b/app/services/experiments_overview_service.rb @@ -39,10 +39,11 @@ class ExperimentsOverviewService 'AND active_tasks.archived = FALSE') .joins('LEFT OUTER JOIN my_modules AS active_completed_tasks ON active_completed_tasks.experiment_id '\ '= experiments.id AND active_completed_tasks.archived = FALSE AND active_completed_tasks.state = 1') + .readable_by_user(@user) .select('experiments.*') .select('COUNT(DISTINCT active_tasks.id) AS task_count') .select('COUNT(DISTINCT active_completed_tasks.id) AS completed_task_count') - .group('experiments.id') + .group('experiments.id, user_assignments.id, user_roles.id') end def filter_records(records) diff --git a/app/services/report_actions/report_content.rb b/app/services/report_actions/report_content.rb index d1a4965be..ff5cdef20 100644 --- a/app/services/report_actions/report_content.rb +++ b/app/services/report_actions/report_content.rb @@ -68,7 +68,7 @@ module ReportActions def generate_experiment_content(experiment_id, my_module_ids) experiment = Experiment.find_by(id: experiment_id) - return if !experiment && !can_read_experiment?(experiment, @user) + return if !experiment && !can_read_experiment?(@user, experiment) experiment_element = save_element!({ 'experiment_id' => experiment.id }, :experiment, nil) generate_my_modules_content(experiment, experiment_element, my_module_ids) diff --git a/app/services/templates_service.rb b/app/services/templates_service.rb index b9346f321..11f2722d2 100644 --- a/app/services/templates_service.rb +++ b/app/services/templates_service.rb @@ -20,7 +20,8 @@ class TemplatesService tmpl_project = team.projects.create!( name: Constants::TEMPLATES_PROJECT_NAME, visibility: :visible, - template: true + template: true, + created_by: team.created_by ) tmpl_project.user_projects.create!(user: team.created_by, role: 'owner') end diff --git a/app/views/projects/show/_experiment_actions_dropdown.html.erb b/app/views/projects/show/_experiment_actions_dropdown.html.erb index 04cc9651f..c5768f46a 100644 --- a/app/views/projects/show/_experiment_actions_dropdown.html.erb +++ b/app/views/projects/show/_experiment_actions_dropdown.html.erb @@ -64,7 +64,7 @@ <% end %> - <% if can_manage_experiment_access?(experiment) %> + <% if can_manage_experiment_users?(experiment) %>
  • <%= link_to edit_access_permissions_project_experiment_path(project, experiment), data: { action: 'remote-modal'} do %> diff --git a/app/views/search/results/partials/_asset_text.html.erb b/app/views/search/results/partials/_asset_text.html.erb index eff66f39d..a11f2776b 100644 --- a/app/views/search/results/partials/_asset_text.html.erb +++ b/app/views/search/results/partials/_asset_text.html.erb @@ -1,4 +1,4 @@ -<% +<% query ||= nil asset_read_allowed = false file_name = asset.blob.metadata["asset_type"] ? asset.blob.metadata["name"] : asset.file_name diff --git a/app/views/shared/sidebar/_experiments.html.erb b/app/views/shared/sidebar/_experiments.html.erb index 10b34687b..02ecb005f 100644 --- a/app/views/shared/sidebar/_experiments.html.erb +++ b/app/views/shared/sidebar/_experiments.html.erb @@ -8,7 +8,7 @@ <%= link_to t('sidebar.experiments.back_button'), projects_path, class: 'sidebar-link back-button' %> <% end %>
    • - <% project.sorted_experiments(@current_sort, view_mode == 'archived').each do |experiment| %> + <% project.sorted_experiments(current_user, @current_sort, view_mode == 'archived').each do |experiment| %>
  • <% if view_mode == 'archived' %> <%= link_to experiment.name, module_archive_experiment_path(experiment), title: experiment.name, class: 'sidebar-link' %> diff --git a/config/initializers/extends/permission_extends.rb b/config/initializers/extends/permission_extends.rb index 649df3dcc..d13478f1c 100644 --- a/config/initializers/extends/permission_extends.rb +++ b/config/initializers/extends/permission_extends.rb @@ -26,6 +26,9 @@ module PermissionExtends TASKS_MANAGE USERS_READ USERS_MANAGE + READ_ARCHIVED + READ_CANVAS + ACTIVITIES_READ ).each { |permission| const_set(permission, "experiment_#{permission.underscore}") } end @@ -78,4 +81,86 @@ module PermissionExtends DELETE_COLUMN ).each { |permission| const_set(permission, "inventory_#{permission.underscore}") } end + + module PredefinedRoles + OWNER_PERMISSIONS = ( + ProjectPermissions.constants.map { |const| ProjectPermissions.const_get(const) } + + ExperimentPermissions.constants.map { |const| ExperimentPermissions.const_get(const) } + + MyModulePermissions.constants.map { |const| MyModulePermissions.const_get(const) } + ) + + NORMAL_USER_PERMISSIONS = [ + ProjectPermissions::READ, + ProjectPermissions::READ_ARCHIVED, + ProjectPermissions::ACTIVITIES_READ, + ProjectPermissions::USERS_READ, + ProjectPermissions::COMMENTS_READ, + ProjectPermissions::COMMENTS_CREATE, + ProjectPermissions::EXPERIMENTS_CREATE, + ExperimentPermissions::READ, + ExperimentPermissions::MANAGE, + ExperimentPermissions::TASKS_MANAGE, + ExperimentPermissions::USERS_MANAGE, + MyModulePermissions::READ, + MyModulePermissions::MANAGE, + MyModulePermissions::RESULTS_MANAGE, + MyModulePermissions::PROTOCOL_MANAGE, + MyModulePermissions::STEPS_MANAGE, + MyModulePermissions::TAGS_MANAGE, + MyModulePermissions::COMMENTS_CREATE, + MyModulePermissions::COMMENTS_MANAGE, + MyModulePermissions::COMMENTS_MANAGE_OWN, + MyModulePermissions::COMPLETE, + MyModulePermissions::UPDATE_STATUS, + MyModulePermissions::STEPS_COMPLETE, + MyModulePermissions::STEPS_UNCOMPLETE, + MyModulePermissions::STEPS_CHECKLIST_CHECK, + MyModulePermissions::STEPS_CHECKLIST_UNCHECK, + MyModulePermissions::STEPS_COMMENTS_CREATE, + MyModulePermissions::STEPS_COMMENTS_DELETE_OWN, + MyModulePermissions::STEPS_COMMENT_UPDATE_OWN, + MyModulePermissions::REPOSITORY_ROWS_ASSIGN, + MyModulePermissions::REPOSITORY_ROWS_MANAGE + ] + + TECHNICIAN_PERMISSIONS = [ + ProjectPermissions::READ, + ProjectPermissions::READ_ARCHIVED, + ProjectPermissions::ACTIVITIES_READ, + ProjectPermissions::USERS_READ, + ProjectPermissions::COMMENTS_READ, + ProjectPermissions::COMMENTS_CREATE, + ExperimentPermissions::READ, + ExperimentPermissions::READ_ARCHIVED, + ExperimentPermissions::ACTIVITIES_READ, + ExperimentPermissions::USERS_READ, + MyModulePermissions::READ, + MyModulePermissions::COMMENTS_CREATE, + MyModulePermissions::COMMENTS_MANAGE_OWN, + MyModulePermissions::COMPLETE, + MyModulePermissions::UPDATE_STATUS, + MyModulePermissions::STEPS_COMPLETE, + MyModulePermissions::STEPS_UNCOMPLETE, + MyModulePermissions::STEPS_CHECKLIST_CHECK, + MyModulePermissions::STEPS_CHECKLIST_UNCHECK, + MyModulePermissions::STEPS_COMMENTS_CREATE, + MyModulePermissions::STEPS_COMMENTS_DELETE_OWN, + MyModulePermissions::STEPS_COMMENT_UPDATE_OWN, + MyModulePermissions::REPOSITORY_ROWS_ASSIGN, + MyModulePermissions::REPOSITORY_ROWS_MANAGE + ] + + VIEWER_PERMISSIONS = [ + ProjectPermissions::READ, + ProjectPermissions::READ_ARCHIVED, + ProjectPermissions::ACTIVITIES_READ, + ProjectPermissions::USERS_READ, + ProjectPermissions::COMMENTS_READ, + ExperimentPermissions::READ, + ExperimentPermissions::READ_ARCHIVED, + ExperimentPermissions::ACTIVITIES_READ, + ExperimentPermissions::USERS_READ, + MyModulePermissions::READ + ] + end end diff --git a/features/support/env.rb b/features/support/env.rb index 7c17c7dc4..e5ecdae3a 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -54,6 +54,31 @@ 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/access_permissions/experiments_controller_spec.rb b/spec/controllers/access_permissions/experiments_controller_spec.rb index 0f0b6468e..f0f4d36cc 100644 --- a/spec/controllers/access_permissions/experiments_controller_spec.rb +++ b/spec/controllers/access_permissions/experiments_controller_spec.rb @@ -8,11 +8,11 @@ describe AccessPermissions::ExperimentsController, type: :controller do let!(:user) { subject.current_user } let!(:team) { create :team, created_by: user } let!(:user_team) { create :user_team, :admin, user: user, team: team } - let!(:project) { create :project, team: team, created_by: user } let!(:experiment) { create :experiment, project: project } - let!(:owner_role) { create :owner_role } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:viewer_user_role) { create :viewer_role } let!(:technician_role) { create :technician_role } + let!(:project) { create :project, team: team, created_by: user } let!(:viewer_user) { create :user, confirmed_at: Time.zone.now } let!(:normal_user_team) { create :user_team, :normal_user, user: viewer_user, team: team } diff --git a/spec/controllers/access_permissions/my_modules_controller_spec.rb b/spec/controllers/access_permissions/my_modules_controller_spec.rb index 417fa9763..acc44a852 100644 --- a/spec/controllers/access_permissions/my_modules_controller_spec.rb +++ b/spec/controllers/access_permissions/my_modules_controller_spec.rb @@ -8,16 +8,16 @@ describe AccessPermissions::MyModulesController, type: :controller do let!(:user) { subject.current_user } let!(:team) { create :team, created_by: user } let!(:user_team) { create :user_team, :admin, user: user, team: team } - let!(:project) { create :project, team: team, created_by: user } let!(:experiment) { create :experiment, project: project } - let!(:owner_role) { create :owner_role } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:viewer_user_role) { create :viewer_role } let!(:technician_role) { create :technician_role } + let!(:project) { create :project, team: team, created_by: user } let!(:user_project) { create :user_project, user: user, project: project } let!(:viewer_user) { create :user, confirmed_at: Time.zone.now } let!(:normal_user_team) { create :user_team, :normal_user, user: viewer_user, team: team } let!(:viewer_user_project) { create :user_project, user: viewer_user, project: project } - let!(:my_module) { create :my_module, experiment: experiment } + let!(:my_module) { create :my_module, experiment: experiment, created_by: experiment.created_by, created_by: user } before do create_user_assignment(my_module, owner_role, user) diff --git a/spec/controllers/access_permissions/projects_controller_spec.rb b/spec/controllers/access_permissions/projects_controller_spec.rb index 93bc29750..4f2a81eb4 100644 --- a/spec/controllers/access_permissions/projects_controller_spec.rb +++ b/spec/controllers/access_permissions/projects_controller_spec.rb @@ -9,7 +9,7 @@ describe AccessPermissions::ProjectsController, type: :controller do let!(:team) { create :team, created_by: user } let!(:user_team) { create :user_team, :admin, user: user, team: team } let!(:project) { create :project, team: team, created_by: user } - let!(:owner_role) { create :owner_role } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:normal_user_role) { create :normal_user_role } let!(:technician_role) { create :technician_role } let!(:user_project) { create :user_project, user: user, project: project } diff --git a/spec/controllers/assets_controller_spec.rb b/spec/controllers/assets_controller_spec.rb index 453d56a37..a601bee61 100644 --- a/spec/controllers/assets_controller_spec.rb +++ b/spec/controllers/assets_controller_spec.rb @@ -6,7 +6,7 @@ describe AssetsController, type: :controller do login_user include_context 'reference_project_structure', { - role: :owner_role, + role: :owner, result_asset: true, step: true, step_asset: true, diff --git a/spec/controllers/canvas_controller_spec.rb b/spec/controllers/canvas_controller_spec.rb index 57881e9b0..552c30767 100644 --- a/spec/controllers/canvas_controller_spec.rb +++ b/spec/controllers/canvas_controller_spec.rb @@ -30,14 +30,14 @@ describe CanvasController do - let!(:task1) { create :my_module, x: 0, y: 1, experiment: experiment } - let!(:task2) { create :my_module, x: 0, y: 2, experiment: experiment } - let!(:task3) { create :my_module, x: 0, y: 3, experiment: experiment } - let!(:task4) { create :my_module, x: 0, y: 4, experiment: experiment } - let!(:task5) { create :my_module, x: 0, y: 5, experiment: experiment } - let!(:task6) { create :my_module, x: 0, y: 6, experiment: experiment } - let!(:task7) { create :my_module, x: 0, y: 7, experiment: experiment } - let!(:task8) { create :my_module, x: 0, y: 8, experiment: experiment } + let!(:task1) { create :my_module, x: 0, y: 1, experiment: experiment, created_by: experiment.created_by } + let!(:task2) { create :my_module, x: 0, y: 2, experiment: experiment, created_by: experiment.created_by } + let!(:task3) { create :my_module, x: 0, y: 3, experiment: experiment, created_by: experiment.created_by } + let!(:task4) { create :my_module, x: 0, y: 4, experiment: experiment, created_by: experiment.created_by } + let!(:task5) { create :my_module, x: 0, y: 5, experiment: experiment, created_by: experiment.created_by } + let!(:task6) { create :my_module, x: 0, y: 6, experiment: experiment, created_by: experiment.created_by } + let!(:task7) { create :my_module, x: 0, y: 7, experiment: experiment, created_by: experiment.created_by } + let!(:task8) { create :my_module, x: 0, y: 8, experiment: experiment, created_by: experiment.created_by } let!(:step_on_task5) { create :step, name: 'task5_step', protocol: task5.protocol } let!(:step_on_task6) { create :step, name: 'task6_step', protocol: task6.protocol } diff --git a/spec/controllers/my_module_comments_controller_spec.rb b/spec/controllers/my_module_comments_controller_spec.rb index ab0f3a038..085bf5aab 100644 --- a/spec/controllers/my_module_comments_controller_spec.rb +++ b/spec/controllers/my_module_comments_controller_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' describe MyModuleCommentsController, type: :controller do login_user include_context 'reference_project_structure' , { - role: :normal_user_role, + role: :normal_user, my_module_comment: true } diff --git a/spec/controllers/my_module_repositories_controller_spec.rb b/spec/controllers/my_module_repositories_controller_spec.rb index 77ca0b71b..fe56a3060 100644 --- a/spec/controllers/my_module_repositories_controller_spec.rb +++ b/spec/controllers/my_module_repositories_controller_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' describe MyModuleRepositoriesController, type: :controller do login_user include_context 'reference_project_structure' , { - role: :normal_user_role + role: :normal_user } let!(:repository) { create :repository, created_by: user, team: team } let!(:repository_row) do @@ -48,7 +48,7 @@ describe MyModuleRepositoriesController, type: :controller do rows_to_assign: [repository_row.id], downstream: true } 3.times do |_i| - child_module = create :my_module, experiment: experiment + child_module = create :my_module, experiment: experiment, created_by: experiment.created_by Connection.create(output_id: parent_my_module.id, input_id: child_module.id) end expect { put :update, params: params_downstream, format: :json } @@ -97,7 +97,7 @@ describe MyModuleRepositoriesController, type: :controller do rows_to_unassign: [repository_row.id], downstream: true } 3.times do |_i| - child_module = create :my_module, experiment: experiment + child_module = create :my_module, experiment: experiment, created_by: experiment.created_by Connection.create(output_id: parent_my_module.id, input_id: child_module.id) create :mm_repository_row, repository_row: repository_row, my_module: child_module, @@ -155,7 +155,7 @@ describe MyModuleRepositoriesController, type: :controller do rows_to_unassign: [repository_row_2.id], downstream: true } 3.times do |_i| - child_module = create :my_module, experiment: experiment + child_module = create :my_module, experiment: experiment, created_by: experiment.created_by Connection.create(output_id: parent_my_module.id, input_id: child_module.id) create :mm_repository_row, repository_row: repository_row_2, my_module: child_module, diff --git a/spec/controllers/my_modules_controller_spec.rb b/spec/controllers/my_modules_controller_spec.rb index 6dcf8ba3d..732840ef4 100644 --- a/spec/controllers/my_modules_controller_spec.rb +++ b/spec/controllers/my_modules_controller_spec.rb @@ -55,7 +55,7 @@ describe MyModulesController, type: :controller do context 'when deleting due_date' do let(:params) { { id: my_module.id, my_module: { due_date: '' } } } let(:my_module) do - create :my_module, :with_due_date, experiment: experiment + create :my_module, :with_due_date, experiment: experiment, created_by: experiment.created_by end it 'calls create activity for removing due date' do @@ -77,7 +77,7 @@ describe MyModulesController, type: :controller do { id: my_module.id, my_module: { due_date: '02/21/2019 23:59' } } end let(:my_module) do - create :my_module, :with_due_date, experiment: experiment + create :my_module, :with_due_date, experiment: experiment, created_by: experiment.created_by end it 'calls create activity for changing due date' do @@ -174,9 +174,9 @@ describe MyModulesController, type: :controller do } end let(:experiment) { create :experiment } - let(:task1) { create :my_module, :archived, experiment: experiment } - let(:task2) { create :my_module, :archived, experiment: experiment } - let(:task3) { create :my_module, :archived, experiment: experiment } + let(:task1) { create :my_module, :archived, experiment: experiment, created_by: experiment.created_by } + let(:task2) { create :my_module, :archived, experiment: experiment, created_by: experiment.created_by } + let(:task3) { create :my_module, :archived, experiment: experiment, created_by: experiment.created_by } let(:user) { controller.current_user } before do diff --git a/spec/controllers/reports_controller_spec.rb b/spec/controllers/reports_controller_spec.rb index 61addb422..a65aefb7b 100644 --- a/spec/controllers/reports_controller_spec.rb +++ b/spec/controllers/reports_controller_spec.rb @@ -7,7 +7,7 @@ describe ReportsController, type: :controller do include_context 'reference_project_structure' - let(:my_module2) { create :my_module, experiment: experiment } + let(:my_module2) { create :my_module, experiment: experiment, created_by: experiment.created_by } let(:report) do create :report, user: user, project: project, team: team, name: 'test repot A1', description: 'test description A1' diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb index 968235270..0f5153a23 100644 --- a/spec/controllers/teams_controller_spec.rb +++ b/spec/controllers/teams_controller_spec.rb @@ -7,7 +7,6 @@ describe TeamsController, type: :controller do let(:user) { subject.current_user } let(:team) { create :team, created_by: user } - let!(:owner_role) { create :owner_role } let!(:user_team) { create :user_team, :admin, user: user, team: team } describe 'POST export_projects' do diff --git a/spec/factories/experiments.rb b/spec/factories/experiments.rb index 5dc2bb616..c4ebd1924 100644 --- a/spec/factories/experiments.rb +++ b/spec/factories/experiments.rb @@ -12,7 +12,7 @@ FactoryBot.define do project { create :project, created_by: user } trait :with_tasks do after(:create) do |e| - create_list :my_module, 3, :with_tag, experiment: e + create_list :my_module, 3, :with_tag, experiment: e, created_by: e.created_by end end diff --git a/spec/factories/my_modules.rb b/spec/factories/my_modules.rb index 1b9c9ba61..b5fb50481 100644 --- a/spec/factories/my_modules.rb +++ b/spec/factories/my_modules.rb @@ -2,6 +2,10 @@ FactoryBot.define do factory :my_module do + transient do + user { create :user } + end + created_by { user } sequence(:name) { |n| "Task-#{n}" } sequence(:y) { |n| n } sequence(:x) { |n| n } diff --git a/spec/factories/user_roles.rb b/spec/factories/user_roles.rb index 7467c4be2..1366c0fa6 100644 --- a/spec/factories/user_roles.rb +++ b/spec/factories/user_roles.rb @@ -10,51 +10,19 @@ FactoryBot.define do factory :normal_user_role do name { I18n.t('user_roles.predefined.normal_user') } - permissions { - [ - ProjectPermissions::READ, - ProjectPermissions::EXPERIMENTS_CREATE, - ProjectPermissions::COMMENTS_CREATE, - ExperimentPermissions::READ, - ExperimentPermissions::MANAGE, - ExperimentPermissions::ARCHIVE, - ExperimentPermissions::RESTORE, - ExperimentPermissions::CLONE, - ExperimentPermissions::TASKS_CREATE, - MyModulePermissions::READ, - MyModulePermissions::COMMENTS_CREATE, - MyModulePermissions::UPDATE_STATUS, - MyModulePermissions::REPOSITORY_ROWS_ASSIGN - ] - } + permissions { PredefinedRoles::NORMAL_USER_PERMISSIONS } predefined { true } end factory :technician_role do name { I18n.t('user_roles.predefined.technician') } - permissions { - [ - ProjectPermissions::READ, - ProjectPermissions::COMMENTS_CREATE, - ExperimentPermissions::READ, - MyModulePermissions::READ, - MyModulePermissions::COMMENTS_CREATE, - MyModulePermissions::UPDATE_STATUS, - MyModulePermissions::REPOSITORY_ROWS_ASSIGN - ] - } + permissions { PredefinedRoles::TECHNICIAN_PERMISSIONS } predefined { true } end factory :viewer_role do name { I18n.t('user_roles.predefined.viewer') } - permissions { - [ - ProjectPermissions::READ, - ExperimentPermissions::READ, - MyModulePermissions::READ - ] - } + permissions { PredefinedRoles::VIEWER_PERMISSIONS } predefined { true } end end diff --git a/spec/jobs/activities/dispatch_webhooks_job_spec.rb b/spec/jobs/activities/dispatch_webhooks_job_spec.rb index 8107cba1b..d4810fae3 100644 --- a/spec/jobs/activities/dispatch_webhooks_job_spec.rb +++ b/spec/jobs/activities/dispatch_webhooks_job_spec.rb @@ -14,7 +14,7 @@ describe Activities::DispatchWebhooksJob do let!(:webhook_1) { create :webhook, activity_filter: activity_filter_1 } let!(:webhook_2) { create :webhook, activity_filter: activity_filter_2 } let!(:webhook_3) { create :webhook, activity_filter: non_matching_activity_filter } - let!(:activity) { create :activity } + let(:activity) { create :activity } it 'enqueues webhook jobs' do ActiveJob::Base.queue_adapter = :test diff --git a/spec/jobs/activities/send_webhook_job_spec.rb b/spec/jobs/activities/send_webhook_job_spec.rb index f67090969..7c587de23 100644 --- a/spec/jobs/activities/send_webhook_job_spec.rb +++ b/spec/jobs/activities/send_webhook_job_spec.rb @@ -3,8 +3,8 @@ require 'rails_helper' describe Activities::SendWebhookJob do - let!(:webhook) { create :webhook } - let!(:activity) { create :activity } + let(:webhook) { create :webhook } + let(:activity) { create :activity } it 'sends the webhook' do stub_request(:post, webhook.url).to_return(status: 200, body: "", headers: {}) diff --git a/spec/jobs/user_assignments/generate_user_assignments_job_spec.rb b/spec/jobs/user_assignments/generate_user_assignments_job_spec.rb index 12ef612b8..3990ca66e 100644 --- a/spec/jobs/user_assignments/generate_user_assignments_job_spec.rb +++ b/spec/jobs/user_assignments/generate_user_assignments_job_spec.rb @@ -3,17 +3,16 @@ require 'rails_helper' module UserAssignments RSpec.describe GenerateUserAssignmentsJob, type: :job do - let!(:owner_role) { create :owner_role } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:viewer_role) { create :viewer_role } let!(:technician_role) { create :technician_role } - let!(:user_one) { create :user } let!(:user_two) { create :user } let!(:user_three) { create :user } let!(:team) { create :team, created_by: user_one } let!(:project) { create :project, team: team, created_by: user_one } - before do + before(:each) do available_roles = [owner_role, viewer_role, technician_role] [user_one, user_two, user_three].each_with_index do |user, i| create :user_team, :admin, user: user, team: team @@ -30,10 +29,10 @@ module UserAssignments context 'experiment' do let!(:experiment) { create :experiment, project: project } - it 'creates user assignments for all users on the project that are not assigned yet' do - expect { - described_class.perform_now(experiment, user_one) - }.to change(UserAssignment, :count).by(3) + it 'user assignments should be created automatically upon experiment creation' do + # check that all users are assigned + experiment.reload + expect([user_one, user_two, user_three] - experiment.user_assignments.reload.map(&:user)).to eq([]) end it 'assigns the same role as the user had on project level' do @@ -47,12 +46,12 @@ module UserAssignments context 'my_module' do let!(:experiment) { create :experiment, project: project } - let!(:my_module) { create :my_module, experiment: experiment } + let!(:my_module) { create :my_module, experiment: experiment, created_by: experiment.created_by, created_by: user_one } - it 'creates user assignments for all users on the project that are not assigned yet' do - expect { - described_class.perform_now(my_module, user_one) - }.to change(UserAssignment, :count).by(3) + it 'user assignments should be created automatically upon my_module creation' do + # check that all users are assigned + my_module.reload + expect([user_one, user_two, user_three] - my_module.user_assignments.reload.map(&:user)).to eq([]) end it 'assigns the same role as the user had on project level' do diff --git a/spec/jobs/user_assignments/propagate_assignment_job_spec.rb b/spec/jobs/user_assignments/propagate_assignment_job_spec.rb index 247aebf45..09b2d6221 100644 --- a/spec/jobs/user_assignments/propagate_assignment_job_spec.rb +++ b/spec/jobs/user_assignments/propagate_assignment_job_spec.rb @@ -3,18 +3,18 @@ require 'rails_helper' module UserAssignments RSpec.describe PropagateAssignmentJob, type: :job do - let!(:owner_role) { create :owner_role } - let!(:technician_role) { create :technician_role } + let!(:technician_role) { create :technician_role } let!(:user_one) { create :user } let!(:user_two) { create :user } let!(:team) { create :team, created_by: user_one } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:project) { create :project, team: team, created_by: user_one } let!(:user_project) { create :user_project, user: user_one, project: project } let!(:experiment_one) { create :experiment, project: project } let!(:experiment_two) { create :experiment, project: project } - let!(:my_module_one) { create :my_module, experiment: experiment_one } - let!(:my_module_two) { create :my_module, experiment: experiment_two } + let!(:my_module_one) { create :my_module, experiment: experiment_one, created_by: experiment_one.created_by } + let!(:my_module_two) { create :my_module, experiment: experiment_two, created_by: experiment_two.created_by } let!(:user_assignment) do create :user_assignment, assignable: project, diff --git a/spec/lib/tasks/my_modules_spec.rb b/spec/lib/tasks/my_modules_spec.rb index 5d2a27f4e..42eaa358a 100644 --- a/spec/lib/tasks/my_modules_spec.rb +++ b/spec/lib/tasks/my_modules_spec.rb @@ -9,7 +9,7 @@ describe 'my_modules:fix_positions' do experiment = create :experiment 100.times do - create :my_module, experiment: experiment + create :my_module, experiment: experiment, created_by: experiment.created_by end # set 10 tasks same position diff --git a/spec/models/experiment_member_spec.rb b/spec/models/experiment_member_spec.rb index d569ee2eb..c05bd1391 100644 --- a/spec/models/experiment_member_spec.rb +++ b/spec/models/experiment_member_spec.rb @@ -3,8 +3,9 @@ require 'rails_helper' describe ExperimentMember, type: :model do - let(:owner_role) { create :owner_role } + # let(:owner_role) { create :owner_role } let!(:user) { create :user } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:project) { create :project } let!(:user_project) { create :user_project, user: user, project: project } let!(:user_assignment) do diff --git a/spec/models/my_module_member_spec.rb b/spec/models/my_module_member_spec.rb index 22104b584..def7b52b5 100644 --- a/spec/models/my_module_member_spec.rb +++ b/spec/models/my_module_member_spec.rb @@ -3,8 +3,9 @@ require 'rails_helper' describe MyModuleMember, type: :model do - let(:owner_role) { create :owner_role } + # let(:owner_role) { create :owner_role } let!(:user) { create :user } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:project) { create :project } let!(:user_project) { create :user_project, user: user, project: project } let!(:user_assignment) do @@ -15,7 +16,7 @@ describe MyModuleMember, type: :model do assigned_by: user end let!(:experiment) { create :experiment, project: project } - let!(:my_module) { create :my_module, experiment: experiment } + let!(:my_module) { create :my_module, experiment: experiment, created_by: user } let(:normal_user_role) { create :normal_user_role } describe '#update' do diff --git a/spec/models/project_member_spec.rb b/spec/models/project_member_spec.rb index 0862a190c..656c4bb61 100644 --- a/spec/models/project_member_spec.rb +++ b/spec/models/project_member_spec.rb @@ -3,9 +3,9 @@ require 'rails_helper' describe ProjectMember, type: :model do - let(:owner_role) { create :owner_role } - let!(:project) { create :project } + let!(:owner_role) { UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) } let!(:user) { create :user } + let!(:project) { create :project, created_by: user } let(:normal_user_role) { create :normal_user_role } let(:subject) { described_class.new(user, project, user) } @@ -84,13 +84,7 @@ describe ProjectMember, type: :model do assigned_by: user end let!(:user_project) { create :user_project, user: user, project: project } - let!(:user_assignment) do - create :user_assignment, - assignable: project, - user: user, - user_role: owner_role, - assigned_by: user - end + let!(:user_assignment) { project.user_assignments.first } it 'removes the user_assignment and user_project' do expect { @@ -157,6 +151,7 @@ describe ProjectMember, type: :model do end it 'does not add an error when role exists' do + project.user_assignments.destroy_all subject.assign = true subject.user_role_id = owner_role.id subject.valid? diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2fe6f1591..2b5ae68ce 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -80,7 +80,7 @@ describe Project, type: :model do end describe 'after create hooks' do - let!(:owner_role) { create :owner_role } + it 'grands owner permissions to project creator' do user = create(:user) diff --git a/spec/permissions/controllers/canvas_controller_spec.rb b/spec/permissions/controllers/canvas_controller_spec.rb index 3d2aa1490..da613e042 100644 --- a/spec/permissions/controllers/canvas_controller_spec.rb +++ b/spec/permissions/controllers/canvas_controller_spec.rb @@ -53,7 +53,7 @@ describe CanvasController, type: :controller do it_behaves_like "a controller action with permissions checking", :post, :update do let(:testable) { my_modules.first } - let(:permissions) { [MyModulePermissions::ARCHIVE] } + let(:permissions) { [MyModulePermissions::MANAGE] } let(:action_params) { { id: experiment.id, remove: "#{my_modules.first.id},#{my_modules.second.id}" } } end diff --git a/spec/permissions/result_permission_spec.rb b/spec/permissions/result_permission_spec.rb index 70add08ab..95d9b06ed 100644 --- a/spec/permissions/result_permission_spec.rb +++ b/spec/permissions/result_permission_spec.rb @@ -8,11 +8,11 @@ describe 'ResultPermissions' do let(:user) { create :user, current_team_id: team.id } let(:team) { create :team } let(:result) { create :result, user: user, my_module: my_module } - let(:my_module) { create :my_module, experiment: experiment } + let(:my_module) { create :my_module, experiment: experiment, created_by: experiment.created_by } let(:experiment) { create :experiment, user: user } before do - create_user_assignment(my_module, (create :owner_role), user) + create_user_assignment(my_module, UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), user) end describe 'can_read_result?' do @@ -40,7 +40,6 @@ describe 'ResultPermissions' do it 'should be false for archived result' do result.archive!(user) - expect(can_manage_result?(user, result)).to be_falsey end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index f81907521..02a01a03e 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,6 +1,7 @@ # 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' @@ -39,7 +40,45 @@ RSpec.configure do |config| # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures config.fixture_path = "#{::Rails.root}/spec/fixtures" - config.use_transactional_fixtures = true + config.use_transactional_fixtures = false + config.before(:suite) do + DatabaseCleaner.clean_with(:truncation) + end + + config.before(:each) do + DatabaseCleaner.strategy = :transaction + end + + config.before(:each, js: true) do + DatabaseCleaner.strategy = :truncation + end + + config.before(:each) do + DatabaseCleaner.start + + # project creation now requires an owner role to be present, as it assigns it to creator + # so we must ensure it always exists + UserRole.exists?(name: I18n.t('user_roles.predefined.owner')) || UserRole.owner_role.save! + end + + config.after(:each) do + DatabaseCleaner.clean + end + + config.before(:all) do + DatabaseCleaner.strategy = :transaction + end + + config.before(:all) do + DatabaseCleaner.start + # project creation now requires an owner role to be present, as it assigns it to creator + # so we must ensure it always exists + UserRole.exists?(name: I18n.t('user_roles.predefined.owner')) || UserRole.owner_role.save! + end + + config.after(:all) do + DatabaseCleaner.clean + end Delayed::Worker.delay_jobs = false diff --git a/spec/requests/api/v1/assets_controller_spec.rb b/spec/requests/api/v1/assets_controller_spec.rb index 9ecc575a4..d78675b16 100644 --- a/spec/requests/api/v1/assets_controller_spec.rb +++ b/spec/requests/api/v1/assets_controller_spec.rb @@ -14,7 +14,7 @@ RSpec.describe 'Api::V1::AssetsController', type: :request do create(:user_team, user: @user, team: @team) - create_user_assignment(@task, (create :owner_role), @user) + create_user_assignment(@task, UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), @user) @valid_headers = { 'Authorization': 'Bearer ' + generate_token(@user.id) } diff --git a/spec/requests/api/v1/checklist_items_controller_spec.rb b/spec/requests/api/v1/checklist_items_controller_spec.rb index 46f169627..0978f0e54 100644 --- a/spec/requests/api/v1/checklist_items_controller_spec.rb +++ b/spec/requests/api/v1/checklist_items_controller_spec.rb @@ -14,7 +14,7 @@ RSpec.describe 'Api::V1::ChecklistsController', type: :request do @checklist = create(:checklist, step: @step) create(:user_team, user: @user, team: @team) - create_user_assignment(@task, (create :owner_role), @user) + create_user_assignment(@task, UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), @user) @valid_headers = { 'Authorization': 'Bearer ' + generate_token(@user.id), diff --git a/spec/requests/api/v1/checklists_controller_spec.rb b/spec/requests/api/v1/checklists_controller_spec.rb index 238ef4d7f..67679198e 100644 --- a/spec/requests/api/v1/checklists_controller_spec.rb +++ b/spec/requests/api/v1/checklists_controller_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'Api::V1::ChecklistsController', type: :request do @protocol = create(:protocol, my_module: @task) @step = create(:step, protocol: @protocol) create(:user_team, user: @user, team: @team) - create_user_assignment(@task, (create :owner_role), @user) + create_user_assignment(@task, UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), @user) @valid_headers = { 'Authorization': 'Bearer ' + generate_token(@user.id), 'Content-Type': 'application/json' diff --git a/spec/requests/api/v1/experiment_user_assignments_controller_spec.rb b/spec/requests/api/v1/experiment_user_assignments_controller_spec.rb index 44770972b..7898427c0 100644 --- a/spec/requests/api/v1/experiment_user_assignments_controller_spec.rb +++ b/spec/requests/api/v1/experiment_user_assignments_controller_spec.rb @@ -8,7 +8,7 @@ RSpec.describe "Api::V1::ExperimentUserAssignmentsController", type: :request do @another_user = create(:user) @team = create(:team, created_by: @user) @normal_user_role = create :normal_user_role - @owner_role = create :owner_role + @owner_role = UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) create(:user_team, user: @user, team: @team, role: :normal_user) create(:user_team, user: @another_user, team: @team, role: :normal_user) @own_project = create(:project, name: Faker::Name.unique.name, created_by: @user, team: @team) diff --git a/spec/requests/api/v1/experiments_controller_spec.rb b/spec/requests/api/v1/experiments_controller_spec.rb index b0a03320a..cfd9481f3 100644 --- a/spec/requests/api/v1/experiments_controller_spec.rb +++ b/spec/requests/api/v1/experiments_controller_spec.rb @@ -16,7 +16,7 @@ RSpec.describe "Api::V1::ExperimentsController", type: :request do create :user_assignment, assignable: @valid_project, user: @user, - user_role: create(:owner_role), + user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), assigned_by: @user @unaccessible_project = create(:project, name: Faker::Name.unique.name, diff --git a/spec/requests/api/v1/project_user_assignments_controller_spec.rb b/spec/requests/api/v1/project_user_assignments_controller_spec.rb index d65fa87c1..ebe3ec482 100644 --- a/spec/requests/api/v1/project_user_assignments_controller_spec.rb +++ b/spec/requests/api/v1/project_user_assignments_controller_spec.rb @@ -13,7 +13,7 @@ RSpec.describe "Api::V1::ProjectUserAssignmentsController", type: :request do @invalid_project = create(:project, name: Faker::Name.unique.name, created_by: @another_user, team: @team, visibility: :hidden) create(:user_project, user: @user, project: @own_project) - create :user_assignment, assignable: @own_project, user: @user, user_role: create(:owner_role), assigned_by: @user + create :user_assignment, assignable: @own_project, user: @user, user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), assigned_by: @user @normal_user_role = create :normal_user_role @valid_headers = { 'Authorization': 'Bearer ' + generate_token(@user.id) } diff --git a/spec/requests/api/v1/projects_controller_spec.rb b/spec/requests/api/v1/projects_controller_spec.rb index 8c464d1fc..91d18752f 100644 --- a/spec/requests/api/v1/projects_controller_spec.rb +++ b/spec/requests/api/v1/projects_controller_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Api::V1::ProjectsController', type: :request do @user = create(:user) @teams = create_list(:team, 2, created_by: @user) create(:user_team, user: @user, team: @teams.first, role: 2) - owner_role = create(:owner_role) + owner_role = UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) # valid_projects 2.times do project = create(:project, name: Faker::Name.unique.name, created_by: @user, team: @teams.first) diff --git a/spec/requests/api/v1/results_controller_spec.rb b/spec/requests/api/v1/results_controller_spec.rb index 4a76e1f76..e998824d4 100644 --- a/spec/requests/api/v1/results_controller_spec.rb +++ b/spec/requests/api/v1/results_controller_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Api::V1::ResultsController', type: :request do create :user_assignment, assignable: @valid_project, user: @user, - user_role: create(:owner_role), + user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), assigned_by: @user @unaccessible_project = create(:project, name: Faker::Name.unique.name, diff --git a/spec/requests/api/v1/steps_controller_spec.rb b/spec/requests/api/v1/steps_controller_spec.rb index 6d153784a..44fcfab8c 100644 --- a/spec/requests/api/v1/steps_controller_spec.rb +++ b/spec/requests/api/v1/steps_controller_spec.rb @@ -10,7 +10,7 @@ RSpec.describe 'Api::V1::StepsController', type: :request do @experiment = create(:experiment, :with_tasks, project: @project) @task = @experiment.my_modules.first create(:user_team, user: @user, team: @team) - create_user_assignment(@task, (create :owner_role), @user) + create_user_assignment(@task, UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), @user) @valid_headers = { 'Authorization': 'Bearer ' + generate_token(@user.id) } diff --git a/spec/requests/api/v1/tables_controller_spec.rb b/spec/requests/api/v1/tables_controller_spec.rb index 3567f42c0..a2ca175bd 100644 --- a/spec/requests/api/v1/tables_controller_spec.rb +++ b/spec/requests/api/v1/tables_controller_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'Api::V1::TablesController', type: :request do @protocol = create(:protocol, my_module: @task) @step = create(:step, protocol: @protocol) create(:user_team, user: @user, team: @team) - create_user_assignment(@task, (create :owner_role), @user) + create_user_assignment(@task, UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), @user) @valid_headers = { 'Authorization': 'Bearer ' + generate_token(@user.id), diff --git a/spec/requests/api/v1/task_user_assignments_controller_spec.rb b/spec/requests/api/v1/task_user_assignments_controller_spec.rb index b286f1ab1..a9758b91e 100644 --- a/spec/requests/api/v1/task_user_assignments_controller_spec.rb +++ b/spec/requests/api/v1/task_user_assignments_controller_spec.rb @@ -8,7 +8,7 @@ RSpec.describe "Api::V1::TaskUserAssignmentsController", type: :request do @another_user = create(:user) @team = create(:team, created_by: @user) @normal_user_role = create :normal_user_role - @owner_role = create :owner_role + @owner_role = UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) create(:user_team, user: @user, team: @team, role: :normal_user) create(:user_team, user: @another_user, team: @team, role: :normal_user) @own_project = create(:project, name: Faker::Name.unique.name, created_by: @user, team: @team) @@ -16,7 +16,7 @@ RSpec.describe "Api::V1::TaskUserAssignmentsController", type: :request do name: Faker::Name.unique.name, project: @own_project, created_by: @user - @own_task = create :my_module, name: Faker::Name.unique.name, experiment: @own_experiment + @own_task = create :my_module, name: Faker::Name.unique.name, experiment: @own_experiment, created_by: @own_experiment.created_by @invalid_project = create :project, name: Faker::Name.unique.name, created_by: @another_user, diff --git a/spec/requests/api/v1/tasks_controller_spec.rb b/spec/requests/api/v1/tasks_controller_spec.rb index e369e5818..c54075943 100644 --- a/spec/requests/api/v1/tasks_controller_spec.rb +++ b/spec/requests/api/v1/tasks_controller_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Api::V1::TasksController', type: :request do @user = create(:user) @teams = create_list(:team, 2, created_by: @user) create(:user_team, user: @user, team: @teams.first, role: 2) - @owner_role = create(:owner_role) + @owner_role = UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) @valid_project = create(:project, name: Faker::Name.unique.name, created_by: @user, team: @teams.first) diff --git a/spec/services/experiments/move_to_project_service_spec.rb b/spec/services/experiments/move_to_project_service_spec.rb index 22841dfee..46a68766e 100644 --- a/spec/services/experiments/move_to_project_service_spec.rb +++ b/spec/services/experiments/move_to_project_service_spec.rb @@ -4,17 +4,16 @@ require 'rails_helper' describe Experiments::MoveToProjectService do let(:team) { create :team, :with_members } + let(:user) { create :user } let(:project) do - create :project, team: team, user_projects: [] + create :project, team: team, created_by: user end let(:new_project) do - create :project, team: team, user_projects: [user_project2] + create :project, team: team, created_by: user end let(:experiment) do create :experiment, :with_tasks, name: 'MyExp', project: project end - let(:user) { create :user } - let(:user_project2) { create :user_project, :normal_user, user: user } let(:service_call) do Experiments::MoveToProjectService.call(experiment_id: experiment.id, project_id: new_project.id, @@ -72,7 +71,7 @@ describe Experiments::MoveToProjectService do end it 'returns an error on validate' do - allow_any_instance_of(Experiment).to(receive(:moveable_projects).and_return([])) + allow_any_instance_of(Experiment).to(receive(:movable_projects).and_return([])) expect(service_call.succeed?).to be_falsey end diff --git a/spec/services/experiments_overview_service_spec.rb b/spec/services/experiments_overview_service_spec.rb index f0bdc04c3..37fe58bf2 100644 --- a/spec/services/experiments_overview_service_spec.rb +++ b/spec/services/experiments_overview_service_spec.rb @@ -6,7 +6,7 @@ describe ExperimentsOverviewService do EXPERIMENTS_CNT = 26 time = Time.new(2019, 8, 1, 14, 35, 0) let!(:user) { create :user } - let!(:project) { create :project } + let!(:project) { create :project, created_by: user } before do @experiments_overview = ExperimentsOverviewService.new(project, user, params) end diff --git a/spec/services/smart_annotations/html_preview_spec.rb b/spec/services/smart_annotations/html_preview_spec.rb index 8be773a22..98549b64a 100644 --- a/spec/services/smart_annotations/html_preview_spec.rb +++ b/spec/services/smart_annotations/html_preview_spec.rb @@ -12,7 +12,7 @@ describe SmartAnnotations::HtmlPreview do created_by: user, last_modified_by: user end - let(:task) { create :my_module, name: 'task', experiment: experiment } + let(:task) { create :my_module, name: 'task', experiment: experiment, created_by: experiment.created_by } describe 'Project annotations with type prj' do it 'returns a html snippet' do diff --git a/spec/services/smart_annotations/permission_eval_spec.rb b/spec/services/smart_annotations/permission_eval_spec.rb index 9b1d80f5a..156fe1a9f 100644 --- a/spec/services/smart_annotations/permission_eval_spec.rb +++ b/spec/services/smart_annotations/permission_eval_spec.rb @@ -15,7 +15,7 @@ describe SmartAnnotations::PermissionEval do create :user_assignment, assignable: project, user: user, - user_role: create(:owner_role), + user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), assigned_by: user end let(:experiment) do @@ -24,7 +24,7 @@ describe SmartAnnotations::PermissionEval do created_by: user, last_modified_by: user end - let(:task) { create :my_module, name: 'task', experiment: experiment } + let(:task) { create :my_module, name: 'task', experiment: experiment, created_by: experiment.created_by } let(:repository) { create :repository, team: team, created_by: user } let(:repository_item) { create :repository_row, repository: repository } diff --git a/spec/services/smart_annotations/tag_to_html_spec.rb b/spec/services/smart_annotations/tag_to_html_spec.rb index fbff77457..a822f6579 100644 --- a/spec/services/smart_annotations/tag_to_html_spec.rb +++ b/spec/services/smart_annotations/tag_to_html_spec.rb @@ -14,7 +14,7 @@ describe SmartAnnotations::TagToHtml do create :user_assignment, assignable: project, user: user, - user_role: create(:owner_role), + user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), assigned_by: user end let(:text) do diff --git a/spec/services/smart_annotations/tag_to_text_spec.rb b/spec/services/smart_annotations/tag_to_text_spec.rb index 63e5aa6a6..a95e9af3b 100644 --- a/spec/services/smart_annotations/tag_to_text_spec.rb +++ b/spec/services/smart_annotations/tag_to_text_spec.rb @@ -14,7 +14,7 @@ describe SmartAnnotations::TagToText do create :user_assignment, assignable: project, user: user, - user_role: create(:owner_role), + user_role: UserRole.find_by(name: I18n.t('user_roles.predefined.owner')), assigned_by: user end diff --git a/spec/services/smart_annotations/text_preview_spec.rb b/spec/services/smart_annotations/text_preview_spec.rb index 41ad34c9b..5a277d8d8 100644 --- a/spec/services/smart_annotations/text_preview_spec.rb +++ b/spec/services/smart_annotations/text_preview_spec.rb @@ -12,7 +12,7 @@ describe SmartAnnotations::TextPreview do created_by: user, last_modified_by: user end - let(:task) { create :my_module, name: 'task', experiment: experiment } + let(:task) { create :my_module, name: 'task', experiment: experiment, created_by: experiment.created_by } describe 'Project annotations' do it 'returns a text snippet' do diff --git a/spec/support/reference_project_structure_context.rb b/spec/support/reference_project_structure_context.rb index 063b23a48..e5def1b23 100644 --- a/spec/support/reference_project_structure_context.rb +++ b/spec/support/reference_project_structure_context.rb @@ -1,7 +1,7 @@ # Example # # include_context 'reference_project_structure', { -# role: :owner_role, +# role: :owner, # result_asset: true, # step: true, # team: @team, @@ -15,17 +15,23 @@ RSpec.shared_context 'reference_project_structure' do |config| config ||= {} let!(:user) { subject.current_user } - let!(:role) { create (config[:role] || :owner_role) } + let!(:role) do + if config[:role] + UserRole.find_by(name: I18n.t("user_roles.predefined.#{config[:role]}")) || create("#{config[:role]}_role") + else + UserRole.find_by(name: I18n.t("user_roles.predefined.owner")) || UserRole.find_by(name: I18n.t('user_roles.predefined.owner')) + end + end let!(:team) { config[:team] || (create :team, created_by: user) } 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!(:project) { role && create(:project, team: team, created_by: user) } + let!(:projects) { role && 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_module) { create :my_module, experiment: experiment, created_by: user } 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] diff --git a/spec/support/shared_examples/controller_action_with_permission_checking.rb b/spec/support/shared_examples/controller_action_with_permission_checking.rb index ff06c73aa..dd5691d45 100644 --- a/spec/support/shared_examples/controller_action_with_permission_checking.rb +++ b/spec/support/shared_examples/controller_action_with_permission_checking.rb @@ -7,7 +7,7 @@ RSpec.shared_examples 'a controller action with permissions checking' do |verb, 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 = 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)