diff --git a/.rubocop.yml b/.rubocop.yml
index 543b1c577..538599c28 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -350,6 +350,9 @@ Style/WhenThen:
Metrics/AbcSize:
Enabled: false
+Metrics/BlockLength:
+ ExcludedMethods: ['describe', 'context']
+
Metrics/ClassLength:
Enabled: false
diff --git a/app/controllers/experiments_controller.rb b/app/controllers/experiments_controller.rb
index 7c5a51381..1259c893f 100644
--- a/app/controllers/experiments_controller.rb
+++ b/app/controllers/experiments_controller.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
class ExperimentsController < ApplicationController
include SampleActions
include TeamsHelper
@@ -237,49 +239,23 @@ class ExperimentsController < ApplicationController
# POST: move_experiment(id)
def move
- project = Project.find_by_id(params[:experiment].try(:[], :project_id))
- old_project = @experiment.project
-
- # Try to move the experiment
- success = true
- if @experiment.moveable_projects(current_user).include?(project)
- success = @experiment.move_to_project(project)
- else
- success = false
- end
-
- if success
- Activity.create(
- type_of: :move_experiment,
- project: project,
- experiment: @experiment,
- user: current_user,
- message: I18n.t(
- 'activities.move_experiment',
- user: current_user.full_name,
- experiment: @experiment.name,
- project_new: project.name,
- project_original: old_project.name
- )
- )
+ service = Experiments::MoveToProjectService
+ .call(experiment_id: @experiment.id,
+ project_id: move_experiment_param,
+ user_id: current_user.id)
+ if service.succeed?
flash[:success] = t('experiments.move.success_flash',
experiment: @experiment.name)
- respond_to do |format|
- format.json do
- render json: { path: canvas_experiment_url(@experiment) }, status: :ok
- end
- end
+ path = canvas_experiment_url(@experiment)
+ status = :ok
else
- respond_to do |format|
- format.json do
- render json: { message: t('experiments.move.error_flash',
- experiment:
- escape_input(@experiment.name)) },
- status: :unprocessable_entity
- end
- end
+ message = t('experiments.move.error_flash',
+ experiment: escape_input(@experiment.name))
+ status = :unprocessable_entity
end
+
+ render json: { message: message, path: path }, status: status
end
def module_archive
@@ -348,6 +324,10 @@ class ExperimentsController < ApplicationController
params.require(:experiment).permit(:name, :description, :archived)
end
+ def move_experiment_param
+ params.require(:experiment).require(:project_id)
+ end
+
def load_projects_tree
# Switch to correct team
current_team_switch(@experiment.project.team) unless @experiment.project.nil?
diff --git a/app/models/experiment.rb b/app/models/experiment.rb
index 851372781..ee99a7bb5 100644
--- a/app/models/experiment.rb
+++ b/app/models/experiment.rb
@@ -255,24 +255,6 @@ class Experiment < ApplicationRecord
clone
end
- def move_to_project(project)
- self.project = project
-
- my_modules.each do |m|
- new_tags = []
- m.tags.each do |t|
- new_tags << t.deep_clone_to_project(project)
- end
- m.my_module_tags.destroy_all
-
- project.tags << new_tags
- m.tags << new_tags
- end
- result = save
- touch(:workflowimg_updated_at) if result
- result
- end
-
# Get projects where user is either owner or user in the same team
# as this experiment
def projects_with_role_above_user(current_user)
diff --git a/app/models/tag.rb b/app/models/tag.rb
index a1b1089ee..4818622de 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
class Tag < ApplicationRecord
include SearchableModel
@@ -48,7 +50,10 @@ class Tag < ApplicationRecord
end
end
- def deep_clone_to_project(project)
+ def clone_to_project_or_return_existing(project)
+ tag = Tag.find_by(project: project, name: name, color: color)
+ return tag if tag
+
Tag.create(
name: name,
color: color,
diff --git a/app/services/experiments/move_to_project_service.rb b/app/services/experiments/move_to_project_service.rb
new file mode 100644
index 000000000..088bbc7b0
--- /dev/null
+++ b/app/services/experiments/move_to_project_service.rb
@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+module Experiments
+ class MoveToProjectService
+ extend Service
+
+ attr_reader :errors
+
+ def initialize(experiment_id:, project_id:, user_id:)
+ @exp = Experiment.find experiment_id
+ @project = Project.find project_id
+ @user = User.find user_id
+ @original_project = @exp&.project
+ @errors = {}
+ end
+
+ def call
+ return self unless valid?
+
+ ActiveRecord::Base.transaction do
+ @exp.project = @project
+
+ @exp.my_modules.each do |m|
+ new_tags = m.tags.map do |t|
+ t.clone_to_project_or_return_existing(@project)
+ end
+ m.my_module_tags.delete_all
+ m.tags = new_tags
+ end
+
+ raise ActiveRecord::Rollback unless @exp.save
+ end
+
+ @errors.merge!(@exp.errors.to_hash) unless @exp.valid?
+
+ track_activity if succeed?
+ self
+ end
+
+ def succeed?
+ @errors.none?
+ end
+
+ private
+
+ def valid?
+ unless @exp && @project && @user
+ @errors[:invalid_arguments] =
+ { 'experiment': @exp,
+ 'project': @project,
+ 'user': @user }
+ .map do |key, value|
+ "Can't find #{key.capitalize}" if value.nil?
+ end.compact
+ return false
+ end
+
+ e = Experiment.find_by(name: @exp.name, project: @project)
+
+ if e
+ @errors[:project_with_exp] =
+ ['Project already contains experiment with this name']
+ false
+ elsif !@exp.moveable_projects(@user).include?(@project)
+ @errors[:target_project_not_valid] =
+ ['Experiment cannot be moved to this project']
+ false
+ else
+ true
+ end
+ end
+
+ def track_activity
+ Activity.create(
+ type_of: :move_experiment,
+ project: @project,
+ experiment: @exp,
+ user: @user,
+ message: I18n.t(
+ 'activities.move_experiment',
+ user: @user,
+ experiment: @exp.name,
+ project_new: @project.name,
+ project_original: @original_project.name
+ )
+ )
+ end
+ end
+end
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 511fce723..1aa7f6690 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -54,11 +54,15 @@ describe ProjectsController, type: :controller do
end
end
+ # rubocop:disable Security/Eval
+ # rubocop:disable Style/EvalWithLocation
(1..PROJECTS_CNT).each do |i|
let!("user_projects_#{i}") do
- create :user_project, project: eval("project_#{i}"), user: user
+ create :user_project, :owner, project: eval("project_#{i}"), user: user
end
end
+ # rubocop:enable Security/Eval
+ # rubocop:enable Style/EvalWithLocation
describe '#index' do
context 'in JSON format' do
diff --git a/spec/factories/experiments.rb b/spec/factories/experiments.rb
index 610c12b20..8743772e8 100644
--- a/spec/factories/experiments.rb
+++ b/spec/factories/experiments.rb
@@ -12,7 +12,7 @@ FactoryBot.define do
project { create :project, created_by: user }
factory :experiment_with_tasks do
after(:create) do |e|
- create_list :my_module, 3, experiment: e
+ create_list :my_module, 3, :with_tag, experiment: e
end
end
end
diff --git a/spec/factories/my_modules.rb b/spec/factories/my_modules.rb
index 3101a427d..c383d794f 100644
--- a/spec/factories/my_modules.rb
+++ b/spec/factories/my_modules.rb
@@ -8,5 +8,8 @@ FactoryBot.define do
workflow_order { MyModule.where(experiment_id: experiment.id).count + 1 }
experiment
my_module_group { create :my_module_group, experiment: experiment }
+ trait :with_tag do
+ tags { create_list :tag, 3, project: experiment.project }
+ end
end
end
diff --git a/spec/factories/tags.rb b/spec/factories/tags.rb
new file mode 100644
index 000000000..5e5a09f41
--- /dev/null
+++ b/spec/factories/tags.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :tag do
+ sequence(:name) { |n| "My tag-#{n}" }
+ project
+ color { Faker::Color.hex_color }
+ end
+end
diff --git a/spec/factories/teams.rb b/spec/factories/teams.rb
index 130f67fb7..a94666a66 100644
--- a/spec/factories/teams.rb
+++ b/spec/factories/teams.rb
@@ -6,5 +6,8 @@ FactoryBot.define do
sequence(:name) { |n| "My team-#{n}" }
description { Faker::Lorem.sentence }
space_taken { 1048576 }
+ trait :with_members do
+ users { create_list :user, 3 }
+ end
end
end
diff --git a/spec/factories/user_project.rb b/spec/factories/user_project.rb
deleted file mode 100644
index 18b5d163c..000000000
--- a/spec/factories/user_project.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-FactoryBot.define do
- factory :user_project do
- role 'owner'
- end
-end
diff --git a/spec/factories/user_projects.rb b/spec/factories/user_projects.rb
new file mode 100644
index 000000000..6ab1aa8e2
--- /dev/null
+++ b/spec/factories/user_projects.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :user_project do
+ user
+ project
+ assigned_by { user }
+ trait :owner do
+ role { UserProject.roles[:owner] }
+ end
+ trait :normal_user do
+ role { UserProject.roles[:normal_user] }
+ end
+ trait :technician do
+ role { UserProject.roles[:technician] }
+ end
+ trait :viewer do
+ role { UserProject.roles[:viewer] }
+ end
+ end
+end
diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb
index d30877fa4..1f5bcbe14 100644
--- a/spec/models/experiment_spec.rb
+++ b/spec/models/experiment_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'rails_helper'
describe Experiment, type: :model do
diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb
index e95c7abef..ded47e60c 100644
--- a/spec/models/tag_spec.rb
+++ b/spec/models/tag_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'rails_helper'
describe Tag, type: :model do
@@ -23,15 +25,46 @@ describe Tag, type: :model do
it { should have_many :my_modules }
end
- describe 'Should be a valid object' do
- it { should validate_presence_of :name }
- it { should validate_presence_of :color }
- it { should validate_presence_of :project }
- it do
- should validate_length_of(:name).is_at_most(Constants::NAME_MAX_LENGTH)
+ describe 'Validations' do
+ describe '#name' do
+ it { should validate_presence_of :name }
+ it do
+ should validate_length_of(:name)
+ .is_at_most(Constants::NAME_MAX_LENGTH)
+ end
end
- it do
- should validate_length_of(:color).is_at_most(Constants::COLOR_MAX_LENGTH)
+
+ describe '#color' do
+ it { should validate_presence_of :color }
+ it do
+ should validate_length_of(:color)
+ .is_at_most(Constants::COLOR_MAX_LENGTH)
+ end
+ end
+ describe '#projects' do
+ it { should validate_presence_of :project }
+ end
+ end
+ describe '.clone_to_project_or_return_existing' do
+ let(:project) { create :project }
+ let(:tag) { create :tag }
+
+ context 'when tag does not exits' do
+ it 'does create new tag for project' do
+ expect do
+ tag.clone_to_project_or_return_existing(project)
+ end.to(change { Tag.where(project_id: project.id).count })
+ end
+ end
+
+ context 'when tag already exists' do
+ it 'return existing tag for project' do
+ Tag.create(name: tag.name, color: tag.color, project: project)
+
+ expect do
+ tag.clone_to_project_or_return_existing(project)
+ end.to_not(change { Tag.where(project_id: project.id).count })
+ end
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 15ab3be5e..a41f965b3 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -187,7 +187,9 @@ describe User, type: :model do
describe '#last_activities' do
let!(:user) { create :user }
let!(:project) { create :project }
- let!(:user_projects) { create :user_project, project: project, user: user }
+ let!(:user_projects) do
+ create :user_project, :viewer, project: project, user: user
+ end
let!(:activity_one) { create :activity, user: user, project: project }
let!(:activity_two) { create :activity, user: user, project: project }
diff --git a/spec/requests/api/v1/users_controller_spec.rb b/spec/requests/api/v1/users_controller_spec.rb
index ba705391d..508b471ec 100644
--- a/spec/requests/api/v1/users_controller_spec.rb
+++ b/spec/requests/api/v1/users_controller_spec.rb
@@ -4,9 +4,9 @@ require 'rails_helper'
RSpec.describe 'Api::V1::UsersController', type: :request do
before :all do
- @user1 = create(:user, email: Faker::Internet.unique.email)
- @user2 = create(:user, email: Faker::Internet.unique.email)
- @user3 = create(:user, email: Faker::Internet.unique.email)
+ @user1 = create(:user)
+ @user2 = create(:user)
+ @user3 = create(:user)
@team1 = create(:team, created_by: @user1)
@team2 = create(:team, created_by: @user2)
@team3 = create(:team, created_by: @user3)
@@ -40,7 +40,7 @@ RSpec.describe 'Api::V1::UsersController', type: :request do
it 'When invalid request, non existing user' do
hash_body = nil
- get api_v1_user_path(id: 123), headers: @valid_headers
+ get api_v1_user_path(id: -1), headers: @valid_headers
expect(response).to have_http_status(403)
expect { hash_body = json }.not_to raise_exception
expect(hash_body['errors'][0]).to include('status': 403)
diff --git a/spec/services/experiments/move_to_project_service_spec.rb b/spec/services/experiments/move_to_project_service_spec.rb
new file mode 100644
index 000000000..67d8e33a9
--- /dev/null
+++ b/spec/services/experiments/move_to_project_service_spec.rb
@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Experiments::MoveToProjectService do
+ let(:team) { create :team, :with_members }
+ let(:project) do
+ create :project, team: team, user_projects: []
+ end
+ let(:new_project) do
+ create :project, team: team, user_projects: [user_project2]
+ 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,
+ user_id: user.id)
+ end
+
+ context 'when call with valid params' do
+ it 'unnasigns experiment from project' do
+ service_call
+
+ expect(project.experiments.pluck(:id)).to_not include(experiment.id)
+ end
+
+ it 'assigns experiment to new_project' do
+ service_call
+
+ expect(new_project.experiments.pluck(:id)).to include(experiment.id)
+ end
+
+ it 'copies tags to new project' do
+ expect { service_call }.to(change { new_project.tags.count })
+ end
+
+ it 'leaves tags on an old project' do
+ experiment # explicit call to create tags
+ expect { service_call }.not_to(change { project.tags.count })
+ end
+
+ it 'sets new project tags to modules' do
+ service_call
+ new_tags = experiment.my_modules.map { |m| m.tags.map { |t| t } }.flatten
+ tags_project_id = new_tags.map(&:project_id).uniq.first
+
+ expect(tags_project_id).to be == new_project.id
+ end
+
+ it 'adds Activity record' do
+ expect { service_call }.to(change { Activity.all.count })
+ end
+ end
+
+ context 'when call with invalid params' do
+ it 'returns an error when can\'t find user and project' do
+ allow(Project).to receive(:find).and_return(nil)
+ allow(User).to receive(:find).and_return(nil)
+
+ expect(service_call.errors).to have_key(:invalid_arguments)
+ end
+
+ it 'returns an error on validate' do
+ FactoryBot.create :experiment, project: new_project, name: 'MyExp'
+ expect(service_call.succeed?).to be_falsey
+ end
+
+ it 'returns an error on saving' do
+ expect_any_instance_of(Experiments::MoveToProjectService)
+ .to receive(:valid?)
+ .and_return(true)
+ FactoryBot.create :experiment, project: new_project, name: 'MyExp'
+
+ expect(service_call.succeed?).to be_falsey
+ end
+
+ it 'rollbacks cloned tags after unsucessful save' do
+ expect_any_instance_of(Experiments::MoveToProjectService)
+ .to receive(:valid?)
+ .and_return(true)
+ FactoryBot.create :experiment, project: new_project, name: 'MyExp'
+ experiment
+
+ expect { service_call }.not_to(change { Tag.count })
+ end
+
+ it 'returns error if teams is not the same' do
+ t = create :team, :with_members
+ project.update(team: t)
+
+ expect(service_call.errors).to have_key(:target_project_not_valid)
+ end
+ end
+end