launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/commentlist-isnt-as-smart-as-it-thinks-it-is into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/commentlist-isnt-as-smart-as-it-thinks-it-is/+merge/119798

Summary
=======
The way comment lists are setup assumes that comments exist on a given page.
This is safe on bugs and merge proposals, b/c there are always comments.

On questions it breaks though, as there are no comments.

Preimp
======
Spoke with Curtis Hovey, Rick Harding.

Implementation
==============
On setup on questions, check for the existence of comments, and only setup the
comment list if they exist--there's no need for it otherwise, so you can just
proceed without it.

QA
==
Load a question with no comments; there should be no JS error.

LoC
===
This is part of disclosure.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/answers/templates/question-index.pt
-- 
https://code.launchpad.net/~jcsackett/launchpad/commentlist-isnt-as-smart-as-it-thinks-it-is/+merge/119798
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/commentlist-isnt-as-smart-as-it-thinks-it-is into lp:launchpad.

=== modified file 'lib/lp/answers/templates/question-index.pt'
--- lib/lp/answers/templates/question-index.pt	2012-07-25 21:49:38 +0000
+++ lib/lp/answers/templates/question-index.pt	2012-08-15 21:30:29 +0000
@@ -21,11 +21,13 @@
             function(Y) {
         Y.on('domready', function() {
             LP.cache.comment_context = LP.cache.context;
-            var container = Y.one('.boardComment').get('parentNode');
-            var cl = new Y.lp.app.comment.CommentList({
-                comment_list_container: container 
-            });
+            var first_comment = Y.one('.boardComment');
+            if (first_comment !== null) {
+              var cl = new Y.lp.app.comment.CommentList({
+                  comment_list_container: first_comment.get('parentNode')
+              });
             cl.render();
+            }
             new Y.lp.answers.subscribers.createQuestionSubscribersLoader();
         });
       });