658547 - The link for new attachments in bugmail should point to the "Edit" page rather than to the attachment content itself

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, enhancement)

Description

[Josh Matthews [:jdm]]

I frequently want to read a patch that I've learned about in an email, but the only link I get is the raw file contents. I'd like a link to the graphical view as well.

Comment 1

[Frédéric Buclin]

What is a graphical view??? You mean the diff view?

Comment 2

[Josh Matthews [:jdm]]

Precisely.

Comment 3

[David Lawrence [:dkl]]

Attachment: Patch to add attachment view/edit/diff links to text emails (v1)

Patch adds attachment view/edit/diff links to text email notifications. The HTML view of bug mail currently links to view and edit but not diff. This would be solved if bug 652332 lands.

dkl

Comment 4

[Frédéric Buclin]

Comment on attachment 534100 [details] [diff] [review]
Patch to add attachment view/edit/diff links to text emails (v1)

I don't think we want all three links. We should keep only one, maybe the edit one, as you can easily go everywhere from there. Also, the Diff view doesn't exist if the attachment is not a patch.

Comment 5

[Max Kanat-Alexander]

This also needs to go into the HTML email in a completely different format than it goes into the text email.

Comment 6

[David Lawrence [:dkl]]

Attachment: Patch to add attachment edit link to text emails (v2)

Ok, this patch just changes the link in bug/format_comment.txt.tmpl to go to the attachment details page. The email template for request/email.txt.tmpl already does this.

The HTML version of the mail relies on quoteUrls and so when bug 652332 is landed in some fashion.

dkl

Comment 7

[Max Kanat-Alexander]

Comment on attachment 535757 [details] [diff] [review]
Patch to add attachment edit link to text emails (v2)

This is the wrong patch.

Comment 8

[David Lawrence [:dkl]]

Attachment: Patch to add attachment edit link to text emails (v2)

Sigh. Was really wasted tired at the end of the day when I uploaded that. Here is the right patch.

dkl

Comment 9

[Frédéric Buclin]

Comment on attachment 535797 [details] [diff] [review]
Patch to add attachment edit link to text emails (v2)

r=LpSolit

Comment 10

[David Lawrence [:dkl]]

trunk:
Committing to: bzr+ssh://[e-mail]/bugzilla/trunk
modified template/en/default/bug/format_comment.txt.tmpl
Committed revision 7843.

dkl

Comment 11

[David Lawrence [:dkl]]

Damn. I saw the approval flag and thought it was + but it was ?. Should I back out the patch until approval or can I get a quick approval and be done with this? 

Sorry 
dkl

Comment 12

[Frédéric Buclin]

This is minor enough to not break stuff, so let's go for 4.2.

Comment 13

[Josh Matthews [:jdm]]

So, I actually dislike the implemented solution more than the original situation that caused me to file this.  If I'm going to look at a patch, it's either going to be in the raw view that lets me fill my screen with the patch contents, or in the coloured diff view. In my opinion, linking to the Edit view is the worst of all worlds.

Comment 14

[Max Kanat-Alexander]

I'm somewhat in accord with jdm on this; I don't really see why we would always link to the edit page, except that it does allow you to at least get other places.

I'm also not sure why we changed this rather broadly-used behavior on the request of a single user.

Comment 15

[Frédéric Buclin]

As said elsewhere, request emails already point to the edit page, and now we are at least consistent between bugmails and flagmails. Then, in the edit page, your attachment is already displayed as raw data, which is exactly the same as the view page; except that from the edit page, you at least have a chance to a) view the attachment in the diff view if it's a patch, b) set flags and other attachment attributes, c) go directly to its bug (which is not possible in view mode).

About the "request of a single user" argument, I also wanted to make it point to something usable and consistent with flagmails well before this bug was filed. Except that I always considered it minor enough to not spend time on it.

Comment 16

[Max Kanat-Alexander]

  Okay. Sounds good to me. :-)