Add support for getsatisfaction for the See Also field

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Creating/Changing Bugs
--
enhancement
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: Thomas D., Assigned: selsky)

Tracking

(Depends on: 1 bug)

unspecified
Bugzilla 4.4
Bug Flags:
approval +

Details

(URL)

Attachments

(1 attachment, 8 obsolete attachments)

(Reporter)

Description

8 years ago
STR

0 Realize that users are being encouraged to commit their feedback (incl. bug reports) at Thunderbird's getsatisfaction support site (http://getsatisfaction.com/mozilla_messaging).
1 On the real bug in bugzilla, you want to create a reference link to some user's getsatisfaction feedback, as it contains bug description, ideas for solving, improving or other useful information
2 try adding a reference URL for relevant getsatisfaction content to the "See also" field of the bug (e.g. http://getsatisfaction.com/mozilla_messaging/topics/relevant_topic)

Actual

cannot add getsatisfaction URLs to See Also
(although it's an official place where mozilla messaging expects people to report bugs)

Expected

allow adding getsatisfaction URLs to See Also (and reflect once again if it really makes sense to restrict this field to certain URLs only, see below)

Further information

I've seen a lot of bmo bugs where people from bug triage posted lists of related getsatisfaction links in comments, which is very ineffecient compared to adding them directly to the bug for reference. As a side note (as has been said before), we really need a field where we can add *any* URL that is relevant for triaging or solving a bug. "See also" field looks like a good place for that, and we can still have all the future API and normalization magic for supported URLs, while leaving any other URLs untouched as they are added.

Comment 1

8 years ago
Why don't you use the URL field for that? The See Also field specified "Add Bug URLs", so we really want to point to a bug report, not some random URL.
(Reporter)

Comment 2

8 years ago
Frédéric, thx for quick reply.
(In reply to comment #1)
> Why don't you use the URL field for that?

1) Unfortunately, URL field does not allow *multiple* URLs to be added (they will not be linkified). E.g., as mentioned in comment 0, we often need to point to multiple getsatisfaction links to provide contextual information and an impression of the popularity of a bug/RFE. And there's no duplicate system at getsatisfaction that I'm aware of, so there's no other way of referencing them all-in-one.
2) Does it make sense to treat getsatisfaction bug reports different from other bug reports and link them in two different spots on the bug's page?
3) So far, my assumption was that the URL field is more for test cases etc., but I can't tell because there's no documentation about URL field in https://bugzilla.mozilla.org/page.cgi?id=fields.html

> The See Also field specified "Add Bug URLs", so we really want to point to a
> bug report, not some random URL.

I understand and appreciate that idea, but then:
- how do I link to bug reports from smaller bug systems that you have not yet listed? It doesn't seem very practical having to open a new bug for each system whose bugs we want to link to
- how do I link efficiently to multiple "bug reports", feature requests, solutions etc. that happen to be at some other place, like mozillazine, mozilla forums, hendrix, product reviews, private blogs etc?

This is not about adding "random URLs", it is about an efficient and scalable way of adding links that provide helpful information for triaging/solving a bug. For more than one link, this is currently not possible, and adding such resources somewhere buried in the comments won't be very useful, let alone posting the actual contents of such external sources into bugzilla comments.

Btw, I am not the first to propose that we need a way of referencing *any* URL (see Tanner M. Young bug 543667, comment 1 and bug 543667, comment 3). 

Notwithstanding, the focus of *this bug* is primarily to have at least getsatisfaction.com included into the list of allowed URLs for "See also".
Blocks: 231429
(Reporter)

Comment 3

8 years ago
Adding some interested parties to CC - Pls note the above comments about the need for/benefit of providing some way of adding multiple "any URLs" to a bug.

Let me add that I fully appreciate and look forward to Max Kanat Alexander's work on improving the functionality of the "see also" field by exploiting APIs etc. It would be nice to find a way how that work could co-exist with the option of adding any relevant URL to some field of the bug.

Comment 4

8 years ago
getsatisfaction appears to be a bug database, so it's a perfectly reasonable target for see also.
(Reporter)

Updated

8 years ago
No longer blocks: 231429
Depends on: 231429
(Reporter)

Comment 5

8 years ago
Roland, Wayne, ^^?

Comment 6

8 years ago
getsatisfaction is analogous to a summomo. however, in the context of the way we are using it,  is functioning as a database.  

Too early in the morning for me to definitively suggest whether we should be using URL field or "related bugs" for gfsn links, but related bugs feels more correct.  Note: take into consideration here, that we might evolve in the future to using a different support venue, and gfsn might not always be what we use
(Assignee)

Comment 7

7 years ago
Created attachment 513290 [details] [diff] [review]
Add support for getsatisfaction.com bugs
Assignee: create-and-change → selsky
Status: NEW → ASSIGNED
Attachment #513290 - Flags: review?(timello)
Comment on attachment 513290 [details] [diff] [review]
Add support for getsatisfaction.com bugs

