email_in.pl should use Bugzilla::Bug->create to create bugs instead of requiring post_bug.cgi

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
Incoming Email
--
enhancement
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

(Blocks: 1 bug)

unspecified
Bugzilla 3.6
Dependency tree / graph
Bug Flags:
approval +

Details

(Whiteboard: [es-ita])

Attachments

(1 attachment, 1 obsolete attachment)

v2
15.01 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
Right now, email_in.pl creates bugs via a sort of hack where it populates a $cgi object and then calls post_bug.cgi. Since we have Bugzilla::Bug->create now, we should just use it.
(Assignee)

Updated

9 years ago
Whiteboard: [es-ita]
(Assignee)

Updated

9 years ago
Depends on: 526165
(Assignee)

Comment 1

9 years ago
Created attachment 409887 [details] [diff] [review]
v1

This does several nice things:

* I have unified the API of email_in.pl and Bug.create from the WebService. Future enhancements to email_in.pl's post_bug functionality should enhance Bug.create just as well.

* I now allow (and expect) people to specify fields without = after them. I originally required the = to avoid the accidental setting of a field during multi-line items, but I think that's hopefully uncommon enough, and not having to specify = is more convenient.

* I fixed the error that can_enter_product throws when you pass it no product.

* I now allow people to specify values for custom multi-select fields when filing a bug via email_in.
Assignee: incoming.email → mkanat
Status: NEW → ASSIGNED
Attachment #409887 - Flags: review?(dkl)

Comment 2

9 years ago
(In reply to comment #1)
> * I now allow people to specify values for custom multi-select fields when
> filing a bug via email_in.

This means I can no longer pass field values having commas in them as you explicitly split on them, right? Pretty uncommon, but this edge case exists.
(Assignee)

Updated

9 years ago
Blocks: 526184
(Assignee)

Comment 3

9 years ago
(In reply to comment #2)
> This means I can no longer pass field values having commas in them as you
> explicitly split on them, right? Pretty uncommon, but this edge case exists.

  Yeah, it does unfortunately mean that. The rest of Bugzilla still works with comma-separated values, just email_in.pl won't.

Comment 4

9 years ago
Comment on attachment 409887 [details] [diff] [review]
v1

>=== modified file 'Bugzilla/Bug.pm'

> sub set_custom_field {

Using |@cf_branch 3.6-branch| throws the following error:

Can't use string ("3.6-branch") as an ARRAY ref while "strict refs" in use.

This error comes from Bugzilla::Bug::_check_multi_select_field() which assumes to always get arrayrefs as values.



>=== modified file 'email_in.pl'

Autocompletion doesn't work anymore for user fields (CC, assignee, QA contact). post_bug.cgi was doing it for us via Bugzilla::User::match_field(). We must fix that before 3.6. As match_field() uses $cgi heavily, we cannot use it directly from here, especially because it does some stuff we don't want. But we can call Bugzilla::User::match() directly, which doesn't depend on $cgi and does just what we want here. This looks simple enough to fix to do it as part of this patch.
Attachment #409887 - Flags: review?(dkl) → review-
(Assignee)

Comment 5

9 years ago
(In reply to comment #4)
> This looks simple enough to fix to do it as part of this patch.

  That's what I would have thought too, but I looked at it and it's not simple, when I actually go to implement it. Unfortunately, match_field handles all of the validation done by user-matching, so I'd have to re-implement it all, at which point I'd be better off re-writing match_field to not use $cgi. (Having it take a hash, and use input_params instead would be fine.) That's something we can *do* for 3.6, but it's something that I think might make this patch harder to review than it already is.
(Assignee)

Comment 6

9 years ago
Created attachment 416450 [details] [diff] [review]
v2

This fixes the multi-select problem.
Attachment #409887 - Attachment is obsolete: true
Attachment #416450 - Flags: review?(LpSolit)

Updated

9 years ago
Attachment #416450 - Flags: review?(LpSolit) → review+

Comment 7

9 years ago
Comment on attachment 416450 [details] [diff] [review]
v2

Everything works fine. Note however that when email_in.pl sends an email back to the email author about wrong data, the encoding is incorrect. Letters with accent are unreadable. I guess this can be fixed in a separate bug? r=LpSolit

Updated

9 years ago
Flags: approval+
(Assignee)

Comment 8

9 years ago
(In reply to comment #7)
> (From update of attachment 416450 [details] [diff] [review])
> Everything works fine. Note however that when email_in.pl sends an email back
> to the email author about wrong data, the encoding is incorrect. Letters with
> accent are unreadable. I guess this can be fixed in a separate bug? r=LpSolit

  Oh, weird. Yeah, that's something that can be fixed in a separate bug--I suspect it's something that's existed since before this bug.
(Assignee)

Comment 9

9 years ago
Checking in email_in.pl;
/cvsroot/mozilla/webtools/bugzilla/email_in.pl,v  <--  email_in.pl
new revision: 1.29; previous revision: 1.28
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.208; previous revision: 1.207
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.303; previous revision: 1.302
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.199; previous revision: 1.198
done
Checking in Bugzilla/WebService/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Bug.pm,v  <--  Bug.pm
new revision: 1.49; previous revision: 1.48
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.291; previous revision: 1.290
done
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Blocks: 534055

Updated

9 years ago
Blocks: 534057

Updated

9 years ago
Blocks: 429431

Updated

8 years ago
Blocks: 547748
(Assignee)

Updated

8 years ago
Keywords: relnote
(Assignee)

Comment 10

8 years ago
Added to the release notes in bug 547466.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.