Load initial comments in the SPV on pageload

app/assets/javascripts/app/views/comment_stream_view.js

@@ -43,9 +43,9 @@ app.views.CommentStream = app.views.Base.extend({
 
   presenter: function(){
     return _.extend(this.defaultPresenter(), {
-      moreCommentsCount : (this.model.interactions.commentsCount() - 3),
-      showExpandCommentsLink : (this.model.interactions.commentsCount() > 3),
-      commentsCount : this.model.interactions.commentsCount()
+      moreCommentsCount: (this.model.interactions.commentsCount() - this.model.comments.length),
+      showExpandCommentsLink: (this.model.interactions.commentsCount() > this.model.comments.length),
+      commentsCount: this.model.interactions.commentsCount()
     });
   },
 
@@ -143,8 +143,9 @@ app.views.CommentStream = app.views.Base.extend({
     if(evt){ evt.preventDefault(); }
     this.model.comments.fetch({
       success: function() {
-        this.$("div.comment.show_comments").addClass("hidden");
+        this.$("div.comment.show-comments").addClass("hidden");
         this.$(".loading-comments").addClass("hidden");
+        this.trigger("commentsExpanded");
       }.bind(this)
     });
   },

app/assets/javascripts/app/views/single-post-viewer/single_post_comment_stream.js

@@ -5,35 +5,42 @@ app.views.SinglePostCommentStream = app.views.CommentStream.extend({
 
   initialize: function(){
     this.CommentView = app.views.ExpandedComment;
-    $(window).on('hashchange',this.highlightPermalinkComment);
+    $(window).on("hashchange", this.highlightPermalinkComment.bind(this));
     this.setupBindings();
-    this.model.comments.fetch({success: function() {
-      setTimeout(this.highlightPermalinkComment, 0);
-    }.bind(this)});
   },
 
   highlightPermalinkComment: function() {
-    if (document.location.hash && $(document.location.hash).length > 0) {
-      var element = $(document.location.hash);
-      var headerSize = 60;
-      $(".highlighted").removeClass("highlighted");
-      element.addClass("highlighted");
-      var pos = element.offset().top - headerSize;
-      $("html,body").animate({scrollTop: pos});
+    if (!document.location.hash) {
+      return;
     }
+
+    var selector = document.location.hash;
+
+    if (this.$(selector).length === 0) {
+      this.once("commentsExpanded", function() { _.defer(this.highlightComment, selector); });
+      this.expandComments();
+    } else {
+      this.highlightComment(selector);
+    }
+  },
+
+  highlightComment: function(selector) {
+    if (this.$(selector).length === 0) {
+      return;
+    }
+
+    var element = this.$(selector);
+    var headerSize = $("nav.navbar-fixed-top").height() + 10;
+    this.$(".highlighted").removeClass("highlighted");
+    element.addClass("highlighted");
+    var pos = element.offset().top - headerSize;
+    window.scroll(0, pos);
   },
 
   postRenderTemplate: function() {
     app.views.CommentStream.prototype.postRenderTemplate.apply(this);
     this.$(".new-comment-form-wrapper").removeClass("hidden");
-  },
-
-  presenter: function(){
-    return _.extend(this.defaultPresenter(), {
-      moreCommentsCount : 0,
-      showExpandCommentsLink : false,
-      commentsCount : this.model.interactions.commentsCount()
-    });
-  },
+    _.defer(this.highlightPermalinkComment.bind(this));
+  }
 });
 // @license-end

app/assets/stylesheets/comments.scss

@@ -1,5 +1,5 @@
 .comment_stream {
-  .show_comments {
+  .show-comments {
     border-top: 1px solid $border-grey;
     line-height: $line-height-computed;
     margin-top: 5px;
@@ -11,8 +11,9 @@
   }
 
   .loading-comments {
-    height: $line-height-computed + 11px; // height of .show_comments: line-height, 10px margin, 1px border
+    height: $line-height-computed + 11px; // height of .show-comments: line-height, 10px margin, 1px border
     margin-top: -$line-height-computed - 11px;
+    text-align: center;
 
     .loader {
       height: 20px;

app/assets/stylesheets/single-post-view.scss

@@ -116,9 +116,10 @@
 
   .no-comments { text-align: center; }
 
-  a {
+  .author-name {
     color: $link-color;
   }
+
   .count {
     float: left;
     i {
@@ -145,6 +146,14 @@
     padding: 10px;
   }
 
+  .show-comments {
+    text-align: center;
+  }
+
+  .loading-comments {
+    text-align: left;
+  }
+
   .count,
   .interaction-avatars {
     line-height: 25px;

app/assets/templates/comment-stream_tpl.jst.hbs

@@ -1,4 +1,4 @@
-<div class="show_comments comment {{#unless showExpandCommentsLink}} hidden {{/unless}}">
+<div class="show-comments comment {{#unless showExpandCommentsLink}} hidden {{/unless}}">
   <div class="media">
     <a href="/posts/{{id}}#comments" class="toggle_post_comments">
       {{t "stream.more_comments" count=moreCommentsCount}}

app/presenters/last_three_comments_decorator.rb

@@ -5,7 +5,7 @@ def initialize(presenter)
 
   def as_json(options={})
     @presenter.as_json.tap do |post|
-      post[:interactions].merge!(:comments => CommentPresenter.as_collection(@presenter.post.last_three_comments))
+      post[:interactions].merge!(comments: CommentPresenter.as_collection(@presenter.post.last_comments(3)))
     end
   end
-end
+end

app/presenters/post_presenter.rb

@@ -23,7 +23,8 @@ def with_initial_interactions
     as_json.tap do |post|
       post[:interactions].merge!(
         likes:    LikeService.new(current_user).find_for_post(@post.id).limit(30).as_api_response(:backbone),
-        reshares: ReshareService.new(current_user).find_for_post(@post.id).limit(30).as_api_response(:backbone)
+        reshares: ReshareService.new(current_user).find_for_post(@post.id).limit(30).as_api_response(:backbone),
+        comments: CommentPresenter.as_collection(@post.last_comments(10))
       )
     end
   end

lib/diaspora/commentable.rb

@@ -11,12 +11,10 @@ def self.included(model)
     end
 
     # @return [Array<Comment>]
-  def last_three_comments
-    return [] if self.comments_count == 0
-    # DO NOT USE .last(3) HERE.  IT WILL FETCH ALL COMMENTS AND RETURN THE LAST THREE
-    # INSTEAD OF DOING THE FOLLOWING, AS EXPECTED (THX AR):
-    self.comments.order('created_at DESC').limit(3).includes(:author => :profile).reverse
-  end
+    def last_comments(count)
+      return [] if comments_count == 0
+      comments.order(:created_at).last(count).includes(author: :profile)
+    end
 
     # @return [Integer]
     def update_comments_counter

spec/javascripts/app/views/comment_stream_view_spec.js

@@ -354,6 +354,18 @@ describe("app.views.CommentStream", function(){
       });
       expect(this.view.$(".loading-comments")).toHaveClass("hidden");
     });
+
+    it("triggers 'commentsExpanded' on success", function() {
+      spyOn(this.view, "trigger");
+      this.view.render();
+      this.view.expandComments();
+      expect(this.view.trigger).not.toHaveBeenCalledWith("commentsExpanded");
+
+      jasmine.Ajax.requests.mostRecent().respondWith({
+        status: 200, responseText: JSON.stringify([])
+      });
+      expect(this.view.trigger).toHaveBeenCalledWith("commentsExpanded");
+    });
   });
 
   describe("openForm", function() {

spec/javascripts/app/views/single-post-view/single_post_comment_stream_spec.js

@@ -1,6 +1,6 @@
 describe("app.views.SinglePostCommentStream", function() {
   beforeEach(function() {
-    this.post = factory.post();
+    this.post = factory.postWithInteractions();
     this.view = new app.views.SinglePostCommentStream({model: this.post});
   });
 
@@ -9,10 +9,91 @@ describe("app.views.SinglePostCommentStream", function() {
       expect(this.view.CommentView).toBe(app.views.ExpandedComment);
     });
 
+    it("calls highlightPermalinkComment on hashchange", function() {
+      spyOn(app.views.SinglePostCommentStream.prototype, "highlightPermalinkComment");
+      this.view.initialize();
+      $(window).trigger("hashchange");
+      expect(app.views.SinglePostCommentStream.prototype.highlightPermalinkComment).toHaveBeenCalled();
+    });
+
     it("calls setupBindings", function() {
       spyOn(app.views.SinglePostCommentStream.prototype, "setupBindings");
       this.view.initialize();
       expect(app.views.SinglePostCommentStream.prototype.setupBindings).toHaveBeenCalled();
     });
   });
+
+  describe("highlightPermalinkComment", function() {
+    beforeEach(function() {
+      this.view.render();
+    });
+
+    it("calls highlightComment if the comment is visible", function() {
+      document.location.hash = "#" + this.post.comments.first().get("guid");
+      spyOn(this.view, "highlightComment");
+      this.view.highlightPermalinkComment();
+      expect(this.view.highlightComment).toHaveBeenCalledWith(document.location.hash);
+    });
+
+    it("doesn't call expandComments if the comment is visible", function() {
+      document.location.hash = "#" + this.post.comments.first().get("guid");
+      spyOn(this.view, "expandComments");
+      this.view.highlightPermalinkComment();
+      expect(this.view.expandComments).not.toHaveBeenCalled();
+    });
+
+    it("calls expandComments if the comment isn't visible yet", function() {
+      document.location.hash = "#404-guid-not-found";
+      spyOn(this.view, "expandComments");
+      this.view.highlightPermalinkComment();
+      expect(this.view.expandComments).toHaveBeenCalled();
+    });
+
+    it("calls hightlightComment after the comments are expanded", function() {
+      document.location.hash = "#404-guid-not-found";
+      spyOn(_, "defer").and.callFake(function(fn, arg) { fn(arg); });
+      spyOn(this.view, "highlightComment");
+      this.view.highlightPermalinkComment();
+      this.view.trigger("commentsExpanded");
+      expect(this.view.highlightComment).toHaveBeenCalledWith(document.location.hash);
+    });
+  });
+
+  describe("highlightComment", function() {
+    beforeEach(function() {
+      this.view.render();
+    });
+
+    it("removes the existing highlighting and highlights the given comment", function() {
+      var firstGuidSelector = "#" + this.post.comments.first().get("guid"),
+          lastGuidSelector = "#" + this.post.comments.last().get("guid");
+      this.view.$(lastGuidSelector).addClass("highlighted");
+      this.view.highlightComment(firstGuidSelector);
+      expect(this.view.$(lastGuidSelector)).not.toHaveClass("highlighted");
+      expect(this.view.$(firstGuidSelector)).toHaveClass("highlighted");
+    });
+  });
+
+  describe("postRenderTemplate", function() {
+    it("calls app.views.CommentStream.prototype.postRenderTemplate", function() {
+      spyOn(app.views.CommentStream.prototype, "postRenderTemplate");
+      this.view.postRenderTemplate();
+      expect(app.views.CommentStream.prototype.postRenderTemplate).toHaveBeenCalled();
+    });
+
+    it("shows the new comment form wrapper", function() {
+      this.view.render();
+      this.view.$(".new-comment-form-wrapper").addClass("hidden");
+      expect(this.view.$(".new-comment-form-wrapper")).toHaveClass("hidden");
+      this.view.postRenderTemplate();
+      expect(this.view.$(".new-comment-form-wrapper")).not.toHaveClass("hidden");
+    });
+
+    it("defers a highlightPermalinkComment call", function() {
+      spyOn(_, "defer").and.callFake(function(fn) { fn(); });
+      spyOn(this.view, "highlightPermalinkComment");
+      this.view.postRenderTemplate();
+      expect(this.view.highlightPermalinkComment).toHaveBeenCalled();
+    });
+  });
 });

spec/presenters/post_presenter_spec.rb

@@ -22,6 +22,7 @@
     before do
       bob.like!(status_message)
       bob.reshare!(status_message)
+      bob.comment!(status_message, "hey")
     end
 
     describe "#with_interactions" do
@@ -42,6 +43,9 @@
       it "works with a user" do
         post_hash = presenter.with_initial_interactions
         expect(post_hash).to be_a Hash
+        expect(post_hash[:interactions][:comments]).to eq(
+          CommentPresenter.as_collection(status_message.last_comments(10))
+        )
         expect(post_hash[:interactions][:likes]).to eq(
           LikeService.new(bob).find_for_post(status_message.id).as_api_response(:backbone)
         )
@@ -53,6 +57,9 @@
       it "works without a user" do
         post_hash = unauthenticated_presenter.with_initial_interactions
         expect(post_hash).to be_a Hash
+        expect(post_hash[:interactions][:comments]).to eq(
+          CommentPresenter.as_collection(status_message.last_comments(10))
+        )
         expect(post_hash[:interactions][:likes]).to eq(
           LikeService.new.find_for_post(status_message.id).as_api_response(:backbone)
         )