>
>=== modified file 'template/en/default/global/user-error.html.tmpl'
>--- template/en/default/global/user-error.html.tmpl	2011-02-16 17:41:08 +0000
>+++ template/en/default/global/user-error.html.tmpl	2011-02-17 23:26:07 +0000
>@@ -259,6 +259,7 @@
>         <li>A ticket in a Trac installation.</li>
>         <li>A b[% %]ug in a MantisBT installation.</li>
>         <li>A b[% %]ug on sourceforge.net.</li>
>+        <li>A b[% %]ug on getsatisfaction.com.</li>

They call 'bug' as 'problem'. So I think it should be 'problem' instead.
Attachment #513290 - Flags: review?(timello) → review-
Comment on attachment 513290 [details] [diff] [review]
Add support for getsatisfaction.com bugs

The small fix can be done in the checkin.

Thanks for the patch Matt! :-)
Attachment #513290 - Flags: review- → review+

Updated

7 years ago
Flags: approval?

Comment 10

7 years ago
Comment on attachment 513290 [details] [diff] [review]
Add support for getsatisfaction.com bugs

You need to specify a "reason" for bug_url_invalid.

Also, as a side note, we should make "show_bug" the default reason if no reason is sent. (That is, make it the last ELSE and use that text if no reason is passed.)
Attachment #513290 - Flags: review-

Updated

7 years ago
Flags: approval?
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Comment 11

7 years ago
Created attachment 518296 [details] [diff] [review]
Add support for getsatisfaction.com bugs v2

Instead of "problem", I changed it to "topic" on the help page since that's what getsatisfaction seems to call it.

Also, I added the reason for bug_url_invalid, as requested.
Attachment #513290 - Attachment is obsolete: true
Attachment #518296 - Flags: review?(timello)
(Assignee)

Comment 12

7 years ago
Created attachment 518301 [details] [diff] [review]
Make "show_bug" the default reason

Make show_bug the default reason by making it the final ELSE.
(Assignee)

Updated

7 years ago
Attachment #518301 - Flags: review?(timello)

Updated

7 years ago
Target Milestone: Bugzilla 4.2 → Bugzilla 5.0
Comment on attachment 518296 [details] [diff] [review]
Add support for getsatisfaction.com bugs v2

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

Could you merge both patches in one?

::: Bugzilla/BugUrl/GetSatisfaction.pm
@@ +22,5 @@
> +use strict;
> +use base qw(Bugzilla::BugUrl);
> +
> +use Bugzilla::Error;
> +use Bugzilla::Util;

You are not using any Bugzilla::Util routine.

