Skip to content

Fixing note creation#123

Open
MarcMichalsky wants to merge 1 commit intosystopia:masterfrom
MarcMichalsky:issue/96
Open

Fixing note creation#123
MarcMichalsky wants to merge 1 commit intosystopia:masterfrom
MarcMichalsky:issue/96

Conversation

@MarcMichalsky
Copy link
Contributor

Adds note creation for contributions created as SEPA mandates.

Moves note creation logic from the Submit API to the Submission class.

This change centralizes the creation of contact and contribution notes
related to Twingle submissions, promoting code reuse and maintainability.

Improves the readability of the Submit API by reducing its complexity.

Fixes systopia#96
/**
* Create contact notes.
*
* @param $contact_id int The contact id.
Copy link
Contributor

Choose a reason for hiding this comment

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

The order of @param is <type hint> <parameter name>.

* @param $profile CRM_Twingle_Profile The twingle profile used.
*
* @return void
* @throws \CiviCRM_API3_Exception
Copy link
Contributor

Choose a reason for hiding this comment

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

\CiviCRM_API3_Exception is an alias of \CRM_Core_Exception and is going to be removed from CiviCRM core. So this line should be removed.

* @param $params array The params array from the submission.
* @param $profile CRM_Twingle_Profile The twingle profile used.
*
* @return void
Copy link
Contributor

Choose a reason for hiding this comment

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

This line has no additional information and therefore can be removed.

/**
* Create contribution notes.
*
* @param $contribution_id int The contribution id.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above for the complete comment block.

* Create contact notes.
*
* @param $contact_id int The contact id.
* @param $params array The params array from the submission.
Copy link
Contributor

Choose a reason for hiding this comment

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

The type hint could be more specific. I assume array<string, mixed>.

->execute();
}
}
CRM_Twingle_Submission::createContactNotes(intval($contact_id), $params, $profile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Type casts to int should be done with (int). However $contact_id should contain an int anyways. Otherwise the assignment should be checked and casted, if necessary. The type is already checked in line 489, though.

}

// Add notes to the contribution.
CRM_Twingle_Submission::createContributionNotes(intval($contribution_id), $params, $profile);;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to add the type hint to $contribution_id = $result_values['sepa_mandate']['entity_id']; (line 737) => $contribution_id = (int) $result_values['sepa_mandate']['entity_id'];

->execute();
}
}
CRM_Twingle_Submission::createContributionNotes(intval($contribution['id']), $params, $profile);;
Copy link
Contributor

Choose a reason for hiding this comment

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

Type cast should be done with (int).

@dontub dontub self-assigned this Oct 27, 2025
@dontub dontub added the status:needs work There is code, but it needs additional work before it should be reviewed label Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:needs work There is code, but it needs additional work before it should be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When using SEPA mandate as a payment method, the contact notes are not getting mapped.

2 participants