Skip to content
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

Issue/1423 #1780

Closed
wants to merge 6 commits into from
Closed

Issue/1423 #1780

wants to merge 6 commits into from

Conversation

mehul0810
Copy link
Contributor

Description

This PR is for #1423

How Has This Been Tested?

Tested this with 2 possible scenarios:

  • API call for all forms (Made sure that goals appear for the forms where goal is set)
  • Api call for individual form

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@@ -1037,6 +1037,13 @@ private function get_form_data( $form_info ) {
$form['info']['tags'] = get_the_terms( $form_info, 'give_forms_tag' );
}

if ( give_is_setting_enabled( give_get_meta( $form_info->ID, '_give_goal_option', true ) ) ) {
$goal_amount = give_get_meta( $form_info->ID, '_give_set_goal', true );
$goal_percentage_completed = round( ( give_get_form_earnings_stats( $form_info->ID ) / $goal_amount ) * 100 );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mehul0810 This code will generate PHP notice.

PHP Warning:  Division by zero in /wp-content/plugins/Give/includes/api/class-give-api.php on line 1042

And also max percentage should be 100
for ref: https://github.com/WordImpress/Give/blob/master/templates/shortcode-goal.php#L50

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravinderk I've updated the code, considering both the above mentioned scenarios

// Check whether any goal is to be achieved for the donation form
$goal_option = give_get_meta( $form_info->ID, '_give_goal_option', true );
$goal_amount = give_get_meta( $form_info->ID, '_give_set_goal', true );
if ( give_is_setting_enabled( $goal_option ) && isset( $goal_amount ) && $goal_amount > 0 ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mehul0810 change this to if ( give_is_setting_enabled( $goal_option ) && $goal_amount )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravinderk I've implemented this change and liked it so much 👍

Copy link
Collaborator

@ravinderk ravinderk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DevinWalker This PR is working fine.

@DevinWalker
Copy link
Member

Closing in favor of #1792 with my code updates.

@mehul0810 mehul0810 deleted the issue/1423 branch December 8, 2017 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants