Updating FactoryGirl to FactoryBot and Rubocop; Restructuring test su…

…ites and directory structure in compliance with Rubocop 0.53.0

.rubocop.yml

@@ -1,7 +1,7 @@
 inherit_from: .rubocop_todo.yml
 
 AllCops:
-  TargetRubyVersion: 2.1
+  TargetRubyVersion: 2.4
   DisplayCopNames: true
   Include:
     - '**/Rakefile'
@@ -10,38 +10,83 @@ AllCops:
 
 Rails:
   Enabled: true
+Rails/FilePath:
+  Exclude:
+    - 'lib/generators/browse_everything/config_generator.rb'
 
 Metrics/ClassLength:
   Max: 130
-
-# Configuration parameters: AllowURI, URISchemes.
 Metrics/LineLength:
   Max: 400
+Metrics/BlockLength:
+  Exclude:
+    - 'spec/views/browse_everything/_files.html.erb_spec.rb'
+    - 'spec/unit/browse_everything_helper_spec.rb'
+    - 'spec/unit/browse_everything/retriever_spec.rb'
+    - 'spec/unit/browse_everything/file_entry_spec.rb'
+    - 'spec/unit/browse_everything/driver/s3_spec.rb'
+    - 'spec/unit/browse_everything/driver/google_drive_spec.rb'
+    - 'spec/unit/browse_everything/driver/file_system_spec.rb'
+    - 'spec/unit/browse_everything/driver/dropbox_spec.rb'
+    - 'spec/unit/browse_everything/driver/box_spec.rb'
+    - 'spec/unit/browse_everything/browser_spec.rb'
+    - 'spec/services/browser_factory_spec.rb'
+    - 'spec/services/browse_everything_session/provider_session_spec.rb'
+    - 'spec/helper/browse_everything_controller_helper_spec.rb'
+    - 'spec/controllers/browse_everything_controller_spec.rb'
+    - 'browse-everything.gemspec'
 
 RSpec/NestedGroups:
   Enabled: false
 
-RSpec/FilePath:
-  Enabled: false
-
 RSpec/LeadingSubject:
   Enabled: false
 
 RSpec/ExampleLength:
   Exclude:
-    - 'spec/unit/dropbox_spec.rb'
-    - 'spec/unit/google_drive_spec.rb'
+    - 'spec/unit/browse_everything/driver/base_spec.rb'
+    - 'spec/unit/browse_everything/driver/box_spec.rb'
+    - 'spec/unit/browse_everything/driver/dropbox_spec.rb'
+    - 'spec/unit/browse_everything/driver/file_system_spec.rb'
+    - 'spec/unit/browse_everything/driver/google_drive_spec.rb'
+    - 'spec/unit/browse_everything/driver/s3_spec.rb'
     - 'spec/features/select_files_spec.rb'
     - 'spec/javascripts/jasmine_spec.rb'
 
 RSpec/MultipleExpectations:
   Exclude:
-    - 'spec/unit/dropbox_spec.rb'
-    - 'spec/unit/google_drive_spec.rb'
+    - 'spec/unit/browse_everything/driver/base_spec.rb'
+    - 'spec/unit/browse_everything/driver/box_spec.rb'
+    - 'spec/unit/browse_everything/driver/dropbox_spec.rb'
+    - 'spec/unit/browse_everything/driver/file_system_spec.rb'
+    - 'spec/unit/browse_everything/driver/google_drive_spec.rb'
+    - 'spec/unit/browse_everything/driver/s3_spec.rb'
     - 'spec/features/**/*'
 
 Style/NumericLiterals:
   MinDigits: 7
 
-Style/IndentationConsistency:
+Layout/IndentationConsistency:
   EnforcedStyle: rails
+
+Naming/FileName:
+  Exclude:
+    - 'browse-everything.gemspec'
+    - 'Gemfile'
+    - 'lib/browse-everything.rb'
+
+Style/MixinUsage:
+  Exclude:
+    - 'spec/unit/browse_everything/driver/s3_spec.rb'
+    - 'spec/unit/browse_everything/driver/google_drive_spec.rb'
+    - 'spec/unit/browse_everything/driver/file_system_spec.rb'
+    - 'spec/unit/browse_everything/driver/dropbox_spec.rb'
+    - 'spec/unit/browse_everything/driver/box_spec.rb'
+    - 'spec/unit/browse_everything/driver/base_spec.rb'
+    - 'spec/unit/browse_everything/browser_spec.rb'
+    - 'spec/services/browser_factory_spec.rb'
+    - 'spec/helper/browse_everything_controller_helper_spec.rb'
+
+Bundler/DuplicatedGem:
+  Exclude:
+    - 'Gemfile'

