launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/extend-privacy-notification-to-comments into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/extend-privacy-notification-to-comments/+merge/69176

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

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.

-- 
https://code.launchpad.net/~jcsackett/launchpad/extend-privacy-notification-to-comments/+merge/69176
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/extend-privacy-notification-to-comments into lp:launchpad.

=== modified file 'lib/lp/app/javascript/privacy.js'
--- lib/lp/app/javascript/privacy.js	2011-07-21 18:54:54 +0000
+++ lib/lp/app/javascript/privacy.js	2011-07-25 22:47:26 +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
@@ -46,7 +50,7 @@
         }
 
         Y.one('.global-notification-close').on('click', function(e) {
-            hide_privacy_notification(true);
+            hide_privacy_notification(highlight);
             e.halt();
         });
     }

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2011-07-08 11:46:41 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2011-07-25 22:47:26 +0000
@@ -106,6 +106,17 @@
 
   <metal:load-lavascript use-macro="context/@@+base-layout-macros/load-javascript" />
 
+  <tal:if condition="request/features/bugs.private_notification.enabled">
+    <script type="text/javascript">
+      var privacy_notification_enabled = true;
+    </script>
+  </tal:if>
+  <tal:if condition="not:request/features/bugs.private_notification.enabled">
+    <script type="text/javascript">
+      var privacy_notification_enabled = false;
+    </script>
+  </tal:if>
+
   <script id="base-layout-load-scripts" type="text/javascript">
     LPS.use('node', 'event-delegate', 'lp', 'lp.app.links', 'lp.app.longpoll', function(Y) {
         Y.on('load', function(e) {

=== 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-25 22:47:26 +0000
@@ -12,8 +12,7 @@
     'BugCommentView',
     'BugCommentXHTMLRepresentation',
     'build_comments_from_chunks',
-    'group_comments_with_activity',
-    ]
+    'group_comments_with_activity', ]
 
 from datetime import (
     datetime,
@@ -333,6 +332,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/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-07-21 18:14:44 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-07-25 22:47:26 +0000
@@ -43,11 +43,12 @@
     /*
      * Display the privacy notification if the bug is private
      */
-    if (bug_private) {
-        Y.on("domready", function () {
+
+    Y.on("domready", function () {
+        if (Y.one(document.body).hasClass('private')) {
             Y.lp.app.privacy.display_privacy_notification();
-        });
-    }
+        }
+    });
 
     /*
      * Check the page for links related to overlay forms and request the HTML

=== 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-25 22:47:26 +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>

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-07-20 21:21:32 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-07-25 22:47:26 +0000
@@ -10,26 +10,6 @@
       <script type='text/javascript' tal:content="string:var yui_base='${yui}';" />
       <script type='text/javascript' id='available-official-tags-js'
               tal:content="view/available_official_tags_js" />
-      <tal:if condition="context/bug/private">
-        <script type="text/javascript">
-          var bug_private = true;
-        </script>
-      </tal:if>
-      <tal:if condition="not:context/bug/private">
-        <script type="text/javascript">
-          var bug_private = false;
-        </script>
-      </tal:if>
-      <tal:if condition="request/features/bugs.private_notification.enabled">
-        <script type="text/javascript">
-          var privacy_notification_enabled = true;
-        </script>
-      </tal:if>
-      <tal:if condition="not:request/features/bugs.private_notification.enabled">
-        <script type="text/javascript">
-          var privacy_notification_enabled = false;
-        </script>
-      </tal:if>
       <script type="text/javascript">
         LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bugtask_index',
                   'lp.bugs.subscribers_list',