Depend on forem_name and forem_email methods on the User model, r…

…ather than to_s.

`to_s` might be used by applications to display other values

Fixes #451
Fixes #526

README.md

@@ -52,21 +52,21 @@ rails g forem:install
 
 ## Set up helper methods in your user model
 
-Forem depends on a `to_s` method being available on your `User` model so that it can display the user's name in posts. Define this in your model like this:
+Forem uses a `forem_name` (which defaults as `to_s`) method being available on your `User` model so that it can display the user's name in posts. Define it in your model like this:
 
 ```ruby
-def to_s
+def forem_name
   name
 end
 ```
 
 Please note that if you are using Devise, User model does not have `name` column by default,
 so you either should use custom migration to add it or use another column (`email` for example).
 
-It also depends on an `email` method for displaying avatars using [Gravatar](http://gravatar.com). If you don't have an `email` attribute on the model, define a new method:
+It also uses an optional `forem_email` method for displaying avatars using [Gravatar](http://gravatar.com). It defaults to `email`. If you don't have an `email` attribute on the model, define a new method:
 
 ```ruby
-def email
+def forem_email
   email_address
 end
 ```

app/helpers/forem/posts_helper.rb

@@ -5,7 +5,7 @@ def forem_avatar(user, options = {})
         # Try to use the user's custom avatar method
         user.try Forem.avatar_user_method.to_sym
       else
-        avatar_url user.try(:email), options
+        avatar_url user.forem_email, options
       end
 
       image_tag image, :alt => "Avatar" if image.present?

app/helpers/forem/topics_helper.rb

@@ -2,7 +2,7 @@ module Forem
   module TopicsHelper
     def link_to_latest_post(topic)
       post = relevant_posts(topic).last
-      text = "#{time_ago_in_words(post.created_at)} #{t("ago_by")} #{post.user}"
+      text = "#{time_ago_in_words(post.created_at)} #{t("ago_by")} #{post.user.forem_name}"
       link_to text, forem.forum_topic_path(post.topic.forum, post.topic, :anchor => "post-#{post.id}", pagination_param => topic.last_page)
     end
 

app/models/forem/nil_user.rb

@@ -1,11 +1,11 @@
 module Forem
   class NilUser
-    def email
+    def forem_email
       "[email protected]"
     end
 
-    def to_s
+    def forem_name
       "[deleted]"
     end
   end
-end
+end

app/views/forem/admin/groups/show.html.erb

@@ -17,7 +17,7 @@
   <div class="row">
     <ul id='members'>
       <% @group.members.each do |member| %>
-        <li><%= member.to_s %> |
+        <li><%= member.forem_name %> |
           <%= link_to t('forem.admin.groups.show.remove_member'), admin_group_member_url(@group, member),
                       method: :delete, data: { confirm: t('are_you_sure') } %></li>
       <% end %>
@@ -45,4 +45,4 @@
       return result.identifier;
     }
   });
-</script>
+</script>

app/views/forem/forums/_forum.html.erb

@@ -6,7 +6,7 @@
       <%= t('forem.forums.index.last_post') -%>
       <span class='last_post'>
         <% if last_post = forum.last_post_for(forem_user) -%>
-          <%= link_to(forem_emojify(last_post.topic.subject), forem.forum_topic_path(forum, last_post.topic)) -%> <%= t('by') %> <%= last_post.user %> 
+          <%= link_to(forem_emojify(last_post.topic.subject), forem.forum_topic_path(forum, last_post.topic)) -%> <%= t('by') %> <%= last_post.user.forem_name %>
           <time datetime="<%= last_post.created_at.to_s(:db) -%>"><%= "#{time_ago_in_words(last_post.created_at)} #{t("ago")}" %></time>
         <% else %>
           <%= t('forem.forums.index.none') -%>

app/views/forem/forums/show.atom.builder

@@ -9,10 +9,10 @@ atom_feed :language => 'en-US' do |feed|
       entry.url forem.forum_topic_url(@forum, item)
       entry.title item.subject
       entry.content forem_format(item.posts.first.text), :type => 'html'
