Drupal\eca\Plugin\Action\ActionBase causes issues for action plugins that extend FieldUpdateActionBase

Moved this to the ECA project, since the implementation of our ActionBase is responsible for this issue when action plugins extend Drupal core's FieldUpdateActionBase.

ECA needs to keep the constructor of the ActionBase unchanged and handle extra stuff in the creator.

Turns out, we can't resolve that in ECA, unfortunately. This is a special case for action plugins that extend FieldUpdateActionBase and override the constructor. I've now created MR!4 here in APITools to "fix" the action plugin such that it still works in a context where the FieldUpdateActionBase base class is being aliased, like by ECA. But it also works without ECA, ofcourse.

@armrus please give the MR!4 a try, ideally in both scenarios, with and without ECA being enabled.

@jurgenhaas i check the MR!4 - all works fine.
There patch from your MR for use before MR is applied to module.

Thanks for testing, setting the status to RTBC, hoping that the maintainers will merge and release this fix.

Hi, thanks for using the module and posting an issue.

I unfortunately can't merge this code in because I'm very unclear what the actual error is and what this fix is for.

I've never used the ECA module, there is nothing in apitools that integrates with it directly.
Regarding this response:

hat sounds like ApiTools has implemented an action plugin specifically for ECA and not as a generic action? Sounds like that implementation in that apitools module is not correct.

I'm not sure what this is referring to.

Regarding this error message:

Drupal\apitools\Plugin\Action\WorkflowStateAction::create(Symfony\Component\DependencyInjection\ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) must be compatible with Drupal\eca\Plugin\Action\ActionBase::create(Symfony\Component\DependencyInjection\ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition): Drupal\eca\Plugin\Action\ActionBase

I don't understand where this error comes from. No apitools classes are extending any eca classes, nor are eca classes extending apitools classes. There's no current integration between the two modules. There must be patches involved that would cause this error, can you maybe list all of the patches or overridden code you're including?

Regarding the MR:

• This line won't work. $eventDispatcher is not a public property nor should it be
• I'm not sure the reason and benefit for calling the parent ::create method.
• I don't think it's sustainable to not be able to ever override the constructor. I'm also not entirely sure again what the error is that's being addressed here.

I don't want to close this issue in case there is a real issue somewhere. Please provide more context as to how the code in these two modules is getting mixed in.

I've found the thread in slack where this started and I now have more context as to why the error message is showing up like this. https://drupal.slack.com/archives/C0287U62CSG/p1697106849930059?thread_t...

I'm still not sure how the alias is working, particularly in such a way that APItools thinks it's overriding an ECA class.

I'm willing to help with a solution, though, the MR/patch laid out here is not going to work.

I talked this over with jurgenhaas. This plugin and class is low-key enough to remove the constructor override.

If I could just ask that you:

• Close this ticket out and create a new one with a very clear description. Example: #3067546: [Webform 6.x] Fix subclassing and stop overriding constructors → . I'd like to make sure it has no reference to ECA or any ECA error messages as that will confuse the origin of the change.
• Please move the MR and update the code to create a setter method for the event dispatcher. As it is right now, it's setting a protected property

Working on it at 📌 Stop overriding constructor in action plugin Active

Work for this is completed in 🐛 Drupal\eca\Plugin\Action\ActionBase causes issues for action plugins that extend FieldUpdateActionBase Closed: duplicate for better context