Closed Bug 1074586 Opened 10 years ago Closed 10 years ago

Add "Bugs of Interest" to the dashboard

Categories

(bugzilla.mozilla.org :: MyDashboard, defect)

Production
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Due Date:

People

(Reporter: dylan, Assigned: dylan)

References

(Blocks 4 open bugs)

Details

User Story

We need to collect the following types of events in a 'bug_interest' table:

e1) Alice adds Bob to the CC list of bug. Bob is now interested in that bug (Ref: Bug 1059417)
e2) Bob files new bugs or bugs moved to a component that Alice watches. Alice is now interested in those bugs. (Ref: Bug 1056445)
e3) *REVISED* Alice is involved with Bug $A and it depends on $B, when $B's resolution changes, Alice is interested in Bug $A.

Except for the transitive relationship between bugs, the events that trigger
bug interest would also cause the bugs to show up in the "involved with and
never visited" query.  It is simpler to say "if a user becomes involved with a
bug, or a bug they are involved with establishes a relationship with another
bug, that bug is of interest".

Parts of Bugzilla that will need to interact with this feature:

p1) New table: bug_interest, defined as (id, bug_id, user_id, timestamp)
p2) Hooks after bug creation and bug update to record the events into bug_interest.
p3) A new search term will be added to Search.pm
p4) The MyDashboard extension will get a query that is the union of "Updated
   since last visit" and new query "interesting bugs" which will pull from the
   events in the bugs_interest table. This query will be referred to in this document as "all interesting bugs".
p5) A new webservice end point will be added, along the lines of BugUserLastVisit.
p6) This API could be read-only or read-write, an UI could be added for the user to arbitarily mark a bug as interesting.
    This is outside of the scope of this feature, however.
p7) There must be a way of clearing interest. Visiting the bug or using "Mark as Visited"
   (Bug 1023405) feature My Dashboard could be used for this, perhaps renamed as "Mark as Read".

Notes:

Making the distinction between "clear last visit" and "clear interest" is potentially confusing.
I suppose when the current query is "all interesting bugs" the "Mark as Read" button would do both.

I have a more advanced proposal, that would add a scoring system to bug_interest events. That will be outlined in separate bug to not bog down this one.

Attachments

(1 file, 3 obsolete files)

We need to collect the following types of events in a 'bug_interest' table:

e1) Alice adds Bob to the CC list of bug. Bob is now interested in that bug (Ref: Bug 1059417)
e2) Bob files new bugs or bugs moved to a component that Alice watches. Alice is now interested in those bugs. (Ref: Bug 1056445)
e3) Alice is involved with Bug $A, and Bug $B is added as a dependency of Bug $A. Alice is now interested in Bug $B, which she may or may not be involved with.

Except for the transitive relationship between bugs, the events that trigger
bug interest would also cause the bugs to show up in the "involved with and
never visited" query.  It is simpler to say "if a user becomes involved with a
bug, or a bug they are involved with establishes a relationship with another
bug, that bug is of interest".

Parts of Bugzilla that will need to interact with this feature:

p1) New table: bug_interest, defined as (id, bug_id, user_id, timestamp)
p2) Hooks after bug creation and bug update to record the events into bug_interest.
p3) A new search term will be added to Search.pm
p4) The MyDashboard extension will get a query that is the union of "Updated
   since last visit" and new query "interesting bugs" which will pull from the
   events in the bugs_interest table. This query will be referred to in this document as "all interesting bugs".
p5) A new webservice end point will be added, along the lines of BugUserLastVisit.
p6) This API could be read-only or read-write, an UI could be added for the user to arbitarily mark a bug as interesting.
    This is outside of the scope of this feature, however.
p7) There must be a way of clearing interest. Visiting the bug or using "Mark as Visited"
   (Bug 1023405) feature My Dashboard could be used for this, perhaps renamed as "Mark as Read".

Notes:

Making the distinction between "clear last visit" and "clear interest" is potentially confusing.
I suppose when the current query is "all interesting bugs" the "Mark as Read" button would do both.

I have a more advanced proposal, that would add a scoring system to bug_interest events. That will be outlined in separate bug to not bog down this one.
Blocks: 1074589
Assignee: nobody → dylan
Ehsan, do the above points (e1, e2, and e3) meet your requirements for an extended "last visited"-type dashboard?
Flags: needinfo?(ehsan.akhgari)
Due Date: 2014-10-07
e1 and e2 are fantastic, and I believe they are the last pieces that we need in order to be able to completely survive without reading any bugmail.

