From 8debdfb596cd8259331e81ec3d74afcf2e65c884 Mon Sep 17 00:00:00 2001
From: bgood <bgood@brocksolutions.com>
Date: Sun, 27 Mar 2016 21:48:30 -0400
Subject: [PATCH 1/3] Add table view for CSV files

---
 app/assets/stylesheets/framework/files.scss   |  7 ++++++
 app/helpers/blob_helper.rb                    | 10 +++++++++
 app/models/blob.rb                            |  2 ++
 app/views/projects/blob/_csv.html.haml        | 22 +++++++++++++++++++
 .../bengood-gitlab-ce-table-view.yml          |  4 ++++
 features/project/source/browse_files.feature  |  4 ++++
 features/steps/project/source/browse_files.rb |  8 +++++++
 spec/models/blob_spec.rb                      |  9 +++++++-
 spec/support/repo_helpers.rb                  | 18 +++++++++++++++
 9 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 app/views/projects/blob/_csv.html.haml
 create mode 100644 changelogs/unreleased/bengood-gitlab-ce-table-view.yml

diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss
index f49d7b92a009..7320d191ca7a 100644
--- a/app/assets/stylesheets/framework/files.scss
+++ b/app/assets/stylesheets/framework/files.scss
@@ -168,6 +168,13 @@
     &.code {
       padding: 0;
     }
+
+    table, th, td {
+      border-collapse: collapse;
+      padding-top: 0;
+      padding-bottom: 5.5px;
+      line-height: 0.5;
+    }
   }
 }
 
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 07ff6fb94889..3a9fd90d6366 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -1,3 +1,5 @@
+require 'csv'
+
 module BlobHelper
   def highlight(blob_name, blob_content, repository: nil, plain: false)
     highlighted = Gitlab::Highlight.highlight(blob_name, blob_content, plain: plain, repository: repository)
@@ -8,6 +10,14 @@ def no_highlight_files
     %w(credits changelog news copying copyright license authors)
   end
 
+  def blob_to_csv(blob, options = {})
+    begin
+      CSV.parse(blob.data, options)
+    rescue CSV::MalformedCSVError
+      nil
+    end
+  end
+
   def edit_blob_link(project = @project, ref = @ref, path = @path, options = {})
     return unless current_user
 
diff --git a/app/models/blob.rb b/app/models/blob.rb
index ab92e8203354..224642631fa8 100644
--- a/app/models/blob.rb
+++ b/app/models/blob.rb
@@ -59,6 +59,8 @@ def to_partial_path
       'download'
     elsif image? || svg?
       'image'
+    elsif csv?
+      'csv'
     elsif text?
       'text'
     else
diff --git a/app/views/projects/blob/_csv.html.haml b/app/views/projects/blob/_csv.html.haml
new file mode 100644
index 000000000000..05c8c485ae49
--- /dev/null
+++ b/app/views/projects/blob/_csv.html.haml
@@ -0,0 +1,22 @@
+.file-content.code
+  .file-content.code.js-syntax-highlight{ class: user_color_scheme }
+    - if blob.csv?
+      - unless blob.empty?
+        .line-numbers
+          - blob.data.lines.each_index do |index|
+            - offset = defined?(first_line_number) ? first_line_number : 1
+            - i = index + offset
+            -# We're not using `link_to` because it is too slow once we get to thousands of lines.
+            %a{ href: "#L#{ i }", id: "L#{ i }", data: { line_number: "#{ i }" } }
+              %i.fa.fa-link
+              = i
+        .blob-content{ data: { blob_id: blob.id } }
+          %pre.code.highlight
+            %table
+              - blob_to_csv(blob).each_with_index do |row, i|
+                %tr
+                  - row.each do |col|
+                    %td
+                      = col
+    - else
+      .nothing-here-block Empty file
\ No newline at end of file
diff --git a/changelogs/unreleased/bengood-gitlab-ce-table-view.yml b/changelogs/unreleased/bengood-gitlab-ce-table-view.yml
new file mode 100644
index 000000000000..968a66c43514
--- /dev/null
+++ b/changelogs/unreleased/bengood-gitlab-ce-table-view.yml
@@ -0,0 +1,4 @@
+---
+title: Display CSV files as tabular data
+merge_request:
+author: Ben Good
diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature
index d4b91fec6e83..f92e77f447c2 100644
--- a/features/project/source/browse_files.feature
+++ b/features/project/source/browse_files.feature
@@ -19,6 +19,10 @@ Feature: Project Source Browse Files
     Given I visit blob file from repo
     And I click link "Raw"
     Then I should see raw file content
