fix: improve return value of Give_Donate_Form::get_goal()

[andrewminion-luminfire]

Bug Report

User Story

As a developer, I want Give_Donate_Form’s get_goal() method to return false if the goal is disabled for a form so that I can use it to determine whether a goal is set without using get_post_meta( $form_id, '_give_goal_option', true ).

Current Behavior

Currently, get_goal() returns the meta value for _give_set_goal regardless of whether the goal is enabled or not. By default, the value is 1.000000. A default goal of $1.00 is unlikely but possible.

Expected Behavior

In my opinion, get_goal() should first check to see if the goal is enabled and if not, return false.

Bug Type

• This bug describes functionality that once worked as expected in version X.X.X.
• This bug describes functionality that never worked as expected.
• I am not sure whether this functionality ever worked as expected.

Steps to Reproduce

1. Write this code:

$form = new Give_Donate_Form( $form_id ); 
$goal = $form->get_goal();
if ( isset( $goal ) ) {
  // Do stuff.
}

Possible Solution

Add a a check to see if the goal is set or not.

Acceptance Criteria

• If a goal is enabled, get_goal() should return the goal.
• If a goal is not enabled, get_goal() should return false, not the default value of 1.000000

[andrewminion-luminfire]

Possible solution: https://gist.github.com/macbookandrew/bb528a7de6bf922464d76515532b92c6#file-give-get-goal-fix-php-L29-L32

[kevinwhoffman]

Hey @andrewminion-luminfire thanks for the request. We're going to take a look at that return value and improve it in Give 2.4.2.

[ravinderk]

Slack Chat Summary

Participants: @DevinWalker @kevinwhoffman @ravinderk
Topic: Should we return multiple types of result from get_goal
Result: Best practice to return one type if possible. We don’t use return type declarations, but they were introduced in PHP 7.0 and allow you to declare the return type in the function signature. https://wiki.php.net/rfc/return_types
Prevents errors with unexpected return values and makes testing easier.

This is still open for discussion whether we can create has_goal function instead of handling user case in get_goal function.

[andrewminion-luminfire]

Adding has_goal would work for me.

Returning only strings from get_goal makes sense, but in my opinion, it should return an empty string if the goal is disabled, rather than the goal value. @ravinderk @DevinWalker @kevinwhoffman