Conversation
The extension fails to enable on CiviCRM 5.75 with a TypeError because entity types are not registered during installation. The MixinScanner only processes installed extensions, so the entity-types-php@2 mixin listener is never loaded for this extension during its own install. - Bundle entity-types-php@2.0.0.mixin.php (copied from Stripe, matching the standard CiviCRM pattern for extensions with custom entities) - Add direct hook_civicrm_entityTypes implementation as a fallback that registers entity types from schema/*.entityType.php regardless of mixin timing. When both fire, the result is identical (same keys). On CiviCRM 6.4+ this is a harmless no-op.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical installation failure on CiviCRM 5.75 caused by a timing issue where entity types were not being registered. By bundling the required mixin and implementing a direct hook for entity type registration, the extension now correctly registers its entities during the installation process, ensuring compatibility across CiviCRM versions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new mixin for auto-registering entity declarations and implements a direct hook in paymentprocessingcore.php to resolve a timing issue during extension installation on CiviCRM 5.75. The review feedback suggests adding an array type hint to the hook parameter for PHPStan level 9 compliance, improving the handling of glob() results to avoid iterating over a [false] array on failure, and adding validation for entity data within the mixin to ensure consistency and robustness.
| * | ||
| * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_entityTypes | ||
| */ | ||
| function paymentprocessingcore_civicrm_entityTypes(&$entityTypes): void { |
There was a problem hiding this comment.
Add the array type hint to the $entityTypes parameter. This ensures compliance with PHPStan level 9 requirements for proper typing and improves code clarity by explicitly defining the expected type for the reference.
function paymentprocessingcore_civicrm_entityTypes(array &$entityTypes): void {References
- PHPStan level 9 compliance (proper types, no mixed where avoidable) (link)
| $files = (array) glob($schemaDir . '/*.entityType.php'); | ||
| foreach ($files as $file) { |
There was a problem hiding this comment.
Casting the result of glob() directly to (array) is problematic because glob() returns false on failure. In PHP, (array) false results in [false], which causes the loop to execute once with an invalid filename, leading to a warning during the include call. Using the null coalescing operator ?: [] is a safer way to handle potential failures.
foreach (glob($schemaDir . '/*.entityType.php') ?: [] as $file) {| $files = (array) glob($mixInfo->getPath('schema/*.entityType.php')); | ||
| foreach ($files as $file) { | ||
| $entity = include $file; | ||
| $entity['module'] = $mixInfo->longName; | ||
| $e->entityTypes[$entity['name']] = $entity; | ||
| } |
There was a problem hiding this comment.
The glob() result should be handled safely to avoid issues if it returns false. Additionally, adding validation for the $entity variable (ensuring it is an array and contains a name) improves robustness and maintains consistency with the implementation in paymentprocessingcore.php. This prevents potential errors if a file in the schema directory is malformed or does not return the expected array.
foreach (glob($mixInfo->getPath('schema/*.entityType.php')) ?: [] as $file) {
$entity = include $file;
if (is_array($entity) && !empty($entity['name'])) {
$entity['module'] = $mixInfo->longName;
$e->entityTypes[$entity['name']] = $entity;
}
}References
- Prefer manual loops with explicit type checks to ensure compliance with strict static analysis and maintain consistency with established codebase patterns.
Response to Gemini review comments1. Add 2. Use 3. Add validation in the mixin file (mixin/entity-types-php@2.0.0.mixin.php:32-37) |
2 participants
Overview
Fixes the extension failing to enable on CiviCRM 5.75 with a TypeError. The entity types (PaymentAttempt, PaymentProcessorCustomer, PaymentWebhook) were not being registered during installation due to a mixin timing issue.
Before
Enabling the extension on CiviCRM 5.75 throws:
The previous backport (#21) added
mixin/polyfill.phpbut the polyfill is gated on!class_exists('CRM_Extension_MixInfo'). SinceMixInfoexists since CiviCRM 5.45, the polyfill never runs on 5.75.After
The extension installs and enables successfully on both CiviCRM 5.75 and 6.4.
Technical Details
CiviCRM's
MixinScanneronly processes installed extensions. During this extension's own installation, theentity-types-php@2mixin listener is never loaded, so entity types are not registered inEntityRepository. WhenCRM_Core_DAO_Base::getEntityDefinition()runs,getEntityNameForClass()returnsnullcausing the TypeError.Two changes fix this:
Bundle
mixin/entity-types-php@2.0.0.mixin.php— copied from the Stripe extension, matching the standard CiviCRM pattern (both Stripe and GoCardless bundle their entity-types mixin). This ensures theMixinScannerfinds the mixin in the extension's own directory.Add direct
hook_civicrm_entityTypesimplementation inpaymentprocessingcore.php— registers entity types by readingschema/*.entityType.phpdirectly, bypassing the mixin timing issue. When both this hook and the mixin fire (normal operation), the result is identical (same data written to the same keys). On CiviCRM 6.4+ this is a harmless no-op.Core overrides
No core overrides.
Comments
paymentprocessingcore.civix.phpfile was intentionally not modified as it is auto-generated by civix.$entityTypes[$entity['name']]with identical data, so the later write simply overwrites with the same content.