[REF] Fix return value on deleting financial type

[seamuslee001]

Overview

The code in the API expects that if a deletion via the del method has been successful it should not return FALSE yet that is what financial Type does

Before

wrong return parameter used

After

correct return is used

ping @mattwire @eileenmcnaughton @colemanw

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@seamuslee001 maybe we should return nothing?

I see

  if (method_exists($bao_name, 'del')) {
    $dao = new $bao_name();
    $dao->id = $params['id'];
    if ($dao->find()) {
      $bao = call_user_func_array([$bao_name, 'del'], $args);
      if ($bao !== FALSE) {
        return civicrm_api3_create_success();
      }

But I also see

    if ($this->_action & CRM_Core_Action::DELETE) {
      $errors = CRM_Financial_BAO_FinancialType::del($this->_id);
      if (!empty($errors)) {
        CRM_Core_Error::statusBounce($errors['error_message'], CRM_Utils_System::url('civicrm/admin/financial/financialType', "reset=1&action=browse"), ts('Cannot Delete'));
      }

Other BAO are inconsistent.... but if we stick with TRUE the above needs fixing

[seamuslee001]

@eileenmcnaughton i have opted to fix the form as i think it makes more sense to stick with returning TRUE as the thing succeeded so lets return TRUE

[eileenmcnaughton]

OK - looks good then