launchpad-reviewers team mailing list archive

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

j.c.sackett has proposed merging lp:~jcsackett/launchpad/decouple-privacy-notifications into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/decouple-privacy-notifications/+merge/69174

Summary
=======
We're trying to extend the privacy on private bugs to other contexts (e.g. bugcomments, branches). As part of that work, we need to decouple the privacy ribbon code from the bugtask code.

A previous branch started this by moving the main functions into privacy.js. This branch starts removing some of the tight coupling between those functions and bugtask_index.js

Preimplementation
=================
None; this is a continuation of previous work.

Implementation
==============
Two changes have been implemented in this branch: the placement of the feature flag guard for the privacy ribbon, and the means bugtasks determine whether to show the ribbon.

The feature flag guard check in the bugtask_template has been moved into the base layout, so code, bugs, teams, &c can have the flag set appropriately.

bugtask_index.pt no longer sets a javascript variable based on the bug's private status; instead, the associated javascript file now uses the body element's private css class status to determine whether to fire off display_privacy_notification; as this is consistent across private contexts, it can be re-used.

Tests
=====
bin/test -vvct bugtask

QA
==
Open a private bug with the privacy feature flag enabled; the ribbon should display as normal.

Confirm that it does not display on a public bug.
-- 
https://code.launchpad.net/~jcsackett/launchpad/decouple-privacy-notifications/+merge/69174
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/decouple-privacy-notifications into lp:launchpad.

=== 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:34:38 +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/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:34:38 +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/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:34:38 +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',