+    
+  Scenario: I browse a csv file
+    Given I browse a csv file
+    Then I should see a table
 
   Scenario: I can create file
     Given I click on "New file" link in repo
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 1cc9e37b0750..6aad5c01c457 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -54,6 +54,14 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   step 'I should see raw file content' do
     expect(source).to eq '' # Body is filled in by gitlab-workhorse
   end
+  
+  step 'I browse a csv file' do
+    visit namespace_project_blob_path(@project.namespace, @project, File.join(sample_csv_commit.id, sample_csv_blob.path))
+  end
+  
+  step 'I should see a table' do
+    expect(find(:css,'div.blob-content')).to have_css('table')
+  end
 
   step 'I click button "Edit"' do
     click_link 'Edit'
diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb
index 03d02b4d382b..27f072dcf842 100644
--- a/spec/models/blob_spec.rb
+++ b/spec/models/blob_spec.rb
@@ -76,7 +76,8 @@ def stubbed_blob(overrides = {})
         language: nil,
         lfs_pointer?: false,
         svg?: false,
-        text?: false
+        text?: false,
+        csv?: false
       )
 
       described_class.decorate(double).tap do |blob|
@@ -102,6 +103,12 @@ def stubbed_blob(overrides = {})
       expect(blob.to_partial_path).to eq 'image'
     end
 
+    it 'handles csv' do
+      blob = stubbed_blob(csv?: true)
+
+      expect(blob.to_partial_path).to eq 'csv'
+    end
+    
     it 'handles text' do
       blob = stubbed_blob(text?: true)
 
diff --git a/spec/support/repo_helpers.rb b/spec/support/repo_helpers.rb
index 73f375c481bf..4ec8319c5b5f 100644
--- a/spec/support/repo_helpers.rb
+++ b/spec/support/repo_helpers.rb
@@ -115,4 +115,22 @@ def sample_compare
       commits: commits
     )
   end
+  
+  def sample_csv_commit
+    OpenStruct.new(
+      id: 'b0343abfb4530c065cd57b74879ee52d1a540e99'
+    )
+  end
+  
+  def sample_csv_blob
+    OpenStruct.new(
+      path: "files/csv/Book1.csv"
+    )
+  end
+      
+  def invalid_csv_commit
+    OpenStruct.new(
+      id: '6ff234d2889b27d91c3442924ef6a100b1fc6f2b'
+    )
+  end
 end
-- 
GitLab


From 46a643f71b268759c94b2573e77ac2f6241a5e56 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 7 Nov 2016 12:37:34 +0000
Subject: [PATCH 2/3] Reduce duplication in CSV blob template

We now re-use most of the `shared/_file_highlight` partial that all text
files uses, and add a special rendering for CSV files rather than
passing the contents through the `highlight` helper.