However, I'm not sure if e3 is what I would want.  I think in practice, in most cases I wouldn't be interested in getting bug $B on my list when it gets added as a dependency for $A.  Because $A gets modified during the dependency setting, it will already be on my list so I can CC myself on $B if desired.  However, if $A depends on $B and $B's resolution changes, I would want $A to appear in my list again because I may be interested to know that for example $A can now make progress since $B is fixed.  And it would be awesome if going to $A at this point would somehow make it obvious that the bug was changed because $B's resolution changes (right now it can be kind of difficult to know that for things that we don't display in the inline history fields on the bug page, such as the CC list changing, or a dependency getting resolved.

Another thing which would be nice to have (but with a lower priority) would be if there was a way for me to mark a bug so that it never shows up in my "bugs of interest" list.  That use case becomes more important when we fix e2, of course, since the user may watch components as a way to get notified about interesting bugs in that component, but they may not actually be interested in every single bug in that component.
Flags: needinfo?(ehsan.akhgari)
Oh, another thing that would be nice would be if we extended e2 to cover the case where a bug was filed in a component that I watch before the "bugs of interest" feature gets deployed.  As things stand right now, such bugs won't appear on the "changed since last visit" dashboard, and I think they will not appear on the "bugs of interest" dashboard either, so there would be no way of knowing about those bugs...
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #3)
> Oh, another thing that would be nice would be if we extended e2 to cover the
> case where a bug was filed in a component that I watch before the "bugs of
> interest" feature gets deployed.  As things stand right now, such bugs won't
> appear on the "changed since last visit" dashboard, and I think they will
> not appear on the "bugs of interest" dashboard either, so there would be no
> way of knowing about those bugs...

It is conceivable to build this information as a one-time custom script. No promises, but I'll consider this after the other goals are met.
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #2)
> [snip]
> However, I'm not sure if e3 is what I would want.  I think in practice, in
> most cases I wouldn't be interested in getting bug $B on my list when it
> gets added as a dependency for $A.  Because $A gets modified during the
> dependency setting, it will already be on my list so I can CC myself on $B
> if desired.  However, if $A depends on $B and $B's resolution changes, I
> would want $A to appear in my list again because I may be interested to know
> that for example $A can now make progress since $B is fixed.
Ah, I believe I misunderstood in the original bug for this. I have collapsed the bug description, and put the revised summary in the User Story field.

> And it would
> be awesome if going to $A at this point would somehow make it obvious that
> the bug was changed because $B's resolution changes (right now it can be
> kind of difficult to know that for things that we don't display in the
> inline history fields on the bug page, such as the CC list changing, or a
> dependency getting resolved.

I believe this is a separate concern, which can be addressed later.

> Another thing which would be nice to have (but with a lower priority) would
> be if there was a way for me to mark a bug so that it never shows up in my
> "bugs of interest" list.  That use case becomes more important when we fix
> e2, of course, since the user may watch components as a way to get notified
> about interesting bugs in that component, but they may not actually be
> interested in every single bug in that component.

Possibly this can be done if the advanced version of this feature is implemented: Bug 1074589.
I believe some sort of scoring system will eventually be required to sort through the deluge of bugs.
Status: NEW → ASSIGNED
User Story: (updated)
This all looks great, Dylan.  Thanks!
Attached patch bug-1074586-feedback.patch (obsolete) — Splinter Review
Working data colllection, Search term, and MyDashboard query
Attachment #8500416 - Flags: feedback?(dkl)
Attached patch bug-1074586-v1.patch (obsolete) — Splinter Review
Bug Interest feature in for review.
Attachment #8500416 - Attachment is obsolete: true
Attachment #8500416 - Flags: feedback?(dkl)
Attachment #8500851 - Flags: review?(dkl)
Comment on attachment 8500851 [details] [diff] [review]
bug-1074586-v1.patch

Review of attachment 8500851 [details] [diff] [review]:
-----------------------------------------------------------------

Watch you line lengths :)

Mostly works well. Just a few little things to revise and then quick r+.

