comments icon indicating copy to clipboard operation
comments copied to clipboard

Dynamic notificationAdmins configuration causes "Element query executed before Craft is fully initialized" warning and closure assignment error

Open romainpoirier opened this issue 10 months ago • 8 comments

Describe the bug

I'm trying to create a dynamic list for the notificationAdmins setting in the Comments plugin (verbb/comments). In my config/comments.php file I use the following code:

use craft\elements\User;

$moderators = [];
foreach (User::find()->group('moderators')->all() as $user) {
    $moderators[] = [
        'email'   => $user->email,
        'enabled' => true,
    ];
}

return [
    '*' => [
        // … other settings …
        'notificationAdmins' => $moderators,
        // … other settings …
    ]
];

While this code works functionally, it produces the following warning:

[craft\elements\db\ElementQuery::prepare] Element query executed before Craft is fully initialized

I then attempted to encapsulate the dynamic query within an arrow function to delay its execution:

'notificationAdmins' => fn() => array_map(
    fn($user) => [
        'email'   => $user->email,
        'enabled' => true,
    ],
    User::find()->group('moderators')->all()
),

However, this produces a PHP error:

Cannot assign Closure to property verbb\comments\models\Settings::$notificationAdmins of type array

I also tried moving the logic into an event listener (executed after Craft is fully initialized):

if (!Craft::$app->getRequest()->getIsConsoleRequest()) {
    Event::on(
        WebApplication::class,
        WebApplication::EVENT_INIT,
        function () {
            $moderators = [];
            $users = User::find()->group('moderators')->all();
            foreach ($users as $user) {
                $moderators[] = [
                    'email'   => $user->email,
                    'enabled' => true,
                ];
            }

            // Attempt to update the Comments plugin configuration.
            // Note: This requires that the Comments plugin allows modifying its settings at runtime.
            $pluginsService = Craft::$app->getPlugins();
            $commentsPlugin = $pluginsService->getPlugin('comments');

            if ($commentsPlugin !== null) {
                // Get the current settings.
                $settings = $commentsPlugin->getSettings();

                // Merge the dynamic moderators into the settings.
                $settings['notificationAdmins'] = $moderators;

                // Update the plugin settings.
                // <<-- I don't know what to put here to transmit the new configuration to the plugin -->

                Craft::info(
                    'Dynamic notificationAdmins updated in the Comments plugin configuration.',
                    __METHOD__
                );
            }
        }
    );
}

The issue is that I don't know how to update or transmit the new configuration to the plugin. I cannot simply assign a closure in config/comments.php because the plugin expects notificationAdmins to be an array. Also, using an event listener to update the configuration post‑initialization does not provide a clear API or method for setting the new value.


Additional context

  • The goal is to have a dynamic list of moderator emails for the notificationAdmins setting that updates based on the users in the "moderators" group.
  • This worked in a static environment but causes warnings and errors in Craft due to the timing of when configuration files are evaluated.
  • The similar plugin example provided here uses an arrow function behind an app-ready guard (e.g., fn() => Craft::$app->globals->getSetByHandle('abandonedCart')->firstReminderSubject), but this approach doesn't work for notificationAdmins because the property is strictly typed as an array.

Request

Could you please advise or provide a solution for updating the dynamic notificationAdmins setting without triggering early query execution or closure assignment errors? Specifically, is there an API or a recommended approach for modifying the Comments plugin settings at runtime in production?

Steps to reproduce

  1. In your config/comments.php file, add the following code:

    use craft\elements\User;
    
    $moderators = [];
    foreach (User::find()->group('moderators')->all() as $user) {
        $moderators[] = [
            'email'   => $user->email,
            'enabled' => true,
        ];
    }
    
    return [
        '*' => [
            // … other settings …
            'notificationAdmins' => $moderators,
            // … other settings …
        ]
    ];
    
  2. Observe that while the setting works functionally, you get the warning:

    [craft\elements\db\ElementQuery::prepare] Element query executed before Craft is fully initialized
    
  3. Next, modify the configuration to wrap the dynamic query in an arrow function:

    'notificationAdmins' => fn() => array_map(
        fn($user) => [
            'email'   => $user->email,
            'enabled' => true,
        ],
        User::find()->group('moderators')->all()
    ),
    
  4. Observe that you now get the PHP error:

    Cannot assign Closure to property verbb\comments\models\Settings::$notificationAdmins of type array
    
  5. Finally, attempt to move the logic into an event listener (executed on WebApplication::EVENT_INIT) as shown above and note that while the dynamic list is calculated, there is no clear API or method provided by the plugin to update its configuration with the new array value.

Craft CMS version

4.4.17

Plugin version

