Merge lp:~jcsackett/launchpad/spam-button-ui into lp:launchpad

[j.c.sackett]

[r=allenap][bug=220535] Adds ui to questions allowing the removal of spam.

Summary
=======
Wouldn't it be nice, when on maintenance rotation, if you could kill spam comments from the UI?

Yeah. This does that for questions.

Less glibly: Previous work added API methods to hide comments on bugs and questions. These could only be accessed via scripts or other means of posting against the API. This branch adds a control (visible to admins and registry experts only) to the footer of messages on questions that allows you to mark the given message as spam via AJAX.

A subsequent branch will add this capability to bugs; that branch, or yet another, will also clean up any duplication that may occur in so doing.

Preimplementation
=================
Curtis Hovey: Implementation and troubleshooting.
Aaron Bentley: Issues with setting up javascript tests.

Implementation
==============
lib/lp/testing/factory.py
-------------------------
Created "makeAdmin" function so you can grab an admin without using sampledata (you routinely end up with ~foo.bar or ~mark via sampledata, and they have super privs.

lib/lp/answers/browser/question.py
-----------------------------------------------------
Created view check to determine whether or not to show the spam controls.

lib/lp/answers/templates/question-index.pt
lib/lp/answers/templates/questionmessage-display.pt
---------------------------------------------------
Added spam controls and javascript hookup.

lib/lp/answers/javascript/question_spam.js
------------------------------------------
Added javascript function which toggles the control between "Mark as spam" and "Mark as not spam", as well as toggles the display as a hidden or not hidden comment and fires off the postback to the API to set the message to hidden.

lib/lp/answers/browser/tests/test_questionmessages.py
-----------------------------------------------------
Tests for view changes.

lib/lp/answers/javascript/tests/test_question_spam.html
lib/lp/answers/javascript/tests/test_question_spam.js
-----------------------------------------------------
Tests for javascript.

Tests
=====
bin/test -vvct test_questionmessages
firefox lib/lp/answers/javascript/tests/test_question_spam.html

QA
==
Open a question with comments while logged in as an admin or registry expert.
Mark a message as spam via the control.
The message will switch to the "hidden" display and the control will change to "Mark as not spam."
Reload the page; the message will still be thusly marked.
Open the same question as anonymous or a user without priveleges (easiest to do in another browser).
The message marked as spam will not display.

Open the question again logged in as admin or registry expert.
Mark the spam message as not spam.
The message will switch to the not hidden display and the control will change to "Mark as spam."
Reload the page; the message will still be marked as not spam.
Open the same question as anon or no privs user; the message will be displayed.

Screenshots
===========
http://people.canonical.com/~jc/images/dealing-with-spam.png

Lint
====
= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/answers/javascript/
  lib/lp/answers/browser/question.py
  lib/lp/answers/browser/tests/test_questionmessages.py
  lib/lp/answers/javascript/question_spam.js
  lib/lp/answers/javascript/tests/
  lib/lp/answers/javascript/tests/test_question_spam.html
  lib/lp/answers/javascript/tests/test_question_spam.js
  lib/lp/answers/templates/question-index.pt
  lib/lp/answers/templates/questionmessage-display.pt
  lib/lp/testing/factory.py

[Gavin Panella (allenap)]

This is an awesome feature, but I think the implementation needs a
little more work.

[1]

+ def canSeeSpamControls(self):
+ return check_permission('launchpad.Moderate', self.context.question)

This is going to be checked for every comment so consider making this
a cached property, or use an instance variable set in initialize()
instead.

[2]

+ <a tal:attributes="id string:mark-spam-${index};"
+ class="js-action sprite edit mark-spam" href="#">
+ <tal:not-spam condition="not: context/visible">
+ Mark as not spam
+ </tal:not-spam>
+ <tal:spam condition="context/visible">
+ Mark as spam
+ </tal:spam>
+ </a>

The text here is repeated in the Javascript:

+var spam_text = "Mark as spam";
+var not_spam_text = "Mark as not spam";

I don't mind if you leave it, but another pattern for doing this could
be:

    <a tal:attributes="id string:mark-spam-${index};"
       class="js-action sprite edit mark-spam" href="#">
         <tal:not-spam condition="not: context/visible">
           <span>Mark as not spam</span>
           <span class="unseen">Mark as spam</span>
         </tal:not-spam>
         <tal:spam condition="context/visible">
           <span class="unseen">Mark as not spam</span>
           <span>Mark as spam</span>
         </tal:spam>
    </a>

Then instead of changing the text you could change which span has the
unseen class:

    function update_text(link) {
        link.all("span").toggleClass("unseen");
    }

[3]

+ var config = {
+ on: {
+ success: function () {},
+ failure: function () {}
+ },

Fishy!

+function toggle_spam_setting(link) {
+ var comment = link.get('parentNode').get('parentNode');
+ var visible = comment.hasClass('adminHiddenComment');
+ var comment_number = parseInt(link.get('id').replace('mark-spam-', ''));
+ comment_number = comment_number - 1;
+ namespace._update_text(link);
+ comment.toggleClass(hidden_class);
+ namespace._set_visibility(comment_number, visible);
+}

The call to _update_text() and comment.toggleClass() should be done in
the success handler passed to named_post(). Also, they should not
toggle but set the UI according to the actual state in case the link
is clicked more than once in quick succession (so my suggestion in [2]
is not quite suitable).

Additionally there should probably be a Y.lazr.anim.red_flash() on
failure (as this is an admin tool it's probably safe to not worry
about a more sophisticated error handler for now), and a green_flash()
on success.

[4]

+namespace._update_text = update_text
...
+namespace._set_visibility = set_visibility

These are private functions and are not called outside of the module,
even for testing. You're probably better off not adding them to the
namespace.

[5]

+}, "0.1", {"requires": ["dom", "event-custom", "node", "lazr.effects",
+ "lp.client"]});

