From 5a123d26ac0765291a8184c82b6619a6ca65104d Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Mon, 25 Feb 2019 15:21:13 +0100 Subject: [PATCH 1/2] Activity model update --- app/models/activity.rb | 146 +++--------------- app/models/experiment.rb | 1 + app/models/my_module.rb | 3 +- app/models/team.rb | 1 + .../experiments/move_to_project_service.rb | 1 + config/initializers/extends.rb | 64 ++++++++ spec/factories/{activity.rb => activities.rb} | 7 + spec/models/activity_spec.rb | 30 +++- 8 files changed, 127 insertions(+), 126 deletions(-) rename spec/factories/{activity.rb => activities.rb} (62%) diff --git a/app/models/activity.rb b/app/models/activity.rb index 12aeee5ee..51715299a 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -1,135 +1,33 @@ +# frozen_string_literal: true + class Activity < ApplicationRecord - include InputSanitizeHelper + enum type_of: Extends::ACTIVITY_TYPES - # after_create :generate_notification - - enum type_of: [ - :create_project, - :rename_project, - :change_project_visibility, - :archive_project, - :restore_project, - :assign_user_to_project, - :change_user_role_on_project, - :unassign_user_from_project, - :create_module, - :clone_module, - :archive_module, - :restore_module, - :change_module_description, - :assign_user_to_module, - :unassign_user_from_module, - :create_step, - :destroy_step, - :add_comment_to_step, - :complete_step, - :uncomplete_step, - :check_step_checklist_item, - :uncheck_step_checklist_item, - :edit_step, - :add_result, - :add_comment_to_result, - :archive_result, - :edit_result, - :create_experiment, - :edit_experiment, - :archive_experiment, - :clone_experiment, - :move_experiment, - :add_comment_to_project, - :edit_project_comment, - :delete_project_comment, - :add_comment_to_module, - :edit_module_comment, - :delete_module_comment, - :edit_step_comment, - :delete_step_comment, - :edit_result_comment, - :delete_result_comment, - :destroy_result, - :start_edit_wopi_file, - :unlock_wopi_file, - :load_protocol_from_file, - :load_protocol_from_repository, - :revert_protocol, - :create_report, - :delete_report, - :edit_report, - :assign_sample, - :unassign_sample, - :complete_task, - :uncomplete_task, - :assign_repository_record, - :unassign_repository_record - ] - - validates :type_of, presence: true - - belongs_to :project, inverse_of: :activities - - # Depricated in SCI-3025 - belongs_to :experiment, inverse_of: :activities, optional: true - # Depricated in SCI-3025 - belongs_to :my_module, inverse_of: :activities, optional: true belongs_to :owner, inverse_of: :activities, class_name: 'User' + belongs_to :subject, polymorphic: true, optional: true - # [MyModule, Experiment, Protoco...? Are we going to list all avaible types?] - belongs_to :subject, polymorphic: true + # For permissions check + belongs_to :project, inverse_of: :activities, optional: true + belongs_to :team, inverse_of: :activities + # Associations for old activity type + belongs_to :experiment, inverse_of: :activities, optional: true + belongs_to :my_module, inverse_of: :activities, optional: true + + validate :activity_version + validates :type_of, :owner, presence: true + validates :subject_type, inclusion: { in: Extends::ACTIVITY_SUBJECT_TYPES, + allow_blank: true } + + def old_activity? + subject.nil? + end private - def generate_notification - if %w(assign_user_to_project - assign_user_to_module unassign_user_from_module).include? type_of - notification_type = :assignment - else - notification_type = :recent_changes - end - - project_m = " - #{project.name}" - if experiment - experiment_m = "| #{I18n.t('search.index.experiment')} - - #{experiment.name}" - end - if my_module - task_m = "| #{I18n.t('search.index.module')} - - #{my_module.name}" - end - - notification = Notification.create( - type_of: notification_type, - title: sanitize_input(message, %w(strong a)), - message: sanitize_input( - "#{I18n.t('search.index.project')} - #{project_m} #{experiment_m} #{task_m}", - %w(strong a) - ), - generator_user_id: user.id - ) - - project.users.each do |project_user| - next if project_user == user - next if !project_user.assignments_notification && - notification.type_of == 'assignment' - next if !project_user.recent_notification && - notification.type_of == 'recent_changes' - UserNotification.create(notification: notification, user: project_user) + def activity_version + if (experiment || my_module) && subject + errors.add(:activity, 'wrong combination of associations') end end end diff --git a/app/models/experiment.rb b/app/models/experiment.rb index 4e13408a3..884b6a4e3 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -24,6 +24,7 @@ class Experiment < ApplicationRecord class_name: 'MyModule' has_many :my_module_groups, inverse_of: :experiment, dependent: :destroy has_many :report_elements, inverse_of: :experiment, dependent: :destroy + # Associations for old activity type has_many :activities, inverse_of: :experiment has_attached_file :workflowimg diff --git a/app/models/my_module.rb b/app/models/my_module.rb index 23425d1f6..daf475004 100644 --- a/app/models/my_module.rb +++ b/app/models/my_module.rb @@ -61,9 +61,10 @@ class MyModule < ApplicationRecord has_many :repository_rows, through: :my_module_repository_rows has_many :user_my_modules, inverse_of: :my_module, dependent: :destroy has_many :users, through: :user_my_modules - has_many :activities, inverse_of: :my_module has_many :report_elements, inverse_of: :my_module, dependent: :destroy has_many :protocols, inverse_of: :my_module, dependent: :destroy + # Associations for old activity type + has_many :activities, inverse_of: :my_module scope :is_archived, ->(is_archived) { where('archived = ?', is_archived) } scope :active, -> { where(archived: false) } diff --git a/app/models/team.rb b/app/models/team.rb index 146ea3c43..87f77f937 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -36,6 +36,7 @@ class Team < ApplicationRecord has_many :tiny_mce_assets, inverse_of: :team, dependent: :destroy has_many :repositories, dependent: :destroy has_many :reports, inverse_of: :team, dependent: :destroy + has_many :activities, inverse_of: :team, dependent: :destroy attr_accessor :without_templates attr_accessor :without_intro_demo diff --git a/app/services/experiments/move_to_project_service.rb b/app/services/experiments/move_to_project_service.rb index 25c259f4f..91749337d 100644 --- a/app/services/experiments/move_to_project_service.rb +++ b/app/services/experiments/move_to_project_service.rb @@ -76,6 +76,7 @@ module Experiments Activity.create( type_of: :move_experiment, project: @project, + team: @project.team, subject: @exp, owner: @user, message: I18n.t( diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index 3dce3347e..a242a9e64 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -83,4 +83,68 @@ class Extends # Hash used for mapping file extensions to custom font awesome icon classes, # 'extension' => 'fa class' FILE_FA_ICON_MAPPINGS = {} + + ################### Activities ################### + ACTIVITY_TYPES = { + create_project: 0, + rename_project: 1, + change_project_visibility: 2, + archive_project: 3, + restore_project: 4, + assign_user_to_project: 5, + change_user_role_on_project: 6, + unassign_user_from_project: 7, + create_module: 8, + clone_module: 9, + archive_module: 10, + restore_module: 11, + change_module_description: 12, + assign_user_to_module: 13, + unassign_user_from_module: 14, + create_step: 15, + destroy_step: 16, + add_comment_to_step: 17, + complete_step: 18, + uncomplete_step: 19, + check_step_checklist_item: 20, + uncheck_step_checklist_item: 21, + edit_step: 22, + add_result: 23, + add_comment_to_result: 24, + archive_result: 25, + edit_result: 26, + create_experiment: 27, + edit_experiment: 28, + archive_experiment: 29, + clone_experiment: 30, + move_experiment: 31, + add_comment_to_project: 32, + edit_project_comment: 33, + delete_project_comment: 34, + add_comment_to_module: 35, + edit_module_comment: 36, + delete_module_comment: 37, + edit_step_comment: 38, + delete_step_comment: 39, + edit_result_comment: 40, + delete_result_comment: 41, + destroy_result: 42, + start_edit_wopi_file: 43, + unlock_wopi_file: 44, + load_protocol_from_file: 45, + load_protocol_from_repository: 46, + revert_protocol: 47, + create_report: 48, + delete_report: 49, + edit_report: 50, + assign_sample: 51, + unassign_sample: 52, + complete_task: 53, + uncomplete_task: 54, + assign_repository_record: 55, + unassign_repository_record: 56 + }.freeze + + ACTIVITY_SUBJECT_TYPES = + %w(User MyModule Experiment Project Report Protocol).freeze end diff --git a/spec/factories/activity.rb b/spec/factories/activities.rb similarity index 62% rename from spec/factories/activity.rb rename to spec/factories/activities.rb index eb91cdb7a..31e7d0217 100644 --- a/spec/factories/activity.rb +++ b/spec/factories/activities.rb @@ -6,5 +6,12 @@ FactoryBot.define do message Faker::Lorem.sentence(10) project subject { create :project } + owner { create :user } + team + trait :old do + project + experiment + subject { nil } + end end end diff --git a/spec/models/activity_spec.rb b/spec/models/activity_spec.rb index 3a329209c..3d0028a92 100644 --- a/spec/models/activity_spec.rb +++ b/spec/models/activity_spec.rb @@ -1,10 +1,23 @@ +# frozen_string_literal: true + require 'rails_helper' describe Activity, type: :model do + subject(:activity) { create :activity } + let(:old_activity) { create :activity, :old } + it 'should be of class Activity' do expect(subject.class).to eq Activity end + it 'is valid' do + expect(activity).to be_valid + end + + it 'is valid (old)' do + expect(old_activity).to be_valid + end + describe 'Database table' do it { should have_db_column :id } it { should have_db_column :my_module_id } @@ -22,9 +35,24 @@ describe Activity, type: :model do it { should belong_to :experiment } it { should belong_to :my_module } it { should belong_to :owner } + it { should belong_to :subject } end - describe 'Should be a valid object' do + describe 'Validations' do it { should validate_presence_of :type_of } + it { should validate_presence_of :owner } + + Extends::ACTIVITY_SUBJECT_TYPES.each do |value| + it { is_expected.to allow_values(value).for(:subject_type) } + end + end + + describe '.old_activity?' do + it 'returns true for old activity' do + expect(old_activity.old_activity?).to be_truthy + end + it 'returns false for activity' do + expect(activity.old_activity?).to be_falsey + end end end From c8ab9f8ff857d98f43b832071cc19b0bd6a6f548 Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Tue, 26 Feb 2019 08:04:05 +0100 Subject: [PATCH 2/2] Fix tests --- app/utilities/delayed_uploader_demo.rb | 2 +- app/utilities/first_time_data_generator.rb | 30 +++++++++++----------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/utilities/delayed_uploader_demo.rb b/app/utilities/delayed_uploader_demo.rb index 920813c61..c0d2a023b 100644 --- a/app/utilities/delayed_uploader_demo.rb +++ b/app/utilities/delayed_uploader_demo.rb @@ -39,7 +39,7 @@ module DelayedUploaderDemo type_of: :add_result, project: my_module.experiment.project, my_module: my_module, - user: current_user, + owner: current_user, created_at: temp_result.created_at, updated_at: temp_result.created_at, message: I18n.t( diff --git a/app/utilities/first_time_data_generator.rb b/app/utilities/first_time_data_generator.rb index bef89486c..799abcddb 100644 --- a/app/utilities/first_time_data_generator.rb +++ b/app/utilities/first_time_data_generator.rb @@ -214,7 +214,7 @@ module FirstTimeDataGenerator # Activity for creating project Activity.new( type_of: :create_project, - user: user, + owner: user, project: project, message: I18n.t( 'activities.create_project', @@ -286,7 +286,7 @@ module FirstTimeDataGenerator # Create module activity Activity.new( type_of: :create_module, - user: user, + owner: user, project: project, my_module: my_module, message: I18n.t( @@ -306,7 +306,7 @@ module FirstTimeDataGenerator ) Activity.new( type_of: :assign_user_to_module, - user: user, + owner: user, project: project, my_module: my_module, message: I18n.t( @@ -340,7 +340,7 @@ module FirstTimeDataGenerator # Activity for creating archived module Activity.new( type_of: :create_module, - user: user, + owner: user, project: project, my_module: archived_module, message: I18n.t( @@ -355,7 +355,7 @@ module FirstTimeDataGenerator # Activity for archiving archived module Activity.new( type_of: :archive_module, - user: user, + owner: user, project: project, my_module: archived_module, message: I18n.t( @@ -376,7 +376,7 @@ module FirstTimeDataGenerator ) Activity.new( type_of: :assign_user_to_module, - user: user, + owner: user, project: project, my_module: archived_module, message: I18n.t( @@ -682,7 +682,7 @@ module FirstTimeDataGenerator type_of: :add_result, project: project, my_module: my_modules[1], - user: user, + owner: user, created_at: temp_result.created_at, updated_at: temp_result.created_at, message: I18n.t( @@ -771,7 +771,7 @@ module FirstTimeDataGenerator type_of: :add_result, project: project, my_module: my_modules[2], - user: user, + owner: user, created_at: temp_result.created_at, updated_at: temp_result.created_at, message: I18n.t( @@ -1160,7 +1160,7 @@ module FirstTimeDataGenerator type_of: :add_result, project: project, my_module: my_modules[5], - user: user, + owner: user, created_at: temp_result.created_at, updated_at: temp_result.created_at, message: I18n.t( @@ -1517,7 +1517,7 @@ module FirstTimeDataGenerator type_of: :create_step, project: my_module.experiment.project, my_module: my_module, - user: step.user, + owner: step.user, created_at: created_at, updated_at: created_at, message: I18n.t( @@ -1532,7 +1532,7 @@ module FirstTimeDataGenerator type_of: :complete_step, project: my_module.experiment.project, my_module: my_module, - user: step.user, + owner: step.user, created_at: completed_on, updated_at: completed_on, message: I18n.t( @@ -1576,7 +1576,7 @@ module FirstTimeDataGenerator ) Activity.new( type_of: :add_comment_to_project, - user: user, + owner: user, project: project, created_at: created_at, updated_at: created_at, @@ -1596,7 +1596,7 @@ module FirstTimeDataGenerator ) Activity.new( type_of: :add_comment_to_module, - user: user, + owner: user, project: my_module.experiment.project, my_module: my_module, created_at: created_at, @@ -1617,7 +1617,7 @@ module FirstTimeDataGenerator ) Activity.new( type_of: :add_comment_to_result, - user: user, + owner: user, project: result.my_module.experiment.project, my_module: result.my_module, created_at: created_at, @@ -1638,7 +1638,7 @@ module FirstTimeDataGenerator ) Activity.new( type_of: :add_comment_to_step, - user: user, + owner: user, project: step.protocol.my_module.experiment.project, my_module: step.protocol.my_module, created_at: created_at,