Merge lp:~jcsackett/launchpad/extend-privacy-notification-to-comments into lp:launchpad

[j.c.sackett]

Extends the new privacy notifications to comments on private bugs.

Summary
=======
We're expanding the ribbon style privacy notification seen right now on bugtasks to other contexts. This branch extends it to bugcomments on private bugs.

Preimplementation
=================
Spoke with Curtis Hovey about bugcomments not being currently covered as part of bugs/bugtasks.

Implementation
==============
lib/lp/app/javascript/privacy.js
--------------------------------
display_privacy_notification() has been modified to accept a parameter indicating if a portlet should be hidden on dismissal of the ribbon. This parameter is passed along to hide_privacy_notification, and defaults to true.

lib/lp/bugs/browser/bugcomment.py
---------------------------------
A property has been added to the view code to determine the "hidden" class to be applied to the privacy ribbon. This is basically cargo culted from the other use of the ribbon in bugtasks.

lib/lp/bugs/templates/bugcomment-index.pt
-----------------------------------------
The privacy notification code has been included in the template, and the notification html has been added.

Tests
=====
bin/test -vvcm lp.bugs.browser.tests
firefox lib/lp/app/javascript/tests/test_privacy.html

QA
==
Check that the comments on a private bug have the ribbon. Check that the comments on a public bug do not.

Also, confirm that the dismissal of the ribbon on private bugs highlights the privacy portlet.

[Ian Booth (wallyworld)]

Thanks for such a detailed mp description. Did you run lint?

35 - 'group_comments_with_activity',
36 - ]
37 + 'group_comments_with_activity', ]

Why the above change? I think the original formatting is correct and is used everywhere else. Could you change this back prior to landing?

I was going to ask about adding a yui test for the display_privacy_notification() method but then noticed there appear to be no yui tests for privacy.js :-(

I originally was also going to ask about the privacy_notice_classes property returning '' and not None and then saw that returning '' makes the tales easier to write so I think that's ok.

review: Approve (code *)

[j.c.sackett (jcsackett)]

The tests are in the dependent branch.

That change in the list formatting is just a typo; I'll fix it before landing this branch.

=== modified file 'lib/lp/app/javascript/privacy.js'
--- lib/lp/app/javascript/privacy.js	2011-07-29 14:54:38 +0000
+++ lib/lp/app/javascript/privacy.js	2011-07-29 14:54:39 +0000
@@ -6,8 +6,12 @@
  *
  * This should be called after the page has loaded e.g. on 'domready'.
  */
-function display_privacy_notification() {
+function display_privacy_notification(highlight_portlet_on_close) {
     /* Check if the feature flag is set for this notification. */
+    var highlight = true;
+    if (highlight_portlet_on_close !== undefined) {
+        highlight = highlight_portlet_on_close;
+    }
     if (privacy_notification_enabled) {
         /* Set a temporary class on the body for the feature flag,
          this is because we have no way to use feature flags in
@@ -47,7 +51,7 @@
         }
 
         Y.one('.global-notification-close').on('click', function(e) {
-            hide_privacy_notification(true);
+            hide_privacy_notification(highlight);
             e.halt();
         });
     }

=== modified file 'lib/lp/bugs/browser/bugcomment.py'
--- lib/lp/bugs/browser/bugcomment.py	2011-05-11 18:00:35 +0000
+++ lib/lp/bugs/browser/bugcomment.py	2011-07-29 14:54:39 +0000
@@ -333,6 +333,13 @@
         return 'Comment %d for bug %d' % (
             self.comment.index, self.context.bug.id)
 
+    @property
+    def privacy_notice_classes(self):
+        if not self.context.bug.private:
+            return 'hidden'
+        else:
+            return ''
+
 
 class BugCommentBoxViewMixin:
     """A class which provides proxied Librarian URLs for bug attachments."""

=== modified file 'lib/lp/bugs/templates/bugcomment-index.pt'
--- lib/lp/bugs/templates/bugcomment-index.pt	2010-09-24 22:30:48 +0000
+++ lib/lp/bugs/templates/bugcomment-index.pt	2011-07-29 14:54:39 +0000
@@ -7,7 +7,28 @@
   i18n:domain="launchpad"
 >
   <body>
+    <metal:block fill-slot="head_epilogue">
+    <script>
+        LPS.use('base', 'node', 'lp.app.privacy', function(Y) {
+            Y.on("domready", function () {
+                if (Y.one(document.body).hasClass('private')) {
+                    Y.lp.app.privacy.display_privacy_notification(false);
+                }
+            });
+        });
+    </script>
+    </metal:block>
     <div metal:fill-slot="main" tal:define="comment view/comment">
+      <tal:if condition="request/features/bugs.private_notification.enabled">
+        <div tal:attributes="class string: global-notification ${view/privacy_notice_classes}">
+          <span class="sprite notification-private"></span>
+          This comment is on a private bug
+          <a href="#" class="global-notification-close">
+            <span class="sprite notification-close"></span>
+            Hide
+          </a>
+        </div>
+      </tal:if>
       <h1 tal:content="view/page_title">Foo doesn't work</h1>
       <tal:comment replace="structure comment/@@+box-expanded-reply">
       </tal:comment>