diff --git a/app/models/concerns/user/project_roles.rb b/app/models/concerns/user/project_roles.rb deleted file mode 100644 index c20d2da55..000000000 --- a/app/models/concerns/user/project_roles.rb +++ /dev/null @@ -1,60 +0,0 @@ -require 'aspector' - -module User::ProjectRoles - extend ActiveSupport::Concern - - aspector do - # Check if user is member of project - around %i( - is_member_of_project? - is_owner_of_project? - is_user_of_project? - is_user_or_higher_of_project? - is_technician_of_project? - is_technician_or_higher_of_project? - is_viewer_of_project? - ) do |proxy, *args, &block| - if args[0] - @user_project = user_projects.where(project: args[0]).take - @user_project ? proxy.call(*args, &block) : false - else - false - end - end - end - - def is_member_of_project?(project) - # This is already checked by aspector, so just return true - true - end - - def is_creator_of_project?(project) - project.created_by == self - end - - def is_owner_of_project?(project) - @user_project.owner? - end - - def is_user_of_project?(project) - @user_project.normal_user? - end - - def is_user_or_higher_of_project?(project) - @user_project.normal_user? or @user_project.owner? - end - - def is_technician_of_project?(project) - @user_project.technician? - end - - def is_technician_or_higher_of_project?(project) - @user_project.technician? or - @user_project.normal_user? or - @user_project.owner? - end - - def is_viewer_of_project?(project) - @user_project.viewer? - end -end diff --git a/app/models/user.rb b/app/models/user.rb index 5bc6add06..9646853ac 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,7 +5,6 @@ class User < ApplicationRecord include SettingsModel include VariablesModel include User::TeamRoles - include User::ProjectRoles include TeamBySubjectModel include InputSanitizeHelper include ActiveStorageConcerns diff --git a/app/models/user_assignment.rb b/app/models/user_assignment.rb index b4a1f2fe9..bd0c728b5 100644 --- a/app/models/user_assignment.rb +++ b/app/models/user_assignment.rb @@ -4,5 +4,5 @@ class UserAssignment < ApplicationRecord belongs_to :assignable, polymorphic: true belongs_to :user_role belongs_to :user - belongs_to :assigned_by, foreign_key: 'assigned_by_id', class_name: 'User' + belongs_to :assigned_by, foreign_key: 'assigned_by_id', class_name: 'User', optional: true end diff --git a/app/models/user_role.rb b/app/models/user_role.rb index b7b5efe4e..5c9827e50 100644 --- a/app/models/user_role.rb +++ b/app/models/user_role.rb @@ -1,14 +1,91 @@ # frozen_string_literal: true class UserRole < ApplicationRecord + before_update :prevent_update, if: :predefined? + validates :name, presence: true, length: { minimum: Constants::NAME_MIN_LENGTH, maximum: Constants::NAME_MAX_LENGTH }, uniqueness: { case_sensitive: false } validates :permissions, presence: true, length: { minimum: 1 } + 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' - belongs_to :last_modified_by, foreign_key: 'last_modified_by_id', class_name: 'User' + 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 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) }, + predefined: true + ) + end + + def self.normal_user_role + new( + name: I18n.t('user_roles.predefined.normal_user'), + permissions: + [ + ProjectPermissions::READ, + ProjectPermissions::CREATE_EXPERIMENTS, + ProjectPermissions::CREATE_COMMENTS, + ExperimentPermissions::READ, + ExperimentPermissions::MANAGE, + ExperimentPermissions::ARCHIVE, + ExperimentPermissions::RESTORE, + ExperimentPermissions::CLONE, + ExperimentPermissions::CREATE_TASKS, + MyModulePermissions::READ, + MyModulePermissions::CREATE_COMMENTS, + MyModulePermissions::ASSIGN_REPOSITORY_ROWS, + MyModulePermissions::CHANGE_FLOW_STATUS, + MyModulePermissions::CREATE_REPOSITORY_SNAPSHOT, + MyModulePermissions::MANAGE_REPOSITORY_SNAPSHOT + ], + predefined: true + ) + end + + def self.technician_role + new( + name: I18n.t('user_roles.predefined.technician'), + permissions: + [ + ProjectPermissions::READ, + ProjectPermissions::CREATE_COMMENTS, + ExperimentPermissions::READ, + MyModulePermissions::READ, + MyModulePermissions::CREATE_COMMENTS, + MyModulePermissions::ASSIGN_REPOSITORY_ROWS, + MyModulePermissions::CHANGE_FLOW_STATUS, + MyModulePermissions::CREATE_REPOSITORY_SNAPSHOT, + MyModulePermissions::MANAGE_REPOSITORY_SNAPSHOT + ], + predefined: true + ) + end + + def self.viewer_role + new( + name: I18n.t('user_roles.predefined.viewer'), + permissions: + [ + ProjectPermissions::READ, + ExperimentPermissions::READ, + MyModulePermissions::READ + ], + predefined: true + ) + end + + private + + def prevent_update + raise ActiveRecord::RecordInvalid, I18n.t('user_roles.predefined.unchangable_error_message') + end end diff --git a/app/permissions/experiment.rb b/app/permissions/experiment.rb index a68eb54b1..b539d9d2c 100644 --- a/app/permissions/experiment.rb +++ b/app/permissions/experiment.rb @@ -59,7 +59,7 @@ Canaid::Permissions.register_for(Experiment) do # experiment: move can :move_experiment do |user, experiment| - can_clone_experiment?(user, experiment) + experiment.permission_granted?(user, ExperimentPermissions::MOVE) end end @@ -125,9 +125,6 @@ Canaid::Permissions.register_for(Comment) do # step: update/delete comment can :manage_comment_in_module do |user, comment| my_module = ::PermissionsUtil.get_comment_module(comment) - project = my_module.experiment.project - # Same check as in `can_manage_comment_in_project?` - project.present? && - (user.is_owner_of_project?(project) || comment.user == user) + comment.user == user || my_module.permission_granted?(user, MyModulePermissions::MANAGE_COMMENTS) end end diff --git a/app/permissions/my_module.rb b/app/permissions/my_module.rb index 3419be5ef..380421ea3 100644 --- a/app/permissions/my_module.rb +++ b/app/permissions/my_module.rb @@ -23,57 +23,56 @@ Canaid::Permissions.register_for(MyModule) do # module: update # result: create, update can :manage_module do |user, my_module| - can_manage_experiment?(user, my_module.experiment) + my_module.permission_granted?(user, MyModulePermissions::MANAGE) end # module: archive can :archive_module do |user, my_module| - can_manage_experiment?(user, my_module.experiment) + my_module.permission_granted?(user, MyModulePermissions::ARCHIVE) end # NOTE: Must not be dependent on canaid parmision for which we check if it's # active # module: restore can :restore_module do |user, my_module| - user.is_user_or_higher_of_project?(my_module.experiment.project) && - my_module.archived? + my_module.archived? && my_module.permission_granted?(user, MyModulePermissions::RESTORE) end # module: move can :move_module do |user, my_module| - can_manage_experiment?(user, my_module.experiment) + my_module.permission_granted?(user, MyModulePermissions::MOVE) end # module: assign/reassign/unassign users can :manage_users_in_module do |user, my_module| - user.is_owner_of_project?(my_module.experiment.project) + my_module.permission_granted?(user, MyModulePermissions::MANAGE_USERS) end # module: assign/unassign repository record # NOTE: Use 'module_page? &&' before calling this permission! can :assign_repository_rows_to_module do |user, my_module| - user.is_technician_or_higher_of_project?(my_module.experiment.project) + my_module.permission_granted?(user, MyModulePermissions::ASSIGN_REPOSITORY_ROWS) end # module: change_flow_status can :change_my_module_flow_status do |user, my_module| - user.is_technician_or_higher_of_project?(my_module.experiment.project) + my_module.permission_granted?(user, MyModulePermissions::CHANGE_FLOW_STATUS) end # module: create comment # result: create comment # step: create comment can :create_comments_in_module do |user, my_module| - can_create_comments_in_project?(user, my_module.experiment.project) + my_module.permission_granted?(user, MyModulePermissions::CREATE_COMMENTS) end # module: create a snapshot of repository item can :create_my_module_repository_snapshot do |user, my_module| - user.is_technician_or_higher_of_project?(my_module.experiment.project) + my_module.permission_granted?(user, MyModulePermissions::CREATE_REPOSITORY_SNAPSHOT) end # module: make a repository snapshot selected can :manage_my_module_repository_snapshots do |user, my_module| - user.is_technician_or_higher_of_project?(my_module.experiment.project) + my_module.permission_granted?(user, MyModulePermissions::MANAGE_REPOSITORY_SNAPSHOT) end end diff --git a/app/permissions/project.rb b/app/permissions/project.rb index a42bdf89f..b31e8c858 100644 --- a/app/permissions/project.rb +++ b/app/permissions/project.rb @@ -76,7 +76,7 @@ Canaid::Permissions.register_for(Project) do # project: create/update/delete tag # module: assign/reassign/unassign tag can :manage_tags do |user, project| - user.is_user_or_higher_of_project?(project) + project.permission_granted?(user, ProjectPermissions::MANAGE_TAGS) end end @@ -92,6 +92,6 @@ Canaid::Permissions.register_for(ProjectComment) do # project: update/delete comment can :manage_comment_in_project do |user, project_comment| project_comment.project.present? && (project_comment.user == user || - project.permission_granted?(user, ProjectPermissions::EDIT_COMMENTS)) + project.permission_granted?(user, ProjectPermissions::MANAGE_COMMENTS)) end end diff --git a/config/initializers/extends/permission_extends.rb b/config/initializers/extends/permission_extends.rb index 687adc167..d5418cae6 100644 --- a/config/initializers/extends/permission_extends.rb +++ b/config/initializers/extends/permission_extends.rb @@ -10,10 +10,9 @@ module PermissionExtends RESTORE CREATE_EXPERIMENTS CREATE_COMMENTS - EDIT_COMMENTS - DELETE_COMMENTS + MANAGE_COMMENTS MANAGE_TAGS - ).each { |permission| const_set(permission, permission.underscore) } + ).each { |permission| const_set(permission, "project_#{permission.underscore}") } end module ExperimentPermissions @@ -24,11 +23,13 @@ module PermissionExtends RESTORE CLONE MOVE - ).each { |permission| const_set(permission, permission.underscore) } + CREATE_TASKS + ).each { |permission| const_set(permission, "experiment_#{permission.underscore}") } end module MyModulePermissions %w( + READ MANAGE ARCHIVE RESTORE @@ -37,9 +38,10 @@ module PermissionExtends ASSIGN_REPOSITORY_ROWS CHANGE_FLOW_STATUS CREATE_COMMENTS + MANAGE_COMMENTS CREATE_REPOSITORY_SNAPSHOT MANAGE_REPOSITORY_SNAPSHOT - ).each { |permission| const_set(permission, permission.underscore) } + ).each { |permission| const_set(permission, "task_#{permission.underscore}") } end module RepositoryPermissions @@ -57,6 +59,6 @@ module PermissionExtends CREATE_COLUMN UPDATE_COLUMN DELETE_COLUMN - ).each { |permission| const_set(permission, permission.underscore) } + ).each { |permission| const_set(permission, "inventory_#{permission.underscore}") } end end diff --git a/config/locales/en.yml b/config/locales/en.yml index a41543113..d9885af9c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1828,6 +1828,14 @@ en: destroy_uo_confirm: "Remove" leave_flash: "Successfuly left team %{team}." + user_roles: + predefined: + owner: "Owner" + normal_user: "User" + technician: "Technician" + viewer: "Viewer" + unchangable_error_message: "Predefined roles can not be changed!" + protocols: protocols_io_import: title_too_long: "... Text is too long so we had to cut it off." diff --git a/db/migrate/20210202214508_create_user_roles_and_assignments.rb b/db/migrate/20210202214508_create_user_roles_and_assignments.rb index df5a75b53..3d6dc65d4 100644 --- a/db/migrate/20210202214508_create_user_roles_and_assignments.rb +++ b/db/migrate/20210202214508_create_user_roles_and_assignments.rb @@ -4,9 +4,10 @@ class CreateUserRolesAndAssignments < ActiveRecord::Migration[6.1] def change create_table :user_roles do |t| t.string :name + t.boolean :predefined, default: false t.string :permissions, array: true, default: [] - t.references :created_by, foreign_key: { to_table: :users }, null: false - t.references :last_modified_by, foreign_key: { to_table: :users }, null: false + t.references :created_by, foreign_key: { to_table: :users }, null: true + t.references :last_modified_by, foreign_key: { to_table: :users }, null: true t.timestamps end @@ -15,7 +16,7 @@ class CreateUserRolesAndAssignments < ActiveRecord::Migration[6.1] t.references :assignable, polymorphic: true, null: false t.references :user, foreign_key: true, null: false t.references :user_role, foreign_key: true, null: false - t.references :assigned_by, foreign_key: { to_table: :users }, null: false + t.references :assigned_by, foreign_key: { to_table: :users }, null: true t.timestamps end diff --git a/db/migrate/20210222123823_migrate_to_new_user_roles.rb b/db/migrate/20210222123823_migrate_to_new_user_roles.rb new file mode 100644 index 000000000..b7fccc161 --- /dev/null +++ b/db/migrate/20210222123823_migrate_to_new_user_roles.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +class MigrateToNewUserRoles < ActiveRecord::Migration[6.1] + def change + reversible do |dir| + dir.up do + owner_role = UserRole.owner_role + owner_role.save! + normal_user_role = UserRole.normal_user_role + normal_user_role.save! + technician_role = UserRole.technician_role + technician_role.save! + viewer_role = UserRole.viewer_role + viewer_role.save! + + create_user_assignments(UserProject.owner, owner_role) + create_user_assignments(UserProject.normal_user, normal_user_role) + create_user_assignments(UserProject.technician, technician_role) + create_user_assignments(UserProject.viewer, viewer_role) + + UserProject.delete_all + end + dir.down do + project_assignments = UserAssignment.joins(:user_role).where(assignable_type: 'Project') + create_user_project( + project_assignments.where(user_role: { name: I18n.t('user_roles.predefined.owner'), predefined: true }), + 'owner' + ) + create_user_project( + project_assignments.where(user_role: { name: I18n.t('user_roles.predefined.normal_user'), predefined: true }), + 'normal_user' + ) + create_user_project( + project_assignments.where(user_role: { name: I18n.t('user_roles.predefined.technician'), predefined: true }), + 'technician' + ) + create_user_project( + project_assignments.where(user_role: { name: I18n.t('user_roles.predefined.viewer'), predefined: true }), + 'viewer' + ) + + UserAssignment.joins(:user_role).where(user_role: { predefined: true }).delete_all + UserRole.where(predefined: true).delete_all + end + end + end + + private + + def create_user_assignments(user_projects, user_role) + user_projects.includes(:user, :project).find_in_batches(batch_size: 100) do |user_project_batch| + user_assignments = [] + user_project_batch.each do |user_project| + user_assignments << UserAssignment.new(user: user_project.user, + assignable: user_project.project, + user_role: user_role) + end + UserAssignment.import(user_assignments) + end + end + + def create_user_project(user_roles, role) + user_roles.includes(:user, :assignable).find_in_batches(batch_size: 100) do |user_role_batch| + user_projects = [] + user_role_batch.each do |user_role| + user_projects << UserProject.new(user: user_role.user, project: user_role.assignable, role: role) + end + UserProject.import(user_projects) + end + end +end diff --git a/db/structure.sql b/db/structure.sql index a824cb8d7..e8a29c461 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2440,7 +2440,7 @@ CREATE TABLE public.user_assignments ( assignable_id bigint NOT NULL, user_id bigint NOT NULL, user_role_id bigint NOT NULL, - assigned_by_id bigint NOT NULL, + assigned_by_id bigint, created_at timestamp(6) without time zone NOT NULL, updated_at timestamp(6) without time zone NOT NULL ); @@ -2609,9 +2609,10 @@ ALTER SEQUENCE public.user_projects_id_seq OWNED BY public.user_projects.id; CREATE TABLE public.user_roles ( id bigint NOT NULL, name character varying, + predefined boolean DEFAULT false, permissions character varying[] DEFAULT '{}'::character varying[], - created_by_id bigint NOT NULL, - last_modified_by_id bigint NOT NULL, + created_by_id bigint, + last_modified_by_id bigint, created_at timestamp(6) without time zone NOT NULL, updated_at timestamp(6) without time zone NOT NULL ); @@ -7335,6 +7336,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20201209165626'), ('20210128105457'), ('20210128105458'), -('20210202214508'); +('20210202214508'), +('20210222123823');