Merge pull request #1848 from urbanrotnik/ur-sci-3541-fixing-wrong-imports

Required fields not imported  [SCI-3541]

app/utilities/protocol_importers/protocol_intermediate_object.rb

@@ -28,9 +28,9 @@ module ProtocolImporters
     def build_steps
       @normalized_protocol_data[:steps].map do |s|
         step = Step.new(step_attributes(s))
-        step.description = StepDescriptionBuilder.generate(s)
         step.assets << AttachmentsBuilder.generate(s)
-        step.tables << TablesBuilder.extract_tables_from_html_string(s[:description][:body])
+        step.tables << TablesBuilder.extract_tables_from_html_string(s[:description][:body], true)
+        step.description = StepDescriptionBuilder.generate(s)
         step
       end
     end

app/utilities/protocol_importers/step_description_builder.rb

@@ -5,6 +5,7 @@ module ProtocolImporters
     def self.generate(step_json)
       return '' unless step_json[:description]
 
+      step_json[:description][:body] = TablesBuilder.remove_tables_from_html(step_json[:description][:body])
       html_string = ApplicationController
                     .renderer
                     .render(template: 'protocol_importers/templates/step_description',

app/utilities/protocol_importers/tables_builder.rb

@@ -2,7 +2,7 @@
 
 module ProtocolImporters
   class TablesBuilder
-    def self.extract_tables_from_html_string(description_string)
+    def self.extract_tables_from_html_string(description_string, remove_first_column_row = false)
       tables = []
 
       doc = Nokogiri::HTML(description_string)
@@ -17,11 +17,21 @@ module ProtocolImporters
           row.css('td').each_with_index do |cell, j|
             two_d_array[i][j] = cell.inner_html
           end
+          two_d_array[i].shift if remove_first_column_row
         end
+        two_d_array.shift if remove_first_column_row
 
         tables << Table.new(contents: { data: two_d_array }.to_json)
       end
       tables
     end
+
+    def self.remove_tables_from_html(description_string)
+      doc = Nokogiri::HTML(description_string)
+      doc.search('table').each do |t|
+        t.swap('<br/><p><i>There was a table here, it was moved to tables section.</i></p>')
+      end
+      doc.css('body').first.inner_html
+    end
   end
 end

app/views/protocol_importers/templates/protocol_description.html.erb

@@ -1,5 +1,5 @@
 <% if @description[:body] %>
-  <p> <%= strip_tags @description[:body] %> </p>
+  <p> <%= sanitize(@description[:body], tags: Constants::PROTOCOLS_DESC_TAGS) %> </p>
   <br/>
 <% end %>
 <% if @description[:image] %>
@@ -11,6 +11,6 @@
 <% @description[:extra_content]&.each do |i| %>
   <% if i[:body].present? %>
     <br/><b><%= strip_tags i[:title] %>:</b> <br/>
-    <%= strip_tags i[:body] %><br/>
+    <%= sanitize(i[:body], tags: Constants::PROTOCOLS_DESC_TAGS) %><br/>
   <% end %>
 <% end %>

app/views/protocol_importers/templates/step_description.html.erb

@@ -1,16 +1,16 @@
 <% if @step_description[:body] %>
-  <p> <%= strip_tags @step_description[:body] %> </p>
+  <p> <%= sanitize(@step_description[:body], tags: Constants::PROTOCOLS_DESC_TAGS) %> </p>
 <% end %>
 
 <% @step_description[:components]&.each do |component| %>
   <% sanitized_component = component.except('type') %>
-  <% sanitized_component[:body] = strip_tags(component[:body]) if component[:body] %>
+  <% sanitized_component[:body] = sanitize(component[:body], tags: Constants::PROTOCOLS_DESC_TAGS) if component[:body] %>
   <%= render partial: "protocol_importers/templates/#{component[:type]}", locals: { item: sanitized_component } %>
 <% end %>
 
 <% @step_description[:extra_content]&.each do |i| %>
   <b><%= strip_tags i[:title] %>:</b> <br/>
-  <%= strip_tags i[:body] %><br/>
+  <%= sanitize(i[:body], tags: Constants::PROTOCOLS_DESC_TAGS) %><br/>
 <% end %>
 
 

config/initializers/constants.rb

@@ -221,6 +221,8 @@ class Constants
     source_id: 'protocolsio/v3'
   }.freeze
 
+  PROTOCOLS_DESC_TAGS = %w(a img i br).freeze
+
   #=============================================================================
   # Other
   #=============================================================================

spec/fixtures/files/protocol_importers/description_with_body_html.json

@@ -1,5 +1,5 @@
 {
   "description": {
-    "body": "<script>alert('boom')</script><div class='col-5'><h5>Text only</h5>"
+    "body": "<script>alert('boom')</script><div class='col-5'><h5>Text only</h5> <img src='nekaj.com'/> <i></i> <b></b> <a href='nekaj.com'>Link tukaj</a> WTF <div class='asd'></div>"
   }
 }

spec/utilities/protocol_importers/step_description_builder_spec.rb

@@ -37,7 +37,7 @@ RSpec.describe ProtocolImporters::StepDescriptionBuilder do
 
     context 'when have only description body' do
       it 'includes paragraph description' do
-        expect(described_class.generate(description_only)).to include('<p> original desc </p>')
+        expect(described_class.generate(description_only)).to include('<p> original desc')
       end
     end
 
@@ -47,7 +47,7 @@ RSpec.describe ProtocolImporters::StepDescriptionBuilder do
       end
 
       it 'strips HTML tags from body values for component' do
-        expect(described_class.generate(description_with_components).scan('alert').size).to be == 0
+        expect(described_class.generate(description_with_components).scan('<script>').size).to be == 0
       end
     end
 
@@ -60,5 +60,11 @@ RSpec.describe ProtocolImporters::StepDescriptionBuilder do
         expect(described_class.generate(description_with_html).scan('script').count).to be == 0
       end
     end
+
+    context 'when have allowed html tags' do
+      it 'does not strip img tags' do
+        expect(described_class.generate(description_with_html).scan('img').size).to eq(1)
+      end
+    end
   end
 end

spec/utilities/protocol_importers/tables_builder_spec.rb

@@ -5,6 +5,7 @@ require 'rails_helper'
 RSpec.describe ProtocolImporters::TablesBuilder do
   # rubocop:disable Metrics/LineLength
   let(:description_string) { '<table><tr><td>1</td><td>2</td><td>3</td><td>4</td><td>5</td><td>6</td><td>7</td><td>8</td><td>9</td><td>10</td></tr><tr><td>a</td><td>b</td><td>c</td><td>d</td><td>e</td><td>f</td><td>g</td><td>h</td><td>a</td><td>a</td></tr><tr><td>1</td><td>1</td><td>1</td><td>2</td><td>3</td><td>4</td><td>5</td><td>1</td><td>1</td><td>1</td></tr><tr><td>1</td><td>1</td><td>1</td><td>1</td><td>1</td><td>1</td><td>1</td><td>1</td><td>1</td><td>1</td></tr><tr><td>asd</td><td>as</td><td>das</td><td>a</td><td>as</td><td>asd</td><td>sad</td><td>sa</td><td>asd</td><td>as124521</td></tr></table><table><tr><td>1</td><td>2</td><td>3</td></tr></table>' }
+  let(:description_string_with_headers) { '<table><tr><td></td><td>1</td><td>2</td><td>3</td></tr><tr><td>A</td><td>d1</td><td>d2</td><td>d3</td></tr><tr><td>B</td><td>c1</td><td>c2</td><td>c3</td></tr></table>' }
   # rubocop:enable Metrics/LineLength
 
   let(:extract_tables_from_string_result) { described_class.extract_tables_from_html_string(description_string) }
@@ -30,5 +31,21 @@ RSpec.describe ProtocolImporters::TablesBuilder do
     it 'returns table with 10 columns' do
       expect(JSON.parse(first_table_in_result.contents)['data'].first.count).to be == 10
     end
+
+    context 'when droping headers' do
+      it 'returns table with 2 rows and 3 columns' do
+        table = described_class.extract_tables_from_html_string(description_string_with_headers, true).first
+
+        expect(JSON.parse(table.contents)['data'].count).to be == 2
+        expect(JSON.parse(table.contents)['data'].first.count).to be == 3
+      end
+    end
+  end
+
+  describe '.remove_tables_from_html' do
+    it 'returns description string without tables' do
+      expect(described_class.remove_tables_from_html(description_string)
+               .scan('<p><i>There was a table here, it was moved to tables section.</i></p>').size).to eq(2)
+    end
   end
 end