-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add "Save entity" derivative #419
base: 8.x-3.x
Are you sure you want to change the base?
Changes from all commits
ed2a37f
95e0f49
9b52169
1714d7c
c2c4c01
ebc189d
6fb10ed
a46cfcc
ba09b7e
4b10458
5bcd4a1
47244f8
889098a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
<?php | ||
|
||
namespace Drupal\rules\Plugin\RulesAction; | ||
|
||
use Drupal\Component\Plugin\Derivative\DeriverBase; | ||
use Drupal\Core\Entity\ContentEntityTypeInterface; | ||
use Drupal\Core\Entity\EntityTypeManagerInterface; | ||
use Drupal\Core\Entity\EntityFieldManagerInterface; | ||
use Drupal\Core\Plugin\Discovery\ContainerDeriverInterface; | ||
use Drupal\Core\StringTranslation\StringTranslationTrait; | ||
use Drupal\Core\StringTranslation\TranslationInterface; | ||
use Drupal\rules\Context\ContextDefinition; | ||
use Symfony\Component\DependencyInjection\ContainerInterface; | ||
|
||
/** | ||
* Derives entity save plugin definitions based on content entity types. | ||
* | ||
* @see \Drupal\rules\Plugin\RulesAction\EntitySave | ||
*/ | ||
class EntitySaveDeriver extends DeriverBase implements ContainerDeriverInterface { | ||
|
||
use StringTranslationTrait; | ||
|
||
/** | ||
* The entity type manager. | ||
* | ||
* @var \Drupal\Core\Entity\EntityTypeManagerInterface | ||
*/ | ||
|
||
protected $entityTypeManager; | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing NL before this line. |
||
* The entity field manager. | ||
* | ||
* @var \Drupal\Core\Entity\EntityFieldManagerInterface; | ||
*/ | ||
protected $entityFieldManager; | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing NL before this line. |
||
* Saves a new EntitySaveDeriver object. | ||
* | ||
* @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager | ||
* The entity type manager. | ||
* @param \Drupal\Core\StringTranslation\TranslationInterface $string_translation | ||
* The string translation service. | ||
*/ | ||
public function __construct(EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, TranslationInterface $string_translation) { | ||
$this->entityTypeManager = $entity_type_manager; | ||
$this->entityFieldManager = $entity_field_manager; | ||
$this->stringTranslation = $string_translation; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public static function create(ContainerInterface $container, $base_plugin_id) { | ||
return new static( | ||
$container->get('entity_type.manager'), | ||
$container->get('entity_field.manager'), | ||
$container->get('string_translation') | ||
); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function getDerivativeDefinitions($base_plugin_definition) { | ||
foreach ($this->entityTypeManager->getDefinitions() as $entity_type_id => $entity_type) { | ||
// Only allow content entities and ignore configuration entities. | ||
if (!$entity_type instanceof ContentEntityTypeInterface) { | ||
continue; | ||
} | ||
|
||
$this->derivatives[$entity_type_id] = [ | ||
'label' => $this->t('Save @entity_type', ['@entity_type' => $entity_type->getLowercaseLabel()]), | ||
'category' => $entity_type->getLabel(), | ||
'entity_type_id' => $entity_type_id, | ||
'context' => [ | ||
'entity' => ContextDefinition::create("entity:$entity_type_id") | ||
->setLabel($entity_type->getLabel()) | ||
->setDescription($this->t('Specifies the @entity_type_label that should be saved permanently.', ['@entity_type_label' => $entity_type->getLabel()])) | ||
->setRequired(TRUE), | ||
'immediate' => ContextDefinition::create('boolean') | ||
->setLabel($this->t('Force saving immediately')) | ||
->setDescription($this->t('Usually saving is postponed till the end of the evaluation, so that multiple saves can be fold into one. If this set, saving is forced to happen immediately.')) | ||
->setDefaultValue(FALSE) | ||
->setRequired(FALSE), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated, but this should be actually required. NULL is invalid, only TRUE or FALSE is fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I change it to TRUE? It was "required = FALSE" check the line 29 above in EntitySave.php. |
||
], | ||
] + $base_plugin_definition; | ||
} | ||
|
||
return $this->derivatives; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,31 +55,31 @@ public function setUp() { | |
$bundle_field_definition->getItemDefinition() | ||
->willReturn($item_definition->reveal()); | ||
$bundle_field_definition->getCardinality()->willReturn(1) | ||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
$bundle_field_definition->getType()->willReturn('string'); | ||
$bundle_field_definition->getLabel()->willReturn('Bundle') | ||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
$bundle_field_definition->getDescription() | ||
->willReturn('Bundle mock description') | ||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
|
||
$bundle_field_definition_required->getItemDefinition() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are all these mocks really needed? Looks like left-overs from EntityCreateTest for me? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the EntityCreateTest file. the $bundle_field_definition_required and $bundle_field_definition_optional are used in willReturn() |
||
->willReturn($item_definition->reveal()); | ||
$bundle_field_definition_required->getCardinality()->willReturn(1) | ||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
$bundle_field_definition_required->getType()->willReturn('string'); | ||
$bundle_field_definition_required->getLabel()->willReturn('Required field') | ||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
$bundle_field_definition_required->getDescription() | ||
->willReturn('Required field mock description') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is that used? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. The test does not work without these lines. All the comments in EntityCreateTest.php are unrelated. I only changed shouldBeCalledTimes(1) to shouldBeCalled() as you suggested during discussion. |
||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
$bundle_field_definition_required->isRequired() | ||
->willReturn(TRUE) | ||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if those are called 0 times, remove them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. those are called 1 time |
||
|
||
$bundle_field_definition_optional->isRequired() | ||
->willReturn(FALSE) | ||
->shouldBeCalledTimes(1); | ||
->shouldBeCalled(); | ||
|
||
// Prepare mocked entity storage. | ||
$entity_type_storage = $this->prophesize(EntityStorageBase::class); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
namespace Drupal\Tests\rules\Integration\Action; | ||
|
||
use Drupal\Core\Entity\EntityInterface; | ||
use Drupal\Core\Entity\EntityStorageBase; | ||
use Drupal\Tests\rules\Integration\RulesEntityIntegrationTestBase; | ||
|
||
/** | ||
|
@@ -33,7 +34,15 @@ public function setUp() { | |
|
||
$this->entity = $this->prophesizeEntity(EntityInterface::class); | ||
|
||
$this->action = $this->actionManager->createInstance('rules_entity_save'); | ||
// Prepare mocked entity storage. | ||
$entity_type_storage = $this->prophesize(EntityStorageBase::class); | ||
|
||
// Return the mocked storage controller. | ||
$this->entityTypeManager->getStorage('test') | ||
->willReturn($entity_type_storage->reveal()); | ||
|
||
// Instantiate the action we are testing. | ||
$this->action = $this->actionManager->createInstance('rules_entity_save:test'); | ||
} | ||
|
||
/** | ||
|
@@ -42,7 +51,21 @@ public function setUp() { | |
* @covers ::summary | ||
*/ | ||
public function testSummary() { | ||
$this->assertEquals('Save entity', $this->action->summary()); | ||
$this->assertEquals('Save test', $this->action->summary()); | ||
} | ||
|
||
/** | ||
* Tests the action execution. | ||
* | ||
* @covers ::execute | ||
*/ | ||
public function testActionExecution() { | ||
$this->entity->save()->shouldBeCalledTimes(1); | ||
|
||
$this->action->setContextValue('entity', $this->entity->reveal()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. gives exception " The entity context is not a valid context." |
||
->setContextValue('immediate', TRUE); | ||
|
||
$this->action->execute(); | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about this __construct()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems ok?