Feature test website admin & fix discovered bugs, cleanup

coyote-team#111 coyote-team#109

.rubocop.yml

@@ -26,3 +26,6 @@ Metrics/MethodLength:
 
 Layout/DotPosition:
   Enabled: false
+
+Style/BlockDelimiters:
+  EnforcedStyle: braces_for_chaining

app/controllers/application_controller.rb

@@ -28,18 +28,20 @@ def clear_search_index
   end
 
   protected
+
   def admin
-    redirect_to(root_url) unless current_user and current_user.admin?
+    redirect_to(root_url) unless current_user.try(:admin?)
   end
 
   def users
-    redirect_to(root_url) if current_user.nil?
+    redirect_to(root_url) unless current_user
   end
 
   def get_users
     @users = User.sorted
   end
+
   def get_groups
-    @groups = Group.all.sort{|a,b| a.to_s<=>b.to_s} 
+    @groups = Group.all.sort { |a,b| a.to_s <=> b.to_s } 
   end
 end

app/controllers/websites_controller.rb

@@ -1,8 +1,7 @@
 class WebsitesController < ApplicationController
-  before_action :admin, only: [:edit, :update, :destroy]
-  before_action :users, only: [:create]
-  before_action :set_website, only: [:show, :edit, :update, :destroy, :check_count]
-  before_action :set_strategies_collection, only: [:new, :edit]
+  before_action :admin, only: %i[new edit update destroy]
+  before_action :set_website, only: %i[show edit update destroy check_count]
+  before_action :set_strategies_collection, only: %i[new edit]
 
   caches_action :check_count, :cache_path => { :cache_path => Proc.new { |c| c.params } }, :expires_in => 5.minutes
 
@@ -86,17 +85,20 @@ def destroy
   end
 
   private
-    # Use callbacks to share common setup or constraints between actions.
-    def set_website
-      @website = Website.find(params[:id])
-    end
+  # Use callbacks to share common setup or constraints between actions.
+  def set_website
+    @website = Website.find(params[:id])
+  end
 
-    def set_strategies_collection
-      @strategies_collection = Strategy.subclasses.map{|s| s = s.new; [s.title, s.class.name]}
+  def set_strategies_collection
+    @strategies_collection = Coyote::Strategies.all.map do |s| 
+      s = s.new 
+      [s.title, s.class.name]
     end
+  end
 
-    # Only allow a trusted parameter "white list" through.
-    def website_params
-      params.require(:website).permit(:title, :url, :strategy)
-    end
+  # Only allow a trusted parameter "white list" through.
+  def website_params
+    params.require(:website).permit(:title,:url,:strategy)
+  end
 end

lib/coyote/strategies.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Coyote
+  # Namespace for site/CMS-specific Coyote code
+  # @see Coyote::Strategies::MCA
+  module Strategies
+    module_function
+
+    # @return [Array<Class>] a list of all strategy classes
+    def all
+      constants.map do |constant_name| 
+        const_get(constant_name)
+      end
+    end
+  end
+end
+
+# Eager-load all strategies so WebsitesController can see them
+Pathname.glob("./lib/coyote/strategies/*.rb").each do |path|
+  require_dependency path.sub_ext("") 
+end

lib/coyote/strategies/mca.rb

@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 module Coyote
-  module Strategy
+  module Strategies
     # Responsible for linking Coyote to the MCA CMS
     # @see https://mcachicago.org/
-    class MCAStrategy
+    class MCA
       def title
         "MCA"
       end

spec/features/adding_and_changing_websites_spec.rb

@@ -0,0 +1,48 @@
+RSpec.describe "Adding and changing a Website" do
+  context "when logged-in as an admin" do
+    include_context "as a logged-in admin user"
+
+    let(:website_attributes) do
+      attributes_for(:website).tap(&:symbolize_keys!)
+    end
+
+    it "succeeds" do
+      click_link "Websites"
+      click_link "New Website"
+      expect(page.current_path).to eq(new_website_path)
+
+      fill_in "Title", with: "XYZ Museum"
+      fill_in "Url",   with: "http://example.com"
+
+      select("MCA",from: "Strategy")
+
+      expect {
+        click_button "Create Website"
+      }.to change(Website,:count).from(0).to(1)
+
+      website = Website.first
+      expect(page.current_path).to eq(website_path(website))
+
+      click_link "Edit"
+      fill_in "Title", with: "ABC Museum"
+
+      expect {
+        click_button "Update Website"
+        website.reload
+      }.to change(website,:title).to("ABC Museum")
+    end
+  end
+
+  context "when logged-in as a user" do
+    include_context "as a logged-in user"
+
+    it "is not allowed" do
+      expect(page).not_to have_link("Websites")
+      visit new_website_path
+      expect(page.current_path).to eq(root_path)
+
+      visit edit_website_path(create(:website))
+      expect(page.current_path).to eq(root_path)
+    end
+  end
+end

spec/lib/strategies_spec.rb

@@ -0,0 +1,7 @@
+require "coyote/strategies"
+
+RSpec.describe Coyote::Strategies do
+  it ".all returns a list of eager-loaded strategy classes" do
+    expect(Coyote::Strategies.all).to match_array([Coyote::Strategies::MCA])
+  end
+end

spec/support/feature_user_login.rb

@@ -12,11 +12,7 @@ def login(user = create(:user),password = "")
         fill_in "Password", with: password
       end
 
-      # When admins login, the system calls out to Travis; we record that interaction to make our tests
-      # deterministic
-      VCR.use_cassette "Travis CI build notification check" do
-        click_button "Log in"
-      end
+      click_button "Log in"
     end
 
     # Completes a user's session