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

Check if entry/lead ID is empty in "gform_post_note_added" action #1447

Merged
merged 2 commits into from Sep 13, 2023

Conversation

zach-adams
Copy link
Contributor

Fixes #1446.

Adds a simple "empty" check on the incoming $lead_id to prevent warnings/notices when "null" is passed under certain circumstances (detailed in issue).

Note: After reviewing the rest of the class I think there's a lot of safety/sanity checks & general updates that could/should be made to this Gravity Forms connector class and there's a lack of PHPUnit tests for this connector as well. Since this PR is relatively simple & straightforward I think it's fine to merge for the time being to fix the immediate notices/warnings problem, and I can submit a more detailed improvement issue/PR in the near future to cover a more proper general refactoring. Let me know if that sounds alright or if you'd prefer to wait on this issue/update until that more proper refactoring is ready

Checklist

  • Project documentation has been updated to reflect the changes in this pull request, if applicable.
  • I have tested the changes in the local development environment (see contributing.md).
  • I have added phpunit tests.

Copy link
Contributor

@kasparsd kasparsd left a comment

Choose a reason for hiding this comment

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

This looks good, thanks for your contribution!

@@ -672,6 +672,11 @@ public function callback_gform_post_note_added( $note_id, $lead_id, $user_id, $u
unset( $note );
unset( $note_type );

// Skip if no entry/lead id (e.g. Save and Continue notifications)
if( empty( $lead_id ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Looks like the CI with linter checks isn't running for this pull request but could you please update it to have the space between if and the opening (?

Suggested change
if( empty( $lead_id ) ) {
if ( empty( $lead_id ) ) {

Copy link
Contributor

@kasparsd kasparsd left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great!

@kasparsd kasparsd merged commit aaf43ea into xwp:develop Sep 13, 2023
1 check passed
@kasparsd kasparsd mentioned this pull request Oct 9, 2023
7 tasks
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.

PHP Warnings/Notices on Gravity Forms note added
2 participants