| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | major |
| Assigned: | tim.plunkett |
| Status: | needs work |
| Issue tags: | actions, Blocks-Layouts, Needs upgrade path, Needs upgrade path tests, rules, VDC |
Issue Summary
Goal
Replace the current actions API by a better one, which provides reasonable UX and fulfills the needs for Rules. Having two incompatible action APIs is bad for Drupal and its ecosystem - so this needs a fix.
Next, blocks&layouts needs a conditions API: #1743686: Condition Plugin System - we want to have conditions and actions with a similar DX and UX. Also, we want a conditions API that Rules can build upon to avoid having two incompatible APIs again.
Proposed resolution
1. Add a new actions API (done!)
2. Add a conditions API that works the same way (done!)
3. Convert core to leverage the new API (remove old action usages and that node operation stuff) (done!)4. Remove action.module
4. Rename the action.module to action_ui.module
What's wrong with actions.module?
- The current way of configuring actions is very unintuitive. It does not make sense for users to switch to another interface for configuring an action, then go back to whereever to use it. Instead, the primary action configuration should happen on the fly in a multi-step form or so. (Yes, there is a use-case for having pre-configured actions, but that's far not the most common one.)
- Actions need multiple parameters as input. Having just one which is identified solely by an "action type" does not suffice, nor does a magic but undefined $context array for further input help. I need to be able to know when I can execute an action, for which I need to have a complete description of the required inputs.
- Then, another thing the current action system does is tracking the number of recursive calls and caring about recursion. I don't think that's something the action system should take care of. It should provide me with an action that I can execute - whether this creates a recursive loop is up to the calling code; usually (think VBO) it won't. Recursions may occur in the regular trigger/rules use-case, but then it's caused by the triggering logic so the check should be there.
So in the end, I see nothing valuable in the existing actions API left. Let's better re-design it from scratch with the tools we already have: Plugins + TypedData definitions for describing inputs (=action parameters).
The new API
Focus
Imo, the focus for core should be on providing an action API that can be configured on the fly - i.e. I select the action, optionally get a configuration form, press submit - it's executed. There is no storage or separate screen for pre-configuration actions required.
Based upon that basic API an UI + storage can be added to build a screen for pre-configuring actions. But I do not even think we need a screen for "pre-configured actions in core?
Defined parameters
We already haved typed data definitions in core, which allow use to describe the needed data similar to what Rules does already in Drupal 7. By doing, so we exactly know what the required action parameters are, so we can determine action plugins that are executable given a certain input (e.g. a node).
Moreover, to fulfill the needs of Rules we need to have that kind of data definition for any input - i.e. any string input needs to be defined as parameter. Thus, action parameter configuration is totally decoupled from forms, forms are just the UI on top of the API. That makes it easy to re-use an action from code also.
Relationship to Conditions
Code-wise a condition is very similar to an action, so we can share most of the code. The difference between actions and conditions really is that conditions impose more restrictions upon the actual implementation: It must return a boolean, and it may not alter the input variables (conditions are not expected to issue changes). As conditions impose restrictions upon actions, we can extend conditions from actions, i.e. have ConditionInterface extends ActionInterface. Likewise we can probably extend some of the required classes or use them directly, while for developers the condition API appears to be a separate one that just works the same way as actions. E.g.
-> have \Drupal\Core\Action\ActionManager
-> have \Drupal\Core\Condition\ConditionManager
So the condition core component would have a dependency on the action component.
Using the new API
As said, actions would be based upon the plugin system, so they are instantiated via a respective plugin manager service. For configuring action methods should be used, e.g. like this:
<?php
drupal_container()->get('action.manager')
->createInstance('node_set_title')
->setArgument('node', $node)
->setArgument('text', 'new title')
->execute();
?>More things required by Rules
Unfortunately there is even more we need to have to make the API usable by Rules also. It mostly boils down to features needed for configuring parameters: The ability to select the argument from an available data input (e.g. to configure 'node'), optionally via a data selector ('node:referenced-node') and the ability to have DataProcessors which process the argument value before it is passed on to the action. The core use-case here would be the Tokens, i.e. have tokens replaced before the inputted text is passed on to the action. Rules can add more data processor plugins then.
To keep things reasonable simple in core I think we should make sure that the API for this is in place, but only provide a simple and focussed UI for it in core; e.g. we do not need to have Rules' data selector (for selection node:author:referenced-node:title) in core, a simple select for choosing the "Node" to go with suffices. Rules, then should be able to override this for its own usage by a more sophisticated UI - this should be rather easy by using dependency injection, e.g. we just need to allow Rules to inject it's own ParameterFormManager later on.
User interface changes
The action pre-configuration screen would go away. The remaining 1% use-case of pre-configuring actions instead of configuring them on the fly could be handled by Rules.
Relatively unchanged.
API changes
The actions API would completely change.
Code
In patch.
Comments
#1
adding tags
#2
Closely related: #1839516: Various subsystems require an Entity Operations API. Introduce one.
#3
Here is some API brainstorming I did together with EclipseGC at BADcamp: http://pastebin.com/WafrnPrF
@entity-operations:
I discussed that with fubhy today and the relationship to actions came to my mind also. I think it's different though, as entiy operations is more about the available operations you can list or link to when listing an entity. So maybe some actions could be exposed as entity operations but its main purpose is quite different I think.
#4
Would love to see this happen and am happy to pitch in on the sandbox once it's up.
#5
Can you clarify where exactly you see the difference between an action and an entity operation?
I think that would be a tremendous help for everyone. Thanks in advance.
#6
Unlike actions, entity operations come with a menu callback that is ready to use for linking to it. Then I think for entity operations it would be best to require them to return a renderable array only (or just have it at the callback), so e.g. view or different view-modes can be an operation also. Example: A commerce order might have "view order" and "view invoice" operations. However, there is no "action" functionality involved there.
Then something like the "edit" operation also differs as it would get the regular entity-form for an entity operation, but an entity update action would have a different looking form that e.g. allows you to populate entity references like node author by referring to existing context variables (e.g. the current user).
#7
mmm, yeah, I rather thought of entity operations à la "delete", but also "clone", "enable", "disable", "publish", "unpublish", "make sticky", "promote to frontpage" though... some entity operations may be configurable, and so are actions... in the end, I see quite some overlap ;)
But OK, let's move forward here independently for now, and only later on double-check how much these two things are duplicating each other. :)
#8
Forcing something to use context does not seem counter to each action providing a form. Sounds like operations are a particular route to a thing, where that thing might be an action with constrained or predetermined values.
#9
We discussed the overlap between actions and operations today on IRC and came to the conclusion that actions CAN be exposed as operations. We would do that through derivative discovery by picking up all actions that want to be picked up as operations.
#10
Thought I'd post my own code example of where this might generally go here: https://gist.github.com/4067136
#11
Ok, I've been speaking to EclipseGc and fubhy; I'm going to work on creating an initial patch for this.
#12
Great!
Adding another thought - I think we should leverage #1845546: Implement validation for the TypedData API for validating parameter configuration once its in. With that we can just put 'constraints' into the parameter-description and leverage it to easily validate parameter values independently from forms. So when you manually execute an action, we can fire up the same validation as during action configuration via forms.
#13
I've created a sandbox for this now: http://drupal.org/sandbox/damiankloip/1853786
It's currently just a rough start, but you have to start somewhere!
#14
#15
Any progress on this?
#16
While I initially didn't grok at all the crossover between operations and actions, I've been actively developing this D7 module: http://drupal.org/project/entity_operations, and sure enough, I've found that there is huge benefit from making some operations be actions: they get exposed to VBO, Services, and more.
It would be great to get people with more D8 experience than I have to look at it.
#17
yes - we started working on #1743686: Condition Plugin System. It comes with a parent executable core component which actions and conditions can re-use. Once we have conditions done, we can simply use the same code as a base for actions. We'd have to allow actions to provide variables/context and obviously migrate the current uses.
#18
#1743686-74: Condition Plugin System already in core
making this major
#19
Even though it's not strictly for this discussion to fully resolve, IMO the definition of automation events has always been a thorny issue. Now that Trigger module has been removed from core, there is nothing in particular remotely governing the nature of events triggering actions. Given #1509164: Use Symfony EventDispatcher for hook system is postponed to D9, we need to officially answer the question as to whose responsibility it will be to "standardize" automation events.
Should I create this as a separate task, or does someone here already have the answer?
#20
I'm afraid we might be falling between two stools. Actions are not plugins (#1788104: Convert actions to plugin sub-system was stopped being worked on I guess in favor of this), while this has also not been progressing lately. What we have on the other hand is very custom DB storage for very custom code and actions neither using plugins, nor config entities (both of which they should). I'm afraid if we are shooting too high and not reaching those goals, then we'll just not move actions forward in any way. That would mean actions would not have any use in core, and could just as well be removed.
(The D8MI perspective is actions in core like dsm or send email should be translatable but there is no way now since they are stored in a blob field serialized, and no general config entity system is used so we can affect this in any way).
#21
I suppose I might take that to mean automation events are out (from core).
On the other hand, Rules in D7 already stores configurations as entities and sports an i18n extension for translating parameter settings (e.g. message for dsm). Apart from the inevitable hard work involved, I don't see why Rules for D8 can't do the same.
If I understood fago right, this issue is similar to the condition plugin system, i.e. to standardize an API for defining/using actions. From the D6 days, Rules implemented its own action system (presumably) to handle multiple typed parameters, among other features. That Rules became more popular than core Trigger for automation meant other modules (e.g. VBO) had to support Rules executables in their own (sometimes convoluted) ways. Defining this API would go a long way towards making maintainers for a number of modules agree on the structure and dynamics of an action.
The fact that we've landed on plugins is no accident (since that's what it was there for). The multilingual aspect (apart from
@Translationannotations) will be up to Rules. The removal of Trigger simplifies this consideration.#22
Yes they are. I see no problem with that, Rules will introduce an "event" plugin system as in D7.
@Gabor: I answered it over at #1788104-14: Convert actions to plugin sub-system.
#23
Erm, acton module *is in core*. Action configuration with human facing text as admin input is in core. It is not out of core. it is in.
#24
Automation events triggering those actions (with conditions) will be defined in Rules.
As for the core Actions module and the text configuration, I presume a
ConfigEntityInterface-based actions API refactoring is in order (for entity-related needs, e.g. translation) in a separate issue. I'm not sure it's realistic to remove it at any point, considering Views is in core and the core Actions module does provide some useful extensions for bulk operations (unless the community supports an upgrade path directly from core Actions to Rules, which will then need #D8CX).This issue's main intent is to unify the interface underlying the advanced actions managed in the Actions admin UI and other possible actions, e.g. managed in Rules. However, if Actions is indeed to become just an API, then most of the remaining Actions code can be split between System and Views. If not, then Actions will provide an overview of available actions provided by all, and then a UI for managing custom, configurable "actions" (e.g. dsm).
In the latter case, a detailed investigation will be required regarding how to treat an action as core-customizable. Should the case be "all actions can be customized", then entity contexts will have to be considered (e.g. for node actions). With typed data in core, it should at least theoretically be less impossible to define the criteria under which an action can be customized in the core UI.
#25
So actions should be plugins a-la conditions now?
#26
I need to sync up with @damiankloip about this, but I've thought about this more from the "please kill hook_user_operations and hook_node_operations" side of things.
In the meantime, assigning to myself.
#27
I will have an update tomorrow. Closed #1788104: Convert actions to plugin sub-system in the meantime.
#28
This depends on #1851086: Replace admin/people with a View and #1895160: Convert admin/content to a View, keep a non-views fallback with no bulk operations.
It will not apply until they are committed.
It introduces a new plugin type called
Operation.This replaces
hook_user_operation()andhook_node_operation(), as wellhook_action_info().However, since many actions actually need to be configured, this patch also introduces a new ConfigEntity,
ActionAn Action has a 1:1 relationship with an Operation. It actually directly mirrors the new Block Entity/Plugin architecture, and works quite well here.
This has a lot of @todos and other cleanups that can take place in the next couple days while those views conversions are finalized.
Purposefully leaving needs work for now.
#29
Sandbox is at http://drupalcode.org/sandbox/tim.plunkett/1698392.git/shortlog/refs/hea...
#30
As I summarized before:
An Action is an instance of an Operation.
An Operation is a potential candidate for an Action.
#31
Ideally this would use \Drupal\Core\Executable\ExecutableInterface.
However, ExecutableInterface is completely wrapped up in Conditions and is unusable on its own. See #1920822: Decouple Drupal\Core\Executable from Drupal\Core\Condition and Drupal\Core\Form\FormInterface.
#32
Okay, got it passing. Here it is with both conversions included.
The upgrade path is blocked on #1998204: config_install_default_config() is not safe to use in hook_update_N().
#33
Hmm yeah we need action plugins. Certainly after reading this patch :)
+++ b/core/lib/Drupal/Core/Annotation/Operation.phpundefined@@ -0,0 +1,69 @@
+ * @return
+ * An array of operations. Each operation is an associative array that may
+ * contain the following key-value pairs:
+ * - label: (required) The label for the operation, displayed in the dropdown
+ * menu.
It's strange and a bit confusing to add @return docs to a class..
+++ b/core/lib/Drupal/Core/Operation/OperationBase.phpundefined@@ -0,0 +1,58 @@
+ /**
+ * {@inheritdoc}
+ */
+ public function execute(array $entities) {
+ foreach ($entities as $entity) {
+ $this->executeSingle($entity);
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function executeSingle($object) {
+ $this->execute(array($object));
+ }
This looks kinda strange for a base implementation... Why are you calling $this->execute() inside executeSingle. Doesn't that trigger an infinite loop?
+++ b/core/lib/Drupal/Core/Operation/OperationManager.phpundefined@@ -0,0 +1,52 @@
+ $this->discovery = new AnnotatedClassDiscovery('Operation', $namespaces, array(), 'Drupal\Core\Annotation\Operation');
+ $this->discovery = new AlterDecorator($this->discovery, 'operation_info');
Hmm don't we need to cache this? (and wasn't there a patch somewhere that enabled caching by default?)
+++ b/core/modules/action/action.moduleundefined@@ -130,568 +113,9 @@ function actions_do($action_ids, $object = NULL, $context = NULL, $a1 = NULL, $a
+ $actions = entity_load_multiple('action', (array) $action_ids);
+ foreach ($actions as $action) {
+ $action->execute(array($object));
Why not use executeSingle here?
+++ b/core/modules/action/lib/Drupal/action/ActionListController.phpundefined@@ -0,0 +1,98 @@
+ $build['action_table'] = parent::render();
+ if (!$this->hasConfigurableActions) {
+ unset($build['action_table']['#header']['operations']);
+ }
+ $build['action_admin_manage_form'] = drupal_get_form(new ActionAdminManageForm(\Drupal::service('plugin.manager.operation')));
+ return $build;
Maybe a newbisch question. But this is part of a entityListingForm thingie. Why does it first build a table by callen parent::render and adds another table on the bottom? EDIT never mind, adding an action is on the same page, if I understand the flow.
+++ b/core/modules/action/lib/Drupal/action/Plugin/Operation/GotoAction.phpundefined@@ -0,0 +1,69 @@
+ public function validate(array &$form, array &$form_state) {
If we have a ConfigurationOperationBase we don't need to insert an empty validate function every single time. Not sure what core policy says about this...
+++ b/core/modules/action/lib/Drupal/action/Plugin/Operation/MessageAction.phpundefined@@ -0,0 +1,97 @@
+ public function validate(array &$form, array &$form_state) {
See :D
+++ b/core/modules/node/lib/Drupal/node/NodeBCDecorator.phpundefined@@ -0,0 +1,16 @@
+class NodeBCDecorator extends EntityBCDecorator implements NodeInterface {
What do we need this here?
+++ b/core/modules/node/lib/Drupal/node/Plugin/Core/Entity/Node.phpundefined@@ -233,4 +234,15 @@ public function getRevisionId() {
+ /**
+ * {@inheritdoc}
+ */
+ public function getBCEntity() {
+ if (!isset($this->bcEntity)) {
+ $this->getPropertyDefinitions();
+ $this->bcEntity = new NodeBCDecorator($this, $this->fieldDefinitions);
+ }
+ return $this->bcEntity;
Same question...
+++ b/core/modules/node/lib/Drupal/node/Plugin/Operation/DemoteNode.phpundefined@@ -0,0 +1,32 @@
+ node_mass_update($entities, array('promote' => NODE_NOT_PROMOTED));
Out of scope for this issue and probably D9, but shouldn't we put this inside some controller holding some utility functions?
+++ b/core/modules/node/lib/Drupal/node/Plugin/Operation/UnpublishByKeywordNode.phpundefined@@ -0,0 +1,75 @@
+ $node->save();
+ break;
a break inside a foreach, is that how we should handle these kinda loops? (just a quesstion :) )
+++ b/core/modules/node/lib/Drupal/node/Plugin/Operation/UnstickyNode.phpundefined@@ -0,0 +1,32 @@
+class UnstickyNode extends OperationBase {
Hmm now I know why you're doing that strange loop thingie in OperationBase... Else you need to implement executeSingle here to call execute. But in the end I still think creating classes with infinite loops in it isn't a good idea...
#34
Pretending each dreditor docblock is numbered:
1) Yep, copy/paste. Will fix.
2) Yes. This is a bit strange. We'll see if I can find an elegant way to solve this.
3) It indeeds needs caching. See #1903346: Solve Trust issues with Plugin Discovery for the default caching.
4) Uhhhhh idk I will :)
5) :)
6) yep, I already considered adding a ConfigurationOperationBase, probably still will
8) That's part of the admin/content patch
9) See 8
10) resolved in the sandbox
11) Already in HEAD
12) See 2
#35
Ideally those two views issues will go in first. But in case they continue to be stalled, here is the above patch decoupled from them.
This addresses 4 and 6 from #34.
#36
The last submitted patch, operations-actions-1846172-35.patch, failed testing.
#37
Just some small comments.
+++ b/core/modules/action/lib/Drupal/action/ActionFormControllerBase.phpundefined@@ -0,0 +1,126 @@
+class ActionFormControllerBase extends EntityFormController {
Feels like a good usecase for abstract.
+++ b/core/modules/action/lib/Drupal/action/Form/ActionAdminManageForm.phpundefined@@ -22,12 +47,16 @@ public function getFormID() {
+ foreach ($this->manager->getDefinitions() as $id => $definition) {
+ if (is_subclass_of($definition['class'], '\Drupal\Core\Operation\ConfigurableOperationInterface')) {
+ $key = Crypt::hashBase64($id);
+ $actions[$key] = $definition['label'] . '...';
+ }
+ }
Why do we need all this crazyness ... Maybe a comment would help.
+++ b/core/modules/action/lib/Drupal/action/Plugin/Operation/GotoAction.phpundefined@@ -0,0 +1,69 @@
+class GotoAction extends OperationBase implements ConfigurableOperationInterface {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function executeSingle($object) {
+ drupal_goto($this->configuration['url']);
Urgs, this will be really hard to rip out later for RedirectResponse ...
+++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleContentTest.phpundefined@@ -209,6 +209,7 @@ function testContentTypeDirLang() {
+ module_enable(array('views'));
It seems to be that you not 100% truncated the patch but yeah, that's not a big deal at all.
+++ b/core/modules/node/lib/Drupal/node/Plugin/Operation/AssignOwnerNode.phpundefined@@ -0,0 +1,100 @@
+ $count = db_query("SELECT COUNT(*) FROM {users}")->fetchField();
...
+ if (is_numeric($this->configuration['owner_uid'])) {
+ $owner_name = db_query("SELECT name FROM {users} WHERE uid = :uid", array(':uid' => $this->configuration['owner_uid']))->fetchField();
...
+ $result = db_query("SELECT uid, name FROM {users} WHERE uid > 0 ORDER BY name");
Just wondering whether we want to use the ContainerFactory and inject stuff in there.
+++ b/core/modules/node/lib/Drupal/node/Plugin/Operation/UnpublishByKeywordNode.phpundefined@@ -0,0 +1,75 @@
+class UnpublishByKeywordNode extends OperationBase implements ConfigurableOperationInterface {
...
+ $elements = node_view(clone $node);
UnpublishByKeyword(Node|Comment) could be abstracted to an action based on entity + entity_view (maybe this is something for a follow up).
#38
I had a long talk with fago, who disliked the name Operation. So that is all Action now.
I need to respond to @dawehner still.
#39
The last submitted patch, actions-1846172-38.patch, failed testing.
#40
missing semicolons
#41
The last submitted patch, actions-1846172-40.patch, failed testing.
#42
Silly mistakes. This should pass. I would appreciate help with docs.
#43
tim.plunkett, you are my hero. That is all.
I did a brief glance at the patch.
1) Why are we doing:
+ if (empty($this->configuration['node'])) {+ $this->configuration['node'] = $entity;
+ }
Which part of code is still assuming nodes here?
2) Am I right to assume that this API no longer has global actions (previously type = 'system', in VBO type = 'entity')? Declaring an action available to all entity types has had a bit of a bumpy UX, so I'd love to see if we could use derivatives to provide one for each entity type.
For instance, we need a "delete entity" action that would work on any entity type, so that we don't need to duplicate code for every entity type we define (it is an action that is always needed).
Of course, this can be a followup.
3) I see that we are defining a confirmation page for deletions only.
VBO tried to tackle this generically, by introducing a setting for requiring confirmation per action.
We might not want to do that in D8, but there's still an open question of which action deserves a confirmation page, which action doesn't,
and whether we can remove the requirement to hand code it each time.
4) Am I right to assume that the API would still allow for actions that are configured on the fly? (like the issue summary claims)
An example would be sending an email, where after selecting the entities (users, for instance), an email is inputted and then sent off
to each one. We don't want to save a configured action entity, because the configuration of the action is discarded right after.
(So, we'd stick the action entity in a tempstore?)
5)
<?php+class SaveComment extends ActionBase {
+ public function execute($comment) {
+ $comment->save();
+ Cache::invalidateTags(array('content' => TRUE));
+ }
+}
?>
Sounds like we want a postExecute() method that would run after all of the passed entities have been processed (for invalidating tags or doing any other cleanup).
#44
Thanks @bojanz, I appreciate it.
I think all 5 of those points are excellent, and still very possible, but very much fodder for follow-ups.
1) That's taken directly from http://api.drupal.org/api/drupal/core%21modules%21action%21action.module...
2) Yes, I'm doing a 1:1 port right now, just to cut down on complexity. I absolutely want to use derivatives to unify stuff like PublishComment and PublishNode. And we still have three system actions: action_message_action, action_goto_action, and action_send_email_action (now MessageAction, GotoAction, and EmailAction).
3) I've already considered adding a form for all actions, but that would be a new UI and interaction pattern, and would need a bunch of new tests. Follow-up!
4) Yes, that should work just fine. entity_create() without a save.
5) +1, follow-up.
#45
Yeah, I think we should do derivates for all the important entity-generic actions. Not having them is bad for UX (what we are seeing with Rules), so I really think we should have that. We can have a generic action working with any entity as well.
Couldn't I just use the plugin, instanstiate it, configure it and execute it also? I think we should be able to that. Having to create an entity if you do not want to store anything feels wrong to me.
#46
Ah! Yeah you could absolutely use the plugin directly.
So, I opened #2001190: Use derivatives for action plugins and #2001196: Add a postExecute() step to action plugins.
What are the next steps before RTBC?
#47
Great work! I did a first quick review. I've not been able to through everything yet, but it should be a start:
+++ b/core/includes/bootstrap.inc@@ -3035,19 +3035,7 @@ function drupal_classloader_register($name, $path) {
+ * @todo We need a new example.
yes
+++ b/core/lib/Drupal/Core/Action/ActionBase.php@@ -0,0 +1,59 @@
+ public function executeMultiple(array $entities) {
+ foreach ($entities as $entity) {
+ $this->execute($entity);
this and execute pointing back looks like a loop?
+++ b/core/lib/Drupal/Core/Action/ActionInterface.php@@ -0,0 +1,44 @@
+ * @todo Extend \Drupal\Core\Executable\ExecutableInterface after
It might not be ideal yet, but still why not immediately extend from it and fix what needs to be fixed in follow-ups? Not using it is just confusing and seems wrong.
+++ b/core/lib/Drupal/Core/Action/ActionInterface.php@@ -0,0 +1,44 @@
+ * @see \Drupal\Core\Annotation\Operation
Left-over?
+++ b/core/lib/Drupal/Core/Action/ActionInterface.php@@ -0,0 +1,44 @@
+ public function execute($object);
$object should be a context? If I have an action that works with an entity, it should be in the context.
Not totally sure how that would work with multiple executing though. Maybe we could just define the multiple stuff to be required that way in the context?
+++ b/core/lib/Drupal/Core/Action/ConfigurableActionInterface.php@@ -0,0 +1,53 @@
+interface ConfigurableActionInterface extends ActionInterface {
+1 to separating that but that's an issue for Conditions as well. Let's treat it as one and just file it as follow-up to split out form interface?
+++ b/core/lib/Drupal/Core/Annotation/Action.php@@ -0,0 +1,56 @@
+ * A URL to redirect to after processing the action.
+ *
+ * @var string (optional)
+ */
That seems wrong to me. The code making use of the action should provide the default redirect, like going back to the node admin screen, not the action imho. Is there a use-case for which we need that?
+++ b/core/lib/Drupal/Core/Annotation/Action.php@@ -0,0 +1,56 @@
+ * @todo Replace with \Drupal\Core\Plugin\Context\Context.
Yes, imho that's critical and should be done for the initial patch.
#48
$this->factory = new DefaultFactory($this->discovery);+ $this->factory = new ContainerFactory($this);
Seems to be unrelated?
+ * Implements hook_user_role_insert().+ */
+function user_user_role_insert(RoleInterface $role) {
+ // Ignore the authenticated and anonymous roles.
+ if (in_array($role->id(), array(DRUPAL_AUTHENTICATED_RID, DRUPAL_ANONYMOUS_RI
+ retur
ouch. Roles should be a configuration parameter, so there is no reason for auto-generating an action per role. I doubt auto-generating actions is a good pattern. Instead, one should select the action "Remove user role" (or add) and configure it on-the-fly. That clutters the UI less and is more user-friendly as well I think. (You could even select multiple roles at once..) If that's complicated to do right now, it's something we might want to do in a follow-up? Or if we really want to keep the separate entries, it should implement a derivative but not auto-generate configs.