Merge branch 'features/new_permissions' of https://github.com/biosistemika/scinote-web into new_perimssions-managing-project-ui

app/models/concerns/user/project_roles.rb

@@ -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

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

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

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

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

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

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

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

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."

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

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

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');