diff --git a/app/assets/javascripts/sitewide/external_protocols.js b/app/assets/javascripts/sitewide/external_protocols.js
index 79b7a625b..a93395187 100644
--- a/app/assets/javascripts/sitewide/external_protocols.js
+++ b/app/assets/javascripts/sitewide/external_protocols.js
@@ -18,8 +18,8 @@ var ExternalProtocols = (function() {
var msg;
msg = key.charAt(0).toUpperCase() + key.slice(1) + ': ' + errors.protocol[key].join(', ');
if ((input.length > 0) && (errors.protocol[key].length > 0)) {
- input.next('span.help-block').html(msg);
- input.parent().addClass('has-error');
+ input.parent().next('span.help-block').html(msg);
+ input.parent().parent().addClass('has-error');
} else if (errors.protocol[key].length > 0) {
modal.find('.general-error > span').append(msg + '
');
}
@@ -72,7 +72,7 @@ var ExternalProtocols = (function() {
}
function initLoadProtocolModalPreview() {
- var externalProtocols = $('.exteral-proocol-result');
+ var externalProtocols = $('.external-protocol-result');
externalProtocols.on('click', 'a[data-action="external-import"]', function(e) {
var link = $(this);
animateSpinner(null, true);
@@ -120,7 +120,6 @@ var ExternalProtocols = (function() {
function initFormSubmits() {
var modal = $('#protocol-preview-modal');
modal.on('click', 'button[data-action=import_protocol]', function() {
- // console.log('click');
var form = modal.find('form');
var hiddenField = form.find('#protocol_protocol_type');
hiddenField.attr('value', $(this).data('import_type'));
diff --git a/app/assets/stylesheets/protocols/preview_modal.scss b/app/assets/stylesheets/protocols/preview_modal.scss
index baa16c6a9..fadd62730 100644
--- a/app/assets/stylesheets/protocols/preview_modal.scss
+++ b/app/assets/stylesheets/protocols/preview_modal.scss
@@ -15,7 +15,7 @@
}
}
- .general-eror {
+ .general-error {
text-align: center;
}
}
diff --git a/app/controllers/external_protocols_controller.rb b/app/controllers/external_protocols_controller.rb
index 06273d70b..619d7f2f4 100644
--- a/app/controllers/external_protocols_controller.rb
+++ b/app/controllers/external_protocols_controller.rb
@@ -47,23 +47,26 @@ class ExternalProtocolsController < ApplicationController
protocol_source: new_params[:protocol_source],
protocol_client_id: new_params[:protocol_client_id],
user_id: current_user.id,
- team_id: @team.id
+ team_id: @team.id,
+ build_with_assets: false
)
- p = service_call.built_protocol
- p.valid? # Get validations errors here
- @protocol = p
if service_call.succeed?
+ @protocol = service_call.built_protocol
+ @protocol&.valid? # Get validations errors here
+
render json: {
html: render_to_string(
partial: 'protocol_importers/import_form.html.erb',
- locals: { protocol: p, steps_json: service_call.serialized_steps }
+ locals: { protocol: @protocol,
+ steps_json: service_call.serialized_steps,
+ steps_assets: service_call.steps_assets }
),
- title: "Import protocol - #{p.name}",
+ title: t('protocol_importers.new.modal_title', protocol_name: @protocol.name),
footer: render_to_string(
partial: 'protocol_importers/preview_modal_footer.html.erb'
),
- validation_errors: { protocol: p.errors.messages }
+ validation_errors: { protocol: @protocol.errors.messages }
}
else
render json: { errors: service_call.errors }, status: 400
@@ -74,7 +77,7 @@ class ExternalProtocolsController < ApplicationController
def create
service_call = ProtocolImporters::ImportProtocolService.call(
protocol_params: create_protocol_params,
- steps_params: JSON.parse(create_steps_params[:steps]),
+ steps_params_json: create_steps_params[:steps],
user_id: current_user.id,
team_id: @team.id
)
diff --git a/app/services/protocol_importers/build_protocol_from_client_service.rb b/app/services/protocol_importers/build_protocol_from_client_service.rb
index 9d083e25f..fdc863533 100644
--- a/app/services/protocol_importers/build_protocol_from_client_service.rb
+++ b/app/services/protocol_importers/build_protocol_from_client_service.rb
@@ -5,13 +5,15 @@ module ProtocolImporters
extend Service
require 'protocol_importers/protocols_io/v3/errors'
- attr_reader :errors, :built_protocol
+ attr_reader :errors, :built_protocol, :steps_assets
- def initialize(protocol_client_id:, protocol_source:, user_id:, team_id:)
+ def initialize(protocol_client_id:, protocol_source:, user_id:, team_id:, build_with_assets: true)
@id = protocol_client_id
@protocol_source = protocol_source
- @user = User.find user_id
- @team = Team.find team_id
+ @user = User.find_by_id user_id
+ @team = Team.find_by_id team_id
+ @build_with_assets = build_with_assets
+ @steps_assets = {}
@errors = {}
end
@@ -26,11 +28,12 @@ module ProtocolImporters
pio = ProtocolImporters::ProtocolIntermediateObject.new(normalized_json: normalized_hash,
user: @user,
- team: @team)
+ team: @team,
+ build_with_assets: @build_with_assets)
@built_protocol = pio.build
- # Protocol with AR errors should not handled here as someting whats wrong.
- # @errors[:protocol] = pio.protocol.errors unless @built_protocol.valid?
+ @steps_assets = pio.steps_assets unless @build_with_assets
+
self
rescue api_errors => e
@errors[e.error_type] = e.message
@@ -48,14 +51,18 @@ module ProtocolImporters
end
def serialized_steps
+ # Serialize steps with nested attributes for Tables and NOT nasted attributes for Assets
+ # We want to avoid creating (downloading) Assets instances on building first time and again on importing/creating,
+ # when both actions are not in a row.
+ # Also serialization does not work properly with paperclip attrs
return nil unless built_protocol
built_protocol.steps.map do |step|
step_hash = step.attributes.symbolize_keys.slice(:name, :description, :position)
- # if step.assets.any?
- # step_hash[:assets_attributes] = step.assets.map { |a| a.attributes.symbolize_keys.except(:id) }
- # end
+ if !@build_with_assets && @steps_assets[step.position].any?
+ step_hash[:attachments] = @steps_assets[step.position]
+ end
if step.tables.any?
step_hash[:tables_attributes] = step.tables.map { |t| t.attributes.symbolize_keys.slice(:contents) }
diff --git a/app/services/protocol_importers/import_protocol_service.rb b/app/services/protocol_importers/import_protocol_service.rb
index e34dbc9c2..8da08d908 100644
--- a/app/services/protocol_importers/import_protocol_service.rb
+++ b/app/services/protocol_importers/import_protocol_service.rb
@@ -6,11 +6,11 @@ module ProtocolImporters
attr_reader :errors, :protocol
- def initialize(protocol_params:, steps_params:, team_id:, user_id:)
- @user = User.find user_id
- @team = Team.find team_id
+ def initialize(protocol_params:, steps_params_json:, team_id:, user_id:)
+ @user = User.find_by_id user_id
+ @team = Team.find_by_id team_id
@protocol_params = protocol_params
- @steps_params = steps_params
+ @steps_params = JSON.parse(steps_params_json) # catch error here
@errors = {}
end
@@ -18,10 +18,17 @@ module ProtocolImporters
return self unless valid?
@protocol = Protocol.new(@protocol_params.merge!(added_by: @user, team: @team))
- @protocol.steps << @steps_params.collect do |step_params|
- Step.new(step_params.merge(user: @user, completed: false))
- # TODO: Manually create file here. "Accept nasted attributes won't work here"
+ @protocol.steps << @steps_params.map do |step_params|
+ # Create step with nested attributes for tables
+ s = Step.new(step_params
+ .symbolize_keys
+ .slice(:name, :position, :description, :tables_attributes)
+ .merge(user: @user, completed: false))
+
+ # 'Manually' create assets here. "Accept nasted attributes won't work assets"
+ s.assets << AttachmentsBuilder.generate(step_params.deep_symbolize_keys, user: @user, team: @team)
+ s
end
@errors[:protocol] = @protocol.errors.messages unless @protocol.save
diff --git a/app/utilities/protocol_importers/attachments_builder.rb b/app/utilities/protocol_importers/attachments_builder.rb
index 7efcb3c6a..503646e3b 100644
--- a/app/utilities/protocol_importers/attachments_builder.rb
+++ b/app/utilities/protocol_importers/attachments_builder.rb
@@ -2,16 +2,27 @@
module ProtocolImporters
module AttachmentsBuilder
- def self.generate(step_json)
+ def self.generate(step_json, user: nil, team: nil)
return [] unless step_json[:attachments]&.any?
step_json[:attachments].map do |f|
Asset.new(file: URI.parse(f[:url]),
- created_by: @user,
- last_modified_by: @user,
- team: @team,
+ created_by: user,
+ last_modified_by: user,
+ team: team,
file_file_name: f[:name])
end
end
+
+ def self.generate_json(step_json)
+ return [] unless step_json[:attachments]&.any?
+
+ step_json[:attachments].map do |f|
+ {
+ name: f[:name],
+ url: f[:url]
+ }
+ end
+ end
end
end
diff --git a/app/utilities/protocol_importers/protocol_intermediate_object.rb b/app/utilities/protocol_importers/protocol_intermediate_object.rb
index 798420cea..ecddee038 100644
--- a/app/utilities/protocol_importers/protocol_intermediate_object.rb
+++ b/app/utilities/protocol_importers/protocol_intermediate_object.rb
@@ -2,12 +2,14 @@
module ProtocolImporters
class ProtocolIntermediateObject
- attr_accessor :normalized_protocol_data, :user, :team, :protocol
+ attr_reader :normalized_protocol_data, :user, :team, :protocol, :steps_assets, :build_with_assets
- def initialize(normalized_json: {}, user:, team:)
+ def initialize(normalized_json: {}, user:, team:, build_with_assets: true)
@normalized_protocol_data = normalized_json.with_indifferent_access[:protocol] if normalized_json
@user = user
@team = team
+ @steps_assets = {}
+ @build_with_assets = build_with_assets
end
def import
@@ -28,7 +30,11 @@ module ProtocolImporters
def build_steps
@normalized_protocol_data[:steps].map do |s|
step = Step.new(step_attributes(s))
- step.assets << AttachmentsBuilder.generate(s)
+ if @build_with_assets
+ step.assets << AttachmentsBuilder.generate(s, user: user, team: team)
+ else
+ @steps_assets[step.position] = AttachmentsBuilder.generate_json(s)
+ end
step.tables << TablesBuilder.extract_tables_from_html_string(s[:description][:body], true)
step.description = StepDescriptionBuilder.generate(s)
step
diff --git a/app/views/protocol_importers/_import_form.html.erb b/app/views/protocol_importers/_import_form.html.erb
index 73157b1bb..e6b528608 100644
--- a/app/views/protocol_importers/_import_form.html.erb
+++ b/app/views/protocol_importers/_import_form.html.erb
@@ -43,7 +43,7 @@
<% protocol.steps.sort_by{ |s| s.position }.each do |step| %>
- <%= render partial: "steps/step.html.erb", locals: { step: step, preview: true } %>
+ <%= render partial: "steps/step.html.erb", locals: { step: step, steps_assets: steps_assets, preview: true, import: true } %>
<% end %>
diff --git a/app/views/protocols/index/_external_protocols_tab.html.erb b/app/views/protocols/index/_external_protocols_tab.html.erb
index 17f3614dc..64d408383 100644
--- a/app/views/protocols/index/_external_protocols_tab.html.erb
+++ b/app/views/protocols/index/_external_protocols_tab.html.erb
@@ -51,6 +51,9 @@
<%= t('protocols.index.external_protocols.list_panel.empty_text') %>
+
Error protocol (click me, should default to default screen)
-
-
-
diff --git a/app/views/steps/_step.html.erb b/app/views/steps/_step.html.erb
index 679c2987c..add2404ac 100644
--- a/app/views/steps/_step.html.erb
+++ b/app/views/steps/_step.html.erb
@@ -1,4 +1,5 @@
<% preview = (defined?(preview) ? preview : false) %>
+<% import = (defined?(import) ? import : false) %>
">
@@ -113,7 +114,12 @@
<% end %>
- <%= render partial: 'steps/attachments/list.html.erb', locals: { step: step, preview: preview } %>
+ <% if import %>
+ <%= render partial: 'steps/attachments/preview_list.html.erb',
+ locals: { attachments: steps_assets[step.position]} %>
+ <% else %>
+ <%= render partial: 'steps/attachments/list.html.erb', locals: { step: step, preview: preview } %>
+ <% end %>
<% unless step.checklists.blank? then %>
@@ -147,10 +153,12 @@
<% end %>
-
- <%= render partial: 'steps/comments.html.erb', locals: { comments: step.last_comments,
- comments_count: step.step_comments.count,
- step: step } %>
+ <% unless import %>
+
+ <%= render partial: 'steps/comments.html.erb', locals: { comments: step.last_comments,
+ comments_count: step.step_comments.count,
+ step: step } %>
+ <% end %>
+
+
+
+
<%= truncate(file_name || file_url, length: Constants::FILENAME_TRUNCATION_LENGTH) %>
+
+
+
+
+
+
+
+
+
+
+ <%= t('protocols.steps.files', count: attachments.size) %>
+
+
+
+
+
+
+ <% attachments.each do |a| %>
+ <%= render partial: 'steps/attachments/new_attachment.html.erb',
+ locals: { file_url: a[:url], file_name: a[:name] } %>
+ <% end %>
+
+
diff --git a/config/locales/protocols/en.yml b/config/locales/protocols/en.yml
index 5e11cf57c..b4f111968 100644
--- a/config/locales/protocols/en.yml
+++ b/config/locales/protocols/en.yml
@@ -1,5 +1,7 @@
en:
protocol_importers:
+ new:
+ modal_title: 'Import protocol - %{protocol_name}'
templates:
amount:
title: 'Amount'
diff --git a/spec/controllers/external_protocols_controller_spec.rb b/spec/controllers/external_protocols_controller_spec.rb
index 94e0cc1c6..0f0de9fbb 100644
--- a/spec/controllers/external_protocols_controller_spec.rb
+++ b/spec/controllers/external_protocols_controller_spec.rb
@@ -110,6 +110,8 @@ describe ExternalProtocolsController, type: :controller do
service = double('success_service')
allow(service).to(receive(:succeed?)).and_return(true)
allow(service).to(receive(:built_protocol)).and_return(protocol)
+ allow(service).to(receive(:serialized_steps)).and_return({}.to_s)
+ allow(service).to(receive(:steps_assets)).and_return([])
allow_any_instance_of(ProtocolImporters::BuildProtocolFromClientService)
.to(receive(:call)).and_return(service)
@@ -148,8 +150,10 @@ describe ExternalProtocolsController, type: :controller do
let(:params) do
{
team_id: team.id,
- protocol_params: {},
- steps_params: {}
+ protocol: {
+ name: 'name',
+ steps: {}.to_s
+ }
}
end
@@ -159,7 +163,7 @@ describe ExternalProtocolsController, type: :controller do
# Setup double
service = double('success_service')
allow(service).to(receive(:succeed?)).and_return(true)
- allow(service).to(receive(:protocol)).and_return({})
+ allow(service).to(receive(:protocol)).and_return(create(:protocol))
allow_any_instance_of(ProtocolImporters::ImportProtocolService).to(receive(:call)).and_return(service)
diff --git a/spec/services/protocol_importers/build_protocol_from_client_service_spec.rb b/spec/services/protocol_importers/build_protocol_from_client_service_spec.rb
index 7351c1b43..3adc78bfe 100644
--- a/spec/services/protocol_importers/build_protocol_from_client_service_spec.rb
+++ b/spec/services/protocol_importers/build_protocol_from_client_service_spec.rb
@@ -9,6 +9,14 @@ describe ProtocolImporters::BuildProtocolFromClientService do
ProtocolImporters::BuildProtocolFromClientService
.call(protocol_client_id: 'id', protocol_source: 'protocolsio/v3', user_id: user.id, team_id: team.id)
end
+ let(:service_call_without_assets) do
+ ProtocolImporters::BuildProtocolFromClientService
+ .call(protocol_client_id: 'id',
+ protocol_source: 'protocolsio/v3',
+ user_id: user.id,
+ team_id: team.id,
+ build_with_assets: false)
+ end
let(:normalized_response) do
JSON.parse(file_fixture('protocol_importers/normalized_single_protocol.json').read)
.to_h.with_indifferent_access
@@ -16,7 +24,7 @@ describe ProtocolImporters::BuildProtocolFromClientService do
context 'when have invalid arguments' do
it 'returns an error when can\'t find user' do
- allow(User).to receive(:find).and_return(nil)
+ allow(User).to receive(:find_by_id).and_return(nil)
expect(service_call.errors).to have_key(:invalid_arguments)
end
@@ -69,5 +77,13 @@ describe ProtocolImporters::BuildProtocolFromClientService do
expect(service_call.built_protocol).to be_instance_of(Protocol)
end
# more tests will be implemented when add error handling to service
+
+ describe 'serialized_steps' do
+ context 'when build without assets' do
+ it 'returns JSON with attachments' do
+ expect(JSON.parse(service_call_without_assets.serialized_steps).first).to have_key('attachments')
+ end
+ end
+ end
end
end
diff --git a/spec/services/protocol_importers/import_protocol_service_spec.rb b/spec/services/protocol_importers/import_protocol_service_spec.rb
index af31a9646..f644f7f49 100644
--- a/spec/services/protocol_importers/import_protocol_service_spec.rb
+++ b/spec/services/protocol_importers/import_protocol_service_spec.rb
@@ -8,19 +8,19 @@ describe ProtocolImporters::ImportProtocolService do
let(:protocol_params) { attributes_for :protocol, :in_public_repository }
let(:steps_params) do
[
- attributes_for(:step).merge!(assets_attributes: [attributes_for(:asset)]),
+ attributes_for(:step).merge!(attachments: [{ name: 'random.jpg', url: 'http://www.example.com/random.jpg' }]),
attributes_for(:step).merge!(tables_attributes: [attributes_for(:table), attributes_for(:table)])
- ]
+ ].to_json
end
let(:service_call) do
ProtocolImporters::ImportProtocolService
- .call(protocol_params: protocol_params, steps_params: steps_params, user_id: user.id, team_id: team.id)
+ .call(protocol_params: protocol_params, steps_params_json: steps_params, user_id: user.id, team_id: team.id)
end
context 'when have invalid arguments' do
it 'returns an error when can\'t find user' do
- allow(User).to receive(:find).and_return(nil)
+ allow(User).to receive(:find_by_id).and_return(nil)
expect(service_call.errors).to have_key(:invalid_arguments)
end
@@ -28,19 +28,20 @@ describe ProtocolImporters::ImportProtocolService do
it 'returns invalid protocol when can\'t save it' do
# step with file without name
steps_invalid_params = [
- attributes_for(:step).merge!(assets_attributes: [attributes_for(:asset).except(:file_file_name)])
- ]
+ attributes_for(:step).except(:name).merge!(tables_attributes: [attributes_for(:table)])
+ ].to_json
s = ProtocolImporters::ImportProtocolService.call(protocol_params: protocol_params,
- steps_params: steps_invalid_params,
+ steps_params_json: steps_invalid_params,
user_id: user.id, team_id: team.id)
-
expect(s.protocol).to be_invalid
end
end
context 'when have valid arguments' do
before do
+ stub_request(:get, 'http://www.example.com/random.jpg').to_return(status: 200, body: '', headers: {})
+
@protocol = Protocol.new
end
diff --git a/spec/utilities/protocol_importers/attachments_builder_spec.rb b/spec/utilities/protocol_importers/attachments_builder_spec.rb
index 49dbbd156..b0c9ffe3b 100644
--- a/spec/utilities/protocol_importers/attachments_builder_spec.rb
+++ b/spec/utilities/protocol_importers/attachments_builder_spec.rb
@@ -8,6 +8,7 @@ RSpec.describe ProtocolImporters::AttachmentsBuilder do
end
let(:generate_files_from_step) { described_class.generate(step) }
let(:first_file_in_result) { generate_files_from_step.first }
+ let(:generate_json_files_from_step) { described_class.generate_json(step) }
before do
stub_request(:get, 'https://pbs.twimg.com/media/Cwu3zrZWQAA7axs.jpg').to_return(status: 200, body: '', headers: {})
@@ -15,7 +16,7 @@ RSpec.describe ProtocolImporters::AttachmentsBuilder do
.to_return(status: 200, body: '', headers: {})
end
- describe 'self.build_assets_for_step' do
+ describe 'self.generate' do
it 'returns array of Asset instances' do
expect(first_file_in_result).to be_instance_of(Asset)
end
@@ -24,4 +25,14 @@ RSpec.describe ProtocolImporters::AttachmentsBuilder do
expect(first_file_in_result).to be_valid
end
end
+
+ describe 'self.generate_json' do
+ it 'returns JSON with 2 items (files)' do
+ expect(generate_json_files_from_step.size).to be == 2
+ end
+
+ it 'returns JSON item with name and url' do
+ expect(generate_json_files_from_step.first.keys).to contain_exactly(:name, :url)
+ end
+ end
end
diff --git a/spec/utilities/protocol_importers/protocol_intermediate_object_spec.rb b/spec/utilities/protocol_importers/protocol_intermediate_object_spec.rb
index dd356b002..a87085f70 100644
--- a/spec/utilities/protocol_importers/protocol_intermediate_object_spec.rb
+++ b/spec/utilities/protocol_importers/protocol_intermediate_object_spec.rb
@@ -5,6 +5,9 @@ require 'rails_helper'
describe ProtocolImporters::ProtocolIntermediateObject do
subject(:pio) { described_class.new(normalized_json: normalized_result, user: user, team: team) }
let(:invalid_pio) { described_class.new(normalized_json: normalized_result, user: nil, team: team) }
+ let(:pio_without_assets) do
+ described_class.new(normalized_json: normalized_result, user: user, team: team, build_with_assets: false)
+ end
let(:user) { create :user }
let(:team) { create :team }
let(:normalized_result) do
@@ -34,5 +37,26 @@ describe ProtocolImporters::ProtocolIntermediateObject do
it { expect(invalid_pio.import).to be_invalid }
it { expect { invalid_pio.import }.not_to(change { Protocol.all.count }) }
end
+
+ context 'when build wihout assets' do
+ it { expect { pio_without_assets.import }.to change { Protocol.all.count }.by(1) }
+ it { expect { invalid_pio.import }.not_to(change { Asset.all.count }) }
+ end
+ end
+
+ describe '.steps_assets' do
+ context 'when have default pio' do
+ it 'retuns empty hash for steps_assets' do
+ pio.build
+ expect(pio.steps_assets).to be == {}
+ end
+ end
+
+ context 'when have pio built without assets' do
+ it 'returns hash with two assets on steps by position' do
+ pio_without_assets.build
+ expect(pio_without_assets.steps_assets.size).to be == 2
+ end
+ end
end
end