@@ +48,5 @@
> +    } else {
> +        ThrowUserError('bug_url_invalid', { url => $value, reason => 'show_bug' });
> +    }
> +    my $topic_name;
> +    if ($uri->path =~ m|^/(?:[^/]+)/([^/]+)|) {

A comment would be nice here...

@@ +59,5 @@
> +    # always go with the HTTP scheme, as that's the default.
> +    $value = "http://getsatisfaction.com/" . $project_name .
> +             "/topics/" . $topic_name;
> +
> +    return new URI($value);

You should be explicit be importing URI module: 'use URI'

Updated

7 years ago
Attachment #518296 - Flags: review?(timello) → review-
Comment on attachment 518301 [details] [diff] [review]
Make "show_bug" the default reason

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

::: template/en/default/global/user-error.html.tmpl
@@ +251,4 @@
>        URLs must start with "http" or "https".
>      [% ELSIF reason == 'path_only' %]
>        You must specify a full URL.
> +    [% ELSIF reason == 'id' %]

This will break Bugzilla::BurUrl since it still uses 'show_bug' reason.
Attachment #518301 - Flags: review?(timello) → review-
(Assignee)

Comment 15

7 years ago
Created attachment 553010 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason v3

The patches have been merged into one.

> You should be explicit be importing URI module: 'use URI'

OK, added.  I was following the examples for LaunchPad, Debian, and Google modules.  Should I patch those files as well?

> This will break Bugzilla::BurUrl since it still uses 'show_bug' reason.

That is handled by the "else" case.  But I've remove the reference to 'show_bug' in Bugzilla::BugUrl to be more clear.
Attachment #518296 - Attachment is obsolete: true
Attachment #518301 - Attachment is obsolete: true
Attachment #553010 - Flags: review?(timello)
(Reporter)

Comment 16

6 years ago
Tiago (review?), ping?

Having to wait more than 4 months to get a patch reviewed might be a discouraging experience for volunteers...
(In reply to Thomas D. from comment #16)
> Tiago (review?), ping?
> 
> Having to wait more than 4 months to get a patch reviewed might be a
> discouraging experience for volunteers...

Hi Matt and Thomas, indeed. Sorry about that. I'm going to review them soon I also have other patches in the queue. Thanks for the reminder!
Comment on attachment 553010 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason v3

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

::: Bugzilla/BugUrl/GetSatisfaction.pm
@@ +46,5 @@
> +    if ($uri->path =~ m|^/([^/]+)/topics/([^/]+)|) {
> +        $project_name = $1;
> +        $topic_name = $2;
> +    } else {
> +        ThrowUserError('bug_url_invalid', { url => $value, reason => 'id' });

I think, instead of having this else, we could catch this pattern in the should_handle method above as we do for the others... so if the url does not match the right pattern the class_for will throw the error.
Attachment #553010 - Flags: review?(timello) → review-
(Assignee)

Comment 19

6 years ago
Created attachment 588802 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason v4

Patch updated as per comment 18.  One question, when would a reason of 'id' ever be used?  We now check the entire URL at once in should_handle.  Or should we break the check into two parts to be able to know that the user is trying to reference a certain type of BugUrl, but just getting the specific URL wrong?
Attachment #553010 - Attachment is obsolete: true
Attachment #588802 - Flags: review?(timello)
(Assignee)

Comment 20

6 years ago
Created attachment 588807 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason v5

Don't use classes we don't need.
Attachment #588802 - Attachment is obsolete: true
Attachment #588807 - Flags: review?(timello)
Attachment #588802 - Flags: review?(timello)
(In reply to Matt Selsky [:selsky] from comment #19)
> Created attachment 588802 [details] [diff] [review]
> Add support for getsatisfaction.com bugs and make show_bug the default
> reason v4
> 
> Patch updated as per comment 18.  One question, when would a reason of 'id'
> ever be used?  We now check the entire URL at once in should_handle.  Or
> should we break the check into two parts to be able to know that the user is
> trying to reference a certain type of BugUrl, but just getting the specific
> URL wrong?

BugUrl::Bugzilla currently uses reason => id and I think it makes more sense for Bugzilla BugUrl type but not for the others.
I think we just want to valid a certain URL and tell if it is one type or another and save that in the database. If in the future for any reason a specific error message is going to be needed, then we implement it.
Comment on attachment 588807 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason v5

>=== added file 'Bugzilla/BugUrl/GetSatisfaction.pm'
>+
>+sub should_handle {
>+    my ($class, $uri) = @_;
>+
>+    # GetSatisfaction URLs only have one form:
>+    #   http(s)://getsatisfaction.com/PROJECT_NAME/topics/TOPIC_NAME
>+    return ($uri->authority =~ /^getsatisfaction.com$/i
>+            and $uri->path =~ m|^/[^/]+/[^/]+/issues/\d+$|) ? 1 : 0;

The example form says 'topics' and the code 'issues'. What is the correct form?

Actually, I think this is wrong. It would accept something like:

http://getsatisfaction.com/AAA/BBB/issues/9999

is this the expected form? I think the example is right but the code is not.
Attachment #588807 - Flags: review?(timello) → review-
(Assignee)

Comment 23

6 years ago
Created attachment 609443 [details] [diff] [review]
patch, v6

I had mistakenly copied and pasted the regex from the Github module.

As per https://getsatisfaction.com/devcommunity/topics/bringing_https_to_our_community_pages_in_detail SSL is now preferred so I updated _check_value to force the scheme to https.
Attachment #588807 - Attachment is obsolete: true
Attachment #609443 - Flags: review?(timello)

Comment 24

6 years ago
Comment on attachment 609443 [details] [diff] [review]
patch, v6

Definitely a candidate for the extension.
Attachment #609443 - Flags: review?(timello) → review-

Updated

6 years ago
Severity: normal → enhancement
(Assignee)

Comment 25

6 years ago
Created attachment 609500 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason, v7

Moved to MoreBugUrl extension.
Attachment #609443 - Attachment is obsolete: true
Attachment #609500 - Flags: review?(timello)
Comment on attachment 609500 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason, v7

r=timello, thanks for the patch!
Attachment #609500 - Flags: review?(timello) → review+

Updated

6 years ago
Flags: approval?

Comment 27

6 years ago
This patch is going to conflict with bug 731727. Holding approval for now.

(and I still have some problems to consider getsatisfaction.com as a bug tracker)
Summary: Cannot add getsatisfaction links/URLs to "See also" field (e.g. for problems reported at Mozilla Messaging's Thunderbird support site) → Add support for getsatisfaction for the See Also field
(Assignee)

Comment 28

6 years ago
Created attachment 616125 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason, v8

Patch updated to resolve conflicts with bug 731727.
Attachment #609500 - Attachment is obsolete: true
Attachment #616125 - Flags: review?(timello)

Comment 29

6 years ago
The patch must be reviewed first.
Flags: approval?
Comment on attachment 616125 [details] [diff] [review]
Add support for getsatisfaction.com bugs and make show_bug the default reason, v8

The patch applies cleanly, it passes in all tests and works as expected.

Thanks for the patch!
Attachment #616125 - Flags: review?(timello) → review+

Updated

6 years ago
Flags: approval?

Updated

6 years ago
Flags: approval? → approval+
Committing to: bzr+ssh://timello%40gmail.com@bzr.mozilla.org/bugzilla/trunk/                                                                                                                                    
modified Bugzilla/BugUrl.pm
modified extensions/MoreBugUrl/Extension.pm
added extensions/MoreBugUrl/lib/GetSatisfaction.pm                                                                                                                                                              
modified extensions/MoreBugUrl/template/en/default/hook/global/user-error-bug_url_invalid_tracker.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 8218.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 32

5 years ago
Added to relnotes for 4.4.
You need to log in before you can comment on or make changes to this bug.