Part of #1971384: [META] Convert page callbacks to controllers.
Convert form defined in user_form_test_current_password to a Controller.

CommentFileSizeAuthor
#8 password_controller-2035689-8.patch4.55 KBplopesc
#8 interdiff.txt1.14 KBplopesc
#5 password_controller-2035689-5.patch4.53 KBplopesc
#5 interdiff.txt2.93 KBplopesc
#3 password_controller-2035689-3.patch4.8 KBplopesc
#3 interdiff.txt1.75 KBplopesc
#1 password_controller-2035689-1.patch4.86 KBplopesc
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented
Assigned: Unassigned » plopesc
Status: Active » Needs review
FileSize
password_controller-2035689-1.patch4.86 KB

Hello

Attaching first approach for this task.

This patch has problems using the paramconverters, given that 'user', instead of taking the uid from path, takes the current user uid.

How should work this converter?

Regards

Status: Needs review » Needs work

The last submitted patch, password_controller-2035689-1.patch, failed testing.

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.75 KB
password_controller-2035689-3.patch4.8 KB

Hello

I fixed my converter error. I found that buildForm method parameter must respect the same name as in routing to be loaded well. I expected the User value in $acconut parameter instead of $user as defined in route.

Then I found another problem. $user entity is loaded fine, but tests still not working given that PhpassHashedPassword class is not yet ready to work with EntityNG values and expect that $user->pass is a string instead of FieldItemInterface.

Now tests passes because user_load funtion returns an instance of UserBCDecorator.

Then I think we have two options:

  • Use getBCDEntity metod to convert User object to its UserBCDecorator and still test passing for now. Once entities converted to EntityNG, test will fail and this line should be removed.
  • Mark this issue as postponed now and come back when entities will be converted to EntityNG.

Attaching patch that follows approach in bullet 1.

Regards

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented

Looks good to me except few minor doc fixes (below).

  1. +++ b/core/modules/user/tests/modules/user_form_test/lib/Drupal/user_form_test/Form/TestCurrentPassword.php
@@ -0,0 +1,75 @@
+ * Base class for implementing system configuration forms.

    Minor: doc needs update. 

  2. +++ b/core/modules/user/tests/modules/user_form_test/lib/Drupal/user_form_test/Form/TestCurrentPassword.php
@@ -0,0 +1,75 @@
+   * Implements \Drupal\Core\Form\FormInterface::validateForm().
...
+   * Implements \Drupal\Core\Form\FormInterface::submitForm().

    Minor: Can we use {@inheritdoc}

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented
Issue tags: +WSCCI-conversion
FileSize
interdiff.txt2.93 KB
password_controller-2035689-5.patch4.53 KB

Hello

Re-rolled using FormBase and following advices in #4

Regards.

disasm’s picture

disasm CreditAttribution: disasm commented
Status: Needs review » Reviewed & tested by the community

Reviewed this, patch applies, tests pass, conversion is straight forward, marking RTBC!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/user/tests/modules/user_form_test/lib/Drupal/user_form_test/Form/TestCurrentPassword.php
@@ -0,0 +1,67 @@
+  /**
+   * {@inheritdoc}
+   *
+   * @param \Drupal\user\Entity\User $user
+   *   The user account.
+   */
+  public function buildForm(array $form, array &$form_state, User $user = NULL) {

In general we typehint with interfaces rather than classes. I think this should be UserInterface.

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.14 KB
password_controller-2035689-8.patch4.55 KB

Typehinting fixed.

Regards.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

Thanks for your work on this issue! Please see #1971384-43: [META] Convert page callbacks to controllers for an update on the routing system conversion process.

disasm’s picture

disasm CreditAttribution: disasm commented
Status: Needs review » Reviewed & tested by the community

Clean and simple! RTBC!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Looks great, thanks!

Committed and pushed to 8.x.

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