Issue #3515924: Add "check your email" page.
Closes #3515924
Merge request reports
Activity
- Resolved by James Shields
- Resolved by James Shields
35 $container->get('entity_type.manager') 36 ); 37 } 38 39 /** 40 * Renders the thank you page. 41 * 42 * @param string $entity_id 43 * The entity ID of the verify email form. 44 * 45 * @return array 46 * A render array. 47 */ 48 public function thankYouPage($entity_id) { 49 // Load the verify_email entity using the entity manager 50 $entity = $this->entityTypeManager->getStorage('verify_email')->load($entity_id); - Comment on lines +48 to +50
If we define the parameter as an entity in the route definition, we can pass the entity in directly and skip the load (see comment in the
VerifyEmailRoutes
class): changed this line in version 2 of the diff
- Resolved by James Shields
106 106 public function testVerifyEmailForm(): void { 107 107 $this->drupalGet('/verify-email-form'); 108 108 $this->assertSession()->statusCodeEquals(200); changed this line in version 2 of the diff
110 111 $this->submitForm([ 111 112 'email' => 'another@test.com', 112 113 ], 'Verify'); 114 115 // Check if we are redirected to the correct 'Thank You' page. 116 // We assume the entity ID for the VerifyEmail form is 'verify_email_test'. 117 // So the thank you page route should be like /verify-email-form/thanks. 113 118 $this->assertSession()->statusCodeEquals(200); 114 $this->assertSession()->statusMessageContains('The message has been sent.'); 119 120 // Check if the correct 'Thank You' page is loaded. 121 // Check the path of the page to ensure we are redirected to the expected 'thanks' route. 122 $this->assertSession()->addressEquals('/verify-email-form/thanks'); 123 124 // Check that the title is 'Thank you'. 125 $this->assertSession()->titleEquals('Thank you'); This is failing because it's testing the actual page title, which is the title with the site name appended, which seems to be "Thank you | Drupal" in the CI. I prefer not to rely on a specific site name when writing tests, so I generally check against the
<h1>
element on the page:changed this line in version 2 of the diff
112 113 ], 'Verify'); 114 115 // Check if we are redirected to the correct 'Thank You' page. 116 // We assume the entity ID for the VerifyEmail form is 'verify_email_test'. 117 // So the thank you page route should be like /verify-email-form/thanks. 113 118 $this->assertSession()->statusCodeEquals(200); 114 $this->assertSession()->statusMessageContains('The message has been sent.'); 119 120 // Check if the correct 'Thank You' page is loaded. 121 // Check the path of the page to ensure we are redirected to the expected 'thanks' route. 122 $this->assertSession()->addressEquals('/verify-email-form/thanks'); 123 124 // Check that the title is 'Thank you'. 125 $this->assertSession()->titleEquals('Thank you'); 126 127 // Check if the page text contains the message confirming the email verification. - Comment on lines +115 to +127
I think the comments in this section are too verbose. These 4 lines are all checking the "Thank you" page loads, so I think a single comment should cover them all, and it could be terser. Something like "Check that after form submission redirection occurs to thank you page with dynamic route of /verify-email-form/thanks."
changed this line in version 2 of the diff
22 * VerifyEmailThankYouController constructor. 23 * 24 * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager 25 */ 26 public function __construct(EntityTypeManagerInterface $entityTypeManager) { 27 $this->entityTypeManager = $entityTypeManager; 28 } 29 30 /** 31 * {@inheritdoc} 32 */ 33 public static function create(ContainerInterface $container) { 34 return new static( 35 $container->get('entity_type.manager') 36 ); 37 } changed this line in version 2 of the diff
55 55 ], 56 56 ], 57 57 ); 58 59 // New route for the "/thanks" path with the controller. 60 $routes["verify_email.$entity_id.thanks"] = new Route( 61 path: '/' . $entity->getPath() . '/thanks', 62 defaults: [ 63 '_controller' => 'Drupal\verify_email\Controller\VerifyEmailThankYouController::thankYouPage', 64 '_title' => 'Thank you', 65 'entity_id' => $entity_id, changed this line in version 4 of the diff
1 <?php 2 3 namespace Drupal\verify_email\Controller; 4 5 use Drupal\Core\Controller\ControllerBase; 6 use Drupal\verify_email\Entity\VerifyEmail; 7 8 /** 9 * Controller for handling the thank you page. 10 */ 11 class VerifyEmailThankYouController extends ControllerBase { Just a small thing, but the class name seems unhelpfully long. I don't think it needs to contain "VerifyEmail", and I don't think "Controller" is needed in the name, as it's in the Controller directory. So I would just call it
ThankYou
.Filename will need to change, and it will need to be updated in
VerifyEmailRoutes
.changed this line in version 5 of the diff