Problem/Motivation

Split off from #3580682: [pp-1] Create a service for user account cancellation. Convert cancel methods to plugins

User has only a few functions left, let's convert them here and update the api in a follow up.
user_cancel
_user_cancel
_user_cancel_session_regenerate
user_cancel_methods

Steps to reproduce

N/A

Proposed resolution

There is a new AccountCancellation service.

  • ::cancel
  • ::cancelMethods

There are two batch callbacks converted:

  • ::regenerateSession
  • ::cancelAccount

Remaining tasks

Review

User interface changes

N/A

Introduced terminology

N/A

API changes

AccountCancellation::cancel
AccountCancellation::cancelMethods

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3620912

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

nicxvan created an issue. See original summary.

nicxvan opened merge request !17003

nicxvan’s picture

nicxvan commented
Title: [ignore] testing issue » Deprecate user.module cancel methods
Issue summary: View changes
nicxvan’s picture

nicxvan commented
Issue summary: View changes
nicxvan’s picture

nicxvan commented
Status: Active » Needs review
danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented
Status: Needs review » Needs work

Reviewed this one. Node IDs in the Docblock need changing. Left one other comment about it being a little odd that some of these are public APIs but I don't feel strongly about it, especially since this is just a replacement of exisiting functionality. Maybe we think about that once this is in.

Not too many test changes needed as part of this either. Makes me wonder if we need to check we have proper coverage.

nicxvan’s picture

nicxvan commented
Status: Needs work » Needs review

Thanks for the review! I updated the change record links.

I also replied to your batch question.

There is a follow up to redesign the api that adds a ton of testing, but we have not been adding testing to these conversions unless we are changing the api surface which this does not.

The test failure is random: builds/core/modules/system/tests/src/FunctionalJavascript/ThemeSettingsFormTest.php:85

danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented
Status: Needs review » Reviewed & tested by the community

Thanks for updating! All feedback addressed. I think this is ready for RBTC now.

godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Needs work

Thanks for all the work on this. It's close, but I have a few comments on the MR.

nicxvan’s picture

nicxvan commented
Status: Needs work » Reviewed & tested by the community
Related issues: +#3621541: Create closure properties for UserController

I addressed all of the feedback, it was pretty minimal so I think self RTBC is ok, I created the follow up. #3621541: [pp-1] Create closure properties for UserController

amateescu’s picture

amateescu commented

Reviewed and posted a few code suggestions, the most important one being that we shouldn't break the 11.4 deprecation from AccountSettingsForm.

  • amateescu committed 9e837365 on main 
    task: #3620912 Deprecate user.module cancel methods

By: nicxvan
By:...
amateescu’s picture

amateescu commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed 9e8373659f2 to main. Thanks!

We'll need a 11.x MR :)

nicxvan opened merge request !17057

nicxvan’s picture

nicxvan commented

Backport was clean except entity resource constructor and the services yml.

It was pretty straightforward, I think I can mark it once tests pass.

Edit: I need to add the new argument to the jsonapi services.

nicxvan’s picture

nicxvan commented
Status: Patch (to be ported) » Reviewed & tested by the community

This is ready!

  • amateescu committed c615ff14 on 11.x 
    task: #3620912 Deprecate user.module cancel methods

By: nicxvan
By:...
amateescu’s picture

amateescu commented
Status: Reviewed & tested by the community » Fixed

Committed c615ff1 and pushed to 11.x. Thanks!

Also published the CR.

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.