2.0.5

Multi-site?

Yes

Additional context

No response

romainpoirier avatar Feb 04 '25 16:02 romainpoirier

I then attempted to encapsulate the dynamic query within an arrow function to delay its execution:

That'll be the preferred approach I believe, but we do need handling on our end to allow this setting to be supplied as a Closure and array.

'notificationAdmins' => fn() => array_map(
    fn($user) => [
        'email'   => $user->email,
        'enabled' => true,
    ],
    User::find()->group('moderators')->all()
),

Updated for the next release. To get this early, run composer require verbb/comments:"dev-craft-4 as 2.0.16".

engram-design avatar Feb 05 '25 03:02 engram-design

Thank you for the update and for implementing the handling of closures in the next release!

I had initially attempted the following approach, but encountered an issue where the closure itself was being stored instead of executed:

'notificationAdmins' => fn() => array_map(
    fn($user) => [
        'email'   => $user->email,
        'enabled' => true,
    ],
    User::find()->group('moderators')->all()
),

This resulted in the following output:

[notificationAdmins] => Closure Object
    (
        [this] => craft\services\Config Object
        (
            [_events:yii\base\Component:private] => Array
                (
                )

            [_eventWildcards:yii\base\Component:private] => Array
                (
                )

            [_behaviors:yii\base\Component:private] => 
            [env] => production
            [configDir] => /var/www/html/config
            [appDefaultsDir] => /var/www/html/vendor/craftcms/cms/src/config/defaults
            [_configSettings:craft\services\Config:private] => Array
                (
                    [general] => craft\config\GeneralConfig Object
                        (
                            [_events:yii\base\Component:private] => Array
                                (
                                )
                            [_eventWildcards:yii\base\Component:private] => Array
                                (
                                )
                            [_behaviors:yii\base\Component:private] => 
                            [_errors:yii\base\Model:private] => 
                            [_validators:yii\base\Model:private] => 
                            [_scenario:yii\base\Model:private] => default
                            [filename:protected] => general
                        )
                )
        )
    )
... [truncated]

As a workaround, I modified it to execute array_map immediately:

'notificationAdmins' => array_map(
    fn($user) => [
        'email'   => $user->email,
        'enabled' => true,
    ],
    User::find()->group('moderators')->all()
),

This correctly returns the expected array, but still triggers the following warning:

2025-02-05 09:16:20 [web.WARNING] [craft\elements\db\ElementQuery::prepare] Element query executed before Craft is fully initialized. Stack trace:
...

Also, will this approach be extended to the Abandoned Cart plugin's configuration? (See [#88](https://github.com/verbb/abandoned-cart/issues/88)).

Looking forward to the update, and thanks again for the quick response!

romainpoirier avatar Feb 05 '25 09:02 romainpoirier

the closure itself was being stored instead of executed

Where did you see this, in project config? On my end, it seems to be storing the array correctly, not the Closure (which wouldn't be possible to store and would throw an error).

Image

Or, are you fetching that yourself somewhere in a module? If so, using $settings->getNotificationAdmins() is the correct approach.

And yep, just working on a suitable pattern for a few other plugins to use as well.

engram-design avatar Feb 05 '25 22:02 engram-design

I see the Closure Object output from my example by using print_r() + exit in the comments.php file to check what is being passed to the plugin’s configuration when it is initialized. Wouldn't this be a good approach to verify that it's working correctly?

However, something is unclear: are you saying that this would be stored in the project config? That wouldn’t be a viable solution since this data depends on the database, and in production, the project config is in read-only mode.

romainpoirier avatar Feb 06 '25 08:02 romainpoirier

Testing in that fashion won't really work, as that's correct you'll just be printing out the literal closure.

But yes, that's indeed a great point. While this will always be dynamic and evaluated in real-time with real users in production, values are still saved in the project config data, which just adds to "noise" in your changes as you commit code to project config. I can confirm this is currently all working for me correctly now, but I think I can improve that DX by removing it from the project config.

engram-design avatar Feb 07 '25 03:02 engram-design

FYI, just pushed a fix for that content stored in project config. As I said, calling the value in comments.php is going to just produce the Closure, but that's why we dynamically evaluate it at runtime.

engram-design avatar Feb 07 '25 20:02 engram-design

Thank you for the explanations! It looks like everything is working as expected.
Do you have an estimated release date for this? I'd like to avoid using dev-craft-4 in production.

romainpoirier avatar Feb 10 '25 15:02 romainpoirier

Should be a release scheduled at the end of the week.

engram-design avatar Feb 11 '25 04:02 engram-design

Updated in 2.0.17

engram-design avatar Jul 18 '25 09:07 engram-design