Modernize contact.module forms

The changes made so far look spot on.

You missed a couple t() in CategoryFormController and MessageFormController.

Also in the other conversions I fixed "Definition of Drupal\..." to be "Contains \Drupal\...", and switched Implements/Overrides to be {@inheritdoc}.

#7: issue-2072897-7.patch queued for re-testing.

+++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
@@ -187,7 +249,7 @@ public function save(array $form, array &$form_state) {
       drupal_mail('contact', 'page_autoreply', $sender->getEmail(), $language_interface->id, $params);

#1889644: Convert drupal_mail_system() to a MailFactory service to allow more flexible replacements is in we can inject mail service.

11: issue-2072897-11.patch queued for re-testing.

Patch Rerolled.

Updated #21 patch with new modifications.

Ignore last patch. Size got too large. Updation to #21 patch with modifications.

New patch without syntax errors and some modifications to #21.

1. +++ b/core/modules/contact/lib/Drupal/contact/CategoryFormController.php
   @@ -2,12 +2,14 @@
    /**
     * @file
   - * Definition of Drupal\contact\CategoryFormController.
   + * Contains Drupal\contact\CategoryFormController.
     */
   

   Contains \Drupal\... with a leading backslash.

2. +++ b/core/modules/contact/lib/Drupal/contact/CategoryFormController.php
   @@ -2,12 +2,14 @@
   +use Drupal\Core\Config\ConfigFactory;
   

   You should be using ConfigFactoryInterface now everywhere in this patch in use, type hints and documentation.

3. +++ b/core/modules/contact/lib/Drupal/contact/CategoryFormController.php
   @@ -15,20 +17,46 @@
    
      /**
   -   * Overrides Drupal\Core\Entity\EntityFormController::form().
   +   * The configuration storage.
   +   *
   +   * @var \Drupal\Core\Config\ConfigFactory
   +   */
   +  protected $configStorage;
   +
   

   This is misnamed.. should be something like $contactSettings and @var is \Drupal\Core\Config\Config.

4. +++ b/core/modules/contact/lib/Drupal/contact/CategoryFormController.php
   @@ -15,20 +17,46 @@
   +   * Constructs a new ConfigImportForm.
   

   Wrong class name, copy & pasted from somewhere.

5. +++ b/core/modules/contact/lib/Drupal/contact/CategoryFormController.php
   @@ -15,20 +17,46 @@
   +   * @param \Drupal\Core\Config\ConfigFactory $config_storage
   +   *   The configuration storage.
   +   */
   +  public function __construct(ConfigFactory $config_storage) {
   

   Variable name and description is wrong, this is $config_factory, not $config_storage.

6. +++ b/core/modules/contact/lib/Drupal/contact/CategoryFormController.php
   @@ -101,29 +129,39 @@ public function save(array $form, array &$form_state) {
            ->save();
        }
    
   +
        $form_state['redirect_route']['route_name'] = 'contact.category_list';
   +    $form_state['redirect'] = 'admin/structure/contact';
   +  }
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function delete(array $form, array &$form_state) {
   +    $form_state['redirect'] = 'admin/structure/contact/manage/' . $this->entity->id() . '/delete';
   +
      }
    
   

   All those changes should be removed.

7. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
   @@ -2,15 +2,21 @@
     * @file
   - * Definition of Drupal\contact\MessageFormController.
   + * Contains Drupal\contact\MessageFormController.
     */
   

   Same here, leading \ before Drupal.

8. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
   @@ -2,15 +2,21 @@
   +use Drupal\Core\Entity\EntityFormControllerNG;
   

   This class no longer exists, remove the use.

9. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
   @@ -25,9 +31,69 @@ class MessageFormController extends ContentEntityFormController {
   +  /**
   +   * The configuration storage.
   +   *
   +   * @var \Drupal\Core\Config\ConfigFactory
   +   */
   +  protected $configStorage;
   

   Same as above, $contactSettings...

10. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
    @@ -25,9 +31,69 @@ class MessageFormController extends ContentEntityFormController {
    +  public function __construct(LanguageManager $language_manager, ConfigFactory $config_storage, FloodInterface $flood, UserStorageControllerInterface $user_storage) {
    

    Same here with configuration storage, this is the factory. Also, LanguageManagerInterface instead of LanguageManager.

11. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
    @@ -25,9 +31,69 @@ class MessageFormController extends ContentEntityFormController {
       public function form(array $form, array &$form_state) {
    +
         $user = \Drupal::currentUser();
         $message = $this->entity;
    

    This should use $this->currentUser().

12. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
    @@ -136,15 +201,15 @@ public function preview(array $form, array &$form_state) {
       public function save(array $form, array &$form_state) {
    -    $user = \Drupal::currentUser();
     
    +    $user = \Drupal::currentUser();
    

    Also here.

13. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
    @@ -211,12 +276,15 @@ public function save(array $form, array &$form_state) {
    +    if ($message->isPersonal() && $user->hasPermission('access user profiles')) {
    +      $uri = $message->getPersonalRecipient()->uri();
           $form_state['redirect_route'] = $message->getPersonalRecipient()->urlInfo();
    +      $form_state['redirect'] = array($uri['path'], $uri['options']);
         }
    

    $uri/$form_state['redirect'] should also be removed.

Made all improvements as per comment #29. Uploading patch and interdiff for review.

1. +++ b/core/modules/contact/lib/Drupal/contact/CategoryFormController.php
   @@ -140,28 +140,16 @@ public function save(array $form, array &$form_state) {
   -
   -    $form_state['redirect_route']['route_name'] = 'contact.category_list';
   -    $form_state['redirect'] = 'admin/structure/contact';
   -  }
   

   Now you removed too much, the redirect_route is still needed.

2. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
   @@ -59,9 +58,9 @@ class MessageFormController extends ContentEntityFormController {
       *   The language manager.
       * @param \Drupal\Core\Config\ConfigFactory $config_storage
       *   The configuration storage.
   

   The documentation here also needs to be udated.

3. +++ b/core/modules/contact/lib/Drupal/contact/MessageFormController.php
   @@ -94,7 +93,7 @@ public static function create(ContainerInterface $container) {
      public function form(array $form, array &$form_state) {
    
   -    $user = \Drupal::currentUser();
   +    $user = \Drupal::$this->currentUser();
        $message = $this->entity;
        $form = parent::form($form, $form_state, $message);
        $form['#attributes']['class'][] = 'contact-form';
   @@ -205,7 +204,7 @@ public function preview(array $form, array &$form_state) {
   
   @@ -205,7 +204,7 @@ public function preview(array $form, array &$form_state) {
       */
      public function save(array $form, array &$form_state) {
    
   -    $user = \Drupal::currentUser();
   +    $user = \Drupal::$this->currentUser();
        $language_interface = \Drupal::languageManager()->getCurrentLanguage();
   

   Without \Drupal, just $this->currentUser().

Update to #30 patch. Made modifications as per review on #33.

Patch in #34 no longer applies. Tagging as needs reroll.

Hasn't been touched in 2 weeks. I'll work on it. :)

This patch applies cleanly at commit 0e6c631

rerolled, and UserStorageController -> UserStorage rename.

1. +++ b/core/modules/contact/src/CategoryForm.php
   @@ -16,20 +18,46 @@
   +  public function __construct(ConfigFactoryInterface $config_factory) {
   +    $this->contactSettings = $config_factory->get('contact.settings');
   ...
   -    $default_category = \Drupal::config('contact.settings')->get('default_category');
   +    $default_category = $this->contactSettings->get('default_category');
   

   Use $this->config(), no reason for injection

2. +++ b/core/modules/contact/src/MessageForm.php
   @@ -53,15 +87,18 @@ public function __construct(EntityManagerInterface $entity_manager, FloodInterfa
   +      $container->get('config.factory'),
   
   @@ -168,15 +205,14 @@ public function preview(array $form, array &$form_state) {
   +    $user = $this->currentUser();
   

   suppose $this->config() should be used as provided by base class

fixed #43.

Looks good -- only two small issues:

1. +++ b/core/modules/contact/src/CategoryForm.php
   @@ -93,33 +93,33 @@ public function validate(array $form, array &$form_state) {
   +    $contactSettings = $this->config('contact.settings');
   ...
   -    $contact_config = \Drupal::config('contact.settings');
   

   Local variables should be $lower_underscored.

   Not sure why $contact_config is touched here at all?

2. +++ b/core/modules/contact/src/MessageForm.php
   @@ -2,16 +2,19 @@
    namespace Drupal\contact;
    
   +
    use Drupal\Component\Utility\String;
   

   Extraneous newline.

thanks for review. fixed.

rerolled. a chunk of code was removed in #1856562: Convert "Subject" and "Message" into Message base fields, and a user_access call fix was committed from #2061977: Replace user_access() calls with $account->hasPermission() in all core modules except user.

Thanks.

+1 rtbc

git ac https://www.drupal.org/files/issues/2072897-49.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 14398  100 14398    0     0  14638      0 --:--:-- --:--:-- --:--:-- 14797
error: patch failed: core/modules/contact/src/CategoryForm.php:43
error: core/modules/contact/src/CategoryForm.php: patch does not apply
error: patch failed: core/modules/contact/src/MessageForm.php:11
error: core/modules/contact/src/MessageForm.php: patch does not apply

Rerolled the patch, but have doubt following line:

-    $sender = clone $this->entityManager->getStorage('user')->load($user->id());
+    $sender = clone $this->userStorage->load($user->id());

Please review.

Declared $languageManager once.

re #53: yes, keeping the $userStorage property wasn't necessary. removed.
Also fixed the reroll issues, and a new "t()" call.

+1 rtbc

Committed 40fbf6b and pushed to 8.x. Thanks!

+++ b/core/modules/contact/src/MessageForm.php
@@ -38,7 +38,7 @@ class MessageForm extends ContentEntityForm {
   /**
    * The language manager service.
    *
-   * @var \Drupal\Core\Language\LanguageManagerInterface
+   * @var \Drupal\Core\Language\LanguageManager
    */
   protected $languageManager;

Reverted this change on commit. This should be typehinted to the interface.