Closed
Bug 852750
Opened 11 years ago
Closed 11 years ago
Flags requested of me, shows only requested flags in non-restricted bugs
Categories
(bugzilla.mozilla.org :: MyDashboard, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: gkw, Assigned: dkl)
References
Details
Attachments
(1 file, 2 obsolete files)
11.24 KB,
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
The flags requested of me section (e.g. feedback?) seems to show only bugs from non-restricted bugs, not from restricted bugs (e.g. s-s bugs). I had 2 feedback? flags set for me from 2 bugs, one s-s and the other not, but I only saw the one from the non-s-s bug in the Dashboard. I am logged in. The s-s bug is bug 849014 and the non-s-s bug is bug 850070. (It will be nice to have confirmation of this, since I'm not too sure myself)
Assignee | ||
Comment 1•11 years ago
|
||
I was not able to explicitly reproduce this issue but it made me think that the way I was querying the database to get the flags using direct SQL is more error prone than should be. This patch simplifies the way I get the flags that are displayed by utilizing the built in functions more for filtering privacy, etc. I want glob to take a look to sanity check before we make this type of change live. dkl
Assignee: nobody → dkl
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #728971 -
Flags: review?(glob)
Comment on attachment 728971 [details] [diff] [review] Patch to simplify how flags are queried for my dashboard (v1) Review of attachment 728971 [details] [diff] [review]: ----------------------------------------------------------------- very nice refactoring :) works and for me shows a flag which the current query incorrectly misses. however this creates a bug object individually for each bug, so the db load is much higher than grabbing all the data in a single query. as discussed on irc, it should be possible to minimise this by using flag matching, then using a single query to grab the bug's status and summary. ::: extensions/MyDashboard/lib/Queries.pm @@ +183,5 @@ > + my $matched = Bugzilla::Flag->match($match_params); > + > + my @flags; > + foreach my $flag (@$matched) { > + next if (!$flag->bug->isopened && !$include_resolved); the checkbox is "resolved", however you're filtering on all closed states, not just resolved. the checkbox's label should probably be changed to "show closed".
Attachment #728971 -
Flags: review?(glob) → review-
Assignee | ||
Comment 3•11 years ago
|
||
Not sure if this is much simpler as far as lines of code as the original but here is a revised version that does not load full bug objects. Also I changed "Show Resolved" to "Show Closed" as well. Please review dkl
Attachment #728971 -
Attachment is obsolete: true
Attachment #729760 -
Flags: review?(glob)
Assignee | ||
Comment 5•11 years ago
|
||
New patch changed to work with commit for bug 856110. dkl
Attachment #729760 -
Attachment is obsolete: true
Attachment #729760 -
Flags: review?(glob)
Attachment #731281 -
Flags: review?(glob)
Comment on attachment 731281 [details] [diff] [review] Patch to simplify how flags are queried for my dashboard (v3) Review of attachment 731281 [details] [diff] [review]: ----------------------------------------------------------------- r=glob this patch contains trailing whitespace, please remove. ::: extensions/MyDashboard/lib/Queries.pm @@ +229,5 @@ > + # 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'}}; no need to do the visibility check again
Attachment #731281 -
Flags: review?(glob) → review+
Assignee | ||
Comment 7•11 years ago
|
||
Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bmo/4.2 modified Bugzilla/Flag.pm modified extensions/MyDashboard/lib/Queries.pm modified extensions/MyDashboard/lib/WebService.pm modified extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl modified extensions/MyDashboard/web/js/flags.js Committed revision 8717.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•