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

Files: 
CommentFileSizeAuthor
#8 password_controller-2035689-8.patch4.55 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 58,712 pass(es).
[ View ]
#8 interdiff.txt1.14 KBplopesc
#5 password_controller-2035689-5.patch4.53 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 58,411 pass(es).
[ View ]
#5 interdiff.txt2.93 KBplopesc
#3 password_controller-2035689-3.patch4.8 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 56,826 pass(es).
[ View ]
#3 interdiff.txt1.75 KBplopesc
#1 password_controller-2035689-1.patch4.86 KBplopesc
FAILED: [[SimpleTest]]: [MySQL] 56,808 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

plopesc’s picture

Assigned:Unassigned» plopesc
Status:Active» Needs review
StatusFileSize
new4.86 KB
FAILED: [[SimpleTest]]: [MySQL] 56,808 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

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

Status:Needs work» Needs review
StatusFileSize
new1.75 KB
new4.8 KB
PASSED: [[SimpleTest]]: [MySQL] 56,826 pass(es).
[ View ]

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

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

Issue tags:+WSCCI-conversion
StatusFileSize
new2.93 KB
new4.53 KB
PASSED: [[SimpleTest]]: [MySQL] 58,411 pass(es).
[ View ]

Hello

Re-rolled using FormBase and following advices in #4

Regards.

disasm’s picture

Status:Needs review» Reviewed & tested by the community

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

webchick’s picture

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

Status:Needs work» Needs review
StatusFileSize
new1.14 KB
new4.55 KB
PASSED: [[SimpleTest]]: [MySQL] 58,712 pass(es).
[ View ]

Typehinting fixed.

Regards.

xjm’s picture

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

Status:Needs review» Reviewed & tested by the community

Clean and simple! RTBC!

webchick’s picture

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.