11368 - Move all bug activity/history onto main bug screen (display it inline with comments)

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement, P2)

Description

[Matthew Tuck [:CodeMachine]]

I have a feeling this one's going to be controversial, but I believe it would be
beneficial.

Currently, normal actions don't log anything in the "Description" section.  I
propose this should be turned into "History", and all bug activity shown here.
Here's why.

(1) The view bug activity is currently is a separate place.  Hence it is
difficult to cross-reference events and comments temporally.
(2) People _want_ this information on the main form.  You already have a
precedent for it - x being made a duplicate of this bug.  "Moving to Mx" is a
very common message, and there are many like it.  Since it will be put there
already, why not automatically generate it?
(3) Since it is in a separate place it takes longer to get to.

So I propose that there be no separate bug activity.  You could make this a
preference, I hope this wouldn't prevent some people still adding comments like
mentioned in (2).

Comment 1

[ekrock's old account (dead)]

Though well intentioned, I'd prefer that we not do this as it would add more
noise to the descriptions.  If anything, I think we should do the reverse and
stop putting "Moving to ..." comments in the description and instead track that
in View Bug Activity only.  My .02.

Comment 2

[Matthew Tuck [:CodeMachine]]

Note that I did mention this could be a user preference.  I personally would
prefer this view, since you don't miss anything.

It's not the reverse to get rid of the "moving to" messages, this is part of the
point.

Comment 3

[Terry Weissman]

[e-mail] is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)

Comment 4

[Dave Miller [:justdave]]

What about doing it the other way around, and including comments in the bug 
activity page?  Or maybe a separate "complete bug history" link which would 
combine the two?

Comment 5

[Chris Yeh]

this would be a lot easier if the sql wasn't so intertwined with the bug 
displays.

Comment 6

[Jesse Ruderman]

Apache's bugs look like this: http://bugs.apache.org/index.cgi/full/393

Comment 7

[Matthew Tuck [:CodeMachine]]

I think the best thing we could do here is to provide all three views, with the
current as default.  As long as it is possible to view them together, I think
the first two problems should be mostly solved.

Maybe make it a user default too.

> Apache's bugs look like this ...

I would hope we could do something a little nicer than that.

Comment 8

[Stephan Niemz]

Adding default QA contact to all open Webtools/Bugzilla bugs lacking one.
Sorry for the spam.

Comment 9

[Wouter Leeuwis]

I am surprised that this issue has been open for so long without it being 
addressed. I am evaluating some bug tracking tools at the moment and one of my 
main requirements is that I have an instant overview of the bug history. Any 
chance of this getting the option to view it as you like and setting that view 
option as default?

Comment 10

[GavinS]

Bug#98123 will/could provide the user param to specify how the user would like 
the bug activity to be displayed.

Comment 11

[Matthew Tuck [:CodeMachine]]

Moving to new Bugzilla product ...

Comment 12

[Dave Miller [:justdave]]

*** Bug 140864 has been marked as a duplicate of this bug. ***

Comment 13

[Bradley Baetz (:bbaetz)]

*** Bug 179840 has been marked as a duplicate of this bug. ***

Comment 14

[Bradley Baetz (:bbaetz)]

*** Bug 191539 has been marked as a duplicate of this bug. ***

Comment 15

[Dave Miller [:justdave]]

*** Bug 109358 has been marked as a duplicate of this bug. ***

Comment 16

[Dave Miller [:justdave]]

*** Bug 224910 has been marked as a duplicate of this bug. ***

Comment 17

[Dave Miller [:justdave]]

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.

Comment 18

[Wayne Mery (:wsmwk)]

user pref or default - but should be configurable

Comment 19

[Frédéric Buclin]

The trunk is now frozen to prepare Bugzilla 2.22. Only bug fixes are accepted, no enhancement bugs. As this bug has a pretty low activity (especially from the assignee), it's retargetted to ---. If you want to work on it and you think you can have it fixed for 2.24, please retarget it accordingly (to 2.24).

Comment 20

[Rémi Zara]

bug 326581 has a patch and proof of concept script to do this with AJAX.
Bug activity is displayed on demand on the bug page, and you retain the possibility to have a separate page.

Comment 21

[Frédéric Buclin]

*** Bug 230058 has been marked as a duplicate of this bug. ***

Comment 22

[Frédéric Buclin]

Looks like I have to fix it myself if we want to see it being fixed before 2012.

Comment 23

[Frédéric Buclin]

*** Bug 345097 has been marked as a duplicate of this bug. ***

Comment 24

[Frédéric Buclin]

This bug is retargetted to Bugzilla 3.2 for one of the following reasons:

- it has no assignee (except the default one)
- we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1)
- it's not a blocker

If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0.

If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug.

If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".

Comment 25

[Jens Bannmann]

(In reply to comment #7)
> I think the best thing we could do here is to provide all three views, with the
> current as default.

Yeah, that would be great. Often I find myself looking over the bug activity, memorizing a time stamp and then going back to the main bug page to find out what the user said about this change.

At my company, we use Atlassian JIRA, and I have grown used to having this feature. For an example, visit http://jira.atlassian.com/browse/JRA-9?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel . Note how comments and activity is mixed, and that you can switch to "just change history" and "just comments", the latter being the default. If you change   it, all issues will display in this mode.
What Bugzilla does better, though, is to group changes to different fields occuring at the same time.

Comment 26

[Olav Vitters]

Don't change the intention of this bug while I'm assigned without also either reassigning or adding a patch.

Comment 27

[Todd Stansell]

Attachment: proof of concept, based on 2.22

Here's my changes to support this feature, based on a customized version of bugzilla 2.22.  This won't actually apply directly to a 2.22 codebase, since we have a fairly customized version in-house.  This is meant more as a possible route to take with this request.  I'll be integrating this into the 3.0 branch soon, so I can get an official patch for that shortly.  I will attach a png of what our version looks like next.

Todd

Comment 28

[Todd Stansell]

Attachment: proof of concept screen capture

Here's a png of a test bug that used the previous code...I'll obsolete the patch too, since it's not an official patch by any means.

Todd

Comment 29

[Todd Stansell]

Attachment: v1 against 3.0rc1

I've got a couple of patches.  One for 3.0rc1 and the other that should apply cleanly to the trunk.  This one is against 3.0rc1.  There's a user preferences (default is off) that determines if we should display activity inline with the comments.  The major changes in the code involves the comments.html.tmpl template where we use Template functions to merge the comments and the activity based on the date of each.

Todd

Comment 30

[Todd Stansell]

Attachment: v1 against trunk (3.1)

Here's the patch against the trunk.  I'll go ahead and add review? to see if this is something we can get a review on...and if folks think this is a good way to go.  One other think I forgot to mention was that the patch includes the ability to link to a show_bug.cgi page and include &activity=on to force the activity to be shown.  Anyway, enough about all this.  I'll await some feedback.

Todd

Comment 31

[Todd Stansell]

Comment on attachment 261741 [details] [diff] [review]
v1 against trunk (3.1)

FYI, I have not tested this code, except for the 3.0rc1 patch.  There's very little difference between that and this one, but someone should actually test this.

Comment 32

[timeless]

Comment on attachment 261741 [details] [diff] [review]
v1 against trunk (3.1)

>+my $inline_activity = defined $cgi->param('activity') ?
>+                      $cgi->param('activity') :
>+		      Bugzilla->user->settings->{'inline_activity'}->{'value'};

indentation here is bad. you're using tabs. don't.

>Index: Bugzilla/Bug.pm
>+            ", bugs_activity.removed, bugs_activity.added, profiles.login_name, profiles.realname

this shouldn't be needed...
we already have a user object.

>Index: Bugzilla/Install.pm
>@@ -57,11 +57,13 @@
>+    inline_activity    => { options => ['on', 'off'], default => 'off' }

this worries me.

>Index: template/en/default/bug/comments.html.tmpl
>+[% operations = [] %]
> [% IF (start_at > 0) %]
>-    [% sort_order = "oldest_to_newest" %]
>+  [%# don't show inline activity on midair collision page %]
i'm pretty sure we use 4 space indentation for [] blocks.
>+  [% sort_order = "oldest_to_newest" %]

>+      [%# This is an activity ... %]

=> These are changes ...

>@@ -144,13 +185,30 @@

>+            Activity
>+          [% END %]
>+          From

No.

Comment From
Change(s) By

>Index: template/en/default/global/setting-descs.none.tmpl
>@@ -39,7 +39,8 @@
>+   "inline_activity"                  => "Show bug activity inline with comments",

no. inline activity should be at least available per bug instead of as a per user preference.

99% of the time most users won't want this. but sometimes a user might want it for a single bug, being forced to turn on a global user pref and then turn it off again.

Comment 33

[Andrés G. Aragoneses]

(In reply to comment #32)
> no. inline activity should be at least available per bug instead of as a per
> user preference.

I don't agree. I am an admin of a Bugzilla installation and I would like to add this view for all bugs and for all users by default. They are really confused currently when I tell them to use the link "View bug activity". Thus, we want a global option feature please.

Comment 34

[Mark Clements]

I was imagining it would work a bit like the search page, which remembers whether your last search was 'basic' or 'advanced' and shows the same page again next time.

I would imagine a global preference to set the default, but that a cookie would store the most recent state for a user and use that next time.  A link on the bug page would allow the user to toggle between the two views easily.

If it doesn't use this method, then I think it should probably have a global setting for the default value (as per comment 33) plus a per-user setting that overrides this.  That will cover most use cases (for example, I would always choose to have in-line activity turned on).

Comment 35

[Frédéric Buclin]

It's fine to have a user pref to control this behavior. There is no need to have a per-bug pref.

Comment 36

[Todd Stansell]

> >+my $inline_activity = defined $cgi->param('activity') ?
> >+                      $cgi->param('activity') :
> >+                  Bugzilla->user->settings->{'inline_activity'}->{'value'};
> 
> indentation here is bad. you're using tabs. don't.

Yep.  Corrected.

> >Index: Bugzilla/Bug.pm
> >+            ", bugs_activity.removed, bugs_activity.added, profiles.login_name, profiles.realname
> 
> this shouldn't be needed...
> we already have a user object.

We need the real name for the activity so we can display it along with the comments.  I'm simply adding this to the existing sql query just as the GetBugComments function works.  Would you rather I change the activity structure so that it returns a user object instead (which would require more changes than just what this bug is for)? -57,11 +57,13 @@
> >+    inline_activity    => { options => ['on', 'off'], default => 'off' }
> 
> this worries me.

Why?  This gives us users who always want to display inline activity that option.  Many of us want this.

> >Index: template/en/default/bug/comments.html.tmpl
> >+[% operations = [] %]
> > [% IF (start_at > 0) %]
> >-    [% sort_order = "oldest_to_newest" %]
> >+  [%# don't show inline activity on midair collision page %]

I thought templates used 2-space indentation for everything.  It's very inconsistent when trying to look through existing documents ... and from the Bugzilla Developer's Guide at:

  http://www.bugzilla.org/docs/developer.html
  "Indentation - HTML and XML Templates should have a 2-space indent."

> >+  [% sort_order = "oldest_to_newest" %]
> 
> >+      [%# This is an activity ... %]
> 
> => These are changes ...

Clarified the comments and used the term "changes" instead of "activity".

> >@@ -144,13 +185,30 @@
> 
> >+            Activity
> >+          [% END %]
> >+          From
> 
> No.
> 
> Comment From
> Change(s) By

Corrected.

> >Index: template/en/default/global/setting-descs.none.tmpl
> >@@ -39,7 +39,8 @@
> >+   "inline_activity"                  => "Show bug activity inline with comments",
> 
> no. inline activity should be at least available per bug instead of as a per
> user preference.
> 
> 99% of the time most users won't want this. but sometimes a user might want it
> for a single bug, being forced to turn on a global user pref and then turn it
> off again.

This isn't true.  We need to give the user the option of viewing inline activity for every bug, all the time.  This is how many of us use it at our company.  We turn it on and never go back.  Many times you want to know which fields changed while you're reading some comment, so having it there all the time is very convenient.

That being said, we might need a UI bit on the show_bug.cgi page that allows you to toggle what's currently being displayed (via the activity=on|off parameter)?  Any idea where this should go?  Or just let folks add "&activity=on|off" to the URL to force it one way or another? :)

After we decide on this, I'll submit an updated patch...

Todd

Comment 37

[Andrés G. Aragoneses]

Todd, any updates on this one? It would be nice to have this fixed on next version. Sorry for the bugspam.

Comment 38

[Todd Stansell]

Sorry about the lack of any updates.  Our company got acquired and we've been dealing with that integration.  Bugzilla changes have taken a backseat.  That being said, we ended up going with allowing folks to override this setting on the show_bug.cgi page by manually adding &activity=on|off to the url.  No UI for this.  Everyone seems pretty happy with just the per-user preference at our company.

I'll work on getting an updated patch against the tip that includes this change and get it posted.

Todd

Comment 39

[Max Kanat-Alexander]

Bugzilla 3.2 is now frozen. Only enhancements blocking 3.2 or specifically approved for 3.2 may be checked in to the 3.2 branch. If you would like to nominate your enhancement for Bugzilla 3.2, set "blocking3.2" tp "?", and either the target milestone will be changed back, or the blocking3.2 flag will be granted, if we will accept this enhancement for Bugzilla 3.2.

Comment 41

[David Marshall]

Attachment: screenshot of Y! approach

Here's a screenshot of a what Yahoo! is doing for inline changes.  The fonts and width are not right, but this is the idea.  The guts of my local patch are forthcoming to serve as a basis for discussion of the overall idea before I prepare a patch against HEAD.

Comment 42

[Rob Siklos [:robzilla]]

Oh - that Yahoo thing is nice!  I would love to see that in bugzilla.

Comment 43

[David Marshall]

Attachment: partial yahoo patch (does not apply to HEAD)

Here's a partial patch of how I ported the capability to our new 3.2.3-rooted trunk.  The key changes are a comments_and_activity method (to Bugzilla::Bug for you guys) that retrieves a list of all comments and activities, tweaking Bugzilla::Bug::GetActivity to return an author object (corresponding with the same behavior in GetComments), and the template changes (included here as most of our custom/bug/comments.html.tmpl).  There are some accompanying CSS changes that I think I imported to our trunk separately, so they aren't included here.

Right now, we have 'change marks' that are distinct from 'marks' for comments.  That's kind of bogus, I'm definitely going to refactor that.

If there aren't significant objections to this approach, I will start to work on a patch against HEAD.

Comment 44

[Max Kanat-Alexander]

hey David, thanks for posting your patch. Surprisingly, I wasn't CC'ed on this bug so I didn't see it for feedback until just now. What I'd like to see is a fixing of bug 492674 first, and then we can just use the Bugzilla::ChangeSet objects for everything instead of having to create a whole new Bug subroutine.

Comment 45

[Frédéric Buclin]

We no longer accept new features for Bugzilla 3.6. Retargetting to 3.8.

Comment 47

[Mike Beltzner [:beltzner, not reading bugmail]]

Note that the solution from bug 539081 is a little more robust than what's suggested in the Yahoo! solution. Ehsan/Johnath's changes also do fun thinks like linkify flag names and have them jump to the most recently associated comment. Swweeeeet!

Comment 48

[Max Kanat-Alexander]

Yeah, Ehsan's stuff full stuff is awesome. I'd like to focus first in this bug on the most basic functionality, and then add in the other bits in other bugs, since some of them may actually be tricky to get fully right.

Comment 49

[Frédéric Buclin]

(In reply to comment #47)
> Ehsan/Johnath's changes also do fun thinks
> like linkify flag names and have them jump to the most recently associated
> comment.

We already have a bug for that. No need to file another dupe (just in case).

Comment 50

[Michael Thomas (Mockodin)]

What happens for activity not associated to a comment? It would seem odd to incorporate that in to a comment body for which it does not apply?

I have a similar patch on standby that I was considering posting, hadn't seen this one yet, I don't wish to step on toes however. I have a working, when the sites it up, sample on http://bugzilla.zplace.com. Not completely bug free atm, if nothing else perhaps it might have some ideas in it that may be useful?

Comment 51

[Max Kanat-Alexander]

(In reply to comment #50)
> What happens for activity not associated to a comment?

  See: http://ehsanakhgari.org/blog/2010-01-07/bugzilla-tweaks-enhanced

Comment 52

[David Marshall]

Anyone who would care to take this on should feel free to do so.  I don't know when (or even if, to be honest) I will be able to contribute any more Yahoo! changes (which would include integrating my quasi-patch into HEAD).

I like Ehsan's idea very much, FWIW.  That may be a more effective way to present the activity information.

I should add that aside from performance improvements, this is by far the most popular and successful change we've made to Y! Bugzilla in my tenure.  It's incredibly useful to have the activity inline with the comments.

Comment 53

[Michael Thomas (Mockodin)]

(In reply to comment #52)
I'll take a look at Ehsan's tweaks this is something we are actively doing here, my development schedule has been extended to I have more time

Comment 54

[Frédéric Buclin]

Reassigning to nobody per comment 52.

Comment 55

[Guy Pyrzak]

I am kinda interested in doing this. I'll see how easy it is to integrate the patches into the head in march if no one takes this first.

Comment 56

[[not reading bugmail]]

If your going to do this, can there be an option to hide all the bug history in your preferences or per bug toggle on and off.  Some of the mockups looks hard to read.

Comment 57

[[not reading bugmail]]

(In reply to comment #53)
> (In reply to comment #52)
> I'll take a look at Ehsan's tweaks this is something we are actively doing
> here, my development schedule has been extended to I have more time

The bugzilla tweaks addon was just updated:
https://addons.mozilla.org/en-US/firefox/addon/187588/

Comment 58

[Max Kanat-Alexander]

reed: TMs are only set to a specific non-.0 release if either:

  (a) The bug has an assignee and they are committing to perform the work for that release.
  (b) The feature has been set as a roadmap feature for that specific release.

  So if you'd like to take this bug and set it to 4.2, then please do. However, there is currently no assignee.

Comment 60

[LegNeato]

Ah, couldn't find this bug when I wrote bug 603261. Figured there must have been an existing one...

Comment 61

[LegNeato]

Attachment: WIP patch against trunk

Comment 62

[LegNeato]

Attachment: How the WIP looks (note I have not worked on the CSS at all, so it is ugggggly)

Comment 63

[LegNeato]

Some features of my WIP:

* User option to turn it off
* Lots of CSS stubs to support tweaking the look
* History items by the same person at the same time of a comment are collapsed into the comment
* History items w/o an associated comment are placed inline (with css so multiple in a row can be treated as one block or each individual date/action can be treated as one block)
* Supports multiple comments and multiple history changesets for a given time (not sure if this could ever happen for comments, but the timestamps are down to the second and I imagine there is the slim possibility of 2 comments happening milliseconds apart)
* Works with all comment sort option prefs
* Output fields and values in the same format as the activity table

Comment 64

[Reed Loden [:reed]]

Comment on attachment 482258 [details] [diff] [review]
WIP patch against trunk

>+    # 2010-10-08 [e-mail]
>+    inline_history     => { options => ['on', 'off'], default => 'on' },

Add the bug #, please.

>+# Templates will need to covert from strings to datetimes

convert

>+if( Bugzilla->user->settings->{'inline_history'}->{'value'} == "on" ) {

eq, not ==. eq/ne is for strings. ==/!= is for numerals. Also, I think the standard style is single-quotes for things like that...

Comment 65

[Max Kanat-Alexander]

It's awesome that you're working on this, Christian! :-)

As far as the screenshot goes, are you re-using the existing comment header code? I'd imagine that you should be, in some way or another, or at least re-using the same CSS classes.

Comment 66

[(no longer active)]

I'm curious how you're dealing with CC changes.  In my experiments with Bugzilla Tweaks, CC changes on a bug turned to be a huge volume of mostly noise data, so I ended up hiding them by default, and adding a context menu entry to show them if needed...  But of course, the context menu solution won't work here.

Comment 67

[LegNeato]

(In reply to comment #65)
> As far as the screenshot goes, are you re-using the existing comment header
> code? I'd imagine that you should be, in some way or another, or at least
> re-using the same CSS classes.

For the case where the history items happened on the same time as the comments I am just injecting them into the comment. I haven't decided if the standalone history items should look like comments or should be visually more lightweight (as the extension does). I'm still playing around with CSS, I'll post some options for review.

(In reply to comment #66)
> I'm curious how you're dealing with CC changes.  In my experiments with
> Bugzilla Tweaks, CC changes on a bug turned to be a huge volume of mostly noise
> data, so I ended up hiding them by default, and adding a context menu entry to
> show them if needed...  But of course, the context menu solution won't work
> here.

I was planning on not showing cc changes at all. I agree it would cause a lot of noise and rarely do people care about the cc changes. If they do, they can view the activity page. If someone has a better idea I'm all ears though.

Also, I'd want to make the cc name display nice, and I believe the change object only stores their id/email. It would add a ton of queries for active bugs to nicely display the cc comings and goings.

Comment 68

[LegNeato]

Attachment: Refreshed WIP, still no nice CSS

Here's an updated WIP...anyone should feel free to play around with the CSS as it is bare-bones/plain/ugly right now

Comment 69

[:glob ✱]

(In reply to comment #68)
> Created attachment 488399 [details] [diff] [review]
> Refreshed WIP, still no nice CSS

using this patch i no longer see bug comments (or activity).

Comment 70

[LegNeato]

Attachment: Refreshed WIP, still no nice CSS v2

Whoops, forgot to diff a file. This should work

Comment 71

[:glob ✱]

> Whoops, forgot to diff a file. This should work

yup, works when displaying a bug; however the comments are still missing when a bug is displayed after making changes.

Comment 72

[LegNeato]

Ok, I can reproduce that. It's only on process_bug.cgi. I'll look into it.

Comment 73

[LegNeato]

Attachment: Refreshed WIP, still no nice CSS v3

Should work on process_bug.cgi now

Comment 74

[LegNeato]

Comment on attachment 488430 [details] [diff] [review]
Refreshed WIP, still no nice CSS v3

Requesting review though I really only want feedback. My gut feeling is this is way too much logic in templates. Of course, the logic deals with handling how to lay out the data, so maybe that's ok. I think it would be cleaner to get a combined list in the cgi and the template just spit out what's there in forward or reverse, switching on object type. Let me know if that's the route that should be taken. We could also land something like this and file a followup to simplify the templates.

Comment 75

[Max Kanat-Alexander]

Comment on attachment 488430 [details] [diff] [review]
Refreshed WIP, still no nice CSS v3

Okay, here are some general comments:

* Adding functions instead of object methods (in Bugzilla::Bug) is definitely vetoed. :-)

* Calling those functions in every single CGI that can possibly display a bug is also not the way to go. Making these things methods of a bug object should resolve the situation.

* The method should probably return a series of Bugzilla::ChangeSet objects, which don't exist, and which should be created in the bug that's blocking this one. A ChangeSet should contain all of the changes and comments made at a certain time on this bug. The ChangeSet objects can be returned in date order by  a new method called $bug->changes.

* inline_history should have several options: all, interesting_changes, off, and should default to interesting_changes. More or less, that means all changes except CC.

* I'm not sure what "util" means as a template name, perhaps that should be something else, or those blocks should actually just be their own templates.

* There should not be any constants written directly into the templates. Instead they should be in Bugzilla::Constants, if you do need them. However, with $bug->changes, I don't think that you would need any of that sorting code in the template.

Comment 76

[LegNeato]

I knew you were going to get me to work on bug 492674 ;-). I'll hopefully have time to address these this week.

Comment 77

[Marc Schumann [:Wurblzap]]

> * inline_history should have several options: all, interesting_changes, off,
> and should default to interesting_changes. More or less, that means all changes
> except CC.

Even better would be all changes I request mail for in my personal preferences.

Comment 78

[Max Kanat-Alexander]

(In reply to comment #77)
> Even better would be all changes I request mail for in my personal preferences.

  That is a really clever idea. At that level, I think the preference would deserve its own bug, and in this bug we'd just always display everything until the pref was added. I think the only problem would be that there are a lot of bugs that I have no role on but still want to see various changes for, and email preferences are based totally on role. Also, I'm not sure that I'd really want to see a different activity stream for each type of role that I have.

  One possibility is that we *don't* add a preference, and instead we just add a TUI'ed JavaScript link on show_bug that expands and collapses the activity, and the activity is *always* the "interesting" activity. (If people want the full activity, they can just go to the History page.) I think I like this idea the best.

Comment 79

[Frédéric Buclin]

Christian: any progress? :)

Comment 80

[LegNeato]

I picked this up again. Sorry for the lag :-/

Comment 81

[bigstijn]

For everyone who does see this functionality already in BMO: bug 668506 describes the extension for BMO, although this enhancement will probably offer more functionality.

Comment 82

[Frédéric Buclin]

We are going to branch for Bugzilla 4.4 next week and this bug is too invasive or too risky to be accepted for 4.4 at this point. The target milestone is set to 5.0 which is our next major release.

I ask the assignee to reassign the bug to the default assignee if you don't plan to work on this bug in the near future, to make it clearer which bugs should be fixed by someone else on time for 5.0.

Comment 83

[Nochum Sossonko [:Natch]]

Isn't this complete already or is it just for BMO?

Comment 84

[David Lawrence [:dkl]]

(In reply to Nochum Sossonko [:Natch] from comment #83)
> Isn't this complete already or is it just for BMO?

Just BMO currently using an extension and this bug is about getting the support in the core Bugzilla code which hasn't started yet.

Comment 85

[Mark Clements]

Is there any reason not to implement it to work the same as the extension?  We have been using it for some time, and as far as I am concerned, it works brilliantly.  I'm not sure there's anything I would change.

For reference, it shows all bug activity except CC fields by default, but adds a 'show CCs' link to the page so you can see them if you need to (which is rare).

I guess adding a user preference (show nothing, show all but CCs, show everything) would be the one thing that would probably required if it was in core, as I guess not everyone will want it available (and there may be some masochists who want CCs to always be shown), but really - is there anything else?

Comment 86

[David Lawrence [:dkl]]

(In reply to Mark Clements from comment #85)
> Is there any reason not to implement it to work the same as the extension? 
> We have been using it for some time, and as far as I am concerned, it works
> brilliantly.  I'm not sure there's anything I would change.
> 
> For reference, it shows all bug activity except CC fields by default, but
> adds a 'show CCs' link to the page so you can see them if you need to (which
> is rare).
> 
> I guess adding a user preference (show nothing, show all but CCs, show
> everything) would be the one thing that would probably required if it was in
> core, as I guess not everyone will want it available (and there may be some
> masochists who want CCs to always be shown), but really - is there anything
> else?

I certainly would be for getting this added to the core Bugzilla code as it does provide a useful function, and since there is a method of getting the history on another page the old way, it falls back nicely for people who do not have javascript enabled. And the preference would work nicely for configuring it's behavior.

dkl

Comment 87

[Albert Ting]

I don't see the harm in just using the BMO extension noted in ticket 668506. IMHO, it is more than sufficient.  Or at least formalize the extension and make it available. This way, one can focus on tasks that don't have a solution.

Comment 88

[:glob ✱]

(In reply to Albert Ting from comment #87)
> Or at least formalize the extension and make it available.

http://bzr.mozilla.org/bugzilla/extensions/InlineHistory/trunk


note: the extension shouldn't be rolled into bugzilla itself using its current implementation.  it uses javascript to inject the change notifications into the correct place on the page, while in upstream code we should update the templates.

the blocker here is it was decided that this feature couldn't be implemented without major work which changes how bugzilla tracks changes to bugs (bug 492674).

Comment 89

[Frédéric Buclin]

(In reply to Byron Jones ‹:glob› from comment #88)
> the blocker here is it was decided that this feature couldn't be implemented
> without major work which changes how bugzilla tracks changes to bugs (bug
> 492674).

I think bug 492674 is orthogonal to this bug. We don't need to fix bug 492674 first before fixing this one. But I agree that when implemented, we should fix templates directly instead of using JS.

Comment 90

[Dylan Hardison [:dylan] (he/him)]

I'm a little lost in the weeds as far as the template part goes.

for the data, I see we check if $bug has a get_activity() method -- I assume this is legacy and Bugzilla::Bug always has a get_activity() method?

Comment 91

[David Lawrence [:dkl]]

(In reply to Dylan William Hardison [:dylan] from comment #90)
> I'm a little lost in the weeds as far as the template part goes.
> 
> for the data, I see we check if $bug has a get_activity() method -- I assume
> this is legacy and Bugzilla::Bug always has a get_activity() method?

4.2 and newer had Bugzilla::Bug::GetBugActivity.

Comment 92

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-11368-unfinished-1.patch

Alright, I think this roughly one third of the problem. I'd like some feedback on the proposed API for this ($bug->comments_with_activity)
and a look-over of the POD documentation I wrote for the undocumented methods Bugzilla::Bug.

the a_comment template BLOCK is not part of this patch as it isn't written yet, and I have to flesh out more methods for the "fake" comments. 

This approach should allow inline history to be toggle-able;
the view (the template) can either call bug.comments or bug.comments_with_activity.

Comment 93

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-11368-unfinished-2.patch

This patch now adds activity items inline with comments. It has a few problems:

1) No styling (but it doesn't look any uglier than non-sandstone bugzilla normally does)
2) collapsed comments don't hide associated activity divs
3) Some formatting is off from the InlineHistory extension, obsolete 0 -> 1 instead of obsolete: true for attachments, obsolete attachments are not styled strikethrough
4) It has not been tested against private attachments.

Once these issues are resolved it should be at feature parity with InlineHistory. Except for all the things I've failed to think of...

Comment 94

[Dylan Hardison [:dylan] (he/him)]

Attachment: bugzilla-inline-history.png

Here is an example of what it looks like. Most of the templating was stolen directly from Inlinehistory, although the translation is not 100%. There is no CSS styling yet either.

Comment 95

[Mark Clements]

(In reply to Dylan William Hardison [:dylan] from comment #94)
> Here is an example of what it looks like. Most of the templating was stolen
> directly from Inlinehistory, although the translation is not 100%.

Can I just say, as someone who uses InlineHistory and who has designed their own custom stylesheets for Bugzilla, I think a lot of people would thank you greatly if the internal implementation generated identical structural mark-up to the InlineHistory extension.

It would be quite a job to have to rewrite our CSS if it all changes suddenly, and I will probably disable the internal version and continue using the extension if my stylesheet doesn't apply cleanly.

On the flip side, it makes no odds to me whether there is built-in CSS styling for these new elements.  That should probably be logged as a separate bug that shouldn't prevent this one being closed (though it may possibly block it being shipped, depending on how the community feels).

Great to see someone tackling this at last!

Comment 96

[Dylan Hardison [:dylan] (he/him)]

(In reply to Mark Clements from comment #95)
> (In reply to Dylan William Hardison [:dylan] from comment #94)
> > Here is an example of what it looks like. Most of the templating was stolen
> > directly from Inlinehistory, although the translation is not 100%.
> 
> Can I just say, as someone who uses InlineHistory and who has designed their
> own custom stylesheets for Bugzilla, I think a lot of people would thank you
> greatly if the internal implementation generated identical structural
> mark-up to the InlineHistory extension.
> 
> It would be quite a job to have to rewrite our CSS if it all changes
> suddenly, and I will probably disable the internal version and continue
> using the extension if my stylesheet doesn't apply cleanly.

Right now the class names are off, but that can be fixed. Are you currently relying on the #h1, #h8, etc fragment identifiers? Other than those, everything should be structurally the same. 

Now, under the hood I am going for a clean separation between the data and the template -- and the template code that produces the comments needs to be maintainable and hackable. :-)

Comment 98

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-11368-1.patch

I managed to break "collapse" on comments with activity again. Probably by changing the structure to match InlineHistory more. It is still a nice amount of progress, I think. It even list old duplicate comments (borrowing _add_duplicates() from inline history verbatim).

Needs POD (t/011pod.t does not pass), the above mentioned "collapse" bug fixed,
and a I wouldn't mind testing it against InlineHistory CSS to see how compatible it is.

Comment 99

[Mark Clements]

(In reply to Dylan William Hardison [:dylan] from comment #96)
> Right now the class names are off, but that can be fixed. Are you currently
> relying on the #h1, #h8, etc fragment identifiers? Other than those,
> everything should be structurally the same. 

I can't speak for other users who may have customised the extension (and so as a general approach, keeping it as close as possible is desirable).  However, from my point of view I don't directly reference these IDs, either in CSS or in links to the page.  

That said, I would recommend some form of unique ID for the elements so that they are linkable (I can't see they would be much use for styling purposes).

The most important things for me are the class names and the general DOM structure.
 
> Now, under the hood I am going for a clean separation between the data and
> the template -- and the template code that produces the comments needs to be
> maintainable and hackable. :-)

That is a very good idea.  It would mean that if you broke compatibility with the extension then - in theory - it should be possible to write an alternative template that would restore it.  However, having said that, it would probably be more work to create a custom template than to re-implement your custom CSS, and the latter would only have to be done once, so I don't think this removes the need for maintaining compatibility in the default template that will ship with Bugzilla.

Comment 100

[Kohei Yoshino [:kohei]]

The Inline History extension used in BMO comes with Bugzilla 6.0. We’ll move it into the core and improve the UI in the future.

Comment 101

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9dfeb650f0e
[components] Closes https://github.com/mozilla-mobile/android-components/issues/11368 - Handle key recovery flow for credit cards storage

Comment 102

[Dave Miller [:justdave]]

I think pulsebot is broken. That was supposed to be issue #11368 in GitHub not in Bugzilla.