diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ce790bc506453c34a70e3c9b945a12dff132a9be..d80624c95470485e3836f157b35ef109ee70b364 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -4,9 +4,15 @@ cache: paths: - vendor/ +before_script: + - bundle install -j $(nproc) --path vendor + test: stage: test script: - - apt-get update -qy - - bundle install --path vendor - - bundle exec rake test + - bundle exec rake test + +rubocop: + stage: test + script: + - bundle exec rubocop diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000000000000000000000000000000000000..e1250785f27be09eea4a2f25948c4093024be09a --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,10 @@ +Metrics/LineLength: + Max: 100 + +# exclude for tests +Metrics/BlockLength: + ExcludedMethods: ['describe', 'it'] + +# don't require documentation on classes / modules +Style/Documentation: + Enabled: false diff --git a/Gemfile b/Gemfile index 232142a2bf64852336edcc573df2cbe8759bcb6c..5556ddc7afd73ccb7097d1dba7c5c31ab8d0f809 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,8 @@ -source "https://rubygems.org" +# frozen_string_literal: true -git_source(:github) {|repo_name| "https://github.com/#{repo_name}" } +source 'https://rubygems.org' + +git_source(:github) { |repo_name| "https://github.com/#{repo_name}" } # Specify your gem's dependencies in excelsior.gemspec gemspec diff --git a/Gemfile.lock b/Gemfile.lock index 9b181ca1ad95637247d7dc8373a8fb075f1b74e2..be055d21bce4012a48e63147e1e29b928688e021 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -51,6 +51,7 @@ GEM minitest (~> 5.1) tzinfo (~> 1.1) arel (9.0.0) + ast (2.4.0) builder (3.2.3) concurrent-ruby (1.0.5) crass (1.0.4) @@ -74,6 +75,10 @@ GEM nio4r (2.3.1) nokogiri (1.8.2) mini_portile2 (~> 2.3.0) + parallel (1.12.1) + parser (2.5.1.0) + ast (~> 2.4.0) + powerpack (0.1.1) rack (2.0.5) rack-test (1.0.0) rack (>= 1.0, < 3) @@ -101,7 +106,16 @@ GEM method_source rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) + rainbow (3.0.0) rake (10.5.0) + rubocop (0.56.0) + parallel (~> 1.10) + parser (>= 2.5) + powerpack (~> 0.1) + rainbow (>= 2.2.2, < 4.0) + ruby-progressbar (~> 1.7) + unicode-display_width (~> 1.0, >= 1.0.1) + ruby-progressbar (1.9.0) rubyzip (1.2.1) simple_xlsx_reader (1.0.2) nokogiri @@ -118,6 +132,7 @@ GEM thread_safe (0.3.6) tzinfo (1.2.5) thread_safe (~> 0.1) + unicode-display_width (1.3.3) websocket-driver (0.7.0) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.3) @@ -130,6 +145,7 @@ DEPENDENCIES excelsior! minitest (~> 5.0) rake (~> 10.0) + rubocop (~> 0.56.0) sqlite3 BUNDLED WITH diff --git a/Rakefile b/Rakefile index 66d80bcc2a60a16933dc90b6f9d044b3d0aa5723..43290b22be16660599c531a2008929205e8ca98f 100644 --- a/Rakefile +++ b/Rakefile @@ -1,11 +1,13 @@ -require "bundler/gem_tasks" -require "rake/testtask" +# frozen_string_literal: true + +require 'bundler/gem_tasks' +require 'rake/testtask' Rake::TestTask.new(:test) do |t| - t.libs << "test" - t.libs << "lib" - t.test_files = FileList["test/**/*_test.rb"] + t.libs << 'test' + t.libs << 'lib' + t.test_files = FileList['test/**/*_test.rb'] t.options = '-b' end -task :default => :test +task default: :test diff --git a/bin/console b/bin/console index 03b778fb0cee875f6cb15ae4cbf3942b8a3c6c48..2b65065a63eb8f897d936dcba22d4b7747f55a85 100755 --- a/bin/console +++ b/bin/console @@ -1,7 +1,8 @@ #!/usr/bin/env ruby +# frozen_string_literal: true -require "bundler/setup" -require "excelsior" +require 'bundler/setup' +require 'excelsior' # You can add fixtures and/or initialization code here to make experimenting # with your gem easier. You can also use a different console, if you like. @@ -10,5 +11,5 @@ require "excelsior" # require "pry" # Pry.start -require "irb" +require 'irb' IRB.start(__FILE__) diff --git a/excelsior.gemspec b/excelsior.gemspec index a449320526bb363941300b01bc20e37bd17a0d06..855a97894f1bea6725894256bc3a6f9be344108f 100644 --- a/excelsior.gemspec +++ b/excelsior.gemspec @@ -1,31 +1,35 @@ -lib = File.expand_path("../lib", __FILE__) +# frozen_string_literal: true + +lib = File.expand_path('lib', __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) -require "excelsior/version" +require 'excelsior/version' Gem::Specification.new do |spec| - spec.name = "excelsior" + spec.name = 'excelsior' spec.version = Excelsior::VERSION - spec.authors = ["Immanuel Häussermann"] - spec.email = ["hai@panter.ch"] + spec.authors = ['Immanuel Häussermann'] + spec.email = ['hai@panter.ch'] - spec.summary = %q{Helps you import data from an excel sheet} - spec.description = %q{Provides a concise DSL to map, validate and import data from an excel sheet into your ruby app} - spec.homepage = "http://github.com/manufaktor/excelsior" - spec.license = "MIT" + spec.summary = 'Helps you import data from an excel sheet' + spec.description = 'Provides a concise DSL to map, validate and import data ' \ + 'from an excel sheet into your ruby app' + spec.homepage = 'http://github.com/manufaktor/excelsior' + spec.license = 'MIT' spec.files = `git ls-files -z`.split("\x0").reject do |f| f.match(%r{^(test|spec|features)/}) end - spec.bindir = "exe" + spec.bindir = 'exe' spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } - spec.require_paths = ["lib"] + spec.require_paths = ['lib'] - spec.add_development_dependency "bundler", "~> 1.16" - spec.add_development_dependency "rake", "~> 10.0" - spec.add_development_dependency "minitest", "~> 5.0" - spec.add_development_dependency "sqlite3" + spec.add_development_dependency 'bundler', '~> 1.16' + spec.add_development_dependency 'minitest', '~> 5.0' + spec.add_development_dependency 'rake', '~> 10.0' + spec.add_development_dependency 'rubocop', '~> 0.56.0' + spec.add_development_dependency 'sqlite3' - spec.add_runtime_dependency "simple_xlsx_reader", "~> 1.0.2" - spec.add_runtime_dependency "rails", ">= 4.0.0" - spec.add_runtime_dependency "activerecord", ">= 4.0.0" + spec.add_runtime_dependency 'activerecord', '>= 4.0.0' + spec.add_runtime_dependency 'rails', '>= 4.0.0' + spec.add_runtime_dependency 'simple_xlsx_reader', '~> 1.0.2' end diff --git a/lib/excelsior.rb b/lib/excelsior.rb index 3e35195cef28c46c7276ada131bb1994c3c24b2b..51f6da223aa021914e6be1df2596bba2cde2b962 100644 --- a/lib/excelsior.rb +++ b/lib/excelsior.rb @@ -1,4 +1,6 @@ -require "excelsior/version" +# frozen_string_literal: true + +require 'excelsior/version' module Excelsior end diff --git a/lib/excelsior/error.rb b/lib/excelsior/error.rb index 849361969f64fc77ba9bf00133ddc582f5a3efe4..aefa2b9fea9da822fe8e56a51e728f60052a691f 100644 --- a/lib/excelsior/error.rb +++ b/lib/excelsior/error.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Excelsior Error = Struct.new(:row, :errors) end diff --git a/lib/excelsior/import.rb b/lib/excelsior/import.rb index d4e0527df19c354f006a701a37f5cdc88238e070..c2525f1471115fe3c4cc28b8179469d6d3c7a83b 100644 --- a/lib/excelsior/import.rb +++ b/lib/excelsior/import.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'simple_xlsx_reader' require 'rails' require 'active_record' @@ -20,7 +22,7 @@ module Excelsior self.source = file || self.class.source_file self.fields = self.class.fields - @doc = ::SimpleXlsxReader.open(self.source) + @doc = ::SimpleXlsxReader.open(source) @sheet = @doc.sheets.first @columns = @sheet.rows.shift @@ -36,7 +38,7 @@ module Excelsior model_class.transaction do insert_rows(&block) - raise ActiveRecord::Rollback if @report.failed > 0 + raise ActiveRecord::Rollback if @report.failed.positive? end else insert_rows(&block) @@ -44,20 +46,17 @@ module Excelsior end def valid? - @errors = fields.to_a.reduce({}) do |acc, f| + @errors = fields.to_a.each_with_object({}) do |f, acc| acc[:missing_column] ||= [] - unless @columns.include?(f[:header]) - acc[:missing_column] << { missing: f[:header] } - end - acc + acc[:missing_column] << { missing: f[:header] } unless @columns.include?(f[:header]) end end private def model_class - self.class.name.gsub("Import", "").constantize + self.class.name.gsub('Import', '').constantize end def add_model_errors(record, index) @@ -84,19 +83,29 @@ module Excelsior def insert_rows(&block) @rows.map.with_index do |row, i| attributes = map_row_values(row, @columns) - if block_given? - begin - result = block.call(attributes) - report_insert - result - rescue - report_failure - end - else - record = model_class.create(attributes) - add_model_errors(record, i) - end + insert_row(attributes, i, &block) + end + end + + def insert_row(attributes, index, &block) + if block_given? + insert_row_with_block(attributes, &block) + else + insert_row_without_block(attributes, index) end end + + def insert_row_with_block(attributes) + result = yield(attributes) + report_insert + result + rescue StandardError + report_failure + end + + def insert_row_without_block(attributes, index) + record = model_class.create(attributes) + add_model_errors(record, index) + end end end diff --git a/lib/excelsior/mapping.rb b/lib/excelsior/mapping.rb index 12b9035ed1df8fce91c6f68ef46d6bfa8748857e..d41ccee9339e78e55ac2e25929f244687a351403 100644 --- a/lib/excelsior/mapping.rb +++ b/lib/excelsior/mapping.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Excelsior module Mapping def self.included(host_class) @@ -17,10 +19,9 @@ module Excelsior end def map_row_values(row, columns) - @fields.to_a.reduce({}) do |acc, field| + @fields.to_a.each_with_object({}) do |field, acc| idx = columns.index(field[:header]) acc[field[:attribute]] = row[idx] - acc end end end diff --git a/lib/excelsior/report.rb b/lib/excelsior/report.rb index ce93fca389325eadf4175985505943178fa22723..616fbce9403cb9e3bd71850bfcf88ae175ab4f08 100644 --- a/lib/excelsior/report.rb +++ b/lib/excelsior/report.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Excelsior Report = Struct.new(:inserted, :failed) do def initialize(*args) diff --git a/lib/excelsior/source.rb b/lib/excelsior/source.rb index fbbe7315e7df15f2464c06daab3e47b1c550a07f..8b9648b309c7b4d4283b5879078040fdf07c3d54 100644 --- a/lib/excelsior/source.rb +++ b/lib/excelsior/source.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Excelsior module Source def self.included(host_class) diff --git a/lib/excelsior/transaction.rb b/lib/excelsior/transaction.rb index d3a2247e7e0b86f4622e3a873eebb6d06dab91db..e0950e76c15be5893ffbb0aa5097ef63cfd309f8 100644 --- a/lib/excelsior/transaction.rb +++ b/lib/excelsior/transaction.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Excelsior module Transaction def self.included(host_class) diff --git a/lib/excelsior/version.rb b/lib/excelsior/version.rb index 8c605ac539fb2fda1925d360e92ca8c6d59354c3..5e6ab00c6cf26ea56300474ac91ec08e70290038 100644 --- a/lib/excelsior/version.rb +++ b/lib/excelsior/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Excelsior - VERSION = "0.1.0" + VERSION = '0.1.0' end diff --git a/test/excelsior_test.rb b/test/excelsior_test.rb index 259dc2dde77f380301c1df9e8675893e15b2c362..2d97bb39f85222a089a73e982c06a53bc59a950c 100644 --- a/test/excelsior_test.rb +++ b/test/excelsior_test.rb @@ -1,5 +1,7 @@ -require "test_helper" -require "excelsior/import" +# frozen_string_literal: true + +require 'test_helper' +require 'excelsior/import' class User < ActiveRecord::Base validates :first_name, presence: true @@ -10,11 +12,11 @@ describe Excelsior do Object.send(:remove_const, :UserImport) if Object.constants.include?(:UserImport) class UserImport < Excelsior::Import - source "test/files/complete.xlsx" + source 'test/files/complete.xlsx' - map "Vorname", to: :first_name - map "Nachname", to: :last_name - map "E-Mail", to: :email + map 'Vorname', to: :first_name + map 'Nachname', to: :last_name + map 'E-Mail', to: :email end @import = UserImport.new @@ -27,19 +29,19 @@ describe Excelsior do describe 'source' do it 'allows setting the source on the class' do import = UserImport.new - assert_equal "test/files/complete.xlsx", import.source + assert_equal 'test/files/complete.xlsx', import.source end it 'allows setting the source on an instance level' do - import = UserImport.new("test/files/missing-column.xlsx") - assert_equal "test/files/missing-column.xlsx", import.source + import = UserImport.new('test/files/missing-column.xlsx') + assert_equal 'test/files/missing-column.xlsx', import.source end end describe 'transaction' do describe 'transaction is not set' do it 'inserts only the valid rows' do - UserImport.new("test/files/missing-first-name.xlsx").run + UserImport.new('test/files/missing-first-name.xlsx').run assert_equal 2, User.count end end @@ -49,17 +51,17 @@ describe Excelsior do Object.send(:remove_const, :UserImport) if Object.constants.include?(:UserImport) class UserImport < Excelsior::Import - source "test/files/complete.xlsx" + source 'test/files/complete.xlsx' transaction true - map "Vorname", to: :first_name - map "Nachname", to: :last_name - map "E-Mail", to: :email + map 'Vorname', to: :first_name + map 'Nachname', to: :last_name + map 'E-Mail', to: :email end end describe 'without a block' do - let(:import) { UserImport.new("test/files/missing-first-name.xlsx") } + let(:import) { UserImport.new('test/files/missing-first-name.xlsx') } before do import.run @@ -78,7 +80,7 @@ describe Excelsior do describe 'with a block' do it 'rolls back if one record raises an error' do - UserImport.new("test/files/missing-first-name.xlsx").run do |v| + UserImport.new('test/files/missing-first-name.xlsx').run do |v| User.create!(v) end @@ -86,7 +88,7 @@ describe Excelsior do end it 'does not roll back if the block does not raise an error' do - UserImport.new("test/files/missing-first-name.xlsx").run do |v| + UserImport.new('test/files/missing-first-name.xlsx').run do |v| User.create(v) end @@ -105,18 +107,18 @@ describe Excelsior do describe '#columns' do it 'returns the columns as defined in the excel' do assert_equal 3, @import.columns.length - assert_equal "E-Mail", @import.columns[0] - assert_equal "Vorname", @import.columns[1] - assert_equal "Nachname", @import.columns[2] + assert_equal 'E-Mail', @import.columns[0] + assert_equal 'Vorname', @import.columns[1] + assert_equal 'Nachname', @import.columns[2] end end describe '#fields' do it 'returns the mapped fields' do assert_equal @import.fields, [ - { attribute: :first_name, header: "Vorname" }, - { attribute: :last_name, header: "Nachname" }, - { attribute: :email, header: "E-Mail" } + { attribute: :first_name, header: 'Vorname' }, + { attribute: :last_name, header: 'Nachname' }, + { attribute: :email, header: 'E-Mail' } ] end end @@ -132,28 +134,30 @@ describe Excelsior do describe 'with a block' do it 'yields the records to the block' do results = @import.run { |v| v } - assert_equal results[0], { - first_name: "Hans", - last_name: "Müller", - email: "hans@mueller.com" - } - assert_equal results[1], { - first_name: "Jögi", - last_name: "Brunz", - email: "jb@runz.com" - } + assert_equal results, [ + { + first_name: 'Hans', + last_name: 'Müller', + email: 'hans@mueller.com' + }, + { + first_name: 'Jögi', + last_name: 'Brunz', + email: 'jb@runz.com' + } + ] end end end describe '#errors' do it 'returns an error when a column is missing in the excel' do - import = UserImport.new("test/files/missing-column.xlsx") + import = UserImport.new('test/files/missing-column.xlsx') assert import.errors[:missing_column].any? end it 'returns the model validation errors' do - import = UserImport.new("test/files/missing-first-name.xlsx").tap(&:run) + import = UserImport.new('test/files/missing-first-name.xlsx').tap(&:run) assert import.errors[:model].any? assert_equal import.errors[:model], [Excelsior::Error.new(3, ["First name can't be blank"])] end @@ -162,7 +166,7 @@ describe Excelsior do describe '#report' do describe 'without a block' do it 'returns the inserted, failed and total number of rows' do - import = UserImport.new("test/files/missing-first-name.xlsx").tap(&:run) + import = UserImport.new('test/files/missing-first-name.xlsx').tap(&:run) assert_equal 2, import.report.inserted assert_equal 1, import.report.failed assert_equal 3, import.report.total @@ -171,9 +175,9 @@ describe Excelsior do describe 'with a block' do it 'returns the inserted, failed and total number of rows' do - import = UserImport.new("test/files/missing-first-name.xlsx") + import = UserImport.new('test/files/missing-first-name.xlsx') import.run do |v| - raise "failure!" if v[:first_name].nil? + raise 'failure!' if v[:first_name].nil? v end assert_equal 2, import.report.inserted diff --git a/test/test_helper.rb b/test/test_helper.rb index 59bb421127c260fc6b139f92fa0431260eb1c937..ec37b64678d8f6df583bb72c155c290b4b83db9c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,11 +1,15 @@ -$LOAD_PATH.unshift File.expand_path("../../lib", __FILE__) -require "excelsior" +# frozen_string_literal: true + +$LOAD_PATH.unshift File.expand_path('../lib', __dir__) +require 'excelsior' require 'active_record' -require "minitest/autorun" +require 'minitest/autorun' -class MiniTest::Spec - before do - User.delete_all +module MiniTest + class Spec + before do + User.delete_all + end end end