New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Done #63 Include a module instead of inherit from a class #67
Changes from 5 commits
8f548af
aeb4704
334a2c1
d879fd0
705ab7f
632ff78
c791ade
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -176,23 +176,26 @@ get "/pages/*id" => 'pages#show', :as => :page, :format => false | |
root :to => 'pages#show', :id => 'home' | ||
``` | ||
|
||
Then modify it to subclass from High Voltage, adding whatever you need: | ||
Then modify it to include the High Voltage static page concern: | ||
|
||
```ruby | ||
# app/controllers/pages_controller.rb | ||
class PagesController < HighVoltage::PagesController | ||
class PagesController < ApplicationController | ||
include HighVoltage::Pageable | ||
|
||
before_filter :authenticate | ||
layout :layout_for_page | ||
|
||
protected | ||
def layout_for_page | ||
case params[:id] | ||
when 'home' | ||
'home' | ||
else | ||
'application' | ||
end | ||
end | ||
protected | ||
|
||
def layout_for_page | ||
case params[:id] | ||
when 'home' | ||
'home' | ||
else | ||
'application' | ||
end | ||
end | ||
end | ||
``` | ||
|
||
|
@@ -203,8 +206,10 @@ the `page_finder_factory` method: | |
|
||
```ruby | ||
# app/controllers/pages_controller.rb | ||
class PagesController < HighVoltage::PagesController | ||
private | ||
class PagesController < ApplicationController | ||
include HighVoltage::Pageable | ||
|
||
private | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be consistent with the rest of the project if |
||
|
||
def page_finder_factory | ||
Rot13PageFinder | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
module HighVoltage::Pageable | ||
extend ActiveSupport::Concern | ||
|
||
included do | ||
layout ->(_) { HighVoltage.layout } | ||
|
||
rescue_from ActionView::MissingTemplate do |exception| | ||
if exception.message =~ %r{Missing template #{page_finder.content_path}} | ||
raise ActionController::RoutingError, "No such page: #{params[:id]}" | ||
else | ||
raise exception | ||
end | ||
end | ||
|
||
if respond_to?(:caches_action) | ||
caches_action :show, if: -> { HighVoltage.action_caching } | ||
end | ||
|
||
if respond_to?(:caches_page) | ||
caches_page :show, if: -> { HighVoltage.page_caching } | ||
end | ||
|
||
hide_action :current_page, :page_finder, :page_finder_factory | ||
end | ||
|
||
def show | ||
render template: current_page | ||
end | ||
|
||
def current_page | ||
page_finder.find | ||
end | ||
|
||
def page_finder | ||
page_finder_factory.new(params[:id]) | ||
end | ||
|
||
def page_finder_factory | ||
HighVoltage::PageFinder | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,41 +1,3 @@ | ||
class HighVoltage::PagesController < ApplicationController | ||
layout Proc.new { |_| HighVoltage.layout } | ||
|
||
if respond_to?('caches_action') | ||
caches_action :show, if: Proc.new { | ||
HighVoltage.action_caching | ||
} | ||
end | ||
|
||
if respond_to?('caches_page') | ||
caches_page :show, if: Proc.new { | ||
HighVoltage.page_caching | ||
} | ||
end | ||
|
||
rescue_from ActionView::MissingTemplate do |exception| | ||
if exception.message =~ %r{Missing template #{page_finder.content_path}} | ||
raise ActionController::RoutingError, "No such page: #{params[:id]}" | ||
else | ||
raise exception | ||
end | ||
end | ||
|
||
def show | ||
render :template => current_page | ||
end | ||
|
||
private | ||
|
||
def current_page | ||
page_finder.find | ||
end | ||
|
||
def page_finder | ||
page_finder_factory.new(params[:id]) | ||
end | ||
|
||
def page_finder_factory | ||
HighVoltage::PageFinder | ||
end | ||
include HighVoltage::Pageable | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,9 @@ | ||
module HighVoltage | ||
class Engine < Rails::Engine | ||
initializer "Require concern path" do |app| | ||
concern_path = "app/controllers/concerns" | ||
app.paths.add(concern_path) unless app.paths.keys.include?(concern_path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe require only my concern, without add to load_path? |
||
require "concerns/high_voltage/pageable.rb" | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,25 +2,22 @@ | |
|
||
describe HighVoltage::PagesController, 'action_caching' do | ||
it 'caches the action when action_caching is set to true', enable_caching: true do | ||
HighVoltage.action_caching = true | ||
|
||
load('high_voltage/pages_controller.rb') | ||
get :show, id: 'exists' | ||
|
||
action_was_cached(:exists).should be_true | ||
expect { | ||
HighVoltage.action_caching = true | ||
concern_reload | ||
get :show, id: :exists | ||
}.to change { action_was_cached(:exists) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is misaligned. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should use
|
||
end | ||
|
||
it 'does not cache the action when action_caching is set to false', enable_caching: true do | ||
HighVoltage.action_caching = false | ||
|
||
load('high_voltage/pages_controller.rb') | ||
get :show, id: 'exists' | ||
|
||
action_was_cached(:exists).should be_false | ||
expect { | ||
HighVoltage.action_caching = false | ||
concern_reload | ||
get :show, id: :exists | ||
}.to_not change { action_was_cached(:exists) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should use
|
||
end | ||
|
||
def action_was_cached(page) | ||
ActionController::Base.cache_store.exist?("views/test.host#{page_path(page)}") | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,3 +17,15 @@ | |
config.include RSpec::Matchers | ||
config.mock_with :rspec | ||
end | ||
|
||
def concern_reload | ||
HighVoltage::PagesController.class_eval do | ||
if respond_to?(:caches_action) | ||
caches_action :show, if: -> { HighVoltage.action_caching } | ||
end | ||
|
||
if respond_to?(:caches_page) | ||
caches_page :show, if: -> { HighVoltage.page_caching } | ||
end | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how correctly reload concern. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be consistent with the rest of the project if
protected
was indented by one level.