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

Change reaction-rule config to save the event with settings #405

Open
wants to merge 11 commits into
base: 8.x-3.x
Choose a base branch
from
38 changes: 15 additions & 23 deletions config/schema/rules.schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,21 @@ rules.reaction.*:
label:
type: label
label: 'Label'
event:
type: string
label: 'Event'
events:
type: sequence
label: 'Events'
sequence:
type: mapping
label: 'Event'
mapping:
event_name:
type: string
label: 'Name'
configuration:
type: sequence
label: 'Configuration'
sequence:
type: mapping
module:
type: string
label: 'Module'
Expand Down Expand Up @@ -184,26 +196,6 @@ rules_expression.rules_rule:
type: rules_expression.[id]
label: 'Actions'

rules_expression.rules_reaction_rule:
type: rules_expression
label: "Reaction Rule"
mapping:
id:
type: string
label: 'Plugin ID'
uuid:
type: string
label: 'UUID'
event:
type: string
label: 'Event'
conditions:
type: rules_expression.[id]
label: 'Conditions'
actions:
type: rules_expression.[id]
label: 'Actions'

rules.context.definition:
type: mapping
label: 'Context definition'
Expand Down
11 changes: 6 additions & 5 deletions src/Engine/RulesComponent.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,12 @@ public function getContextDefinitions() {
*/
public function addContextDefinitionsFrom(ConfigEntityInterface $rules_config) {
if ($rules_config instanceof ReactionRuleConfig) {
$event_name = $rules_config->getEvent();
// @todo Use setter injection for the service.
$event_definition = \Drupal::service('plugin.manager.rules_event')->getDefinition($event_name);
foreach ($event_definition['context'] as $context_name => $context_definition) {
$this->addContextDefinition($context_name, $context_definition);
foreach ($rules_config->getEventNames() as $event_name) {
// @todo Use setter injection for the service.
$event_definition = \Drupal::service('plugin.manager.rules_event')->getDefinition($event_name);
foreach ($event_definition['context'] as $context_name => $context_definition) {
$this->addContextDefinition($context_name, $context_definition);
}
}
}
return $this;
Expand Down
39 changes: 32 additions & 7 deletions src/Entity/ReactionRuleConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* config_export = {
* "id",
* "label",
* "event",
* "events",
* "module",
* "description",
* "tag",
Expand Down Expand Up @@ -121,11 +121,16 @@ class ReactionRuleConfig extends ConfigEntityBase {
protected $module = 'rules';

/**
* The event name this reaction rule is reacting on.
* The events this reaction rule is reacting on.
*
* @var string
* Events array. The array is numerically indexed and contains arrays with the
* following structure:
* - event_name: string with the event machine name.
* - configuration: an array containing the event configuration.
*
* @var array
*/
protected $event;
protected $events;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should initialize this as empty array with "[]".


/**
* Sets a Rules expression instance for this Reaction rule.
Expand Down Expand Up @@ -208,10 +213,30 @@ public function getTag() {
}

/**
* Returns the event on which this rule will trigger.
* Gets configuration of all events the rule is reacting on.
*
* @return array
* The events array. The array is numerically indexed and contains arrays
* with the following structure:
* - event_name: string with the event machine name.
* - configuration: an array containing the event configuration.
Copy link
Owner

Choose a reason for hiding this comment

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

Description sentences should start upper-case

*/
public function getEvents() {
return $this->events;
}

/**
* Gets fully qualified names of all events the rule is reacting on.
*
* @return string[]
* The array of fully qualified event names of the rule.
*/
public function getEvent() {
return $this->event;
public function getEventNames() {
$names = [];
foreach ($this->events as $event) {
$names[] = $event['event_name'];
}
return $names;
}

/**
Expand Down
18 changes: 11 additions & 7 deletions src/Entity/ReactionRuleStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,10 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
protected function getRegisteredEvents() {
$events = [];
foreach ($this->loadMultiple() as $rules_config) {
$event = $rules_config->getEvent();
if ($event && !isset($events[$event])) {
$events[$event] = $event;
foreach ($rules_config->getEventNames() as $event_name) {
if (!isset($events[$event_name])) {
$events[$event_name] = $event_name;
}
}
}
return $events;
Expand All @@ -106,10 +107,13 @@ public function save(EntityInterface $entity) {

// After the reaction rule is saved, we need to rebuild the container,
// otherwise the reaction rule will not fire. However, we can do an
// optimization: if the event was already registered before, we do not have
// to rebuild the container.
if (empty($events_before[$entity->getEvent()])) {
$this->drupalKernel->rebuildContainer();
// optimization: if every event was already registered before, we do not
// have to rebuild the container.
foreach ($entity->getEventNames() as $event_name) {
if (empty($events_before[$event_name])) {
$this->drupalKernel->rebuildContainer();
break;
}
}

return $return;
Expand Down
2 changes: 1 addition & 1 deletion src/EventSubscriber/GenericEventSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public function onRulesEvent(Event $event, $event_name) {
// another rule.
foreach ($triggered_events as $triggered_event) {
// @todo Only load active reaction rules here.
$configs = $storage->loadByProperties(['event' => $triggered_event]);
$configs = $storage->loadByProperties(['events.*.event_name' => $triggered_event]);

// Loop over all rules and execute them.
foreach ($configs as $config) {
Expand Down
3 changes: 2 additions & 1 deletion src/Form/ReactionRuleAddForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ public function form(array $form, FormStateInterface $form_state) {
}
}

$form['event'] = [
$form['events']['#tree'] = TRUE;
$form['events'][]['event_name'] = [
'#type' => 'select',
'#title' => $this->t('React on event'),
'#options' => $options,
Expand Down
17 changes: 11 additions & 6 deletions src/Form/ReactionRuleEditForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,17 @@ public static function create(ContainerInterface $container) {
public function form(array $form, FormStateInterface $form_state) {
$this->addLockInformation($form);

$event_name = $this->entity->getEvent();
$event_definition = $this->eventManager->getDefinition($event_name);
$form['event']['#markup'] = $this->t('Event: @label (@name)', [
'@label' => $event_definition['label'],
'@name' => $event_name,
]);
foreach ($this->entity->getEventNames() as $key => $event_name) {
$event_definition = $this->eventManager->getDefinition($event_name);
$form['event'][$key] = [
'#type' => 'item',
'#title' => $this->t('Events') . ':',
'#markup' => $this->t('@label (@name)', [
'@label' => $event_definition['label'],
'@name' => $event_name,
]),
];
}
$form_handler = $this->entity->getExpression()->getFormHandler();
$form = $form_handler->form($form, $form_state);
return parent::form($form, $form_state);
Expand Down
44 changes: 39 additions & 5 deletions tests/src/Kernel/EventIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function testUserLoginEvent() {
$config_entity = $this->storage->create([
'id' => 'test_rule',
'expression_id' => 'rules_rule',
'event' => 'rules_user_login',
'events' => [['event_name' => 'rules_user_login']],
'configuration' => $rule->getConfiguration(),
]);
$config_entity->save();
Expand All @@ -81,7 +81,7 @@ public function testUserLogoutEvent() {
$config_entity = $this->storage->create([
'id' => 'test_rule',
'expression_id' => 'rules_rule',
'event' => 'rules_user_logout',
'events' => [['event_name' => 'rules_user_logout']],
'configuration' => $rule->getConfiguration(),
]);
$config_entity->save();
Expand All @@ -108,7 +108,7 @@ public function testCronEvent() {
$config_entity = $this->storage->create([
'id' => 'test_rule',
'expression_id' => 'rules_rule',
'event' => 'rules_system_cron',
'events' => [['event_name' => 'rules_system_cron']],
'configuration' => $rule->getConfiguration(),
]);
$config_entity->save();
Expand All @@ -134,7 +134,7 @@ public function testSystemLoggerEvent() {
$config_entity = $this->storage->create([
'id' => 'test_rule',
'expression_id' => 'rules_rule',
'event' => 'rules_system_logger_event',
'events' => [['event_name' => 'rules_system_logger_event']],
'configuration' => $rule->getConfiguration(),
]);
$config_entity->save();
Expand All @@ -161,7 +161,7 @@ public function testInitEvent() {
$config_entity = $this->storage->create([
'id' => 'test_rule',
'expression_id' => 'rules_rule',
'event' => KernelEvents::REQUEST,
'events' => [['event_name' => KernelEvents::REQUEST]],
'configuration' => $rule->getConfiguration(),
]);
$config_entity->save();
Expand All @@ -185,4 +185,38 @@ public function testInitEvent() {
$this->assertRulesLogEntryExists('action called');
}

/**
* Test that rules config supports multiple events.
*/
public function testMultipleEvents() {
$rule = $this->expressionManager->createRule();
$rule->addCondition('rules_test_true');
$rule->addAction('rules_test_log');

$config_entity = $this->storage->create([
'id' => 'test_rule',
'expression_id' => 'rules_rule',
]);
$config_entity->set('events', [
['event_name' => 'rules_user_login'],
['event_name' => 'rules_user_logout'],
]);
$config_entity->set('configuration', $rule->getConfiguration());
$config_entity->save();
Copy link
Owner

Choose a reason for hiding this comment

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

This should test using the getter also.


// The logger instance has changed, refresh it.
$this->logger = $this->container->get('logger.channel.rules');

$account = User::create(['name' => 'test_user']);
// Invoke the hook manually which should trigger the rules_user_login event.
rules_user_login($account);
// Invoke the hook manually which should trigger the rules_user_logout
// event.
rules_user_logout($account);
Copy link
Owner

Choose a reason for hiding this comment

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

Nice that this works already!


// Test that the action in the rule logged something.
$this->assertRulesLogEntryExists('action called');
$this->assertRulesLogEntryExists('action called', 1);
}

}