-      entry.updated(item.updated_at.strftime("%Y-%m-%dT%H:%M:%SZ")) 
+      entry.updated(item.updated_at.strftime("%Y-%m-%dT%H:%M:%SZ"))
 
       entry.author do |author|
-        author.name item.user.to_s
+        author.name item.user.forem_name
       end
     end
   end

app/views/forem/posts/_post.html.erb

@@ -22,7 +22,7 @@
           <% if post.user.is_a?(Forem::NilUser) %>
             <%= t(:deleted) %>
           <% else %>
-            <%= link_to_if Forem.user_profile_links, post.user, [main_app, post.user] %>
+            <%= link_to_if Forem.user_profile_links, post.user.forem_name, [main_app, post.user] %>
           <% end %>
         </div>
         <div class='icon'><%= forem_avatar(post.user, :size => 60) %></div>
@@ -36,7 +36,7 @@
 
         <% if post.reply_to %>
           <div class='in-reply-to'>
-            <%= link_to "#{t("forem.post.in_reply_to")} #{post.reply_to.user}", "#post-#{post.reply_to.id}" %>
+            <%= link_to "#{t("forem.post.in_reply_to")} #{post.reply_to.user.forem_name}", "#post-#{post.reply_to.id}" %>
           </div>
         <% end %>
 
@@ -54,7 +54,7 @@
               <% end %>
               <% if can?(:destroy_post, post.topic.forum) %>
                 <%= link_to t('delete', :scope => 'forem.topic'), forem.forum_topic_post_path(post.forum, post.topic, post), :method => :delete, data: { :confirm => t("are_you_sure") }, :class => "btn btn-danger" %>
-              <% end %>  
+              <% end %>
             <% end %>
           </div>
         <% end %>

app/views/forem/posts/_reply_to_post.html.erb

@@ -2,7 +2,7 @@
 <div id='post_<%= post.id %>' class='reply_to_post post <%= cycle('odd', 'even') -%> col-md-10'>
   <div class='user col-md-2'>
     <div class='username'>
-      <%= link_to_if Forem.user_profile_links, post.user, [main_app, post.user] %>
+      <%= link_to_if Forem.user_profile_links, post.user.forem_name, [main_app, post.user] %>
     </div>
     <div class='icon'><%= forem_avatar(post.user, :size => 60) %></div>
   </div>
@@ -13,8 +13,8 @@
 
     <% if post.reply_to %>
       <span class='in_reply_to'>
-        <%= link_to "#{t("forem.post.in_reply_to")} #{post.reply_to.user}", "#post-#{post.reply_to.id}" %>
+        <%= link_to "#{t("forem.post.in_reply_to")} #{post.reply_to.user.forem_name}", "#post-#{post.reply_to.id}" %>
       </span>
     <% end %>
   </div>
-</div>
+</div>

app/views/forem/topics/_topic.html.erb

@@ -18,7 +18,7 @@
   <td class='byline'>
     <%= new_since_last_view_text(topic) %>
     <div class='subject'><%= link_to forem_emojify(topic.subject), forem.forum_topic_path(@forum, topic) %></div>
-    <div class='started-by'><%= t "started_by" %><%= relevant_posts(topic).first.user %></div>
+    <div class='started-by'><%= t "started_by" %><%= relevant_posts(topic).first.user.forem_name %></div>
   </td>
   <td class='latest-post text-center'>
     <%= link_to_latest_post(topic) -%>

lib/forem.rb

@@ -45,6 +45,17 @@ def forem_approved_to_post?
         def forem_spammer?
           forem_state == 'spam'
         end
+
+        # Using +to_s+ by default for backwards compatibility
+        def forem_name
+          to_s
+        end unless method_defined? :forem_name
+
+        # Using +email+ by default for backwards compatibility. This attribute
+        # it's optional
+        def forem_email
+          try :email
+        end unless method_defined? :forem_email
       end
     end
 

spec/features/user_link_spec.rb

@@ -13,7 +13,7 @@
       visit forum_topic_path(post.forum, post.topic)
 
       # There should be at least one link on the page with the user's name
-      first(:link, post.user.to_s).click
+      first(:link, post.user.forem_name).click
 
       page.should have_content("A user's page!")
     end
@@ -25,7 +25,7 @@
     it "user name is not a link" do
       visit forum_topic_path(post.forum, post.topic)
       # There should be no links on the page with the user's name
-      Nokogiri::HTML(page.body).css("a").none? { |a| a.text.include?(post.user.to_s) }
+      Nokogiri::HTML(page.body).css("a").none? { |a| a.text.include?(post.user.forem_name) }
     end
   end
 end

spec/lib/generators/forem/dummy/dummy_generator.rb

@@ -50,6 +50,7 @@ def test_dummy_config
       template "config/initializers/devise.rb", "#{dummy_path}/config/initializers/devise.rb", :force => true
       template "db/migrate/1_create_users.rb", "#{dummy_path}/db/migrate/1_create_users.rb", :force => true
       template "db/migrate/2_create_admins.rb", "#{dummy_path}/db/migrate/2_create_admins.rb", :force => true
+      template "db/migrate/3_create_refunery_yoosers.rb", "#{dummy_path}/db/migrate/3_create_refunery_yoosers.rb", :force => true
       template "Rakefile", "#{dummy_path}/Rakefile", :force => true
       inject_into_file "#{dummy_path}/config/environments/test.rb",
                   "\n  config.action_mailer.default_url_options = { :host => 'www.example.com' }\n",

spec/lib/generators/forem/dummy/templates/app/models/refunery/yooser.rb

@@ -1,5 +1,5 @@
 module Refunery
   class Yooser < ActiveRecord::Base
-
+    self.table_name = 'refunery_yoosers'
   end
 end

spec/lib/generators/forem/dummy/templates/app/models/user.rb

@@ -5,6 +5,10 @@ class User < ActiveRecord::Base
   devise :database_authenticatable
 
   def to_s
+    fail # to_s should not be called in tests
+  end
+
+  def forem_name
     login
   end
 

spec/lib/generators/forem/dummy/templates/app/views/layouts/application.html.erb

@@ -12,7 +12,7 @@
     <div class='flash <%= key %>'><%= message %></div>
   <% end %>
   <% if user_signed_in? %>
-    Signed in as <%= forem_user %>
+    Signed in as <%= forem_user.forem_name %>
     <%= link_to "Sign out", main_app.destroy_user_session_path, :method => :delete %>
   <% end %>
 

spec/models/user_spec.rb

@@ -6,11 +6,62 @@
 describe User do
   subject { User.new }
 
-  it "can read forums" do
-    assert subject.can_read_forem_forums?
+  describe Forem::DefaultPermissions do
+    it "can read forums" do
+      assert subject.can_read_forem_forums?
+    end
+
+    it "can read a given forum" do
+      assert subject.can_read_forem_forum?(Forem::Forum.new)
+    end
   end
 
-  it "can read a given forum" do
-    assert subject.can_read_forem_forum?(Forem::Forum.new)
+  describe "Profile methods" do
+    context "with custom methods" do
+      describe "#forem_email" do
+        it "responds to our own email method" do
+          subject.should receive :email
+          subject.forem_email
+        end
+      end
+
+      describe "#forem_name" do
+        it "responds to our own name method" do
+          subject.should receive :login
+          subject.forem_name
+        end
+      end
+    end
+
+    context "with defaults" do
+      # Using a class without custom methods
+      subject { Refunery::Yooser.new }
+
+      before do
+        @original_class_name = Forem.user_class
+        Forem.user_class = "Refunery::Yooser"
+        Forem.decorate_user_class!
+      end
+
+      after do
+        Forem.user_class = @original_class_name.to_s
+      end
+
+      describe "#forem_email" do
+        it "defaults with 'email'" do
+          subject.should respond_to :forem_email
+          subject.should receive(:email)
+          subject.forem_email
+        end
+      end
+
+      describe "#forem_name" do
+        it "defaults with 'to_s'" do
+          subject.should respond_to :forem_name
+          subject.should receive :to_s
+          subject.forem_name
+        end
+      end
+    end
   end
 end