::: extensions/MyDashboard/Extension.pm
@@ +69,5 @@
> +            modification_time => { TYPE    => 'DATETIME',
> +                                   NOTNULL => 1 }
> +        ],
> +        INDEXES => [
> +            bug_interest_idx         => { FIELDS => [qw(bug_id user_id)], },

TYPE => 'UNIQUE'

@@ +138,5 @@
> +                               WHERE product_id = ?
> +                                 AND (component_id = ?
> +                                  OR  component_id IS NULL
> +                                  OR  ? LIKE CONCAT(component_prefix, '%'))";
> +    my ($user_ids) = $dbh->selectcol_arrayref($query, undef, $self->product_id, $self->id, $self->name);

cache the results in case bug_end_of_update is called after bug_end_of_create one or more times for the same component

::: extensions/MyDashboard/lib/BugInterest.pm
@@ +35,5 @@
> +
> +sub id            { return $_[0]->{id}            }
> +sub bug_id        { return $_[0]->{bug_id}        }
> +sub user_id       { return $_[0]->{user_id}       }
> +sub modification_time { return $_[0]->{modification_time} }

nit: line up {

@@ +47,5 @@
> +        $interest->update();
> +        return $interest;
> +    }
> +    else {
> +        return $class->create({ user_id => $user_id, bug_id => $bug_id, modification_time => $timestamp });

nit:

return $class->create({
    user_id           => $user_id,
    bug_id            => $bug_id,
    modification_time => $timestamp
});

::: extensions/MyDashboard/lib/WebService.pm
@@ +134,5 @@
> +
> +    my $bug_ids = $params->{bug_ids}
> +        or ThrowCodeError('param_required', { function => 'MyDashboard.bug_interest_unmark', param => 'type' });
> +
> +    my @bug_ids = ref($bug_ids) ? @$bug_ids : ( $bug_ids );

s/type/bug_ids/

Also shorten to:

$params->{bug_ids}
    || ThrowCodeError('param_required', { function => 'MyDashboard.bug_interest_unmark', param => 'bug_ids' });
my @bug_ids = ref($params->{bug_ids}) ? @{ $params->{bug_ids} } : ( $params->{bug_ids} );

::: extensions/MyDashboard/web/styles/mydashboard.css
@@ +51,1 @@
>  

nit: remove blank line

::: js/bug.js
@@ +188,5 @@
> +                        console.log("failed to unmark interest: "
> +                            + res.responseText);
> +                },
> +            };
> +

nit: remove extra line

@@ +191,5 @@
> +            };
> +
> +            YAHOO.util.Connect.setDefaultPostHeader('application/json', true);
> +            YAHOO.util.Connect.asyncRequest('POST', 'jsonrpc.cgi', callbacks,
> +                args)

bring args back up to the line above
Attachment #8500851 - Flags: review?(dkl) → review-
Attached patch bug-1074586-v2.patch (obsolete) — Splinter Review
(In reply to David Lawrence [:dkl] from comment #9)
> Watch you line lengths :)

Currently, set to 120 based on previous conversations.
Possibly 100 is more consistent? I know the line length is longer than the 80 we have for upstream. I see the specific constructs that should be shorter, however. :)

> TYPE => 'UNIQUE'
Woops!

> @@ +138,5 @@
> > +                               WHERE product_id = ?
> > +                                 AND (component_id = ?
> > +                                  OR  component_id IS NULL
> > +                                  OR  ? LIKE CONCAT(component_prefix, '%'))";
> > +    my ($user_ids) = $dbh->selectcol_arrayref($query, undef, $self->product_id, $self->id, $self->name);
> 
> cache the results in case bug_end_of_update is called after
> bug_end_of_create one or more times for the same component

Added. I also removed the unused method above this. Maybe we'll need it later, but for right now it was bloat. :)

> ::: extensions/MyDashboard/lib/BugInterest.pm
fixed.

> 
> @@ +47,5 @@
> > +        $interest->update();
> ...
> nit:
fixed.

> 
> ::: extensions/MyDashboard/lib/WebService.pm
> s/type/bug_ids/
Fixed. 

> Also shorten to:

Fixed, but I prefer using postfix "unless" in this case (reads better)

> ::: extensions/MyDashboard/web/styles/mydashboard.css
> nit: remove blank line
Fixed.

> ::: js/bug.js
> @@ +188,5 @@
> > +                        console.log("failed to unmark interest: "
> > +                            + res.responseText);
> > +                },
> > +            };
> > +
> 
> nit: remove extra line
> bring args back up to the line above

Fixed, and the adjacent code too for consistency.
Attachment #8500851 - Attachment is obsolete: true
Attachment #8502788 - Flags: review?(dkl)
There was some noise relating to the double-encoding email bug; this version has been corrected.
Attachment #8502788 - Attachment is obsolete: true
Attachment #8502788 - Flags: review?(dkl)
Attachment #8502791 - Flags: review?(dkl)
Comment on attachment 8502791 [details] [diff] [review]
bug-1074586-v2.1.patch

Review of attachment 8502791 [details] [diff] [review]:
-----------------------------------------------------------------

Can fix on commit. r=dkl

::: Bugzilla/Search.pm
@@ +359,5 @@
> +    bug_interest_ts => {
> +        _non_changed => \&_bug_interest_ts,
> +        _default     => \&_invalid_operator,
> +    },
> + 

nit: remove whitespace on commit

::: extensions/MyDashboard/Extension.pm
@@ +103,5 @@
> +    my $query = "SELECT user_id FROM component_watch
> +                               WHERE product_id = ?
> +                                 AND (component_id = ?
> +                                  OR  component_id IS NULL
> +                                  OR  ? LIKE CONCAT(component_prefix, '%'))";

nit:

my $query = "SELECT user_id FROM component_watch
              WHERE product_id = ?
		    AND (component_id = ?
		         OR component_id IS NULL
		         OR ? LIKE CONCAT(component_prefix, '%'))";

@@ +192,5 @@
> +    my %old_cc = map { $_->id => $_ } grep { defined } @{ $old_bug->cc_users };
> +    my @added = grep { not $old_cc{ $_->id } } grep { defined } @{ $bug->cc_users };
> +    foreach my $user (@added) {
> +        Bugzilla::Extension::MyDashboard::BugInterest->mark($user->id, $bug->id, $timestamp);
> +    }

Lets omit this if the changer is the one being added to the cc list.

::: extensions/MyDashboard/lib/WebService.pm
@@ +128,4 @@
>      return { result => { $type => $results }};
>  }
>  
> +sub bug_interest_unmark {

Sorry I missed before. Need a REST API method for this. Not sure if we should use PUT or DELETE for this. PUT is normally for updating a record and DELETE for totally removing a record. Technically we are removing the entries but I suppose in the abstract case we are merely updating our interesting bugs list.

sub rest_resources {
    return [
        qr{^/bug_interest_unmark$}, {
            PUT => {
                method => 'bug_interest_unmark'
            }
        }
    ];
}
Attachment #8502791 - Flags: review?(dkl) → review+
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   1f84551..ccea670  master -> master
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
This commit broke our Dockerized BMO for ReviewBoard. It is now failing with errors like:

   $ $TESTDIR/testing/bugzilla.py create-bug TestProduct TestComponent bug5
+  Traceback (most recent call last):
+    File "/home/vagrant/version-control-tools/testing/bugzilla.py", line 96, in <module>
+      sys.exit(main(sys.argv[1:]))
+    File "/home/vagrant/version-control-tools/testing/bugzilla.py", line 37, in main
+      client.put(bug)
+    File "/home/vagrant/version-control-tools/pylib/Bugsy/bugsy/bugsy.py", line 109, in put
+      raise BugsyException(result['message'])
+  bugsy.bugsy.BugsyException: Message: DBD::mysql::db selectcol_arrayref failed: Table 'bugs.component_watch' doesn't exist [for Statement "SELECT user_id FROM component_watch
+                    WHERE product_id = ?
+                      AND (component_id = ?
+                           OR component_id IS NULL
+                           OR ? LIKE CONCAT(component_prefix, '%'))"] at /var/lib/bugzilla/bugzilla/extensions/MyDashboard/Extension.pm line 109.

Because the component watching feature doesn't work with a fresh database on the BMO code base (not sure if there is a bug on that), we have disabled the component watching feature in our Dockerized BMO.

Unfortunately, it appears this patch assumes the component watching feature is enabled. When run against an install without component watching, it fails.

I can pin our Dockerized BMO install to the commit before this patch as a stop-gap to get our tests to pass.

Potential solutions:

1) Change this patch so it doesn't require component watching (possibly while this patch is backed out)
2) Fix BMO code base to work with component watching from a fresh database
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Backing out this commit. We either move it to the component watching component or make dependency on component watching optional.

That said, we really shouldn't have to disable component watching. If there isn't already a bug for that, one should be filed.

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   ccea670..2f5045a  master -> master
Status: REOPENED → ASSIGNED
It is looking like dkl will have fixex the problems that require component watching to be disabled.
I'm resolving this bug (and re-applying the new feature) so that this gets out in the next push.
Additional discussion about ComponentWatching causing a clean install to fail can be directed to Bug 1082113, which is fixing this.

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   41e58e2..b348b6a  master -> master
Status: ASSIGNED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Fixed typo / syntax error.

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   b348b6a..6d2defe  master -> master
Summary: New Feature: Bugs of Interest → Add "Bugs of Interest" to the dashboard
Blocks: 1276801
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: