Convert two page callbacks in language_elements_test.module to the new controller system

Working on this

Here goes my first patch, be gentle!

phiit, good work on your first patch. Its close. I noticed a few whitespace issues. Always good to catch these as it makes the committer's life a lot easier. Plus there's a fatal error with the name of the form classes. These need to match what's in the routing.yml file.

+++ b/core/modules/language/tests/language_elements_test/lib/Drupal/language_elements_test/Form/LanguageConfigurationElement.php
@@ -0,0 +1,57 @@
+class LanguageConfigurationElement extends FormBase {
+
+  /**
+     * {@inheritdoc}
+     */
+  public function getFormID() {
+      return 'language_elements_configuration_element';
+  }

There are some whitespace issues with the @inheritdoc comment and the return clause, use only 2 spaces.

+++ b/core/modules/language/tests/language_elements_test/lib/Drupal/language_elements_test/Form/LanguageConfigurationElementTest.php
@@ -0,0 +1,46 @@
+ * @file
+ * Contains \Drupal\language_elements_test\Form\LanguageConfigurationElementTestForm.
+ */
+
+namespace Drupal\language_elements_test\Form;
+
+use Drupal\Core\Form\FormBase;
+
+/**
+ * A form containing a language select element.
+ */
+class LanguageConfigurationElementTest extends FormBase {

The class name needs to match whats in the routing.yml file. Otherwise you get nasty errors:

Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The service definition "\drupal\language_elements_test\form\languageconfigurationelementform" does not exist. in Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition() (line 875 of /home/lucas/websites/drupal/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php).

Ahh, got it. Yeah I wasn't that sure about the class names, I'll give it another go. And somehow I missed a bunch of these whitespaces errors from c&p.

Fixed class names on routing. Interdiff attached for comparison.

@phiit, this looks good now.

+++ b/core/modules/language/tests/language_elements_test/lib/Drupal/language_elements_test/Form/LanguageConfigurationElement.php
@@ -0,0 +1,57 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, array &$form_state) {
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function submitForm(array &$form, array &$form_state) {

+++ b/core/modules/language/tests/language_elements_test/lib/Drupal/language_elements_test/Form/LanguageConfigurationElementTest.php
@@ -0,0 +1,46 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, array &$form_state) {
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function submitForm(array &$form, array &$form_state) {

Hm. If we're not implementing any code in those methods, I don't think these should be here. Else you're going to end up canceling the upstream validate/submit code that's happening in FormBase, no?

I left them there as the class extending FormBase need to implement these methods or they need to be declared abstract. How should I proceed with the new patch?

Continued working on this issue.

From the same meta issue:
Issue #1979034: Convert views_test_data_element_form() to a Controller has the same empty methods committed.
Issue #2035689: Convert user_form_test_current_password to a Controller doesn't have validateForm method and it's committed.

I've verified that the methods submitForm and validateForm are indeed abstract. Which way is preferred in this case? a) leave them out or b) use empty methods or c) do something else.

@phiit: What I just told you in person was actually completely wrong as I completely misunderstood this issue.

+++ b/core/modules/language/tests/language_elements_test/lib/Drupal/language_elements_test/Form/LanguageConfigurationElement.php
@@ -0,0 +1,57 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, array &$form_state) {
+  }

+++ b/core/modules/language/tests/language_elements_test/lib/Drupal/language_elements_test/Form/LanguageConfigurationElementTest.php
@@ -0,0 +1,46 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, array &$form_state) {
+  }

This can be removed, it's already in FormBase

Re @webchick: FormBase does not actually do any validation and submitting. Also there's a test in core that checks that the language element submit actually is called. So the empty submit should be fine as is.

Empty methods removed.

Ahh, sorry for not being precise. The submitForm() actually needs to stay, because that's not in FormBase. Only validateForm() should go. Could you re-roll for that real quick? That would be awesome!

Yes, great. Thanks a lot!

So this has been sitting for a while and core changed in the meantime:

+++ b/core/modules/language/tests/language_elements_test/language_elements_test.routing.yml
@@ -0,0 +1,13 @@
+  pattern: '/language-tests/language_configuration_element'
...
+  pattern: '/language-tests/language_configuration_element_test'

This should be:

  path: '/language....'

Ahh alright, changes made.

Yay, thanks. Let's see if the bot likes this...

Cool, let's do this.

#20: core-language-elements-test-routing-2086499-20.patch queued for re-testing.

#20: core-language-elements-test-routing-2086499-20.patch queued for re-testing.

I think this was done in #2102653: Convert test form callbacks to new form controller

Nope, this is still needed.

Needed PSR-4 switch.

And removed dead code.

Removed the $pid per Jibran's review

Simple enough.

Committed 5106b55 and pushed to 8.x. Thanks!