Bug 1146769: cc activity isn't shown

4 files changed, 77 insertions, 48 deletions

diff --git a/extensions/BugModal/lib/ActivityStream.pm b/extensions/BugModal/lib/ActivityStream.pm
index 4d399854f..b66352031 100644
--- a/extensions/BugModal/lib/ActivityStream.pm
+++ b/extensions/BugModal/lib/ActivityStream.pm
@@ -24,10 +24,12 @@ use Bugzilla::Constants;
 #       user_id => actor user-id
 #       comment => optional, comment added
 #       id      => unique identifier for this change-set
+#       cc_only => boolean
 #       activty => [
 #           {
 #               who     => user object
 #               when    => time (string)
+#               cc_only => boolean
 #               changes => [
 #                   {
 #                       fieldname   => field name :)
@@ -55,7 +57,7 @@ sub activity_stream {
         foreach my $change_set (@$stream) {
             $change_set->{id} = $change_set->{comment}
                 ? 'c' . $change_set->{comment}->count
-                : 'a' . ($change_set->{time} - $base_time) . '.' . $change_set->{user_id};
+                : 'a' . ($change_set->{time} - $base_time) . '_' . $change_set->{user_id};
             $change_set->{activity} = [
                 sort { $a->{fieldname} cmp $b->{fieldname} }
                 @{ $change_set->{activity} }
@@ -91,6 +93,7 @@ sub _add_activity_to_stream {
     my ($stream, $time, $user_id, $data) = @_;
     foreach my $entry (@$stream) {
         next unless $entry->{time} == $time && $entry->{user_id} == $user_id;
+        $entry->{cc_only} = $entry->{cc_only} && $data->{cc_only};
         push @{ $entry->{activity} }, $data;
         return;
     }
@@ -98,6 +101,7 @@ sub _add_activity_to_stream {
         time     => $time,
         user_id  => $user_id,
         comment  => undef,
+        cc_only  => $data->{cc_only},
         activity => [ $data ],
     };
 }
@@ -138,13 +142,13 @@ sub _add_activities_to_stream {
 
     # envelope, augment and tweak
     foreach my $operation (@$raw_activity) {
-        # until we can toggle their visibility, skip CC changes
-        $operation->{changes} = [ grep { $_->{fieldname} ne 'cc' } @{ $operation->{changes} } ];
-        next unless @{ $operation->{changes} };
 
         # make operation.who an object
         $operation->{who} = Bugzilla::User->new({ name => $operation->{who}, cache => 1 });
 
+        # we need to track operations which are just cc changes
+        $operation->{cc_only} = 1;
+
         for (my $i = 0; $i < scalar(@{$operation->{changes}}); $i++) {
             my $change = $operation->{changes}->[$i];
 
@@ -245,6 +249,11 @@ sub _add_activities_to_stream {
                 }
                 $i--;
             }
+
+            # track cc-only
+            if ($change->{fieldname} ne 'cc') {
+                $operation->{cc_only} = 0;
+            }
         }
 
         _add_activity_to_stream($stream, date_str_to_time($operation->{when}), $operation->{who}->id, $operation);
diff --git a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl
index 4bc860996..7ea74b469 100644
--- a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl
+++ b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl
@@ -8,25 +8,29 @@
 
 <div id="comment-actions">
   <button type="button" id="comment-tags-btn" style="display:none" class="minor">Tags &#9662;</button>
-  <button type="button" id="comment-toggle-btn" class="minor">Comments &#9662;</button>
+  <button type="button" id="view-menu-btn" class="minor">View &#9662;</button>
 </div>
 
-<menu id="comment-toggle-menu" type="context" style="display:none">
-  <menuitem id="comment-reset" label="Reset"></menuitem>
+<menu id="view-menu" type="context" style="display:none">
+  <menuitem id="view-reset" label="Reset"></menuitem>
   <hr>
-  <menuitem id="comment-collapse-all" label="Collapse All"></menuitem>
-  <menuitem id="comment-expand-all" label="Expand All"></menuitem>
+  <menuitem id="view-collapse-all" label="Collapse All"></menuitem>
+  <menuitem id="view-expand-all" label="Expand All"></menuitem>
+  <menuitem id="view-comments-only" label="Comments Only"></menuitem>
   <hr>
-  <menuitem id="comments-only" label="Comments Only"></menuitem>
+  <menuitem id="view-toggle-cc" label="Show CC Changes"></menuitem>
   [% Hook.process('comments-toggle-menu') %]
 </menu>
 
 [%
   PROCESS bug/time.html.tmpl;
 
-  activity_counter = 0;
   FOREACH change_set IN bug.activity_stream;
-    '<div class="change-set" id="' _ change_set.id _ '">';
+    IF change_set.cc_only;
+      '<div class="change-set cc-only" id="' _ change_set.id _ '" style="display:none">';
+    ELSE;
+      '<div class="change-set" id="' _ change_set.id _ '">';
+    END;
 
     extra_class = "";
     IF change_set.user_id == bug.assigned_to.id;
@@ -38,15 +42,14 @@
     IF change_set.comment;
       PROCESS comment_header comment=change_set.comment;
     ELSE;
-      activity_counter = activity_counter + 1;
-      PROCESS activity_header activities=change_set.activity id=activity_counter;
+      PROCESS activity_header activities=change_set.activity id=change_set.id;
     END;
 
     IF change_set.comment;
       PROCESS comment_body comment=change_set.comment;
     END;
     FOREACH activity IN change_set.activity;
-      PROCESS activity_body activity=activity;
+      PROCESS activity_body activity=activity change_set=change_set;
     END;
 
     '</div>';
@@ -219,16 +222,17 @@
 
 [%
   BLOCK activity_body;
-    '<div class="activity">';
-    has_cc = 0;
+    IF activity.cc_only && !change_set.cc_only;
+      '<div class="activity cc-only" style="display:none">';
+    ELSE;
+      '<div class="activity">';
+    END;
 
     FOREACH change IN activity.changes;
-      '<div class="change">';
-      class = "";
-
-      IF change.fieldname == 'cc';
-        has_cc = 1;
-        class = "activity-cc";
+      IF change.fieldname == 'cc' && !activity.cc_only && !change_set.cc_only;
+        '<div class="change cc-only" style="display:none">';
+      ELSE;
+        '<div class="change">';
       END;
 
       IF change.attachid;
diff --git a/extensions/BugModal/web/bug_modal.css b/extensions/BugModal/web/bug_modal.css
index c3093fa83..76f2f443d 100644
--- a/extensions/BugModal/web/bug_modal.css
+++ b/extensions/BugModal/web/bug_modal.css
@@ -488,6 +488,10 @@ td.flag-requestee {
     outline: 2px solid #0095dd;
 }
 
+.change-set .comment, .change-set .change {
+    padding-bottom: 1px;
+}
+
 .change-head {
     width: 100%;
     background: #eee;
@@ -529,10 +533,8 @@ td.flag-requestee {
     padding: 2px 2px 0 0 !important;
 }
 
-.comment-spinner {
-    font-family: monospace;
-    white-space: nowrap;
-    vertical-align: bottom;
+.change-spinner {
+    width: 29px;
 }
 
 .comment-text {
diff --git a/extensions/BugModal/web/comments.js b/extensions/BugModal/web/comments.js
index f0d689b3e..f9852cd0e 100644
--- a/extensions/BugModal/web/comments.js
+++ b/extensions/BugModal/web/comments.js
@@ -11,32 +11,30 @@ $(function() {
     // comment collapse/expand
 
     function toggleChange(spinner, forced) {
-        // find and cache the id
-        var id = spinner.data('cid');
-        if (!id) {
-            id = spinner.attr('id').match(/\d+$/)[0];
-            spinner.data('cid', id);
-        }
+        var spinnerID = spinner.attr('id');
+        var id = spinnerID.substring(spinnerID.indexOf('-') + 1);
+
+        var activitySelector = $('#view-toggle-cc').data('shown') === '1' ? '.activity' : '.activity:not(.cc-only)';
 
         // non-comment toggle
-        if (spinner.attr('id').substr(0, 1) == 'a') {
+        if (spinnerID.substr(0, 1) == 'a') {
             var changeSet = spinner.parents('.change-set');
             if (forced == 'hide') {
-                changeSet.find('.activity').hide();
+                changeSet.find(activitySelector).hide();
                 changeSet.find('.gravatar').css('width', '16px').css('height', '16px');
                 $('#ar-' + id).hide();
                 spinner.text('+');
             }
             else if (forced == 'show' || forced == 'reset') {
-                changeSet.find('.activity').show();
+                changeSet.find(activitySelector).show();
                 changeSet.find('.gravatar').css('width', '32px').css('height', '32px');
                 $('#ar-' + id).show();
                 spinner.text('-');
             }
             else {
-                changeSet.find('.activity').slideToggle('fast', function() {
+                changeSet.find(activitySelector).slideToggle('fast', function() {
                     $('#ar-' + id).toggle();
-                    if (changeSet.find('.activity:visible').length) {
+                    if (changeSet.find(activitySelector + ':visible').length) {
                         changeSet.find('.gravatar').css('width', '32px').css('height', '32px');
                         spinner.text('-');
                     }
@@ -69,7 +67,7 @@ $(function() {
             $('#ct-' + id).hide();
             if (BUGZILLA.user.id !== 0)
                 $('#ctag-' + id).hide();
-            $('#c' + id).find('.activity').hide();
+            $('#c' + id).find(activitySelector).hide();
             $('#c' + id).find('.comment-tags').hide();
             $('#c' + id).find('.comment-tags').hide();
             $('#c' + id).find('.gravatar').css('width', '16px').css('height', '16px');
@@ -84,7 +82,7 @@ $(function() {
             $('#ct-' + id).show();
             if (BUGZILLA.user.id !== 0)
                 $('#ctag-' + id).show();
-            $('#c' + id).find('.activity').show();
+            $('#c' + id).find(activitySelector).show();
             $('#c' + id).find('.comment-tags').show();
             $('#c' + id).find('.comment-tags').show();
             $('#c' + id).find('.gravatar').css('width', '32px').css('height', '32px');
@@ -93,7 +91,7 @@ $(function() {
         }
         else {
             $('#ct-' + id).slideToggle('fast', function() {
-                $('#c' + id).find('.activity').toggle();
+                $('#c' + id).find(activitySelector).toggle();
                 $('#c' + id).find('.comment-tags').toggle();
                 if ($('#ct-' + id + ':visible').length) {
                     spinner.text('-');
@@ -127,40 +125,56 @@ $(function() {
             toggleChange($(this));
         });
 
-    // comment and tag menus
+    // view and tag menus
 
-    $('#comment-reset')
+    $('#view-reset')
         .click(function() {
             $('.change-spinner:visible').each(function() {
                 toggleChange($(this), 'reset');
             });
         });
 
-    $('#comment-collapse-all')
+    $('#view-collapse-all')
         .click(function() {
             $('.change-spinner:visible').each(function() {
                 toggleChange($(this), 'hide');
             });
         });
 
-    $('#comment-expand-all')
+    $('#view-expand-all')
         .click(function() {
             $('.change-spinner:visible').each(function() {
                 toggleChange($(this), 'show');
             });
         });
 
-    $('#comments-only')
+    $('#view-comments-only')
         .click(function() {
             $('.change-spinner:visible').each(function() {
                 toggleChange($(this), this.id.substr(0, 3) === 'cs-' ? 'show' : 'hide');
             });
         });
 
+    $('#view-toggle-cc')
+        .click(function() {
+            var that = $(this);
+            var item = $('.context-menu-item.hover');
+            if (that.data('shown') === '1') {
+                that.data('shown', '0');
+                item.text('Show CC Changes');
+                $('.cc-only').hide();
+            }
+            else {
+                that.data('shown', '1');
+                item.text('Hide CC Changes');
+                $('.cc-only').show();
+            }
+        });
+
     $.contextMenu({
-        selector: '#comment-toggle-btn',
+        selector: '#view-menu-btn',
         trigger: 'left',
-        items: $.contextMenu.fromMenu($('#comment-toggle-menu'))
+        items: $.contextMenu.fromMenu($('#view-menu'))
     });
 
     function updateTagsMenu() {