Closed
Bug 657564
Opened 14 years ago
Closed 12 years ago
Create a "Payment" form
Categories
(bugzilla.mozilla.org :: General, enhancement)
Tracking
()
RESOLVED
INVALID
People
(Reporter: glob, Assigned: dkl)
Details
Attachments
(4 files)
14.24 KB,
patch
|
glob
:
review-
|
Details | Diff | Splinter Review |
7.74 KB,
patch
|
glob
:
review-
|
Details | Diff | Splinter Review |
4.56 KB,
patch
|
glob
:
review-
|
Details | Diff | Splinter Review |
2.99 KB,
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
(from bug 645732 comment 12)
We would love to have a form that can add info to an already existing bug.
The concept will be that the bug created by "Budget Form" will be approved and then you can use another form to add payment info after the event.
The edit "Payment Form" should be formed after this: http://etherpad.mozilla.com:9000/remo-form-payment
(and same visual style as the eps.mentorship form)
the url should be https://bugzilla.mozilla.org/form.reps.payment
Is that possible?
Comment 1•14 years ago
|
||
Although it's certainly more secure than email, I'm not sure we want payment data in Bugzilla. Might want to run this past a few folks first.
Comment 2•14 years ago
|
||
I suspect Bugzilla is actually 90% of the way to PCI compliance because of segregated networks and standalone databases and whatnot, but the ease with which someone could accidentally move a bug out of a restricted product puts the permanence of the security in question. A standalone app to accept and store this date would likely be much safer.
Comment 3•14 years ago
|
||
s/date/data/
Also of note - we're not encrypting any of this data when it's stored, either.
Comment 4•14 years ago
|
||
I sent Jim an email asking him about this. I'll let you know what he says.
Comment 5•14 years ago
|
||
Jim says they should send their reimbursement requests to mozilla@bill.com.
Assignee | ||
Comment 6•14 years ago
|
||
So I assume this new payment form is no longer needed then?
dkl
a standalone app makes more sense, and we already have one (bill.com).
marking as wontfix as per comment 5.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX
Comment 8•14 years ago
|
||
I am sorry, but I don't see how we can ask contributors to send their info to bill.com.. Besides we first have to approve those requests and then have someone to actually pay them on these payment info.
How are we going to manage approval, tracking and peer visibility through bill.com?
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment 9•14 years ago
|
||
Pierros - Why don't you meet with Bree in accounting, who manages bill.com, explain to her your program, and see what you can figure out.
Comment 10•14 years ago
|
||
Thanks Liz, William has already planned a meeting with accounting today, and I will update the bug accordingly.
Comment 11•14 years ago
|
||
We just had a meeting with Legal today and Finance yesterday.
I updated the Etherpad to reflect the changes. We are not going to store bank details to bugzilla. Instead they are going to be prompted to email this info to William Q (so that he can process those through bill.com)
Please proceed with the form creation.
Comment 12•14 years ago
|
||
(In reply to comment #11)
> We are not going to store
> bank details to bugzilla. Instead they are going to be prompted to email
> this info to William Q (so that he can process those through bill.com)
That's even worse. At least a form post on Bugzilla would have been encrypted in transit to us. Email gets sent in the clear over the net, ripe for anyone on the path to sniff it and get those juicy bank details.
Comment 13•14 years ago
|
||
FWIW, bill.com does allow you to upload directly to them via their secure web page. I think it requires we set up an account for the user first though. :(
Comment 14•14 years ago
|
||
:( .. I don't think that we should have an account for each contributor on bill.com.
So the only viable solution is to have the info on bugzilla as initially requested? (is email such a problem?)
Thanks!
Comment 15•14 years ago
|
||
William and Pierros - You're planning to follow the recommendations of our accounting department, right?
Chris - Can you weigh in from a security perspective?
Comment 16•14 years ago
|
||
We have had problems with forms and "user error" in the past cloning bugs. We also don't encrypt any of this data within the database, as justdave has pointed out. This data just can't be inside of Bugzilla.
We just shouldn't be taking this data in on Bugzilla. Why can't set examples with these members to use email encryption?
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → WONTFIX
Comment 17•14 years ago
|
||
I've reviewed in the detail the whole payment process used for the Mozilla Reps program with the finance team and it was they're recommendation to have Mozilla Reps send me via email their bank details for reimbursement. We're following they're recommendation.
Comment 18•14 years ago
|
||
Ping, because this is a blocker for Mozilla Reps program to start, do we have any ETA?
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment 19•14 years ago
|
||
(In reply to comment #18)
> Ping, because this is a blocker for Mozilla Reps program to start, do we
> have any ETA?
eta for what? a form on bugzilla which will hold bank details?
Comment 20•14 years ago
|
||
Did I say that ? :)
Comment 11 and this etherpad has the details:
http://etherpad.mozilla.com:9000/remo-form-payment
This is what we did on bug 657563 also.
Thanks!
Assignee | ||
Comment 21•14 years ago
|
||
Working on this.
Assignee: nobody → dkl
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 22•14 years ago
|
||
Here is a patch that creates a page.cgi form that will append information to a current bug report. The form will prefill a comment and also attach two files to the bug report, expense form and receipts. I will commit to stage-tip as well so feedback can be given.
dkl
Attachment #535485 -
Flags: review?(glob)
Assignee | ||
Comment 23•14 years ago
|
||
pierros, you can give this a try at
https://bugzilla-stage-tip.mozilla.org/page.cgi?id=remo-form-payment.html
Dkl
Comment 24•14 years ago
|
||
Just tested it. Works like a charm :)
Thanks David!
After landing on production please don't forget to create the alias url. (same goes for the other 2 bugs)
Reporter | ||
Comment 25•14 years ago
|
||
Comment on attachment 535485 [details] [diff] [review]
Patch to create payment form (v1)
Review of attachment 535485 [details] [diff] [review]:
-----------------------------------------------------------------
looks good; just a few things to tidy up :)
::: extensions/BMO/Extension.pm
@@ +806,5 @@
> + my $token = $input->{'token'};
> +
> + check_token_data($token, 'remo_form_payment');
> +
> + detaint_natural($bug_id);
nit: tabs
@@ +810,5 @@
> + detaint_natural($bug_id);
> + my $bug = Bugzilla::Bug->check($bug_id);
> +
> + # Make sure the user can attach to this bug
> + if (!$user->can_edit_product($bug->product_obj->id)) {
this should be $bug->user->{'canedit'}
@@ +886,5 @@
> + delete_token($token);
> +
> + # Define the variables and functions that will be passed to the UI template.
> + $vars->{'attachment'} = $attachment;
> + $vars->{'bugs'} = [ new Bugzilla::Bug($bug_id) ];
you should be able to pass in the existing $bug object here.
::: extensions/BMO/template/en/default/pages/comment-remo-form-payment.txt.tmpl
@@ +15,5 @@
> + # Copyright (C) 1998 Netscape Communications Corporation. All
> + # Rights Reserved.
> + #
> + # Contributor(s): Gervase Markham <gerv@gerv.net>
> + #%]
the boilerplate needs updating, mozilla foundation, year, name, etc.
::: extensions/BMO/template/en/default/pages/remo-form-payment.html.tmpl
@@ +15,5 @@
> + # Portions created by Mozilla are Copyright (C) 2008 Mozilla
> + # Corporation. All Rights Reserved.
> + #
> + # Contributor(s): Reed Loden <reed@mozilla.com>
> + # David Tran <dtran@mozilla.com>
the boilerplate needs updating, mozilla foundation, year, name, etc.
@@ +24,5 @@
> +[% PROCESS global/header.html.tmpl
> + title = "Mozilla Reps Payment Form"
> + style_urls = [ 'extensions/BMO/web/styles/moz_reps.css' ]
> + javascript_urls = [ 'extensions/BMO/web/js/form_validate.js',
> + 'js/util.js',
please remove tabs
@@ +117,5 @@
> +</tr>
> +
> +<tr class="even">
> + <td colspan="2">
> + <strong>Have you already received payment for this event? <span style="color: red;">*</span></strong>
don't mark this checkbox as mandatory; it's confusing as it implies that it must be checked.
@@ +161,5 @@
> +
> +</form>
> +
> +<p>
> + <strong><span style="color: red;">*</span></strong> - Required field<br />
nit: we're not xhtml, <br> not <br/>
Attachment #535485 -
Flags: review?(glob) → review-
Reporter | ||
Comment 26•14 years ago
|
||
just thinking some more about this; we should also check that the bug provided is in the correct product and component, to try to catch typos.
bonus points for showing the entered bug's summary when focus leaves the bug id input field.
Assignee | ||
Comment 27•14 years ago
|
||
Thanks
(In reply to comment #25)
> @@ +886,5 @@
> > + delete_token($token);
> > +
> > + # Define the variables and functions that will be passed to the UI template.
> > + $vars->{'attachment'} = $attachment;
> > + $vars->{'bugs'} = [ new Bugzilla::Bug($bug_id) ];
>
> you should be able to pass in the existing $bug object here.
I got the design from process_bug.cgi which states in the comments:
# $bug->update() does not update the internal structure of
# the bug sufficiently to display the bug with the new values.
# (That is, if we just passed in the old Bug object, we'd get
# a lot of old values displayed.)
So I figured it was safest just to reload a fresh bug object.
dkl
Assignee | ||
Comment 28•14 years ago
|
||
Thanks glob. Here is a new patch that addresses your changes as well as adds the bonus points of getting the bug id's status and summary and displaying it beneath the id field.
dkl
Attachment #535721 -
Flags: review?(glob)
Comment 29•14 years ago
|
||
I'm not sure which patch to apply this comment to or if it gets a negative review... the last patch on here doesn't even touch the line in question which makes me think it was already broken before?
This produces an Internal Server Error when you load it.
file error - parse error - hook/global/user-error-errors.html.tmpl line 43: unexpected token ("remo_payment_invalid_product')
[% ELSIF error == "remo_payment_invalid_product' %]
Comment 30•14 years ago
|
||
(In reply to comment #17)
> I've reviewed in the detail the whole payment process used for the Mozilla
> Reps program with the finance team and it was they're recommendation to have
> Mozilla Reps send me via email their bank details for reimbursement. We're
> following they're recommendation.
What I'm saying now is in no way a stop-ship, that's up to you guys and finance to decide. The way it's set up now is the procedure Finance has in place. I think that procedure is very wrong, but that's my personal opinion, and in no way should it stop this from getting put up for use, since we're doing what they said.
Personally I'd be very unwilling to send you bank account information via email without it being encrypted. That's just asking for my bank account info to get stolen by someone sniffing packets on the internet. My personal recommendation would be for you guys provide a GPG public key which can be used to encrypt the emails (and we can put the ascii-armored version of it right in the form so they can copy/paste it to use for encrypting).
Comment 31•14 years ago
|
||
Comment on attachment 535721 [details] [diff] [review]
Patch to create payment form (v2)
Review of attachment 535721 [details] [diff] [review]:
-----------------------------------------------------------------
::: extensions/BMO/template/en/default/hook/global/user-error-errors.html.tmpl
@@ +40,4 @@
> be performed by a [% terms.Bugzilla %] admin. Please file a bug report
> under 'bugzilla.mozilla.org:Administration' requesting the new field.
>
> +[% ELSIF error == "remo_payment_invalid_product' %]
Guess it is in this patch... ^^^ mismatched quotes.
Attachment #535721 -
Flags: review?(glob) → review-
Comment 32•14 years ago
|
||
This page produces an ISE if you try to access it without being logged in.
DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`bugstage02/tokens`, CONSTRAINT `fk_tokens_userid_profiles_userid` FOREIGN KEY (`userid`) REFERENCES `profiles` (`userid`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO tokens (userid, issuedate, token, tokentype, eventdata)
VALUES (?, NOW(), ?, ?, ?)"] at /data/www/bugzilla.mozilla.org/Bugzilla/Token.pm line 425
Bugzilla::Token::_create_token(0, 'session', 'remo_form_payment') called at /data/www/bugzilla.mozilla.org/Bugzilla/Token.pm line 172
Bugzilla::Token::issue_session_token('remo_form_payment') called at /data/www/bugzilla.mozilla.org/extensions/BMO/Extension.pm line 911
Bugzilla::Extension::BMO::_remo_form_payment('HASH(0x90b49ec)') called at /data/www/bugzilla.mozilla.org/extensions/BMO/Extension.pm line 147
Bugzilla::Extension::BMO::page_before_template('Bugzilla::Extension::BMO=HASH(0x8967f6c)', 'HASH(0x896b500)') called at /data/www/bugzilla.mozilla.org/Bugzilla/Hook.pm line 33
Bugzilla::Hook::process('page_before_template', 'HASH(0x896b500)') called at /data/www/bugzilla.mozilla.org/page.cgi line 84
Assignee | ||
Comment 33•14 years ago
|
||
Thanks for finding those justdave. I have fixed both in the BZR repo now.
dkl
Assignee | ||
Comment 34•14 years ago
|
||
Comment on attachment 535721 [details] [diff] [review]
Patch to create payment form (v2)
glob. I fixed the problems justdave found in BZR. Please still review the bits dealing with ajax and other issues you found before.
Thanks dkl
Attachment #535721 -
Flags: review- → review?(glob)
Reporter | ||
Comment 35•14 years ago
|
||
Comment on attachment 535721 [details] [diff] [review]
Patch to create payment form (v2)
Review of attachment 535721 [details] [diff] [review]:
-----------------------------------------------------------------
if you enter an invalid bug number, you get a "data.result is undefined" javascript error, and the bug id input is disabled.
you don't need to re-request the bug information if the value hasn't changed.
the summary of the test should start with "Bug ". ie.
Bug 12345 - VERIFIED - ...
instead of
12345 - VERIFIED - ...
and "Bug 12345" should be a link to the bug in question (open in a new window).
you could test the bug's product and component in javascript, and reject invalid bugs prior to submission.
there's a javascript error when you submit:
document.getElementById("short_desc") is null
the generated comment needs to say that it's a payment request.
::: .htaccess
@@ +37,4 @@
> RewriteRule ^form[\.:]reps[\.:]mentorship$ enter_bug.cgi?product=Mozilla+Reps&format=mozreps
> RewriteRule ^form[\.:]reps[\.:]budget$ enter_bug.cgi?product=Mozilla+Reps&format=remo-budget
> RewriteRule ^form[\.:]reps[\.:]swag$ enter_bug.cgi?product=Mozilla+Reps&format=remo-swag
> +RewriteRule ^form[\.:]reps[\.:]payment$ page.cgi?id=remo-form-payment
this needs to be page.cgi?id=remo-form-payment.html
Attachment #535721 -
Flags: review?(glob) → review-
Assignee | ||
Comment 36•14 years ago
|
||
Thanks glob. Here is a new patch which addresses all of your changes. I have committed and should be testable on stage-tip.
dkl
Attachment #536232 -
Flags: review?(glob)
Reporter | ||
Comment 37•14 years ago
|
||
Comment on attachment 536232 [details] [diff] [review]
Patch to create payment form (v3)
if you resubmit the form (submit, back, submit again), you get an ISE:
Undefined subroutine Fh::slice
at template\en\default\global\hidden-fields.html.tmpl line 60
the crux of this issue is you're using check_token_data(), which is designed to work with the admin pages only -- it uses admin/confirm-action.html.tmpl -- and the name of the upload field is hard-coded as "data".
it's probably easiest at this stage to copy the token validation code from post_bug, however you can't use the same template as it won't post to the correct cgi; just throw a user error stating that they can't submit the same form twice, with a link to the form which will allow them to create a new request.
if we are pressed for time, i'm happy for the current code to go live as long as this is resolved soonish.
Attachment #536232 -
Flags: review?(glob) → review-
Assignee | ||
Comment 38•14 years ago
|
||
Thanks again for the review. I have reworked the token code and submitting a patch with those changes for review.
dkl
Attachment #536315 -
Flags: review?(glob)
Reporter | ||
Comment 39•14 years ago
|
||
Comment on attachment 536315 [details] [diff] [review]
Patch to create payment form (v4)
r=glob :)
Attachment #536315 -
Flags: review?(glob) → review+
Assignee | ||
Comment 40•14 years ago
|
||
This is live now. Please reopen if there are any issues.
https://bugzilla.mozilla.org/form.reps.payment
dkl
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Comment 41•14 years ago
|
||
Sorry for the late request, but I talked to Alex Fowler about this and he recommends that a note be added to the form that says something like "Please black out any bank account information included on receipts before attaching them."
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 42•14 years ago
|
||
William and Pierros - With respect to how reps send William their bank wire info, Alex agreed with Dave that sending by email is too insecure. He suggested that you modify the process just a little by asking them to send you the info either by fax or snail mail (or call you and give it to you over the phone). Hopefully it won't be a problem to make that minor change.
Assignee | ||
Comment 43•14 years ago
|
||
(In reply to comment #41)
> Sorry for the late request, but I talked to Alex Fowler about this and he
> recommends that a note be added to the form that says something like "Please
> black out any bank account information included on receipts before attaching
> them."
Will add that now.
(In reply to comment #42)
> William and Pierros - With respect to how reps send William their bank wire
> info, Alex agreed with Dave that sending by email is too insecure. He
> suggested that you modify the process just a little by asking them to send
> you the info either by fax or snail mail (or call you and give it to you
> over the phone). Hopefully it won't be a problem to make that minor change.
Can some provide the exact wording that would like for this and I will get it added to the form.
dkl
Assignee | ||
Updated•14 years ago
|
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 44•14 years ago
|
||
(In reply to comment #43)
> (In reply to comment #42)
> > William and Pierros - With respect to how reps send William their bank wire
> > info, Alex agreed with Dave that sending by email is too insecure. He
> > suggested that you modify the process just a little by asking them to send
> > you the info either by fax or snail mail (or call you and give it to you
> > over the phone). Hopefully it won't be a problem to make that minor change.
>
> Can some provide the exact wording that would like for this and I will get
> it added to the form.
>
> dkl
Ping? Is this still needed?
dkl
Assignee | ||
Updated•12 years ago
|
Flags: needinfo?(pierros)
Comment 45•12 years ago
|
||
Hey dkl you can close this, as the process now included a password protected file send to accounting (when bank info is involved) for over a year now. Thanks!
Flags: needinfo?(pierros)
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 12 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•