-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#211, currentTotal variable is used till 4.6, therefore retrieved data… #212
base: 4.7-dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -151,14 +151,12 @@ | |
// Hide payment if total is 0 and no more participants. | ||
if ($('#priceset').length) { | ||
additionalParticipants = cj("#additional_participants").val(); | ||
// The currentTotal is already being calculated in Form/Contribution/Main.tpl. | ||
if(typeof currentTotal !== 'undefined') { | ||
if (currentTotal == 0 && !additionalParticipants) { | ||
// This is also hit when "Going back", but we already have stripe_token. | ||
debugging('ozlkf'); | ||
// This should not happen on Confirm Contribution, but seems to on 4.6 for some reason. | ||
//return true; | ||
} | ||
currentTotal = $('#pricevalue').data('raw-total'); | ||
if (currentTotal == 0 && !additionalParticipants) { | ||
// This is also hit when "Going back", but we already have stripe_token. | ||
debugging('ozlkf'); | ||
// This should not happen on Confirm Contribution, but seems to on 4.6 for some reason. | ||
//return true; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If currentTotaal == 0 and there are no additional participants, shouldn't we return true? I'm not sure why that is commented out in the original or in your fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comments tell the story.....it was breaking 4.6 transactions.
Do you mind testing this PR with that line uncommented on 4.6? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @h-c-c maybe it's better to have separate branches for 4.6 and 4.7? This issue still exists and there doesn't seem to be any progress on testing it on 4.6. btw, this commit progressivetech@b255663 is probably the better one to create as a PR. |
||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currentTotal is used till CiviCRM version 4.6.x. In 4.7 currentTotal is never set and hence the code tries to validate payment details which should not be in case total amount is 0. An alternate method which will work for all version of civicrm is to retrieve data stored in pricevalue.