Bug 852750 - Flags requested of me, shows only requested flags in non-restricted bugs

r=glob

4 files changed, 81 insertions, 78 deletions

diff --git a/extensions/MyDashboard/lib/Queries.pm b/extensions/MyDashboard/lib/Queries.pm
index 9f4935c9d..69af01d79 100644
--- a/extensions/MyDashboard/lib/Queries.pm
+++ b/extensions/MyDashboard/lib/Queries.pm
@@ -13,6 +13,8 @@ use Bugzilla;
 use Bugzilla::Bug;
 use Bugzilla::CGI;
 use Bugzilla::Search;
+use Bugzilla::Flag;
+use Bugzilla::Status qw(is_open_state);
 use Bugzilla::Util qw(format_time datetime_from);
 
 use Bugzilla::Extension::MyDashboard::Util qw(open_states quoted_open_states);
@@ -162,7 +164,7 @@ sub query_bugs {
 }
 
 sub query_flags {
-    my ($type, $include_resolved) = @_;
+    my ($type, $include_closed) = @_;
     my $user     = Bugzilla->user;
     my $dbh      = Bugzilla->dbh;
     my $date_now = DateTime->now(time_zone => $user->timezone);
@@ -170,87 +172,88 @@ sub query_flags {
     ($type ne 'requestee' || $type ne 'requester')
         || ThrowCodeError('param_required', { param => 'type' });
 
-    my $attach_join_clause = "flags.attach_id = attachments.attach_id";
-    if (Bugzilla->params->{insidergroup} && !$user->in_group(Bugzilla->params->{insidergroup})) {
-        $attach_join_clause .= " AND attachments.isprivate < 1";
+    my $match_params = { status => '?' };
+
+    if ($type eq 'requestee') {
+        $match_params->{'requestee_id'} = $user->id;
+    }
+    else {
+        $match_params->{'setter_id'} = $user->id;
     }
 
-    my $query =
-    # Select columns describing each flag, the bug/attachment on which
-    # it has been set, who set it, and of whom they are requesting it.
-    " SELECT flags.id AS id,
-             flagtypes.name AS type,
-             flags.status AS status,
-             flags.bug_id AS bug_id,
-             bugs.bug_status AS bug_status,
-             bugs.short_desc AS bug_summary,
-             flags.attach_id AS attach_id,
-             attachments.ispatch AS is_patch,
-             requesters.login_name AS requester,
-             requestees.login_name AS requestee,
-             " . $dbh->sql_date_format('flags.modification_date', '%Y-%m-%d %H:%i') . " AS updated
-        FROM flags 
-             LEFT JOIN attachments
-                  ON ($attach_join_clause)
-             INNER JOIN flagtypes
-                  ON flags.type_id = flagtypes.id
-             INNER JOIN bugs
-                  ON flags.bug_id = bugs.bug_id
-             LEFT JOIN profiles AS requesters
-                  ON flags.setter_id = requesters.userid
-             LEFT JOIN profiles AS requestees
-                  ON flags.requestee_id  = requestees.userid
-             LEFT JOIN bug_group_map AS bgmap
-                  ON bgmap.bug_id = bugs.bug_id
-             LEFT JOIN cc AS ccmap
-                  ON ccmap.who = " . $user->id . "
-                  AND ccmap.bug_id = bugs.bug_id ";
-
-    # Limit query to pending requests and open bugs only
-    $query .= " WHERE flags.status = '?' ";
-
-    # Limit to open bugs only unless want to include resolved
-    if (!$include_resolved) {
-        $query .= " AND bugs.bug_status IN (" . join(',', quoted_open_states()) . ") ";
+    my $matched = Bugzilla::Flag->match($match_params);
+
+    return [] if !@$matched;
+
+    my @unfiltered_flags;
+    my %all_bugs; # Use hash to filter out duplicates
+    foreach my $flag (@$matched) {
+        next if ($flag->attach_id && $flag->attachment->isprivate && !$user->is_insider);
+
+        my $data = {
+            id          => $flag->id,
+            type        => $flag->type->name,
+            status      => $flag->status,
+            attach_id   => $flag->attach_id,
+            is_patch    => $flag->attach_id ? $flag->attachment->ispatch : 0,
+            bug_id      => $flag->bug_id,
+            requester   => $flag->setter->login,
+            requestee   => $flag->requestee ? $flag->requestee->login : '',
+            updated     => $flag->modification_date,
+        };
+        push(@unfiltered_flags, $data);
+
+        # Record bug id for later retrieval of status/summary
+        $all_bugs{$flag->{'bug_id'}}++;
     }
 
-    # Weed out bug the user does not have access to
-    $query .= " AND ((bgmap.group_id IS NULL)
-                     OR bgmap.group_id IN (" . $user->groups_as_string . ")
-                     OR (ccmap.who IS NOT NULL AND cclist_accessible = 1) 
-                     OR (bugs.reporter = " . $user->id . " AND bugs.reporter_accessible = 1)
-                     OR (bugs.assigned_to = " . $user->id .") ";
-    if (Bugzilla->params->{useqacontact}) {
-        $query .= " OR (bugs.qa_contact = " . $user->id . ") ";
+    # Filter the bug list based on permission to see the bug
+    my %visible_bugs = map { $_ => 1 } @{ $user->visible_bugs([ keys %all_bugs ]) };
+
+    return [] if !scalar keys %visible_bugs;
+
+    # Get all bug statuses and summaries in one query instead of loading
+    # many separate bug objects
+    my $bug_rows = $dbh->selectall_arrayref("SELECT bug_id, bug_status, short_desc
+                                               FROM bugs
+                                              WHERE " . $dbh->sql_in('bug_id', [ keys %visible_bugs ]),
+                                            { Slice => {} });
+    foreach my $row (@$bug_rows) {
+        $visible_bugs{$row->{'bug_id'}} = {
+            bug_status => $row->{'bug_status'},
+            short_desc => $row->{'short_desc'}
+        };
     }
-    $query .= ") ";
 
-    # Order the records (within each group).
-    my $group_order_by = " GROUP BY flags.bug_id ORDER BY flags.modification_date, flagtypes.name";
+    # Now drop out any flags for bugs the user cannot see
+    # or if the user did not want to see closed bugs
+    my @filtered_flags;
+    foreach my $flag (@unfiltered_flags) {
+        # Skip this flag if the bug is not visible to the user
+        next if !$visible_bugs{$flag->{'bug_id'}};
 
-    my $flags = [];
-    if ($type eq 'requestee') {
-        $flags = $dbh->selectall_arrayref($query .
-                                          " AND requestees.login_name = ? " .
-                                          $group_order_by,
-                                          { Slice => {} }, $user->login);
-    }
+        # Skip closed unless user requested closed bugs
+        next if (!$include_closed
+                 && !is_open_state($visible_bugs{$flag->{'bug_id'}}->{'bug_status'}));
 
-    if ($type eq 'requester') {
-        $flags = $dbh->selectall_arrayref($query .
-                                          " AND requesters.login_name = ? " .
-                                          $group_order_by,
-                                          { Slice => {} }, $user->login);
-    }
+        # Include bug status and summary with each flag
+        $flag->{'bug_status'}  = $visible_bugs{$flag->{'bug_id'}}->{'bug_status'};
+        $flag->{'bug_summary'} = $visible_bugs{$flag->{'bug_id'}}->{'short_desc'};
 
-    # Format the updated date specific to the user's timezone and add the fancy version
-    foreach my $flag (@$flags) {
+        # Format the updated date specific to the user's timezone
+        # and add the fancy human readable version
         $flag->{'updated'} = format_time($flag->{'updated'});
         my $date_then = datetime_from($flag->{'updated'});
+        $flag->{'updated_epoch'} = $date_then->epoch;
         $flag->{'updated_fancy'} = time_ago($date_then, $date_now);
+
+        push(@filtered_flags, $flag);
     }
 
-    return $flags;
+    return [] if !@filtered_flags;
+
+    # Sort by most recently updated
+    return [ sort { $b->{'updated_epoch'} <=> $a->{'updated_epoch'} } @filtered_flags ];
 }
 
 1;
diff --git a/extensions/MyDashboard/lib/WebService.pm b/extensions/MyDashboard/lib/WebService.pm
index 1a9c212f4..69f43f322 100644
--- a/extensions/MyDashboard/lib/WebService.pm
+++ b/extensions/MyDashboard/lib/WebService.pm
@@ -87,8 +87,8 @@ sub run_flag_query {
                             { function => 'MyDashboard.run_flag_query',
                               param    => 'type' });
 
-    my $include_resolved = $params->{include_resolved} || 0;
-    my $results = query_flags($type, $include_resolved);
+    my $include_closed = $params->{include_closed} || 0;
+    my $results = query_flags($type, $include_closed);
 
     return { result => { $type => $results }};
 }
diff --git a/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl b/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl
index 55936f0bf..8b2dfd8bf 100644
--- a/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl
+++ b/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl
@@ -123,8 +123,8 @@
         <span id="requestee_count_refresh" class="bz_default_hidden">
           <span class="items_found" id="requestee_flags_found">0 flags found</span>
           | <a class="refresh" href="javascript:void(0);" id="requestee_refresh">Refresh</a>
-          | <input type="checkbox" id="requestee_resolved">
-            <label for="requestee_resolved">Show resolved</label>
+          | <input type="checkbox" id="requestee_closed">
+            <label for="requestee_closed">Include Closed</label>
         </span>
         <div id="requestee_table"></div>
       </div>
@@ -136,8 +136,8 @@
         <span id="requester_count_refresh" class="bz_default_hidden">
           <span class="items_found" id="requester_flags_found">0 flags found</span>
           | <a class="refresh" href="javascript:void(0);" id="requester_refresh">Refresh</a>
-          | <input type="checkbox" id="requester_resolved">
-            <label for="requester_resolved">Show resolved</label>
+          | <input type="checkbox" id="requester_closed">
+            <label for="requester_closed">Include Closed</label>
         </span>
         <div id="requester_table"></div>
       </div>
diff --git a/extensions/MyDashboard/web/js/flags.js b/extensions/MyDashboard/web/js/flags.js
index 29afefabb..9980c00fb 100644
--- a/extensions/MyDashboard/web/js/flags.js
+++ b/extensions/MyDashboard/web/js/flags.js
@@ -26,7 +26,7 @@ YUI({
     var updateFlagTable = function (type) {
         if (!type) return;
 
-        var include_resolved = Y.one('#' + type + '_resolved').get('checked') ? 1 : 0;
+        var include_closed = Y.one('#' + type + '_closed').get('checked') ? 1 : 0;
 
         counter = counter + 1;
 
@@ -49,7 +49,7 @@ YUI({
             version: "1.1",
             method:  "MyDashboard.run_flag_query",
             id:      counter,
-            params:  { type : type, include_resolved: include_resolved }
+            params:  { type : type, include_closed: include_closed }
         };
 
         var stringified = Y.JSON.stringify(json_object);
@@ -143,7 +143,7 @@ YUI({
     Y.one('#requestee_refresh').on('click', function(e) {
         updateFlagTable('requestee');
     });
-    Y.one('#requestee_resolved').on('change', function(e) {
+    Y.one('#requestee_closed').on('change', function(e) {
         updateFlagTable('requestee');
     });
 
@@ -183,7 +183,7 @@ YUI({
     Y.one('#requester_refresh').on('click', function(e) {
         updateFlagTable('requester');
     });
-    Y.one('#requester_resolved').on('change', function(e) {
+    Y.one('#requester_closed').on('change', function(e) {
         updateFlagTable('requester');
     });
 });