If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Hendrix should be able to submit to alternate destinations depending on product

RESOLVED FIXED

Status

Webtools Graveyard
Hendrix
RESOLVED FIXED
11 years ago
a year ago

People

(Reporter: reed, Assigned: reed)

Tracking

Dependency tree / graph

Details

Attachments

(1 attachment, 6 obsolete attachments)

(Assignee)

Description

11 years ago
Created attachment 260370 [details] [diff] [review]
patch - v1

Due to recent requests such as bug 375879 and others, it would be nice if Hendrix had the ability to submit to alternate newsgroups rather than just the hardcoded one in its configuration file.

This patch adds support for this request. It includes this priority order for choosing the newsgroup to use:
1) HENDRIX_NEWSGROUP (env. variable)
2) 'newsgroup' variable from POST (must be listed in the @newsgroup array
3) the first entry in the @newsgroup array

I haven't tested this...
Attachment #260370 - Flags: review?(gerv)
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

11 years ago
Created attachment 260419 [details] [diff] [review]
patch - v2

Rewrote the patch, as per our discussion on IRC.

I think I addressed everything we talked about.
Attachment #260370 - Attachment is obsolete: true
Attachment #260419 - Flags: review?(gerv)
Attachment #260370 - Flags: review?(gerv)
(Assignee)

Updated

11 years ago
Summary: Hendrix should be able to submit to alternate newsgroups via a POST variable → Hendrix should be able to submit to alternate newsgroups depending on product
Comment on attachment 260419 [details] [diff] [review]
patch - v2

>+# Map products to newsgroups
>+my %product_newsgroup_map = (
>+	"Firefox" => "mozilla.feedback.firefox",
>+	"Thunderbird" => "mozilla.feedback.thunderbird",
>+	"Mozilla Suite" => "mozilla.feedback.suite",
>+	"SeaMonkey" => "mozilla.feedback.seamonkey",
>+	"Sunbird" => "mozilla.feedback.sunbird",
>+	"Camino" => "mozilla.feedback.camino",
>+	"Bon Echo" => "mozilla.feedback.firefox",
>+	"Gran Paradiso" => "mozilla.feedback.firefox",
>+	"Minefield" => "mozilla.feedback.firefox",
>+	"Other" => "mozilla.feedback.other"
>+);

I just thought: using a hash means that we lose control of the order of the products in the dropdown. Currently, they are in an order with the most-used products first. Among other things, this means that Firefox is first and therefore the default, which is a good feature to preserve.

Should we switch to storing it initially in an array, which we can then hashify?

Would it make sense to have the first version send everything to mozilla.feedback, so we can decouple the checking-in of this patch from the creation of new newsgroups?

>+# The DNS blacklists by which to check sender IP addresses
> my $rbl = Net::RBLClient->new(
>     lists => [
>-        'opm.blitzed.org',
>+        'dnsbl.ahbl.org',

I'm sure you know what you are doing, but could you explain?

Gerv
Attachment #260419 - Flags: review?(gerv) → review-
Also, perhaps we could fix bug 374995 at the same time. I believe the Perl modules we already require support sending email. We just need to enhance the configuration part a little bit to allow specifying newsgroups or email addresses. We could probably grep for "@" in the string to differentiate.

Gerv
(Assignee)

Updated

11 years ago
Blocks: 376067
(Assignee)

Updated

11 years ago
Blocks: 374995
(Assignee)

Comment 4

11 years ago
Created attachment 261097 [details] [diff] [review]
patch - v3

Addressed review comments (I think...) and added support for the request in bug 374995. This code is untested.

As to your question about the change in the DNSBL, opm.blitzed.org was shutdown a while back, so I replaced it with another DNSBL (dnsbl.ahbl.org) that acts pretty close to how opm worked. If you want, I can change this in another bug with another patch.
Attachment #260419 - Attachment is obsolete: true
Attachment #261097 - Flags: review?(gerv)
(Assignee)

Updated

11 years ago
Summary: Hendrix should be able to submit to alternate newsgroups depending on product → Hendrix should be able to submit to alternate destinations depending on product
(Assignee)

Comment 5

11 years ago
Comment on attachment 261097 [details] [diff] [review]
patch - v3

>+[% destination.search('@') ? "To:" : "Newsgroups:" %] [% destination %]

I wonder if this needs a '+' next to one of the '%' characters to ensure proper whitespacing in output... Not sure.
(Assignee)

Comment 6

11 years ago
Comment on attachment 261097 [details] [diff] [review]
patch - v3

Yeah, this needs some more work. New patch soon.
Attachment #261097 - Attachment is obsolete: true
Attachment #261097 - Flags: review?(gerv)
(Assignee)

Comment 7

11 years ago
Created attachment 261108 [details] [diff] [review]
patch - v4

Ok, I decided to leave the product_destination_map as a hash but add a second array with a list of products to display in the drop-down of the main Hendrix page. I purposely did this, as there may be a time where we don't want something on the main Hendrix page, but we do want Hendrix to support it.

I also added "eBay Companion" as a valid product and pointed it to "mozilla.feedback.companion.ebay". I did not add it to the products_list.
Attachment #261108 - Flags: review?(gerv)
Comment on attachment 261108 [details] [diff] [review]
patch - v4

Assuming v4 has been tested, r=gerv :-)

Gerv
Attachment #261108 - Flags: review?(gerv) → review+
(Assignee)

Comment 9

11 years ago
Created attachment 261219 [details] [diff] [review]
patch - v5

I put this through some rigorous tests, rewrote some parts, added some preventative checks, etc. This patch does work. I will land this tomorrow, but I'm still requesting review on it to make sure I didn't miss anything.
Attachment #261108 - Attachment is obsolete: true
Attachment #261219 - Flags: review?(gerv)
(Assignee)

Comment 10

11 years ago
Created attachment 261231 [details] [diff] [review]
patch - v5.1

I noticed a bug in Hendrix when I was doing some more testing, so this fixes it.

Basically, $vars->{'message'} was not being set before the throwError() call, so errors did not have the user's message displayed, as the text states.
Attachment #261219 - Attachment is obsolete: true
Attachment #261231 - Flags: review?(gerv)
Attachment #261219 - Flags: review?(gerv)
(Assignee)

Comment 11

11 years ago
Created attachment 261244 [details] [diff] [review]
patch - v6

Address review comments.
Attachment #261231 - Attachment is obsolete: true
Attachment #261244 - Flags: review?(gerv)
Attachment #261231 - Flags: review?(gerv)
Comment on attachment 261244 [details] [diff] [review]
patch - v6

r=gerv.

Gerv
Attachment #261244 - Flags: review?(gerv) → review+
(Assignee)

Comment 13

11 years ago
I did s/==/eq/ to fix a bug.

Checking in index.cgi;
/cvsroot/mozilla/webtools/hendrix/index.cgi,v  <--  index.cgi
new revision: 1.13; previous revision: 1.12
done
Checking in template/index.html.tmpl;
/cvsroot/mozilla/webtools/hendrix/template/index.html.tmpl,v  <--  index.html.tmpl
new revision: 1.17; previous revision: 1.16
done
Checking in template/message-headers.txt.tmpl;
/cvsroot/mozilla/webtools/hendrix/template/message-headers.txt.tmpl,v  <--  message-headers.txt.tmpl
new revision: 1.3; previous revision: 1.2
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.