diff --git a/README.md b/README.md index f794e39..508fc3f 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ If things are not working make sure that: * You've generated a config file with your `app_id` as detailed above. * Your user object responds to an `id` or `email` method. -* Your current user is accessible in your controllers as `current_user` or `@user`, if not in `config/initializers/intercom.rb`: +* Your current user is accessible in your controllers as `current_user`. If it is not, set it in `config/initializers/intercom.rb`: ```ruby config.user.current = Proc.new { current_user_object } ``` diff --git a/lib/intercom-rails/proxy.rb b/lib/intercom-rails/proxy.rb index b096951..f9564eb 100644 --- a/lib/intercom-rails/proxy.rb +++ b/lib/intercom-rails/proxy.rb @@ -71,6 +71,13 @@ def config_variable_set?(variable_name) config.send(variable_name).present? end + def unsaved? + return proxied_object.new_record? if proxied_object.respond_to?(:new_record?) + return !proxied_object.persisted? if proxied_object.respond_to?(:persisted?) + return proxied_object.new? if proxied_object.respond_to?(:new?) + false + end + def identity_present? self.class.identity_attributes.any? { |attribute_name| (proxied_object.is_a?(Hash) && proxied_object[attribute_name].present?) || (proxied_object.respond_to?(attribute_name) && proxied_object.send(attribute_name).present?) } end diff --git a/lib/intercom-rails/proxy/company.rb b/lib/intercom-rails/proxy/company.rb index b9be8b0..f8c0f02 100644 --- a/lib/intercom-rails/proxy/company.rb +++ b/lib/intercom-rails/proxy/company.rb @@ -24,7 +24,7 @@ def self.current_in_context(search_object) end def valid? - return false if company.blank? || company.respond_to?(:new_record?) && company.new_record? + return false if company.blank? || unsaved? return false if config.company.exclude_if.present? && config.company.exclude_if.call(company) company.present? && identity_present? end diff --git a/lib/intercom-rails/proxy/user.rb b/lib/intercom-rails/proxy/user.rb index eaba494..4d8be11 100644 --- a/lib/intercom-rails/proxy/user.rb +++ b/lib/intercom-rails/proxy/user.rb @@ -9,10 +9,7 @@ class User < Proxy proxy_delegator :name proxy_delegator :created_at - PREDEFINED_POTENTIAL_USER_OBJECTS = [ - Proc.new { current_user }, - Proc.new { @user } - ] + PREDEFINED_POTENTIAL_USER_OBJECTS = [Proc.new { current_user }] def self.potential_user_objects if config.current.present? @@ -46,7 +43,7 @@ def standard_data end def valid? - return false if user.blank? || user.respond_to?(:new_record?) && user.new_record? + return false if user.blank? || unsaved? return false if excluded? identity_present? end diff --git a/lib/rails/generators/intercom/config/config_generator.rb b/lib/rails/generators/intercom/config/config_generator.rb index 5fb6eb3..c1ff1ab 100644 --- a/lib/rails/generators/intercom/config/config_generator.rb +++ b/lib/rails/generators/intercom/config/config_generator.rb @@ -20,9 +20,9 @@ def create_config_file introduction = <<-desc Intercom will automatically insert its javascript before the closing '' tag on every page where it can find a logged-in user. Intercom by default -looks for logged-in users, in the controller, via 'current_user' and '@user'. +looks for logged-in users, in the controller, via 'current_user'. -Is the logged-in user accessible via either 'current_user' or '@user'? [Yn] +Is the logged-in user accessible via 'current_user'? [Yn] desc print "#{introduction.strip} " diff --git a/lib/rails/generators/intercom/config/intercom.rb.erb b/lib/rails/generators/intercom/config/intercom.rb.erb index cb9a3ff..6a9abef 100644 --- a/lib/rails/generators/intercom/config/intercom.rb.erb +++ b/lib/rails/generators/intercom/config/intercom.rb.erb @@ -27,7 +27,7 @@ IntercomRails.config do |config| # == Current user method/variable # The method/variable that contains the logged in user in your controllers. - # If it is `current_user` or `@user`, then you can ignore this + # If it is `current_user`, then you can ignore this # <%- if @current_user -%> config.user.current = Proc.new { <%= @current_user %> } diff --git a/spec/auto_include_filter_spec.rb b/spec/auto_include_filter_spec.rb index aec9a0e..52f2ae8 100644 --- a/spec/auto_include_filter_spec.rb +++ b/spec/auto_include_filter_spec.rb @@ -59,6 +59,17 @@ def with_current_user_method render_content("Hello world") end + def with_user_instance_variable_but_no_current_user + @user = dummy_user + render_content("Hello world") + end + + def with_unsaved_user_instance_variable + @user = dummy_user + def @user.persisted?; false; end + render_content("Hello world") + end + def with_admin_instance_variable @admin = dummy_user(:email => 'eoghan@intercom.io', :name => 'Eoghan McCabe') render_content("Hello world") @@ -80,8 +91,9 @@ def render_content(body) end def current_user - raise NameError if params[:action] != 'with_current_user_method' - dummy_user(:email => 'ciaran@intercom.io', :name => 'Ciaran Lee') + raise NameError if params[:action] == 'with_user_instance_variable_but_no_current_user' + return dummy_user(:email => 'ciaran@intercom.io', :name => 'Ciaran Lee') if params[:action] == 'with_current_user_method' + @user end end @@ -122,6 +134,16 @@ def current_user expect(response.body).to include("Ciaran Lee") end + it 'has no intercom script if @user is set but current_user is not defined' do + get :with_user_instance_variable_but_no_current_user + expect(response.body).to eq("Hello world") + end + + it 'has no intercom script if current_user is not persisted' do + get :with_unsaved_user_instance_variable + expect(response.body).to eq("Hello world") + end + it 'finds user using config.user.current proc' do IntercomRails.config.user.current = Proc.new { @admin } get :with_admin_instance_variable diff --git a/spec/proxy/company_spec.rb b/spec/proxy/company_spec.rb index 55784da..2d7a959 100644 --- a/spec/proxy/company_spec.rb +++ b/spec/proxy/company_spec.rb @@ -25,4 +25,10 @@ def id search_object = nil expect(ProxyCompany.new(search_object).valid?).to eq(false) end + + it 'is invalid when the company reports persisted? == false' do + company = dummy_company + def company.persisted?; false; end + expect(ProxyCompany.new(company).valid?).to eq(false) + end end diff --git a/spec/proxy/user_spec.rb b/spec/proxy/user_spec.rb index b553dc5..3de7243 100644 --- a/spec/proxy/user_spec.rb +++ b/spec/proxy/user_spec.rb @@ -21,14 +21,30 @@ def current_user expect(@user_proxy.user).to eq(DUMMY_USER) end - it 'finds user instance variable' do + it 'ignores a @user instance variable' do object_with_instance_variable = Object.new - object_with_instance_variable.instance_eval do - @user = DUMMY_USER - end + object_with_instance_variable.instance_variable_set(:@user, DUMMY_USER) - @user_proxy = ProxyUser.current_in_context(object_with_instance_variable) - expect(@user_proxy.user).to eq(DUMMY_USER) + expect { ProxyUser.current_in_context(object_with_instance_variable) }.to raise_error(IntercomRails::NoUserFoundError) + end + + { + 'current_user' => ->(search_object, user) { search_object.define_singleton_method(:current_user) { user } }, + 'config.user.current' => ->(search_object, user) { + search_object.instance_variable_set(:@user, user) + IntercomRails.config.user.current = Proc.new { @user } + }, + }.each do |source, assign| + it "does not use a user reporting persisted? false from #{source}" do + unsaved_user = dummy_user(:email => 'not-saved@intercom.io') + def unsaved_user.persisted? + false + end + search_object = Object.new + assign.call(search_object, unsaved_user) + + expect { ProxyUser.current_in_context(search_object) }.to raise_error(IntercomRails::NoUserFoundError) + end end it 'finds config user' do @@ -131,14 +147,14 @@ def some_date expect(ProxyUser.new(user).valid?).to be(true) end - it 'considers new records to be invalid' do - new_record_user = dummy_user(:email => 'not-saved@intercom.io', :name => 'New Record') - - def new_record_user.new_record? - true + { 'new_record?' => true, 'persisted?' => false, 'new?' => true }.each do |predicate, unsaved_value| + it "is invalid when the user reports #{predicate} == #{unsaved_value}" do + user = dummy_user(:email => 'not-saved@intercom.io') + user.define_singleton_method(predicate) { unsaved_value } + expect(ProxyUser.new(user).valid?).to be(false) + user.define_singleton_method(predicate) { !unsaved_value } + expect(ProxyUser.new(user).valid?).to be(true) end - - expect(ProxyUser.new(new_record_user).valid?).to be(false) end it 'includes custom data from intercom custom data' do