Skip to content
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

View helper methods #5493

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions devise_road_map.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
view-helpers-method-test branch, notes in views test files
8 changes: 7 additions & 1 deletion lib/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ module Controllers
autoload :UrlHelpers, 'devise/controllers/url_helpers'
end

module Views
autoload :Helpers, 'devise/views/helpers'
end

module Hooks
autoload :Proxy, 'devise/hooks/proxy'
end
Expand Down Expand Up @@ -274,7 +278,7 @@ module Test
# Define a set of modules that are called when a mapping is added.
mattr_reader :helpers
@@helpers = Set.new
@@helpers << Devise::Controllers::Helpers
@@helpers.merge [Devise::Controllers::Helpers, Devise::Views::Helpers]

# Private methods to interface with Warden.
mattr_accessor :warden_config
Expand Down Expand Up @@ -455,11 +459,13 @@ def self.omniauth(provider, *args)
# Include helpers in the given scope to AC and AV.
def self.include_helpers(scope)
ActiveSupport.on_load(:action_controller) do
include Devise::Views::Helpers
include scope::Helpers if defined?(scope::Helpers)
include scope::UrlHelpers
end

ActiveSupport.on_load(:action_view) do
include Devise::Views::Helpers
include scope::UrlHelpers
end
end
Expand Down
1 change: 0 additions & 1 deletion lib/devise/controllers/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ module ClassMethods
#
def devise_group(group_name, opts = {})
mappings = "[#{ opts[:contains].map { |m| ":#{m}" }.join(',') }]"

class_eval <<-METHODS, __FILE__, __LINE__ + 1
def authenticate_#{group_name}!(favorite = nil, opts = {})
unless #{group_name}_signed_in?
Expand Down
39 changes: 39 additions & 0 deletions lib/devise/views/helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
module Devise
module Views
module Helpers

# Generates helpers that do something based on mappings authentication status,
# thereby avoiding the use of repetitive if/else blocks
#
# Generated Methods => signed_in_user & signed_out_user
#
# signed_in_user do
# <h1>Hello <%= @user.name %> </h1>
# end
#
# signed_out_user do
# <%= link_to "Sign In", new_user_session_path %>
# end
#
# These methods can also be used at the controller level:
#
# def some_action
# signed_in_user {render @users}
# signed_out_user {redirect_to root_path}
# end

def self.define_helpers(mapping)
mapping = mapping.name
class_eval <<-METHODS, __FILE__, __LINE__ + 1
def signed_in_#{mapping}(&block)
block.call if #{mapping}_signed_in?
end

def signed_out_#{mapping}(&block)
block.call unless #{mapping}_signed_in?
end
METHODS
end
end
end
end
10 changes: 10 additions & 0 deletions test/controllers/helper_methods_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

class ApiController < ActionController::Metal
include Devise::Controllers::Helpers
include Devise::Views::Helpers
end

class HelperMethodsTest < Devise::ControllerTestCase
Expand All @@ -13,6 +14,10 @@ class HelperMethodsTest < Devise::ControllerTestCase
assert_includes @controller.class.ancestors, Devise::Controllers::Helpers
end

test 'includes Devise::Views::Helpers' do
assert_includes @controller.class.ancestors, Devise::Views::Helpers
end

test 'does not respond_to helper or helper_method' do
refute_respond_to @controller.class, :helper
refute_respond_to @controller.class, :helper_method
Expand All @@ -21,4 +26,9 @@ class HelperMethodsTest < Devise::ControllerTestCase
test 'defines methods like current_user' do
assert_respond_to @controller, :current_user
end

test 'defines methods like signed_in_user' do
assert_respond_to @controller, :signed_in_user
end

end
22 changes: 22 additions & 0 deletions test/views/helper_methods_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true
require 'test_helper'

module ActionView
include Devise::Views::Helpers
end

class ViewsHelperMethodsTest < Devise::IntegrationTest

test 'Action View includes Devise::Views::Helpers' do
assert_includes ActionView.ancestors, Devise::Views::Helpers

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert_includes ActionView.ancestors, Devise::Views::Helpers
assert_includes ActionView.ancestors, Devise::Views::Helpers

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get this pulled?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review. Could we get this pull request submitted?

end

test 'ActionView defines signed_in_user' do
assert ActionView.instance_methods.include?(:signed_in_user)
end

test 'ActionView defines signed_out_user' do
assert ActionView.instance_methods.include?(:signed_out_user)
end

end