diff --git a/.overcommit.yml b/.overcommit.yml
new file mode 100644
index 000000000..ae6a4ce07
--- /dev/null
+++ b/.overcommit.yml
@@ -0,0 +1,5 @@
+PreCommit:
+ RuboCop:
+ enabled: true
+ on_warn: fail
+ problem_on_unmodified_line: ignore
diff --git a/Gemfile b/Gemfile
index 2fe9e3e77..2dfd5cee3 100644
--- a/Gemfile
+++ b/Gemfile
@@ -109,6 +109,7 @@ group :development, :test do
gem 'byebug'
gem 'factory_bot_rails'
gem 'listen', '~> 3.0'
+ gem 'overcommit'
gem 'pry'
gem 'pry-byebug'
gem 'pry-rails'
diff --git a/Gemfile.lock b/Gemfile.lock
index 12680e154..76978c064 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -156,6 +156,8 @@ GEM
mail
case_transform (0.2)
activesupport
+ childprocess (0.9.0)
+ ffi (~> 1.0, >= 1.0.11)
climate_control (0.2.0)
cliver (0.3.2)
coderay (1.1.2)
@@ -241,6 +243,7 @@ GEM
concurrent-ruby (~> 1.0)
i18n-js (3.0.3)
i18n (~> 0.6, >= 0.6.6)
+ iniparse (1.4.4)
jaro_winkler (1.5.1)
jbuilder (2.7.0)
activesupport (>= 4.2.0)
@@ -330,6 +333,9 @@ GEM
oauth2 (~> 1.1)
omniauth (~> 1.2)
orm_adapter (0.5.0)
+ overcommit (0.46.0)
+ childprocess (~> 0.6, >= 0.6.3)
+ iniparse (~> 1.4)
paperclip (5.3.0)
activemodel (>= 4.2.0)
activesupport (>= 4.2.0)
@@ -590,6 +596,7 @@ DEPENDENCIES
nokogiri (~> 1.8.1)
omniauth
omniauth-linkedin-oauth2
+ overcommit
paperclip (~> 5.3)
pg (~> 0.18)
phantomjs
@@ -637,4 +644,4 @@ RUBY VERSION
ruby 2.4.4p296
BUNDLED WITH
- 1.16.6
+ 1.17.1
diff --git a/app/models/experiment.rb b/app/models/experiment.rb
index 69d2926af..851372781 100644
--- a/app/models/experiment.rb
+++ b/app/models/experiment.rb
@@ -98,12 +98,6 @@ class Experiment < ApplicationRecord
end
end
- def modules_without_group
- MyModule.where(experiment_id: id)
- .where(my_module_group: nil)
- .where(archived: false)
- end
-
def active_module_groups
my_module_groups.joins(:my_modules)
.where('my_modules.archived = ?', false)
@@ -227,125 +221,8 @@ class Experiment < ApplicationRecord
true
end
- # This method generate the workflow image and saves it as
- # experiment attachment
def generate_workflow_img
- require 'graphviz'
-
- graph = GraphViz.new(:G,
- type: :digraph,
- use: :neato)
-
- graph[:size] = '4,4'
- graph.node[color: Constants::COLOR_ALTO,
- style: :filled,
- fontcolor: Constants::COLOR_EMPEROR,
- shape: 'circle',
- fontname: 'Arial',
- fontsize: '16.0']
-
- graph.edge[color: Constants::COLOR_ALTO]
-
- label = ''
- subg = {}
-
- # Draw orphan modules
- if modules_without_group
- modules_without_group.each do |my_module|
- graph
- .subgraph(rank: 'same')
- .add_nodes("Orphan-#{my_module.id}",
- label: label,
- pos: "#{my_module.x / 10},-#{my_module.y / 10}!")
- end
- end
-
- # Draw grouped modules
- if my_module_groups.many?
- my_module_groups.each_with_index do |group, gindex|
- subgraph_name = "cluster-#{gindex}"
- subg[subgraph_name] = graph.subgraph(rank: 'same')
- group.ordered_modules.each_with_index do |my_module, index|
- if my_module.outputs.any?
- parent = subg[subgraph_name]
- .add_nodes("#{subgraph_name}-#{index}",
- label: label,
- pos: "#{my_module.x / 10},-#{my_module.y / 10}!")
-
- my_module.outputs.each_with_index do |output, i|
- child_mod = MyModule.find_by_id(output.input_id)
- child_node = subg[subgraph_name]
- .add_nodes("#{subgraph_name}-O#{child_mod.id}-#{i}",
- label: label,
- pos: "#{child_mod.x / 10},-#{child_mod.y / 10}!")
-
- subg[subgraph_name].add_edges(parent, child_node)
- end
- elsif my_module.inputs.any?
- parent = subg[subgraph_name]
- .add_nodes("#{subgraph_name}-#{index}",
- label: label,
- pos: "#{my_module.x / 10},-#{my_module.y / 10}!")
-
- my_module.inputs.each_with_index do |input, i|
- child_mod = MyModule.find_by_id(input.output_id)
- child_node = subg[subgraph_name]
- .add_nodes("#{subgraph_name}-I#{child_mod.id}-#{i}",
- label: label,
- pos: "#{child_mod.x / 10},-#{child_mod.y / 10}!")
-
- subg[subgraph_name].add_edges(child_node, parent)
- end
- end
- end
- end
- else
- my_module_groups.each do |group|
- group.ordered_modules.each_with_index do |my_module, index|
- if my_module.outputs.any?
- parent = graph.add_nodes("N-#{index}",
- label: label,
- pos: "#{my_module.x / 10},-#{ my_module.y / 10}!")
-
- my_module.outputs.each_with_index do |output, i|
- child_mod = MyModule.find_by_id(output.input_id)
- child_node = graph
- .add_nodes("N-O#{child_mod.id}-#{i}",
- label: label,
- pos: "#{child_mod.x / 10},-#{child_mod.y / 10}!")
- graph.add_edges(parent, child_node)
- end
- elsif my_module.inputs.any?
- parent = graph.add_nodes("N-#{index}",
- label: label,
- pos: "#{my_module.x / 10},-#{my_module.y / 10}!")
- my_module.inputs.each_with_index do |input, i|
- child_mod = MyModule.find_by_id(input.output_id)
- child_node = graph
- .add_nodes("N-I#{child_mod.id}-#{i}",
- label: label,
- pos: "#{child_mod.x / 10},-#{child_mod.y / 10}!")
- graph.add_edges(child_node, parent)
- end
- end
- end
- end
- end
-
- file_location = Tempfile.open(['wimg', '.png'],
- Rails.root.join('tmp'))
-
- graph.output(png: file_location.path)
-
- begin
- file = File.open(file_location)
- self.workflowimg = file
- file.close
- save
- touch(:workflowimg_updated_at)
- rescue => ex
- logger.error ex.message
- end
+ Experiments::GenerateWorkflowImageService.call(experiment_id: id)
end
# Clone this experiment to given project
@@ -371,7 +248,7 @@ class Experiment < ApplicationRecord
end
# Copy modules without group
- clone.my_modules << modules_without_group.map do |m|
+ clone.my_modules << my_modules.without_group.map do |m|
m.deep_clone_to_experiment(current_user, clone)
end
clone.save
diff --git a/app/models/my_module.rb b/app/models/my_module.rb
index 28936dc8f..9991aeaa9 100644
--- a/app/models/my_module.rb
+++ b/app/models/my_module.rb
@@ -68,11 +68,13 @@ class MyModule < ApplicationRecord
scope :is_archived, ->(is_archived) { where('archived = ?', is_archived) }
scope :active, -> { where(archived: false) }
scope :overdue, -> { where('my_modules.due_date < ?', Time.current.utc) }
+ scope :without_group, -> { active.where(my_module_group: nil) }
scope :one_day_prior, (lambda do
where('my_modules.due_date > ? AND my_modules.due_date < ?',
Time.current.utc,
Time.current.utc + 1.day)
end)
+ scope :workflow_ordered, -> { order(workflow_order: :asc) }
# A module takes this much space in canvas (x, y) in database
WIDTH = 30
diff --git a/app/models/my_module_group.rb b/app/models/my_module_group.rb
index bfe4493af..37c36adc9 100644
--- a/app/models/my_module_group.rb
+++ b/app/models/my_module_group.rb
@@ -10,10 +10,6 @@ class MyModuleGroup < ApplicationRecord
optional: true
has_many :my_modules, inverse_of: :my_module_group, dependent: :nullify
- def ordered_modules
- my_modules.order(workflow_order: :asc)
- end
-
def deep_clone_to_experiment(current_user, experiment)
clone = MyModuleGroup.new(
created_by: created_by,
@@ -21,12 +17,12 @@ class MyModuleGroup < ApplicationRecord
)
# Get clones of modules from this group, save them as hash
- cloned_modules = ordered_modules.each_with_object({}) do |m, hash|
- hash[m.id] = m.deep_clone_to_experiment(current_user, experiment)
- hash
+ cloned_modules = my_modules.workflow_ordered.each_with_object({}) do |m, h|
+ h[m.id] = m.deep_clone_to_experiment(current_user, experiment)
+ h
end
- ordered_modules.each do |m|
+ my_modules.workflow_ordered.each do |m|
# Copy connections
m.inputs.each do |inp|
Connection.create(
diff --git a/app/services/experiments/generate_workflow_image_service.rb b/app/services/experiments/generate_workflow_image_service.rb
new file mode 100644
index 000000000..71ca146dc
--- /dev/null
+++ b/app/services/experiments/generate_workflow_image_service.rb
@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+module Experiments
+ class GenerateWorkflowImageService
+ extend Service
+ require 'graphviz'
+
+ attr_reader :errors
+
+ def initialize(experiment_id:)
+ @exp = Experiment.find experiment_id
+ @graph = GraphViz.new(:G, type: :digraph, use: :neato)
+
+ @graph[:size] = '4,4'
+ @graph.node[color: Constants::COLOR_ALTO,
+ style: :filled,
+ fontcolor: Constants::COLOR_EMPEROR,
+ shape: 'circle',
+ fontname: 'Arial',
+ fontsize: '16.0']
+
+ @graph.edge[color: Constants::COLOR_ALTO]
+ @errors = []
+ end
+
+ def call
+ draw_diagram
+ save_file
+ self
+ end
+
+ def succeed?
+ @errors.none?
+ end
+
+ private
+
+ def draw_diagram
+ # Draw orphan nodes
+ @exp.my_modules.without_group.each do |my_module|
+ @graph.subgraph(rank: 'same').add_nodes(
+ "Orphan-#{my_module.id}",
+ label: '',
+ pos: "#{my_module.x / 10},-#{my_module.y / 10}!"
+ )
+ end
+
+ # Draw grouped modules
+ subg = {}
+ @exp.my_module_groups.each_with_index do |group, gindex|
+ subgraph_id = "cluster-#{gindex}"
+ subg[subgraph_id] = @graph.subgraph(rank: 'same')
+ nodes = {}
+
+ group.my_modules.workflow_ordered.each_with_index do |my_module, index|
+ # draw nodes
+ node = subg[subgraph_id].add_nodes(
+ "#{subgraph_id}-#{index}",
+ label: '',
+ pos: "#{my_module.x / 10},-#{my_module.y / 10}!"
+ )
+ nodes[my_module.id] = node
+ end
+
+ # draw edges
+ group.my_modules.workflow_ordered.each do |m|
+ m.outputs.each do |output|
+ parent_node = nodes[m.id]
+ child_node = nodes[output.input_id]
+ subg[subgraph_id].add_edges(parent_node, child_node)
+ end
+ end
+ end
+ end
+
+ def save_file
+ file = Tempfile.open(%w(wimg .png), Rails.root.join('tmp'))
+ @graph.output(png: file.path)
+ @exp.workflowimg = file
+ file.close
+ @exp.save
+ @exp.touch(:workflowimg_updated_at)
+ rescue StandardError => ex
+ logger.error ex.message
+ end
+ end
+end
diff --git a/app/services/service.rb b/app/services/service.rb
new file mode 100644
index 000000000..3485ed0a2
--- /dev/null
+++ b/app/services/service.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module Service
+ def call(*args)
+ new(*args).call
+ end
+end
diff --git a/app/views/reports/new/modal/_experiment_contents_inner.html.erb b/app/views/reports/new/modal/_experiment_contents_inner.html.erb
index 8ce2c1384..71c7ff6ee 100644
--- a/app/views/reports/new/modal/_experiment_contents_inner.html.erb
+++ b/app/views/reports/new/modal/_experiment_contents_inner.html.erb
@@ -13,7 +13,7 @@
<% experiment.my_module_groups.each do |my_module_group| %>
<% if my_module_group.my_modules.exists? then %>
- <% my_module_group.ordered_modules.each do |my_module| %>
+ <% my_module_group.my_modules.workflow_ordered.each do |my_module| %>
<%= form.check_box "modules[#{my_module.id}]", label: my_module.name %>
@@ -22,13 +22,10 @@
<% end %>
- <% modules_without_group = experiment.modules_without_group %>
- <% if modules_without_group.exists? %>
- <% modules_without_group.each do |my_module| %>
-
- <%= form.check_box "modules[#{my_module.id}]", label: my_module.name %>
-
- <% end %>
+ <% experiment.my_modules.without_group.each do |my_module| %>
+
+ <%= form.check_box "modules[#{my_module.id}]", label: my_module.name %>
+
<% end %>
diff --git a/app/views/reports/new/modal/_project_contents_inner.html.erb b/app/views/reports/new/modal/_project_contents_inner.html.erb
index 84de808e8..6304104de 100644
--- a/app/views/reports/new/modal/_project_contents_inner.html.erb
+++ b/app/views/reports/new/modal/_project_contents_inner.html.erb
@@ -19,7 +19,7 @@
<% experiment.my_module_groups.each do |my_module_group| %>
<% next unless my_module_group.my_modules.is_archived(false).exists? %>
- <% my_module_group.ordered_modules.is_archived(false).each do |my_module| %>
+ <% my_module_group.my_modules.workflow_ordered.is_archived(false).each do |my_module| %>
<%= form.check_box "modules[#{my_module.id}]", label: my_module.name %>
@@ -27,13 +27,10 @@
<% end %>
- <% modules_without_group = experiment.modules_without_group %>
- <% if modules_without_group.exists? %>
- <% modules_without_group.each do |my_module| %>
-
- <%= form.check_box "modules[#{my_module.id}]", label: my_module.name %>
-
- <% end %>
+ <% experiment.my_modules.without_group.each do |my_module| %>
+
+ <%= form.check_box "modules[#{my_module.id}]", label: my_module.name %>
+
<% end %>
diff --git a/spec/factories/experiments.rb b/spec/factories/experiments.rb
index 498c591b1..610c12b20 100644
--- a/spec/factories/experiments.rb
+++ b/spec/factories/experiments.rb
@@ -1,26 +1,19 @@
+# frozen_string_literal: true
+
FactoryBot.define do
- factory :experiment do
- name { Faker::Name.unique.name }
- description 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit.'
- end
-
- factory :experiment_one, class: Experiment do
- name 'My Experiment One'
- description 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit.'
- association :project, factory: :project
- association :created_by, factory: :user, email: Faker::Internet.email
- association :last_modified_by,
- factory: :user,
- email: Faker::Internet.email
- end
-
- factory :experiment_two, class: Experiment do
- name Faker::Name.name
- description 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit.'
- association :project, factory: :project
- association :created_by, factory: :user, email: Faker::Internet.email
- association :last_modified_by,
- factory: :user,
- email: Faker::Internet.email
+ factory :experiment, class: Experiment do
+ transient do
+ user { create :user }
+ end
+ sequence(:name) { |n| "Experiment-#{n}" }
+ description { Faker::Lorem.sentence }
+ created_by { user }
+ last_modified_by { user }
+ project { create :project, created_by: user }
+ factory :experiment_with_tasks do
+ after(:create) do |e|
+ create_list :my_module, 3, experiment: e
+ end
+ end
end
end
diff --git a/spec/factories/my_module_groups.rb b/spec/factories/my_module_groups.rb
index 60d899b7d..368eb3b2f 100644
--- a/spec/factories/my_module_groups.rb
+++ b/spec/factories/my_module_groups.rb
@@ -1,5 +1,7 @@
+# frozen_string_literal: true
+
FactoryBot.define do
factory :my_module_group do
- experiment { Experiment.first || create(:experiment_two) }
+ experiment
end
end
diff --git a/spec/factories/my_modules.rb b/spec/factories/my_modules.rb
index ebd2d3b24..3101a427d 100644
--- a/spec/factories/my_modules.rb
+++ b/spec/factories/my_modules.rb
@@ -1,10 +1,12 @@
+# frozen_string_literal: true
+
FactoryBot.define do
factory :my_module do
- name 'My first module'
- x 0
- y 0
- workflow_order 0
- experiment { Experiment.first || create(:experiment_one) }
- my_module_group { MyModuleGroup.first || create(:my_module_group) }
+ sequence(:name) { |n| "Task-#{n}" }
+ x { Faker::Number.between(1, 100) }
+ y { Faker::Number.between(1, 100) }
+ workflow_order { MyModule.where(experiment_id: experiment.id).count + 1 }
+ experiment
+ my_module_group { create :my_module_group, experiment: experiment }
end
end
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 525f10a33..240245a22 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -1,10 +1,12 @@
+# frozen_string_literal: true
+
FactoryBot.define do
factory :project do
- created_by { User.first || association(:project_user) }
- team { Team.first || association(:project_team) }
- archived false
- name 'My project'
- visibility 'hidden'
+ sequence(:name) { |n| "My project-#{n}" }
+ association :created_by, factory: :user
+ team { create :team, created_by: created_by }
+ archived { false }
+ visibility { 'hidden' }
end
factory :project_user, class: User do
@@ -14,11 +16,4 @@ FactoryBot.define do
password 'asdf1243'
password_confirmation 'asdf1243'
end
-
- factory :project_team, class: Team do
- created_by { User.first || association(:project_user) }
- name 'My team'
- description 'Lorem ipsum dolor sit amet, consectetuer adipiscing eli.'
- space_taken 1048576
- end
end
diff --git a/spec/factories/teams.rb b/spec/factories/teams.rb
index c623caedb..130f67fb7 100644
--- a/spec/factories/teams.rb
+++ b/spec/factories/teams.rb
@@ -1,8 +1,10 @@
+# frozen_string_literal: true
+
FactoryBot.define do
factory :team do
- created_by { User.first || create(:user) }
- name 'My team'
- description 'Lorem ipsum dolor sit amet, consectetuer adipiscing eli.'
- space_taken 1048576
+ association :created_by, factory: :user
+ sequence(:name) { |n| "My team-#{n}" }
+ description { Faker::Lorem.sentence }
+ space_taken { 1048576 }
end
end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 1e1c42b50..efff4c751 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -1,8 +1,10 @@
+# frozen_string_literal: true
+
FactoryBot.define do
factory :user do
full_name 'admin'
initials 'AD'
- email Faker::Internet.unique.email
+ sequence(:email) { |n| "user-#{n}@example.com" }
password 'asdf1243'
password_confirmation 'asdf1243'
current_sign_in_at DateTime.now
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 53270d9b5..1819e4ed5 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
require 'rails_helper'
describe Project, type: :model do
@@ -39,22 +41,30 @@ describe Project, type: :model do
it { should have_many :report_elements }
end
- describe 'Should be a valid object' do
- let(:user) { create :user }
- let(:team) { create :team, created_by: user }
- it { should validate_presence_of :visibility }
- it { should validate_presence_of :team }
- it do
- should validate_length_of(:name).is_at_least(Constants::NAME_MIN_LENGTH)
- .is_at_most(Constants::NAME_MAX_LENGTH)
+ describe 'Validations' do
+ describe '#visibility' do
+ it { should validate_presence_of :visibility }
end
- it 'should have a unique name scoped to team' do
- create :project, created_by: user, last_modified_by: user, team: team
- project_two = build :project, created_by: user,
- last_modified_by: user,
- team: team
- expect(project_two).to_not be_valid
+ describe '#team' do
+ it { should validate_presence_of :team }
+ end
+
+ describe '#name' do
+ it 'should be at least 2 long and max 255 long' do
+ should validate_length_of(:name)
+ .is_at_least(Constants::NAME_MIN_LENGTH)
+ .is_at_most(Constants::NAME_MAX_LENGTH)
+ end
+
+ it 'should be uniq per project and team' do
+ first_project = create :project
+ second_project = build :project,
+ name: first_project.name,
+ team: first_project.team
+
+ expect(second_project).to_not be_valid
+ end
end
end
end
diff --git a/spec/services/experiments/generate_workflow_image_service_spec.rb b/spec/services/experiments/generate_workflow_image_service_spec.rb
new file mode 100644
index 000000000..62465d7dd
--- /dev/null
+++ b/spec/services/experiments/generate_workflow_image_service_spec.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Experiments::GenerateWorkflowImageService do
+ let(:experiment) { create :experiment_with_tasks }
+ let(:params) { { experiment_id: experiment.id } }
+
+ context 'when succeed' do
+ it 'succeed? returns true' do
+ expect(described_class.call(params).succeed?).to be_truthy
+ end
+
+ it 'worklfow image of experiment is updated' do
+ old_filename = experiment.workflowimg_file_name
+ described_class.call(params)
+ experiment.reload
+
+ expect(experiment.workflowimg_file_name).not_to be == old_filename
+ end
+ end
+end