.rubocop_todo.yml

@@ -36,13 +36,6 @@ RSpec/DescribeClass:
   Exclude:
     - 'spec/javascripts/jasmine_spec.rb'
 
-# Offense count: 2
-# Configuration parameters: Max.
-RSpec/ExampleLength:
-  Exclude:
-    - 'spec/features/select_files_spec.rb'
-    - 'spec/javascripts/jasmine_spec.rb'
-
 # Offense count: 2
 RSpec/MultipleExpectations:
   Max: 2
@@ -83,12 +76,6 @@ Style/ClassAndModuleChildren:
 Style/Documentation:
   Enabled: false
 
-# Offense count: 1
-# Configuration parameters: ExpectMatchingDefinition, Regex, IgnoreExecutableScripts.
-Style/FileName:
-  Exclude:
-    - 'lib/browse-everything.rb'
-
 # Offense count: 2
 # Configuration parameters: MinBodyLength.
 Style/GuardClause:
@@ -102,7 +89,7 @@ Style/MultilineBlockChain:
 
 # Offense count: 1
 # Cop supports --auto-correct.
-Style/MultilineBlockLayout:
+Layout/MultilineBlockLayout:
   Exclude:
     - 'spec/unit/dropbox_spec.rb'
 
@@ -111,7 +98,7 @@ Style/MultilineBlockLayout:
 # NamePrefix: is_, has_, have_
 # NamePrefixBlacklist: is_, has_, have_
 # NameWhitelist: is_a?
-Style/PredicateName:
+Naming/PredicateName:
   Exclude:
     - 'spec/**/*'
     - 'app/helpers/browse_everything_helper.rb'

Gemfile

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 source 'https://rubygems.org'
 
 # Specify your gem's dependencies in browse_everything.gemspec
@@ -31,14 +33,12 @@ else
   when /^5.1/
     gem 'capybara', '~> 2.13.0'
   when /^4.2/
-    gem 'responders', '~> 2.0'
-    gem 'sass-rails', '>= 5.0'
     gem 'coffee-rails', '~> 4.1.0'
     gem 'json', '~> 1.8'
+    gem 'responders', '~> 2.0'
+    gem 'sass-rails', '>= 5.0'
   end
 end
 # END ENGINE_CART BLOCK
 
-unless File.exist?(file)
-  eval_gemfile File.expand_path('spec/test_app_templates/Gemfile.extra', File.dirname(__FILE__))
-end
+eval_gemfile File.expand_path('spec/test_app_templates/Gemfile.extra', File.dirname(__FILE__)) unless File.exist?(file)

Rakefile

@@ -1,4 +1,5 @@
-#!/usr/bin/env rake
+# frozen_string_literal: true
+
 require 'bundler/gem_tasks'
 
 Dir.glob('tasks/*.rake').each { |r| import r }

app/controllers/browse_everything_controller.rb

@@ -1,4 +1,6 @@
-require File.expand_path('../../helpers/browse_everything_helper', __FILE__)
+# frozen_string_literal: true
+
+require File.expand_path('../helpers/browse_everything_helper', __dir__)
 
 class BrowseEverythingController < ActionController::Base
   layout 'browse_everything'
@@ -10,17 +12,20 @@ class BrowseEverythingController < ActionController::Base
     provider_session.token = provider.token unless provider.nil? || provider.token.blank?
   end
 
+  def provider_contents
+    raise BrowseEverythingHelper::NotImplementedError, 'No provider supported' if provider.nil?
+    raise BrowseEverythingHelper::NotAuthorizedError, 'Not authorized' unless provider.authorized?
+
+    @provider_contents ||= provider.contents(browse_path)
+  end
+
   def index
     render layout: !request.xhr?
   end
 
   # Either render the link to authorization or render the files
   # provider#show method is invoked here
   def show