Are event-custom or lazr.effects used?

[6]

+function setup_spam_links() {
+ Y.on('click', function(e) {
+ e.halt()
+ var that = this;
+ namespace.toggle_spam_setting(that);
+ }, '.mark-spam');
+}

I couldn't find docs...

review: Needs Fixing

[j.c.sackett (jcsackett)]

> This is an awesome feature, but I think the implementation needs a
> little more work.
>
>
> [1]
>
> + def canSeeSpamControls(self):
> + return check_permission('launchpad.Moderate', self.context.question)
>
> This is going to be checked for every comment so consider making this
> a cached property, or use an instance variable set in initialize()
> instead.

I've gone ahead and made it a cachedProperty. My understanding was that check_permission did some caching already, but I guess I was mistaken.

> [3]
>
> + var config = {
> + on: {
> + success: function () {},
> + failure: function () {}
> + },
>
> Fishy!

Agreed! That was a quick hack while I was exploring the problem. As you mention below, I meant to create the success handler to call the other functions.

> +function toggle_spam_setting(link) {
> + var comment = link.get('parentNode').get('parentNode');
> + var visible = comment.hasClass('adminHiddenComment');
> + var comment_number = parseInt(link.get('id').replace('mark-spam-', ''));
> + comment_number = comment_number - 1;
> + namespace._update_text(link);
> + comment.toggleClass(hidden_class);
> + namespace._set_visibility(comment_number, visible);
> +}
>
> The call to _update_text() and comment.toggleClass() should be done in
> the success handler passed to named_post(). Also, they should not
> toggle but set the UI according to the actual state in case the link
> is clicked more than once in quick succession (so my suggestion in [2]
> is not quite suitable).
>
> Additionally there should probably be a Y.lazr.anim.red_flash() on
> failure (as this is an admin tool it's probably safe to not worry
> about a more sophisticated error handler for now), and a green_flash()
> on success.

I agree on adding redflash to failure. Green flash on success has two problems, so I've elected not to add it. 1) There's already a visual indicator of success (the bg color of the comment changes), so I don't think further animation is needed. 2) b/c of the way the flash animation occurs, it interferes with the bg color change -- I've looked at the lazr.anim code, and I don't see a way to tell it to green flash to any color but #FFFFFF.

> [4]
>
> +namespace._update_text = update_text
> ...
> +namespace._set_visibility = set_visibility
>
> These are private functions and are not called outside of the module,
> even for testing. You're probably better off not adding them to the
> namespace.

Thanks! I wasn't sure what was appropriate here. I'll remove them from the namespace.

> [5]
>
> +}, "0.1", {"requires": ["dom", "event-custom", "node", "lazr.effects",
> + "lp.client"]});
>
> Are event-custom or lazr.effects used?