This also simplifies our tests.
---
 app/assets/stylesheets/framework/files.scss   | 10 +++---
 app/helpers/blob_helper.rb                    |  4 +--
 app/models/blob.rb                            |  2 --
 app/views/projects/blob/_csv.html.haml        | 30 +++++-----------
 app/views/shared/_file_highlight.html.haml    |  5 ++-
 features/project/source/browse_files.feature  |  4 ---
 features/steps/project/source/browse_files.rb |  8 -----
 spec/helpers/blob_helper_spec.rb              | 21 +++++++++++
 spec/models/blob_spec.rb                      |  7 ----
 spec/support/repo_helpers.rb                  | 18 ----------
 .../shared/_file_hightlight.html.haml_spec.rb | 35 +++++++++++++++++++
 11 files changed, 76 insertions(+), 68 deletions(-)
 create mode 100644 spec/views/shared/_file_hightlight.html.haml_spec.rb

diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss
index 7320d191ca7a..b35ffdd82745 100644
--- a/app/assets/stylesheets/framework/files.scss
+++ b/app/assets/stylesheets/framework/files.scss
@@ -169,11 +169,13 @@
       padding: 0;
     }
 
-    table, th, td {
+    table.csv {
       border-collapse: collapse;
-      padding-top: 0;
-      padding-bottom: 5.5px;
-      line-height: 0.5;
+      display: inline;
+
+      th {
+        font-weight: bold;
+      }
     }
   }
 }
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 3a9fd90d6366..b88886ba0fc2 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -10,9 +10,9 @@ def no_highlight_files
     %w(credits changelog news copying copyright license authors)
   end
 
-  def blob_to_csv(blob, options = {})
+  def parse_csv(blob_data, options = {}, &block)
     begin
-      CSV.parse(blob.data, options)
+      CSV.parse(blob_data, options, &block)
     rescue CSV::MalformedCSVError
       nil
     end
diff --git a/app/models/blob.rb b/app/models/blob.rb
index 224642631fa8..ab92e8203354 100644
--- a/app/models/blob.rb
+++ b/app/models/blob.rb
@@ -59,8 +59,6 @@ def to_partial_path
       'download'
     elsif image? || svg?
       'image'
-    elsif csv?
-      'csv'
     elsif text?
       'text'
     else
diff --git a/app/views/projects/blob/_csv.html.haml b/app/views/projects/blob/_csv.html.haml
index 05c8c485ae49..3b49f83cf303 100644
--- a/app/views/projects/blob/_csv.html.haml
+++ b/app/views/projects/blob/_csv.html.haml
@@ -1,22 +1,8 @@
-.file-content.code
-  .file-content.code.js-syntax-highlight{ class: user_color_scheme }
-    - if blob.csv?
-      - unless blob.empty?
-        .line-numbers
-          - blob.data.lines.each_index do |index|
-            - offset = defined?(first_line_number) ? first_line_number : 1
-            - i = index + offset
-            -# We're not using `link_to` because it is too slow once we get to thousands of lines.
-            %a{ href: "#L#{ i }", id: "L#{ i }", data: { line_number: "#{ i }" } }
-              %i.fa.fa-link
-              = i
-        .blob-content{ data: { blob_id: blob.id } }
-          %pre.code.highlight
-            %table
-              - blob_to_csv(blob).each_with_index do |row, i|
-                %tr
-                  - row.each do |col|
-                    %td
-                      = col
-    - else
-      .nothing-here-block Empty file
\ No newline at end of file
+%table.table-striped.table-bordered.csv
+  - parse_csv(csv, headers: :first_row, return_headers: true) do |row|
+    %tr
+      - row.each do |_, column|
+        - if row.header_row?
+          %th= column
+        - else
+          %td= column
diff --git a/app/views/shared/_file_highlight.html.haml b/app/views/shared/_file_highlight.html.haml
index e26693bf5b96..33bfc7a886c6 100644
--- a/app/views/shared/_file_highlight.html.haml
+++ b/app/views/shared/_file_highlight.html.haml
@@ -13,4 +13,7 @@
           = link_icon
           = i
   .blob-content{data: {blob_id: blob.id}}
-    = highlight(blob.path, blob.data, repository: repository, plain: blob.no_highlighting?)
+    - if blob.csv?
+      = render 'csv', csv: blob.data
+    - else
+      = highlight(blob.path, blob.data, repository: repository, plain: blob.no_highlighting?)
diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature
index f92e77f447c2..d4b91fec6e83 100644
--- a/features/project/source/browse_files.feature
+++ b/features/project/source/browse_files.feature
@@ -19,10 +19,6 @@ Feature: Project Source Browse Files
     Given I visit blob file from repo
     And I click link "Raw"
     Then I should see raw file content
-    
-  Scenario: I browse a csv file
-    Given I browse a csv file
-    Then I should see a table
 
   Scenario: I can create file
     Given I click on "New file" link in repo
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 6aad5c01c457..1cc9e37b0750 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -54,14 +54,6 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   step 'I should see raw file content' do
     expect(source).to eq '' # Body is filled in by gitlab-workhorse
   end
-  
-  step 'I browse a csv file' do
-    visit namespace_project_blob_path(@project.namespace, @project, File.join(sample_csv_commit.id, sample_csv_blob.path))
-  end
-  
-  step 'I should see a table' do
-    expect(find(:css,'div.blob-content')).to have_css('table')
-  end
 
   step 'I click button "Edit"' do
     click_link 'Edit'
diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb
index a43a7238c708..216d5afdcf97 100644
--- a/spec/helpers/blob_helper_spec.rb
+++ b/spec/helpers/blob_helper_spec.rb
@@ -68,6 +68,27 @@ def test(input):
     end
   end
 
+  describe '#parse_csv' do
+    it 'parses CSV data' do
+      expect { |block| parse_csv("foo,bar\noof,rab", &block) }
+        .to yield_successive_args(%w(foo bar), %w(oof rab))
+    end
+
+    it 'passes options to CSV.parse' do
+      options = { foo: :foo, bar: :bar }
+
+      expect(CSV).to receive(:parse).with(anything, options)
+
+      parse_csv('foo', options)
+    end
+
+    it 'rescues MalformedCSVError' do
+      expect(CSV).to receive(:parse).and_raise(CSV::MalformedCSVError)
+
+      expect { parse_csv('') }.not_to raise_error
+    end
+  end
+
   describe "#edit_blob_link" do
     let(:namespace) { create(:namespace, name: 'gitlab' )}
     let(:project) { create(:project, namespace: namespace) }
diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb
index 27f072dcf842..2718f1a87941 100644
--- a/spec/models/blob_spec.rb
+++ b/spec/models/blob_spec.rb
@@ -77,7 +77,6 @@ def stubbed_blob(overrides = {})
         lfs_pointer?: false,
         svg?: false,
         text?: false,
-        csv?: false
       )
 
       described_class.decorate(double).tap do |blob|
@@ -103,12 +102,6 @@ def stubbed_blob(overrides = {})
       expect(blob.to_partial_path).to eq 'image'
     end
 
-    it 'handles csv' do
-      blob = stubbed_blob(csv?: true)
-
-      expect(blob.to_partial_path).to eq 'csv'
-    end
-    
     it 'handles text' do
       blob = stubbed_blob(text?: true)
 
diff --git a/spec/support/repo_helpers.rb b/spec/support/repo_helpers.rb
index 4ec8319c5b5f..73f375c481bf 100644
--- a/spec/support/repo_helpers.rb
+++ b/spec/support/repo_helpers.rb
@@ -115,22 +115,4 @@ def sample_compare
       commits: commits
     )
   end
-  
-  def sample_csv_commit
-    OpenStruct.new(
-      id: 'b0343abfb4530c065cd57b74879ee52d1a540e99'
-    )
-  end
-  
-  def sample_csv_blob
-    OpenStruct.new(
-      path: "files/csv/Book1.csv"
-    )
-  end
-      
-  def invalid_csv_commit
-    OpenStruct.new(
-      id: '6ff234d2889b27d91c3442924ef6a100b1fc6f2b'
-    )
-  end
 end
diff --git a/spec/views/shared/_file_hightlight.html.haml_spec.rb b/spec/views/shared/_file_hightlight.html.haml_spec.rb
new file mode 100644
index 000000000000..5fa0f5413086
--- /dev/null
+++ b/spec/views/shared/_file_hightlight.html.haml_spec.rb
@@ -0,0 +1,35 @@
+require 'rails_helper'
+
+describe 'shared/_file_highlight.html.haml' do
+  let(:file) { 'shared/file_highlight.html.haml' }
+
+  context 'for csv blobs' do
+    it 'renders a CSV partial' do
+      blob = spy(csv?: true, data: nil)
+      stub_template 'shared/_csv.html.haml' => 'CSV file'
+
+      render file, blob: blob
+
+      expect(rendered).to match('CSV file')
+    end
+  end
+
+  context 'for other blobs' do
+    it 'highlights the blob' do
+      blob = instance_double(
+        'Blob',
+        data: 'data',
+        path: 'path',
+        no_highlighting?: true,
+        id: 'id',
+        csv?: false
+      )
+      repo = double
+
+      expect(view).to receive(:highlight)
+        .with('path', 'data', repository: repo, plain: true)
+
+      render file, blob: blob, repository: repo
+    end
+  end
+end
-- 
GitLab


From e5b1f0e93b9b6037a3993c86a7ea099592863622 Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Mon, 7 Nov 2016 13:06:20 -0600
Subject: [PATCH 3/3] Align line-numbers with CSV content; scroll on overflow

---
 app/assets/stylesheets/framework/files.scss    | 18 ++++++++++++++++++
 .../stylesheets/framework/highlight.scss       |  7 ++++++-
 app/views/shared/_file_highlight.html.haml     |  2 +-
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss
index b35ffdd82745..87bbcaca2a98 100644
--- a/app/assets/stylesheets/framework/files.scss
+++ b/app/assets/stylesheets/framework/files.scss
@@ -172,9 +172,21 @@
     table.csv {
       border-collapse: collapse;
       display: inline;
+      border: 0;
 
       th {
         font-weight: bold;
+        border-top: 0;
+      }
+
+      th,
+      td {
+        &:first-of-type {
+          border-left: 0;
+        }
+        &:last-of-type {
+          border-right: 0;
+        }
       }
     }
   }
@@ -191,3 +203,9 @@ span.idiff {
     border-bottom-right-radius: 2px;
   }
 }
+
+.blob-content {
+  white-space: nowrap;
+  overflow: auto;
+  border-left: 1px solid $table-border-gray;
+}
diff --git a/app/assets/stylesheets/framework/highlight.scss b/app/assets/stylesheets/framework/highlight.scss
index 07c8874bf038..3147c3c0628f 100644
--- a/app/assets/stylesheets/framework/highlight.scss
+++ b/app/assets/stylesheets/framework/highlight.scss
@@ -17,7 +17,6 @@
     overflow-y: hidden;
     white-space: pre;
     word-wrap: normal;
-    border-left: 1px solid;
 
     code {
       display: inline-block;
@@ -43,6 +42,12 @@
     text-align: right;
     float: left;
 
+    &.table-numbers {
+      a {
+        height: 38px;
+      }
+    }
+
     a {
       font-family: $monospace_font;
       display: block;
diff --git a/app/views/shared/_file_highlight.html.haml b/app/views/shared/_file_highlight.html.haml
index 33bfc7a886c6..419c28c6b1e1 100644
--- a/app/views/shared/_file_highlight.html.haml
+++ b/app/views/shared/_file_highlight.html.haml
@@ -1,7 +1,7 @@
 - repository = nil unless local_assigns.key?(:repository)
 
 .file-content.code.js-syntax-highlight
-  .line-numbers
+  .line-numbers{class: ('table-numbers' if blob.csv?)}
     - if blob.data.present?
       - link_icon = icon('link')
       - link = blob_link if defined?(blob_link)
-- 
GitLab