-    raise BrowseEverythingHelper::NotImplementedError, 'No provider supported' if provider.nil?
-    raise BrowseEverythingHelper::NotAuthorizedError, 'Not authorized' unless provider.authorized?
-
-    @provider_contents = provider.contents(browse_path)
     render partial: 'files', layout: !request.xhr?
   rescue StandardError => error
     # Should an error be raised, log the error and redirect the use to reauthenticate
@@ -37,16 +42,18 @@ def auth
 
   def resolve
     selected_files = params[:selected_files] || []
-    @links = selected_files.collect do |file|
-      provider_key, uri = file.split(/:/)
+    selected_links = selected_files.collect do |file|
+      provider_key_value, uri = file.split(/:/)
+      provider_key = provider_key_value.to_sym
       (url, extra) = browser.providers[provider_key].link_for(uri)
       result = { url: url }
       result.merge!(extra) unless extra.nil?
       result
     end
+
     respond_to do |format|
       format.html { render layout: false }
-      format.json { render json: @links }
+      format.json { render json: selected_links }
     end
   end
 
@@ -56,17 +63,19 @@ def provider_session
       @provider_session ||= BrowseEverythingSession::ProviderSession.new(session: session, name: provider_name)
     end
 
+    # Generate the markup for the authentication link
+    # @return [String, nil] the markup for the link (if it exists)
     def auth_link
       @auth_link ||= if provider.present?
                        link, data = provider.auth_link
                        provider_session.data = data
                        link = "#{link}&state=#{provider.key}" unless link.to_s.include?('state')
                        link
-                     end # else nil, implicitly
+                     end
     end
 
     def browse_path
-      @path ||= params[:path] || ''
+      @browse_path ||= params[:path] || ''
     end
 
     # Browser state cannot persist between requests to the Controller
@@ -80,18 +89,19 @@ def provider_name_from_state
     end
 
     def provider_name
-      @provider_name ||= params[:provider] || provider_name_from_state
+      @provider_name ||= params[:provider] || provider_name_from_state || browser.providers.each_key.to_a.first
     end
 
     # Retrieve the Driver for each request
     # @return [BrowseEverything::Driver::Base]
     def provider
-      browser.providers[provider_name]
+      browser.providers[provider_name] || browser.first_provider
     end
 
     helper_method :auth_link
     helper_method :browser
     helper_method :browse_path
     helper_method :provider
     helper_method :provider_name
+    helper_method :provider_contents
 end

app/helpers/browse_everything_helper.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module BrowseEverythingHelper
   def array_to_hidden_fields(array, key)
     fields = array.to_query(key).split(Rack::Utils::DEFAULT_SEP).collect do |pair|

app/helpers/font_awesome_version_helper.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module FontAwesomeVersionHelper
   def font_awesome_four?
     Bundler.environment.specs['font-awesome-rails'].first.version >= Gem::Version.new('4')

app/services/browse_everything_session.rb

@@ -1,4 +1,6 @@
 
+# frozen_string_literal: true
+
 class BrowseEverythingSession
   class Base
     def initialize(session:)

app/services/browse_everything_session/provider_session.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 # Object for handling session cookies containing cached values
 class BrowseEverythingSession
   class ProviderSession < Base

app/services/browser_factory.rb

@@ -1,11 +1,14 @@
 
+# frozen_string_literal: true
+
 class BrowserFactory
   class << self
     def build(session:, url_options:)
       browser = BrowseEverything::Browser.new(url_options)
-      browser.providers.values.each do |provider_handler|
+      browser.providers.each_value do |provider_handler|
         # The authentication token must be set here
-        provider_handler.token = BrowseEverythingSession::ProviderSession.for(session: session, name: provider_handler.key.to_sym).token
+        provider_session = BrowseEverythingSession::ProviderSession.for(session: session, name: provider_handler.key.to_sym)
+        provider_handler.token = provider_session.token if provider_session.token
       end
       browser
     end

app/views/browse_everything/_files.html.erb

@@ -8,7 +8,7 @@
         <th role="columnheader">Modified</th>
       </tr>
     </thead>
-    <% @provider_contents.each_with_index do |file, index| %>
+    <% provider_contents.each_with_index do |file, index| %>
       <% path = browse_everything_engine.contents_path(provider_name, file.id) %>
       <% parent = params[:parent] %>
       <% if file.container? || provider.config[:max_upload_file_size].blank? %>