Nope, they've been removed. The perils of cargo culting.

> [6]
>
> +function setup_spam_links() {
> + Y.on('click', function(e) {
> + e.halt()
> + var that = this;
> + namespace.toggle_spam_setting(that);
> + }, '.mark-spam');
> +}
>
> I couldn't find docs for Y.on(). It's a little confusing because the
> selector at the end is easily overlooked. Consider the following:
>
> function setup_spam...

[Gavin Panella (allenap)]

Looks great. Once more small thing, then land it :)

[9]

+ @with_celebrity_logged_in('admin')
+ def makeAdministrator(self, name=None, email=None, password=None):
+ from lp.testing.sampledata import ADMIN_EMAIL
+ if name = None:
+ name = self.getUniqueString()
+ if email = None:
+ email = '%s@%s.com' % (
+ self.getUniqueString(), self.getUniqueString())
+ if password = None:
+ password = self.getUniqueString()

I suspect you're doing more than you need to; makePerson() already
does this.

+ login(ADMIN_EMAIL)

This shouldn't be needed any more.

+ user = self.makePerson(name=name,
+ email=email,
+ password=password)
+ administrators = getUtility(ILaunchpadCelebrities).admin
+ administrators.addMember(user, administrators.teamowner)
+ return user

I /think/ the following should work:

    @with_celebrity_logged_in('admin')
    def makeAdministrator(self, name=None, email=None, password=None):
        user = self.makePerson(name=name,
                               email=email,
                               password=password)
        administrators = getUtility(ILaunchpadCelebrities).admin
        administrators.addMember(user, administrators.teamowner)
        return user

review: Approve

[Robert Collins (lifeless)]

I have only one thing to say.

\ /
 \ /
  \ /
   \ /
    o

(Yes, thats super-raised-hands)

=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py	2011-04-28 15:09:58 +0000
+++ lib/lp/answers/browser/question.py	2011-05-09 14:53:12 +0000
@@ -1113,6 +1113,10 @@
         else:
             return "boardCommentBody"
 
+    @cachedproperty
+    def canSeeSpamControls(self):
+        return check_permission('launchpad.Moderate', self.context.question)
+
     def getBoardCommentCSSClass(self):
         css_classes = ["boardComment"]
         if not self.context.visible:

=== renamed file 'lib/lp/answers/browser/tests/test_questioncomment_visibility.py' => 'lib/lp/answers/browser/tests/test_questionmessages.py'
--- lib/lp/answers/browser/tests/test_questioncomment_visibility.py	2011-04-26 15:44:42 +0000
+++ lib/lp/answers/browser/tests/test_questionmessages.py	2011-05-09 14:53:12 +0000
@@ -9,6 +9,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.testing.pages import find_tag_by_id
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.testing import (
     BrowserTestCase,
@@ -20,7 +21,7 @@
 
     layer = DatabaseFunctionalLayer
 
-    def _makeQuestionWithHiddenComment(self, questionbody=None):
+    def makeQuestionWithHiddenComment(self, questionbody=None):
         administrator = getUtility(ILaunchpadCelebrities).admin.teamowner
         with person_logged_in(administrator):
             question = self.factory.makeQuestion()
@@ -28,38 +29,27 @@
             removeSecurityProxy(comment).message.visible = False
         return question
 
-    def _getUserForTest(self, team=None):
-        person = self.factory.makePerson()
-        if team is not None:
-            with person_logged_in(team.teamowner):
-                team.addMember(person, team.teamowner)
-        return person
-
     def test_admin_can_see_comments(self):
         comment_text = "You can't see me."
-        question = self._makeQuestionWithHiddenComment(comment_text)
-        admin_team = getUtility(ILaunchpadCelebrities).admin
-        administrator = self._getUserForTest(admin_team)
-        view = self.getViewBrowser(
-            context=question, user=administrator)
+        question = self.makeQuestionWithHiddenComment(comment_text)
+        administrator = self.factory.makeAdministrator()
+        view = self.getViewBrowser(context=question, user=administrator)
         self.assertTrue(
            comment_text in view.contents,
            "Administrator cannot see the hidden comment.")
 
     def test_registry_can_see_comments(self):
         comment_text = "You can't see me."
-        question = self._makeQuestionWithHiddenComment(comment_text)
-        registry_team = getUtility(ILaunchpadCelebrities).registry_experts
-        registry_expert = self._getUserForTest(registry_team)
-        view = self.getViewBrowser(
-            context=question, user=registry_expert)
+        question = self.makeQuestionWithHiddenComment(comment_text)
+        registry_expert = self.factory.makeRegistryExpert()
+        view = self.getViewBrowser(context=question, user=registry_expert)
         self.assertTrue(
            comment_text in view.contents,
            "Registy member cannot see the hidden comment.")
 
     def test_anon_cannot_see_comments(self):
         comment_text = "You can't see me."
-        question = self._makeQuestionWithHiddenComment(comment_text)
+        question = self.makeQuestionWithHiddenComment(comment_text)
         view = self.getViewBrowser(context=question, no_login=True)
         self.assertFalse(
            comment_text in view.contents,
@@ -67,8 +57,47 @@
 
     def test_random_cannot_see_comments(self):
         comment_text = "You can't see me."
-        question = self._makeQuestionWithHiddenComment(comment_text)
+        question = self.makeQuestionWithHiddenComment(comment_text)
         view = self.getViewBrowser(context=question)
         self.assertFalse(
            comment_text in view.contents,
            "Random user can see the hidden comment.")
+
+
+class TestQuestionSpamControls(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def makeQuestionWithMessage(self):
+        administrator = getUtility(ILaunchpadCelebrities).admin.teamowner
+        question = self.factory.makeQuestion()
+        body = self.factory.getUniqueString()
+        with person_logged_in(administrator):
+            question.addComment(administrator, body)
+        return question
+
+    def test_admin_sees_spam_control(self):
+        question = self.makeQuestionWithMessage()
+        administrator = self.factory.makeAdministrator()
+        view = self.getViewBrowser(context=question, user=administrator)
+        spam_link = find_tag_by_id(view.contents, 'mark-spam-1')
+        self.assertIsNot(None, spam_link)
+
+    def test_registry_sees_spam_control(self):
+        question = self.makeQuestionWithMessage()
+        registry_expert = self.factory.makeRegistryExpert()
+        view = self.getViewBrowser(context=question, user=registry_expert)
+        spam_link = find_tag_by_id(view.contents, 'mark-spam-1')
+        self.assertIsNot(None, spam_link)
+
+    def test_anon_doesnt_see_spam_control(self):
+        question = self.makeQuestionWithMessage()
+        view = self.getViewBrowser(context=question, no_login=True)
+        spam_link = find_tag_by_id(view.contents, 'mark-spam-1')
+        self.assertIs(None, spam_link)
+
+    def test_random_doesnt_see_spam_control(self):
+        question = self.makeQuestionWithMessage()
+        view = self.getViewBrowser(context=question)
+        spam_link = find_tag_by_id(view.contents, 'mark-spam-1')
+        self.assertIs(None, spam_link)

=== added directory 'lib/lp/answers/javascript'
=== added file 'lib/lp/answers/javascript/question_spam.js'
--- lib/lp/answers/javascript/question_spam.js	1970-01-01 00:00:00 +0000
+++ lib/lp/answers/javascript/question_spam.js	2011-05-09 14:53:12 +0000
@@ -0,0 +1,73 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Animation for IBugTask:+subscribe LaunchpadForm.
+ * Also used in "Edit subscription" advanced overlay.
+ *
+ * @namespace Y.lp.answers.question_spam
+ * @requires  dom, node, lazr.effects
+ */
+YUI.add('lp.answers.question_spam', function(Y) {
+var namespace = Y.namespace('lp.answers.question_spam');
+
+var hidden_class = "adminHiddenComment";
+var spam_text = "Mark as spam";
+var not_spam_text = "Mark as not spam";
+
+function update_comment(link, comment) {
+    var text = link.get('text').trim();
+    if (text == spam_text) {
+        comment.removeClass(hidden_class);
+        link.set('text', not_spam_text);
+    } else {
+        comment.addClass(hidden_class);
+        link.set('text', spam_text);
+    }
+}
+
+function set_visibility(parameters, callbacks) {
+    var question = LP.cache.context
+    var lp_client = new Y.lp.client.Launchpad()
+    var config = {
+        on: {
+            success: callbacks.success,
+            failure: callbacks.failure
+            },
+        parameters: parameters
+        }
+    lp_client.named_post(
+        question.self_link, 'setCommentVisibility', config)
+}
+
+function toggle_spam_setting(link) {
+    var comment = link.get('parentNode').get('parentNode');
+    var visible = comment.hasClass('adminHiddenComment');
+    var comment_number = parseInt(link.get('id').replace('mark-spam-', ''));
+    comment_number = comment_number - 1;
+    parameters = {
+        visible: visible,
+        comment_number: comment_number
+        };
+    set_visibility(parameters, {
+        // We use red flash on failure so admins no it didn't work.
+        // There's no green flash on success, b/c the change in bg
+        // color provides an immediate visual cue.
+        success: function () { 
+            update_comment(link, comment);
+            comment.toggleClass(hidden_class);
+            },
+        failure: function () {
+            Y.lazr.anim.red_flash({node:comment});
+            }
+        });
+}
+namespace.toggle_spam_setting = toggle_spam_setting;
+
+function setup_spam_links() {
+  Y.on('click', function(e) {
+      e.halt();
+      namespace.toggle_spam_setting(this);
+  }, '.mark-spam');
+}
+namespace.setup_spam_links = setup_spam_links
+}, "0.1", {"requires": ["dom", "node", "lazr.anim", "lp.client"]});

=== added directory 'lib/lp/answers/javascript/tests'
=== added file 'lib/lp/answers/javascript/tests/test_question_spam.html'
--- lib/lp/answers/javascript/tests/test_question_spam.html	1970-01-01 00:00:00 +0000
+++ lib/lp/answers/javascript/tests/test_question_spam.html	2011-05-09 14:53:12 +0000
@@ -0,0 +1,80 @@
+<html>
+  <head>
+    <title>Launchpad Question Spam Controls</title>
+    <!-- YUI 3.0 Setup -->
+    <script type="text/javascript" src="../../../../canonical/launchpad/icing/yui/yui/yui.js"></script>
+    <script type="text/javascript"
+      src="../../../../canonical/launchpad/icing/lazr/build/lazr.js"></script>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/icing/yui/cssreset/reset.css"/>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/icing/yui/cssfonts/fonts.css"/>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/icing/yui/cssbase/base.css"/>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/javascript/test.css" />
+    <script type="text/javascript" src="../../../app/javascript/client.js"></script>
+    <script type="text/javascript" src="../../../app/javascript/lp.js"></script>
+
+    <!-- Additional module dependency setup -->
+    <script type="text/javascript">
+        var LP = {
+            cache: {
+                context: {
+                    self_link: 'http:example.com'
+                    }
+                }
+            };
+    </script>
+
+    <!-- The module under test -->
+    <script type="text/javascript" src="../question_spam.js"></script>
+
+    <!-- The test suite -->
+    <script type="text/javascript" src="test_question_spam.js"></script>
+  </head>
+  <body class="yui-skin-sam">
+
+    <!-- The example markup required by the script to run -->
+    <div id="expected-id">
+    <div class="boardComment">
+      <div class="boardCommentDetails">
+        <a href="#" class="sprite person">TestyMcTest</a>
+        said
+        <span title="">2 hours ago</span>:
+        <span style="float:right;"> #1</span>
+      </div>
+
+      <div class="boardCommentBody"><p>Foo bar baz.</p></div>
+
+      <div class="boardCommentFooter">
+        <a id="mark-spam-1" class="js-action sprite edit" href="#">
+            Mark as spam
+        </a>
+
+      </div>
+    </div>
+
+    <div id="hidden-comment" class="boardComment adminHiddenComment">
+      <div class="boardCommentDetails">
+        <a href="#" class="sprite person">TestyMcTest</a>
+        said
+        <span title="">1 hours ago</span>:
+        <span style="float:right;"> #2</span>
+      </div>
+
+      <div class="boardCommentBody"><p>Click here for a diploma!</p></div>
+
+      <div class="boardCommentFooter">
+        <a id="mark-spam-2" class="js-action sprite edit" href="#">
+            Mark as not spam
+        </a>
+
+      </div>
+    </div>
+    </div>
+
+    <!-- The test output -->
+    <div id="log"></div>
+  </body>
+</html>

=== added file 'lib/lp/answers/javascript/tests/test_question_spam.js'
--- lib/lp/answers/javascript/tests/test_question_spam.js	1970-01-01 00:00:00 +0000
+++ lib/lp/answers/javascript/tests/test_question_spam.js	2011-05-09 14:53:12 +0000
@@ -0,0 +1,93 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ */
+
+YUI({
+    base: '../../../../canonical/launchpad/icing/yui/',
+    filter: 'raw', combine: false,
+    fetchCSS: false,
+    }).use('test', 'console', 'node', 'node-event-simulate',
+           'lp.answers.question_spam', function(Y) {
+
+    var suite = new Y.Test.Suite("lp.answers.question_spam Tests");
+
+    suite.add(new Y.Test.Case({
+        name: 'question_spam',
+
+        setUp: function() {
+            // Monkeypatch LP to avoid network traffic and to allow
+            // insertion of test data.
+            window.LP = {
+                links: {},
+                cache: {}
+            };
+            Y.lp.client.Launchpad = function() {};
+            Y.lp.client.Launchpad.prototype.named_post =
+                function(url, func, config) {
+                    LP.cache.call_data = {
+                        called_url: url, 
+                        called_func: func,
+                        called_config: config
+                    }
+                    // our setup assumes success, so we just do the
+                    // success callback.
+                    config.on.success();
+                };
+            LP.cache.context = {
+                self_link: 'https://launchpad.dev/api/devel/questions/fake'
+            };
+        },
+
+        test_mark_as_spam: function () {
+            link = Y.one('#mark-spam-1');
+            comment = Y.one('.boardComment');
+            Y.lp.answers.question_spam.toggle_spam_setting(link);
+            Y.Assert.isTrue(comment.hasClass('adminHiddenComment'));
+            Y.Assert.areEqual('Mark as not spam', link.get('text'),
+                'Link text should be \'Mark as not spam\'');
+            Y.Assert.areEqual(
+                'https://launchpad.dev/api/devel/questions/fake',
+                LP.cache.call_data.called_url, 'Call with wrong url.');
+            Y.Assert.areEqual(
+                'setCommentVisibility', LP.cache.call_data.called_func,
+                'Call with wrong func.');
+            Y.Assert.isFalse(
+                LP.cache.call_data.called_config.parameters.visible);
+            Y.Assert.areEqual(
+                0, LP.cache.call_data.called_config.parameters.comment_number,
+                'Called with wrong wrong comment number.')
+            },
+        
+        test_mark_as_not_spam: function () {
+            link = Y.one('#mark-spam-2');
+            comment = Y.one('#hidden-comment');
+            Y.lp.answers.question_spam.toggle_spam_setting(link);
+            Y.Assert.isFalse(comment.hasClass('adminHiddenComment'));
+            Y.Assert.areEqual('Mark as spam', link.get('text'),
+                'Link text should be \'Mark as spam\'')
+            Y.Assert.areEqual(
+                'https://launchpad.dev/api/devel/questions/fake',
+                LP.cache.call_data.called_url, 'Call with wrong url.');
+            Y.Assert.areEqual(
+                'setCommentVisibility', LP.cache.call_data.called_func,
+                'Call with wrong func.');
+            Y.Assert.isTrue(
+                LP.cache.call_data.called_config.parameters.visible);
+            Y.Assert.areEqual(
+                1, LP.cache.call_data.called_config.parameters.comment_number,
+                'Called with wrong wrong comment number.')
+            },
+
+        }));
+
+    // Lock, stock, and two smoking barrels.
+    Y.Test.Runner.add(suite);
+
+    var console = new Y.Console({newestOnTop: false});
+    console.render('#log');
+
+    Y.on('domready', function() {
+        Y.Test.Runner.run();
+        });
+});
+

=== modified file 'lib/lp/answers/templates/question-index.pt'
--- lib/lp/answers/templates/question-index.pt	2011-04-25 22:12:39 +0000
+++ lib/lp/answers/templates/question-index.pt	2011-05-09 14:53:12 +0000
@@ -6,8 +6,8 @@
       i18n:domain="launchpad"
 >
   <head>
-    <style metal:fill-slot="head_epilogue"
-           type="text/css" media="screen">
+    <metal:block fill-slot="head_epilogue">
+    <style type="text/css" media="screen">
   div.confirmBox {
     margin: 0;
     padding-right: 0.5em;
@@ -16,6 +16,15 @@
     font-size: smaller;
   }
     </style>
+    <script type="text/javascript">
+        LPS.use('base', 'node', 'event',
+                'lp.answers.question_spam', function(Y) {
+        Y.on('domready', function() {
+            Y.lp.answers.question_spam.setup_spam_links();
+        });
+      });
+    </script>
+    </metal:block>
   </head>
   <body>
     <metal:registering fill-slot="registering">
@@ -90,7 +99,6 @@
         </div>
       </div>
 
-
       <tal:message repeat="message view/visible_messages">
       <div tal:replace="structure message/@@+display" />
       </tal:message>

=== modified file 'lib/lp/answers/templates/questionmessage-display.pt'
--- lib/lp/answers/templates/questionmessage-display.pt	2011-04-27 18:57:39 +0000
+++ lib/lp/answers/templates/questionmessage-display.pt	2011-05-09 14:53:12 +0000
@@ -1,10 +1,12 @@
 <tal:root
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
+  define="index context/index"
   omit-tag="">
 <div
   tal:define="css_classes view/getBoardCommentCSSClass"
-  tal:attributes="class string:${css_classes}">
+  tal:attributes="class string:${css_classes};
+                  id string:comment-${index}">
   <div class="boardCommentDetails">
     <tal:bestanswer condition="view/isBestAnswer">
       <img src="/@@/favourite-yes" style="float:right;" alt="Best"
@@ -41,5 +43,19 @@
               value="This Solved My Problem" />
     </form>
   </div>
+
+  <div
+    tal:condition="view/canSeeSpamControls"
+    class="boardCommentFooter">
+    <a tal:attributes="id string:mark-spam-${index};"
+       class="js-action sprite edit mark-spam" href="#">
+         <tal:not-spam condition="not: context/visible">
+         Mark as not spam
+         </tal:not-spam>
+         <tal:spam condition="context/visible">
+         Mark as spam
+         </tal:spam>
+    </a>
+  </div>
 </div>
 </tal:root>

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-05-03 04:39:43 +0000
+++ lib/lp/testing/factory.py	2011-05-09 14:53:12 +0000
@@ -285,6 +285,7 @@
     run_with_login,
     temp_dir,
     time_counter,
+    with_celebrity_logged_in,
     )
 from lp.translations.enums import RosettaImportStatus
 from lp.translations.interfaces.potemplate import IPOTemplateSet
@@ -486,6 +487,15 @@
         login_as(person)
         return person
 
+    @with_celebrity_logged_in('admin')
+    def makeAdministrator(self, name=None, email=None, password=None):
+        user = self.makePerson(name=name,
+                               email=email,
+                               password=password)
+        administrators = getUtility(ILaunchpadCelebrities).admin
+        administrators.addMember(user, administrators.teamowner)
+        return user
+
     def makeRegistryExpert(self, name=None, email='[e-mail]',
                            password='test'):
         from lp.testing.sampledata import ADMIN_EMAIL