Problem/Motivation

In the service definition content_moderation.services.yml we have

content_moderation.workspace_subscriber:
  class: Drupal\content_moderation\EventSubscriber\WorkspaceSubscriber
  arguments: ['@entity_type.manager', '@?workspaces.association']
  tags:
    - { name: event_subscriber }

Notice that @?workspaces.association is optional.

In the class itself \Drupal\content_moderation\EventSubscriber\WorkspaceSubscriber we have

/**
 * Constructs a new WorkspaceSubscriber instance.
 *
 * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager
 *   The entity type manager service.
 * @param \Drupal\workspaces\WorkspaceAssociationInterface $workspaceAssociation
 *   The workspace association service.
 */
public function __construct(
  protected readonly EntityTypeManagerInterface $entityTypeManager,
  protected readonly WorkspaceAssociationInterface $workspaceAssociation
) {}

Notice that $workspaceAssociation is required.

Proposed resolution

Make $workspaceAssociation optional in \Drupal\content_moderation\EventSubscriber\WorkspaceSubscriber.

Remaining tasks

Implement the proposed resolution.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

Release notes snippet

Nope.

Issue fork drupal-3411308

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

ndf created an issue. See original summary.

Keshav Patel made their first commit to this issue’s fork.

ndf’s picture

Status: Active » Needs review

ndf’s picture

@Keshev Patel, thanks for the MR. Can you please explain why you opted for the required parameter solution? This is helpful for other people to review your MR.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Yes an explanation is needed. Also a test case

pooja saraah’s picture

In the original service definition, the second argument is specified as @?workspaces.association. The ? indicates that this argument is optional, meaning that the WorkspaceSubscriber class can accept either the workspaces.association service or no service at all.

In the modified service definition, the optional operator ? has been removed from the second argument. This means that the WorkspaceSubscriber class now explicitly expects the @workspaces.association service as a required argument.

Purpose of the Change:
The modification aligns the service definition with the actual constructor requirements of the WorkspaceSubscriber class. By removing the optional operator, the service definition accurately reflects that the WorkspaceSubscriber class expects the @workspaces.association service to be provided during instantiation.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

dstorozhuk’s picture

After apply the patch from merge request !5978 I have that error when trying to enable/disable any module:
The service "content_moderation.workspace_subscriber" has a dependency on a non-existent service "workspaces.association".

amateescu’s picture

Category: Bug report » Task
Issue summary: View changes
Issue tags: -Needs tests

In the service definition, the workspaces.association service must be marked as optional because it might not exist (for example when the Workspaces module is not enabled).

Note that this is not a bug because \Drupal\content_moderation\EventSubscriber\WorkspaceSubscriber is an event subscriber class, and it shouldn't be instantiated unless it needs to react to the WorkspacePrePublishEvent event, which is only invoked when the Workspaces module is enabled. If some other module instantiates it.. well, it shouldn't :)

If we want to make a change in core though, we need to mark the argument as optional in \Drupal\content_moderation\EventSubscriber\WorkspaceSubscriber::__construct(). Updated the issue summary to reflect that.

stefan.korn made their first commit to this issue’s fork.

stefan.korn’s picture

Thanks for the explanation.

As mentioned in #3242564: Workspaces can't be published from the command line devel quits with WSOD on "Event info". And yes, this is happening if the content moderation module is installed and the workspaces module is not.

Basically

Drupal::service('event_dispatcher')->getListeners();

is failing then.

It seems to me that getListeners() without event argument is a valid call, therefore I am wondering if this should really end in error here (no exceptions for the method pointing in that direction either) even if the workspaces module is not installed.

And the difference between service definition (optional) and implementation (required) does also seem not really straightforward.

I have amended the MR to make the change in the implementation and not the service definition.

amateescu’s picture

Status: Needs work » Reviewed & tested by the community

Ok, let's do this, the change is harmless and allows the class to be instantiated :)

  • catch committed a60890d4 on 11.x
    Issue #3411308 by stefan.korn, ndf, amateescu: WorkspaceSubscriber...

  • catch committed 0a15c5b8 on 10.3.x
    Issue #3411308 by stefan.korn, ndf, amateescu: WorkspaceSubscriber...
catch’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

stefan.korn’s picture

  • catch committed a60890d4 on 11.0.x
    Issue #3411308 by stefan.korn, ndf, amateescu: WorkspaceSubscriber...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.