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

[REF] Remove unused lines from loadObjects #18389

Merged
merged 1 commit into from
Sep 6, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 6, 2020

Overview

Minor code cleanup

Before

Unreachable code muddying the waters

After

poof

Technical Details

loadObjects is only called from one place. As we can see the contribution object is
always passed in so we don't need to handle the possibility of it being anything other than a
BAO (I tweaked the error handling around that line too to make it clearer where it was
coming from)

Screen Shot 2020-09-07 at 9 05 57 AM

Comments

loadObjects is only called from one place. As we can see the contribution object is
always passed in so we don't need to handle the possibility of it being anything other than a
BAO (I tweaked the error handling around that line too to make it clearer where it was
coming from)
@civibot
Copy link

civibot bot commented Sep 6, 2020

(Standard links)

@civibot civibot bot added the master label Sep 6, 2020
CRM_Core_Error::debug_log_message("Could not find contribution record: {$contribution->id} in IPN request: " . print_r($input, TRUE));
echo "Failure: Could not find contribution record for {$contribution->id}<p>";
return FALSE;
throw new CRM_Core_Exception('Failure: Could not find contribution record for ' . (int) $contribution->id, NULL, ['context' => "Could not find contribution record: {$contribution->id} in IPN request: " . print_r($input, TRUE)]);
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton I would note that I think this will cause HTTP 500 errors to be thrown rather than HTTP 200 back to the IPN

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 - except you already merged the try-catch to wrap around it

Copy link
Contributor

Choose a reason for hiding this comment

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

ah ok I guess so right

@seamuslee001 seamuslee001 merged commit 4cc604b into civicrm:master Sep 6, 2020
@seamuslee001 seamuslee001 deleted the pp branch September 6, 2020 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants