New comment highlighting: Remove server-side pv_hex stuff.

Previously, the server would check the user's previous visits when
rendering a comment page and add comment-period-N classes to comments
depending on where they fell in relation to those visits.  The client
side would then add or remove a new-comment class to every comment with
the appropriate (or older) comment-period class on first load or when
the previous visit selection changed.

This removes that server-side addition of comment-period-N classes and
replaces it with ScrollUpdater-based updating of comments based on their
actual timestamps. The goal is to reduce some server-side ugliness and
extraneous memcached lookups.

r2/r2/controllers/api.py

@@ -3057,16 +3057,12 @@ class ApiController(RedditController):
         link=VByName('link_id'),
         sort=VMenu('morechildren', CommentSortMenu, remember=False),
         children=VCommentIDs('children'),
-        pv_hex=VPrintable(
-            "pv_hex", 40,
-            docs={"pv_hex": "(optional) a previous-visits token"}),
         mc_id=nop(
             "id",
             docs={"id": "(optional) id of the associated MoreChildren object"}),
     )
     @api_doc(api_section.links_and_comments)
-    def GET_morechildren(self, form, jquery, link, sort, children,
-                          pv_hex, mc_id):
+    def GET_morechildren(self, form, jquery, link, sort, children, mc_id):
         """Retrieve additional comments omitted from a base comment tree.
 
         When a comment tree is rendered, the most relevant comments are
@@ -3082,9 +3078,6 @@ class ApiController(RedditController):
         call is replacing. This is needed only for the HTML UI's purposes and
         is optional otherwise.
 
-        `pv_hex` is part of the reddit gold "previous visits" feature. It is
-        optional and deprecated.
-
         **NOTE:** you may only make one request at a time to this API endpoint.
         Higher concurrency will result in an error being returned.
 
@@ -3105,9 +3098,6 @@ class ApiController(RedditController):
             if not link or not link.subreddit_slow.can_view(c.user):
                 return abort(403,'forbidden')
 
-            if pv_hex:
-                c.previous_visits = g.cache.get(pv_hex)
-
             if children:
                 builder = CommentBuilder(link, CommentSortMenu.operator(sort),
                                          children=children,
@@ -3139,9 +3129,6 @@ class ApiController(RedditController):
                 # morechildren link
                 jquery.things(str(mc_id)).remove()
                 jquery.insert_things(a, append = True)
-
-                if pv_hex:
-                    jquery.rehighlight_new_comments()
         finally:
             if lock:
                 lock.release()

r2/r2/controllers/front.py

@@ -365,13 +365,6 @@ class FrontController(RedditController):
 
         if previous_visits:
             displayPane.append(CommentVisitsBox(previous_visits))
-            # Used in later "more comments" renderings
-            pv_hex = md5(repr(previous_visits)).hexdigest()
-            g.cache.set(pv_hex, previous_visits, time=g.comment_visits_period)
-            c.previous_visits_hex = pv_hex
-
-        # Used in template_helpers
-        c.previous_visits = previous_visits
 
         if c.site.allows_referrers:
             c.referrer_policy = "always"

r2/r2/lib/pages/pages.py

@@ -1386,10 +1386,7 @@ class CommentsPanel(Templated):
 
 class CommentVisitsBox(Templated):
     def __init__(self, visits, *a, **kw):
-        self.visits = []
-        for visit in reversed(visits):
-            pretty = timesince(visit, precision=60)
-            self.visits.append(pretty)
+        self.visits = list(reversed(visits))
         Templated.__init__(self, *a, **kw)
 
 class LinkInfoPage(Reddit):

r2/r2/lib/template_helpers.py

@@ -238,19 +238,6 @@ def class_dict():
     res = ', '.join(classes)
     return unsafe('{ %s }' % res)
 
-def calc_time_period(comment_time):
-    # Set in front.py:GET_comments()
-    previous_visits = c.previous_visits
-
-    if not previous_visits:
-        return ""
-
-    rv = ""
-    for i, visit in enumerate(previous_visits):
-        if comment_time > visit:
-            rv = "comment-period-%d" % i
-
-    return rv
 
 def comment_label(num_comments=None):
     if not num_comments:
@@ -327,15 +314,10 @@ def replace_render(listing, item, render_func):
             else:
                 replacements['timesince'] = simplified_timesince(item._date)
 
-            replacements['time_period'] = calc_time_period(item._date)
-
         # compute the last edited time here so we don't end up caching it
         if hasattr(item, "editted") and not isinstance(item.editted, bool):
             replacements['lastedited'] = simplified_timesince(item.editted)
 
-        # Set in front.py:GET_comments()
-        replacements['previous_visits_hex'] = c.previous_visits_hex
-
         renderer = render_func or item.render
         res = renderer(style = style, **replacements)
 

r2/r2/models/link.py

@@ -1297,7 +1297,7 @@ class Comment(Thing, Printable):
 
             item.editted = getattr(item, "editted", False)
 
-            item.render_css_class = "comment %s" % CachedVariable("time_period")
+            item.render_css_class = "comment"
 
             #will get updated in builder
             item.num_children = 0
@@ -1434,8 +1434,6 @@ class MoreComments(Printable):
         return False
 
     def __init__(self, link, depth, parent_id=None):
-        from r2.lib.wrapped import CachedVariable
-
         if parent_id is not None:
             id36 = utils.to36(parent_id)
             self.parent_id = parent_id
@@ -1446,7 +1444,6 @@ class MoreComments(Printable):
         self.depth = depth
         self.children = []
         self.count = 0
-        self.previous_visits_hex = CachedVariable("previous_visits_hex")
 
     @property
     def _fullname(self):

r2/r2/public/static/js/jquery.reddit.js

@@ -689,14 +689,6 @@ $.apply_stylesheet = function(cssText) {
     
 };
 
-$.rehighlight_new_comments = function() {
-  checked = $(".comment-visits-box input:checked");
-  if (checked.length > 0) {
-    var v = checked[0].value;
-    highlight_new_comments(v);
-  }
-}
-
 /* namespace globals for cookies -- default prefix, security and domain */
 var default_cookie_domain
 $.default_cookie_domain = function(domain) {

r2/r2/public/static/js/reddit.js

@@ -482,7 +482,7 @@ function togglemessage(elem) {
   }
 }
 
-function morechildren(form, link_id, sort, children, depth, pv_hex) {
+function morechildren(form, link_id, sort, children, depth) {
     $(form).html(reddit.status_msg.loading)
         .css("color", "red");
     var id = $(form).parents(".thing.morechildren:first").thing_id();
@@ -492,7 +492,6 @@ function morechildren(form, link_id, sort, children, depth, pv_hex) {
         children: children,
         depth: depth,
         id: id,
-        pv_hex: pv_hex,
     };
     $.request('morechildren', child_params, undefined, undefined,
               undefined, true);
@@ -1297,18 +1296,6 @@ function show_unfriend(account_fullname) {
         });
 }
 
-function highlight_new_comments(period) {
-  var i;
-  for (i = 0 ; i <= 9; i++) {
-    items = $(".comment-period-" + i);
-    if (period >= 0 && i >= period) {
-      items.addClass("new-comment");
-    } else {
-      items.removeClass("new-comment");
-    }
-  }
-}
-
 function save_href(link) {
   if (!link.attr("srcurl")){
     link.attr("srcurl", link.attr("href"));

r2/r2/public/static/js/timetext.js

@@ -47,17 +47,10 @@
         }
 
         var $el = $(el)
-        var timestamp = $el.data('timestamp')
-        var isoTimestamp
+        var timestamp = r.utils.parseTimestamp($el)
         var text
         var age
 
-        if (!timestamp) {
-            isoTimestamp = $el.attr('datetime')
-            timestamp = Date.parse(isoTimestamp)
-            $el.data('timestamp', timestamp)
-        }
-
         age = (now - timestamp) / 1000
 
         if (this.opts.maxage !== false && age > this.opts.maxage) {

r2/r2/public/static/js/ui.js

@@ -66,6 +66,8 @@ r.ui.init = function() {
 
     r.ui.initLiveTimestamps()
 
+    r.ui.initNewCommentHighlighting()
+
     r.ui.initTimings()
 }
 
@@ -106,6 +108,40 @@ r.ui.initLiveTimestamps = function() {
     }
 }
 
+r.ui.initNewCommentHighlighting = function() {
+  if (!r.config.gold || !$('body').hasClass('comments-page')) {
+    return;
+  }
+
+  $visitSelector = $('#comment-visits');
+  if ($visitSelector.length === 0) {
+    return;
+  }
+
+  $(document).on('new_things_inserted', r.ui.highlightNewComments);
+  $visitSelector.on('change', r.ui.highlightNewComments);
+  r.ui.highlightNewComments();
+}
+
+r.ui.highlightNewComments = function() {
+  var $comments = $('.comment');
+  var selectedVisitTimestamp = $('#comment-visits').val();
+  var selectedVisit;
+
+  if (selectedVisitTimestamp) {
+    selectedVisit = Date.parse(selectedVisitTimestamp);
+  }
+
+  $comments.each(function() {
+    var $commentEl = $(this);
+    var $timeEl = $commentEl.find('.tagline time');
+    var commentTime = r.utils.parseTimestamp($timeEl);
+    var shouldHighlight = !!selectedVisit && commentTime > selectedVisit;
+
+    $commentEl.toggleClass('new-comment', shouldHighlight);
+  });
+}
+
 r.ui.initTimings = function() {
   // return if we're not configured for sending stats
   if (!r.config.pageInfo.actionName || !r.config.stats_domain) {

r2/r2/public/static/js/utils.js

@@ -27,6 +27,19 @@ r.utils = {
         }
     },
 
+    parseTimestamp: function($el) {
+      var timestamp = $el.data('timestamp')
+      var isoTimestamp
+
+      if (!timestamp) {
+        isoTimestamp = $el.attr('datetime')
+        timestamp = Date.parse(isoTimestamp)
+        $el.data('timestamp', timestamp)
+      }
+
+      return timestamp
+    },
+
     joinURLs: function(/* arguments */) {
         return _.map(arguments, function(url, idx) {
             if (idx > 0 && url && url[0] != '/') {

r2/r2/templates/commentvisitsbox.html

@@ -20,22 +20,24 @@
 ## reddit Inc. All Rights Reserved.
 ###############################################################################
 
+<%!
+
+from r2.lib.utils import timesince
+
+%>
+
 <div class="rounded gold-accent comment-visits-box">
   <div class="title">
     ${_("Highlight comments posted since previous visit:")}&#32;
-    <select id="comment-visits" onchange="highlight_new_comments(this.value)">
-      %for i, visit in enumerate(thing.visits, start=1):
-        <option value="${len(thing.visits) - i}"
-          %if i == 1:
+    <select id="comment-visits">
+      %for i, visit in enumerate(thing.visits):
+        <option value="${visit.isoformat()}"
+          %if i == 0:
             selected="selected"
           %endif
-        >${visit} ${_("ago")}</option>
+        >${timesince(visit, precision=60)} ${_("ago")}</option>
       %endfor
-      <option value="-1">${_("no highlighting")}</option>
+      <option value="">${_("no highlighting")}</option>
     </select>
   </div>
 </div>
-
-<script type="text/javascript">
-  $(function() { highlight_new_comments(${len(thing.visits) - 1}) })
-</script>

r2/r2/templates/morechildren.html

@@ -33,7 +33,7 @@
 <span class="morecomments">
 <a style="font-size: smaller; font-weight: bold" class="button"
    id="more_${thing._fullname}" href="javascript:void(0)"
-   onclick="return morechildren(this, '${thing.link_name}', '${thing.sort}', '${",".join(cids)}', ${thing.depth}, '${thing.previous_visits_hex}')">
+   onclick="return morechildren(this, '${thing.link_name}', '${thing.sort}', '${",".join(cids)}', ${thing.depth})">
 ${_("load more comments")}
 <span class="gray">&nbsp;(${thing.count} ${ungettext("reply", "replies", thing.count)})</span>
 </a>