Convert user_cancel_confirm() to a new-style Form object [#2403729]

Part of #1971384: [META] Convert page callbacks to controllers

Convert this page callback to a new-style Form object, using the instructions on http://drupal.org/node/1800686

There is currently a wrapper method at Drupal\user\Controller\UserController::confirmCancel().

Found while working on: #1800174: Add missing type hinting to User module docblocks

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category	Task because it is refactoring which doesn't change functionality.
Unfrozen changes	Unfrozen because it refactors deprecated code. See: #1971384: [META] Convert page callbacks to controllers
Disruption	Minor disruption because not many areas of code are in the business of removing user accounts.

Comment	File	Size	Author
#3	2403729-confirm-cancel-3.patch	8.01 KB	kim.pepper

Comments

Comment #1

kim.pepper

English

🏄‍♂️🇦🇺Sydney, Australia

commented 8 January 2015 at 02:37

Assigned:

Unassigned

» kim.pepper

I will look at this.

Comment #2

kim.pepper

English

🏄‍♂️🇦🇺Sydney, Australia

commented 8 January 2015 at 02:51

\Drupal\user\Form\UserCancelForm was first created in #1946466: Convert all confirm_form() in user.module and user.pages.inc to the new form interface and convert route but is not related to what this callback is for. It is for confirming cancellation via an emailed cancellation link.

Comment #3

kim.pepper

English

🏄‍♂️🇦🇺Sydney, Australia

commented 8 January 2015 at 03:19

Assigned:	kim.pepper	» Unassigned
Status:	Active	» Needs review

Status	File	Size
new	2403729-confirm-cancel-3.patch	8.01 KB

Removed user_cancel_confirm() and put logic into \Drupal\user\Controller\UserController::confirmCancel

Added some dependency injection, and used $this->redirect() from base class.

Comment #4

mile23

English

Seattle, WA

commented 9 January 2015 at 01:34

Title:	Convert user_cancel_confirm to a new-style Form object	» Convert user_cancel_confirm() to a new-style Form object
Issue summary:	View changes
Status:	Needs review	» Reviewed & tested by the community

At times like this, I wish SimpleTest tests had a @covers option and that people had used it. :-)

But the test is Drupal\user\Tests\UserCancelTest, and it looks like it covers this behavior.

So glad we can make changes like this:

+++ b/core/modules/user/src/Controller/UserController.php
@@ -42,9 +50,10 @@ class UserController extends ControllerBase {
-  public function __construct(DateFormatter $date_formatter, UserStorageInterface $user_storage) {
+  public function __construct(DateFormatter $date_formatter, UserStorageInterface $user_storage, UserDataInterface $user_data) {

Adding a beta evaluation, marking it as unfrozen because it's DX and DrupalWTF otherwise.

Comment #5

alexpott

he/they

English

🇪🇺🌍

commented 12 January 2015 at 13:55

Status:

Reviewed & tested by the community

» Fixed

Yep these wrappers should have been removed. Committed 5f2a572 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the summary.

Can someone open a follow to move the lonely theme function in user.pages.inc to user.module?

Comment #6

12 January 2015 at 13:57

alexpott committed 5f2a572 on 8.0.x

Issue #2403729 by kim.pepper: Convert user_cancel_confirm() to a new-...

Comment #7

cosmicdreams commented 12 January 2015 at 15:25

/**
   * Constructs a UserController object.
   *
   * @param \Drupal\Core\Datetime\DateFormatter $date_formatter
   *   The date formatter service.
   * @param \Drupal\user\UserStorageInterface $user_storage
   *   The user storage.
   */
  public function __construct(DateFormatter $date_formatter, UserStorageInterface $user_storage, UserDataInterface $user_data) {
    $this->dateFormatter = $date_formatter;
    $this->userStorage = $user_storage;
    $this->userData = $user_data;
  }

I was reading over the commit and found that the constructor's code comment wasn't updated to describe the new parameter.

Should we open a follow up issue for that?