Problem/Motivation

Services should not keep the container. This refactors two scenarios where that occurs.

Steps to reproduce

Proposed resolution

The Executor uses the Factory directly to pass on the dependencies, rather than delegating this, which allows removing the create method.

For the PasswordReset data producer a test is written to document the current behaviour. Then the controller instantiation from the container is moved to a HTTP sub-request that calls the controller directly. This allows injecting the HTTP kernel rather than the entire container.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork graphql-3587709

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:

About issue forks

Comments

kingdutch created an issue. See original summary.

kingdutch opened merge request !98

kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Status: Active » Needs review
kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Issue summary: View changes
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Thanks, approach looks good to me. Just minor comments.

kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Status: Needs work » Needs review

I rebased the work to fix the comment about the container, that was part of the other issue that's now merged.

I've added a type intersection (supported in PHP 8.1) to indicate MailManager is a MockObject which should satisfy PHPStan in the test :) That should've addressed all comments.

kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • kingdutch committed 2ac6b7ff on 5.x 
    fix: #3587709 Services should not keep the container

Status: Fixed » Closed (fixed)

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