Merge pull request #13 from avonderluft/test_coverage

Improving test coverage

CHANGELOG.md

@@ -1,8 +1,11 @@
 # Changelog
 
-## v1.0.8 - TBD
+## v1.0.7.2 - 9/27/2023
 
-- Switched CI from Buildkite to Github actions
+- Pulled will_paginate option out in favor of Kaminari
+- Switched CI from Buildkite to [Github actions](https://github.com/avonderluft/occams/actions/workflows/rubyonrails.yml)
+- Improved test coverage to 99+
+- Removed backward compatibility to Rails prior to 6.x
 
 ## v1.0.7.1 - 9/22/2023
 

Gemfile

@@ -28,6 +28,7 @@ group :test do
   gem 'coveralls_reborn',         '~> 0.28.0', require: false
   gem 'diffy',                    '~> 3.4.2'
   gem 'equivalent-xml',           '~> 0.6.0'
+  gem 'minitest',                 '~> 5.20.0'
   gem 'minitest-reporters',       '~> 1.6.1'
   gem 'mocha',                    '~> 2.1.0', require: false
   gem 'rails-controller-testing', '~> 1.0.5'

README.md

@@ -44,7 +44,6 @@ ocCaM'S, pronounced "AH-kums" is a nod to [Occam's Razor](https://en.wikipedia.o
 
 * File attachments are handled by [ActiveStorage](https://github.com/rails/rails/tree/master/activestorage). Make sure that you can run appropriate migrations by running: `rails active_storage:install` and then `rake db:migrate`
 * Image resizing is done with [ImageMagick](http://www.imagemagick.org/script/download.php), so make sure it's installed
-* Pagination is handled by [kaminari](https://github.com/amatsuda/kaminari) or [will_paginate](https://github.com/mislav/will_paginate). Please add one of those to your Gemfile.
 
 ## Compatibility
 

app/controllers/concerns/occams/paginate.rb

@@ -1,14 +1,9 @@
 # frozen_string_literal: true
 
 module Occams::Paginate
-  # Wrapper to deal with WillPaginate vs Kaminari nonsense
   def occams_paginate(scope, per_page: 50)
-    if defined?(WillPaginate)
-      scope.paginate(page: params[:page], per_page: per_page)
-    elsif defined?(Kaminari)
-      scope.page(params[:page]).per(per_page)
-    else
-      scope
-    end
+    return unless defined?(Kaminari)
+
+    scope.page(params[:page]).per(per_page)
   end
 end

app/controllers/occams/cms/content_controller.rb

@@ -74,10 +74,10 @@ def load_cms_page
   # Getting page and setting content_cache and fragments data if we need to
   # serve translation data
   def find_cms_page_by_full_path(full_path)
-    @cms_page = if Rails.env == 'production'
-                  @cms_site.pages.published.find_by!(full_path: full_path)
-                else
+    @cms_page = if Rails.env == 'development'
                   @cms_site.pages.find_by!(full_path: full_path)
+                else
+                  @cms_site.pages.published.find_by!(full_path: full_path)
                 end
 
     @cms_page.translate!

app/helpers/occams/cms_helper.rb

@@ -62,15 +62,12 @@ def cms_site_detect
       Occams::Cms::Site.find_site(host, path)
     end
 
-    # Wrapper to deal with Kaminari vs WillPaginate
     def occams_paginate(collection)
       return unless collection
 
-      if defined?(WillPaginate)
-        will_paginate collection
-      elsif defined?(Kaminari)
-        paginate collection, theme: 'occams'
-      end
+      return unless defined?(Kaminari)
+
+      paginate collection, theme: 'occams'
     end
   end
 end

app/models/occams/cms/file.rb

@@ -22,13 +22,8 @@ class Occams::Cms::File < ActiveRecord::Base
   # -- Callbacks ---------------------------------------------------------------
   before_validation :assign_label, on: :create
   before_create :assign_position
-  # active_storage attachment behavior changed in rails 6 - see PR#892 for details
-  if Rails::VERSION::MAJOR >= 6
-    before_save :process_attachment
-  else
-    after_save :process_attachment
-  end
-
+  # active_storage attachment behavior changed in rails 6
+  before_save :process_attachment
   after_save :clear_page_content_cache
 
   # -- Validations -------------------------------------------------------------

app/models/occams/cms/fragment.rb

@@ -10,12 +10,8 @@ class Occams::Cms::Fragment < ActiveRecord::Base
   attr_reader :files
 
   # -- Callbacks ---------------------------------------------------------------
-  # active_storage attachment behavior changed in rails 6 - see PR#892 for details
-  if Rails::VERSION::MAJOR >= 6
-    before_save :remove_attachments, :add_attachments
-  else
-    after_save :remove_attachments, :add_attachments
-  end
+  # active_storage attachment behavior changed in rails 6
+  before_save :remove_attachments, :add_attachments
 
   # -- Relationships -----------------------------------------------------------
   belongs_to :record, polymorphic: true, touch: true

lib/occams/content/tags/children.rb

@@ -29,7 +29,7 @@ def initialize(context:, params: [], source: nil)
     @list = ''
     # ActiveRecord_Associations_CollectionProxy
     @page_children = context.children.order(:position).to_ary
-    if Rails.env == 'production'
+    unless Rails.env == 'development'
       @page_children.delete_if { |child| !child.is_published }
     end
     @page_children.delete_if { |child| @exclude.include? child.slug }

lib/occams/seeds/file/exporter.rb

@@ -23,14 +23,10 @@ def export!
         # writing content
         begin
           ::File.binwrite(::File.join(path, ::File.basename(file_path)), file.attachment.download)
+          Occams.logger.info("[CMS SEEDS] Exported File \t #{file.attachment.filename}")
         rescue Errno::ENOENT, OpenURI::HTTPError
-          message = "[CMS SEEDS] No physical File \t #{file.attachment.filename}"
-          Occams.logger.warn(message)
-          next
+          Occams.logger.warn("[CMS SEEDS] No physical File \t #{file.attachment.filename}")
         end
-
-        message = "[CMS SEEDS] Exported File \t #{file.attachment.filename}"
-        Occams.logger.info(message)
       end
     end
   end

lib/occams/seeds/file/importer.rb

@@ -41,11 +41,9 @@ def import!
 
         if fresh_file
           if file.save
-            message = "[CMS SEEDS] Imported File \t #{file_path}"
-            Occams.logger.info(message)
+            Occams.logger.info("[CMS SEEDS] Imported File \t #{file_path}")
           else
-            message = "[CMS SEEDS] Failed to import File \n#{file.errors.inspect}"
-            Occams.logger.warn(message)
+            Occams.logger.warn("[CMS SEEDS] Failed to import File \n#{file.errors.inspect}")
           end
         end
 

lib/occams/seeds/layout/importer.rb

@@ -41,11 +41,9 @@ def import_layout(path, parent)
         )
 
         if layout.save
-          message = "[CMS SEEDS] Imported Layout \t #{layout.identifier}"
-          Occams.logger.info(message)
+          Occams.logger.info("[CMS SEEDS] Imported Layout \t #{layout.identifier}")
         else
-          message = "[CMS SEEDS] Failed to import Layout \n#{layout.errors.inspect}"
-          Occams.logger.warn(message)
+          Occams.logger.warn("[CMS SEEDS] Failed to import Layout \n#{layout.errors.inspect}")
         end
       end
 

lib/occams/seeds/page/importer.rb

@@ -79,8 +79,7 @@ def import_page(path, parent)
           page.fragments.where(identifier: old_frag_identifiers - new_frag_identifiers).destroy_all
 
         else
-          message = "[CMS SEEDS] Failed to import Page \n#{page.errors.inspect}"
-          Occams.logger.warn(message)
+          Occams.logger.warn("[CMS SEEDS] Failed to import Page \n#{page.errors.inspect}")
         end
       end
 

lib/occams/seeds/snippet/importer.rb

@@ -29,11 +29,9 @@ def import!
           )
 
           if snippet.save
-            message = "[CMS SEEDS] Imported Snippet \t #{snippet.identifier}"
-            Occams.logger.info(message)
+            Occams.logger.info("[CMS SEEDS] Imported Snippet \t #{snippet.identifier}")
           else
-            message = "[CMS SEEDS] Failed to import Snippet \n#{snippet.errors.inspect}"
-            Occams.logger.warn(message)
+            Occams.logger.warn("[CMS SEEDS] Failed to import Snippet \n#{snippet.errors.inspect}")
           end
         end
 

lib/occams/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Occams
-  VERSION = '1.0.7.1'
+  VERSION = '1.0.7.2'
 end

test/controllers/occams/admin/cms/files_controller_test.rb

@@ -227,6 +227,16 @@ def test_update
     assert_equal 'image.jpg', @file.attachment.filename.to_s
   end
 
+  def test_update_failure
+    r :put, occams_admin_cms_site_file_path(site_id: @site, id: @file), params: { file: {
+      label: ''
+    } }
+    assert_response :success
+    assert_template :edit
+    assert assigns(:file)
+    assert_equal 'Failed to update file', flash[:danger]
+  end
+
   def test_destroy
     file_count        = -> { Occams::Cms::File.count }
     attachment_count  = -> { ActiveStorage::Attachment.count }

test/controllers/occams/admin/cms/pages_controller_test.rb

@@ -464,6 +464,12 @@ def test_get_toggle_branch
     assert_equal [], session[:cms_page_tree]
   end
 
+  def test_get_toggle_branch_no_record
+    r :get, toggle_branch_occams_admin_cms_site_page_path(site_id: @site, id: @page), xhr: true, params: { format: :js }
+    assert_response :success
+    assert_equal [@page.id.to_s], session[:cms_page_tree]
+  end
+
   def test_reorder
     page_one = occams_cms_pages(:child)
     page_two = @site.pages.create!(

test/controllers/occams/admin/cms/revisions/base_controller_test.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require_relative '../../../../../test_helper'
+
+class Occams::Admin::Cms::Revisions::BaseControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @rev_bc = Occams::Admin::Cms::Revisions::BaseController.new
+  end
+
+  def test_unimplemented_methods
+    %i[load_record record_path].each do |m|
+      assert_raises RuntimeError, 'not implemented' do
+        @rev_bc.send(m)
+      end
+    end
+  end
+end

test/controllers/occams/admin/cms/snippets_controller_test.rb

@@ -141,7 +141,7 @@ def test_reorder
       identifier: 'test'
     )
     assert_equal 0, snippet_one.position
-    assert_equal 1, snippet_two.position
+    assert_equal 2, snippet_two.position
 
     r :put, reorder_occams_admin_cms_site_snippets_path(site_id: @site), params: {
       order: [snippet_two.id, snippet_one.id]

test/controllers/occams/cms/content_controller_test.rb

@@ -281,6 +281,6 @@ def test_with_translation_with_snippet
     I18n.locale = translation.locale
 
     get occams_cms_render_page_path(cms_path: '')
-    assert_equal 'test snippet content test', response.body
+    assert_equal 'test ## snippet content test', response.body
   end
 end

test/fixtures/occams/cms/snippets.yml

@@ -2,5 +2,14 @@ default:
   site: default
   label: Default Snippet
   identifier: default
-  content: snippet content
+  content: "## snippet content"
+  markdown: false
   position: 0
+
+markdown:
+  site: default
+  label: Default Markdown Snippet
+  identifier: markdown
+  content: "## snippet content"
+  markdown: true
+  position: 1

test/helpers/cms_helper_test.rb

@@ -83,18 +83,18 @@ def test_cms_fragment_render_with_files
   end
 
   def test_cms_snippet_content
-    assert_equal 'snippet content', cms_snippet_content(:default)
-    assert_equal 'snippet content', cms_snippet_content(:default, @cms_site)
+    assert_equal '## snippet content', cms_snippet_content(:default)
+    assert_equal '## snippet content', cms_snippet_content(:default, @cms_site)
     assert_equal '', cms_snippet_content(:invalid)
   end
 
   def test_cms_snippet_content_with_site_detection
     @cms_site = nil
-    assert_equal 'snippet content', cms_snippet_content(:default)
+    assert_equal '## snippet content', cms_snippet_content(:default)
   end
 
   def test_cms_snippet_render
-    assert_equal 'snippet content', cms_snippet_render(:default)
+    assert_equal '## snippet content', cms_snippet_render(:default)
   end
 
   def test_cms_snippet_render_with_tags_and_context

test/lib/configuration_test.rb

@@ -33,8 +33,4 @@ def test_initialization_overrides
     Occams.configuration.cms_title = 'New Title'
     assert_equal 'New Title', Occams.configuration.cms_title
   end
-
-  def test_version
-    refute Occams::VERSION.blank?
-  end
 end

test/lib/content/tag_test.rb

@@ -51,4 +51,11 @@ def test_tag_nodes_with_nested_tag
     assert nodes[1].is_a?(ContentTagTest::TestTag)
     assert_equal ' content', nodes[2]
   end
+
+  def test_content_on_base_class
+    tag = Occams::Content::Tag.new(context: nil)
+    assert_raise Occams::Content::Tag::Error do
+      tag.content
+    end
+  end
 end

test/lib/content/tags/children_test.rb

@@ -12,6 +12,7 @@ class ContentTagsChildrenTest < ActiveSupport::TestCase
     @child2 = @site.pages.create!(layout: @layout, parent: @parent, label: 'Child2', slug: 'child2')
     @child3 = @site.pages.create!(layout: @layout, parent: @parent, label: 'Child3', slug: 'child3')
     @child4 = @site.pages.create!(layout: @layout, parent: @parent, label: 'Child4', slug: 'child4')
+    @child5 = @site.pages.create!(layout: @layout, parent: @parent, label: 'Child5', slug: 'child5', is_published: false)
   end
 
   def test_init
@@ -51,8 +52,10 @@ def test_render_with_exclusions
       context: @parent,
       params: [{ 'exclude' => 'child2,child3' }]
     )
-    html = "<ul id=\"children\">\n  <li><a href=/parent/child1>Child1</a></li>\n\
-  <li><a href=/parent/child4>Child4</a></li>\n</ul>"
+    html = "<ul id=\"children\">\n\
+  <li><a href=/parent/child1>Child1</a></li>\n\
+  <li><a href=/parent/child4>Child4</a></li>\n\
+</ul>"
     assert_equal html, tag.render
   end
 

test/lib/content/tags/fragment_test.rb

@@ -46,6 +46,9 @@ def test_fragment_new_record
   def test_content
     tag = Occams::Content::Tag::Fragment.new(context: @page, params: ['content'])
     assert_equal 'content', tag.content
+    assert_raises RuntimeError, 'Form field rendering not implemented for this Tag' do
+      tag.form_field
+    end
   end
 
   def test_content_new_record

test/lib/content/tags/snippet_test.rb

@@ -27,7 +27,12 @@ def test_snippet_new_record
 
   def test_content
     tag = Occams::Content::Tag::Snippet.new(context: @page, params: ['default'])
-    assert_equal 'snippet content', tag.content
+    assert_equal '## snippet content', tag.content
+  end
+
+  def test_markdown_content
+    tag = Occams::Content::Tag::Snippet.new(context: @page, params: ['markdown'])
+    assert_equal "<h2 id=\"snippet-content\">snippet content</h2>\n", tag.content
   end
 
   def test_content_new_record

test/lib/content/tags/template_test.rb

@@ -24,6 +24,7 @@ def test_content
       params: ['path/to/template']
     )
     assert_equal '<%= render template: "path/to/template" %>', tag.content
+    assert_equal true, tag.allow_erb?
   end
 
   def test_render

test/lib/revisions_test.rb

@@ -201,7 +201,7 @@ def test_restore_from_revision_with_wrong_revision_type
     assert_no_difference -> { @snippet.revisions.count } do
       @snippet.restore_from_revision(revision)
       @snippet.reload
-      assert_equal 'snippet content', @snippet.content
+      assert_equal '## snippet content', @snippet.content
     end
   end
 
@@ -217,7 +217,7 @@ def test_creation_with_limit
 
       @snippet.reload
       revision = @snippet.revisions.last
-      assert_equal ({ 'content' => 'snippet content' }), revision.data
+      assert_equal ({ 'content' => '## snippet content' }), revision.data
     end
   end
 end