Clarify how intercom-rails resolves the current user

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 }
 ```

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

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

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

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 '</body>'
 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} "

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 %> }

spec/auto_include_filter_spec.rb

@@ -59,6 +59,17 @@ def with_current_user_method
     render_content("<body>Hello world</body>")
   end
 
+  def with_user_instance_variable_but_no_current_user
+    @user = dummy_user
+    render_content("<body>Hello world</body>")
+  end
+
+  def with_unsaved_user_instance_variable
+    @user = dummy_user
+    def @user.persisted?; false; end
+    render_content("<body>Hello world</body>")
+  end
+
   def with_admin_instance_variable
     @admin = dummy_user(:email => 'eoghan@intercom.io', :name => 'Eoghan McCabe')
     render_content("<body>Hello world</body>")
@@ -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("<body>Hello world</body>")
+  end
+
+  it 'has no intercom script if current_user is not persisted' do
+    get :with_unsaved_user_instance_variable
+    expect(response.body).to eq("<body>Hello world</body>")
+  end
+
   it 'finds user using config.user.current proc' do
     IntercomRails.config.user.current = Proc.new { @admin }
     get :with_admin_instance_variable

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

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