Convert system_date_time_formats() to a new style controller

I tried to assign this issue to myself. Going to take a shot at this.

Here's a first stab at this:

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
+/**
+ * Created by JetBrains PhpStorm.
+ * User: karen
+ * Date: 5/19/13
+ * Time: 11:21 AM
+ * To change this template use File | Settings | File Templates.
+ */

@file missing

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+

Not needed.

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
\ No newline at end of file

newline missing.

+++ b/core/modules/system/system.routing.ymlundefined
@@ -87,3 +87,10 @@ date_format_localize_reset:
\ No newline at end of file

Same as above

In general you should also remove the old code.

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
+ * Created by JetBrains PhpStorm.
+ * User: karen
+ * Date: 5/19/13
+ * Time: 11:21 AM
+ * To change this template use File | Settings | File Templates.

ups :)

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
+use Drupal\Core\Config\ConfigFactory;
+use Drupal\Core\Controller\ControllerInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;

None of this classes are actually used. Phpstorm shows you the code in grey if it's not used (note: plugin annotation aren't seen).

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
+class DateTimeController {
...
+  public function formats() {

I'm sorry both these needs some docs.

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+

Wow that's a lot of space

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,83 @@
\ No newline at end of file

+++ b/core/modules/system/system.routing.ymlundefined
@@ -87,3 +87,10 @@ date_format_localize_reset:
\ No newline at end of file

Missing endline at the end of the file.

Cleaned up the comments as per above.

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,69 @@
\ No newline at end of file

newline still missing :)

ok

no here

Let's test it.

We have to remove system_date_time_formats in system.admin.inc.

• Removed system_date_time_formats in system.admin.inc.
• Fixed docs.

For the test.

While manually testing the patch I realized that this would break adding a date format and get redirected to admin/config/regional/date-time/formats
which breaks with this change (due to the route system).

We can fix the tests by removing /formats, which is not a problem, as we don't have local tabs here.

This one might fix the two test fails.

missing the patch, here it is

Let's fix the failures.

I'm at a table at the Portland sprint right now. We're looking at this patch and scratching our heads over the

use Symfony\Component\DependencyInjection\ContainerInterface;

which is not actually used in the file. We suspect it will be advantageous to actually use ContainerInterface in the future. We'll grab Crell when he gets here.

We got some feedback form Crell. Mainly I'm not sure if it makes sense to take out system_date_time_formats() yet as there could be a better answer that replaces that functionality everywhere. I think the that function and related functions need to be a service but it's likely that ideal should stay out of the way of the present conversion issue.

I do think "DateTimeController" is too general. Thoughts?

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,102 @@
+class DateTimeController implements ControllerInterface {

What about DateFormatsListing?

+++ b/core/modules/system/lib/Drupal/system/Controller/DateTimeController.phpundefined
@@ -0,0 +1,102 @@
+    // @todo, Crell suggests replacing the direct usage of system_get_date_formats()
+    // with this protected method that gets the config system out of the container
+    // and then does the same task as system_get_date_formats(). I'm not sure
+    // If that should be done now or in a bulk replacement of system_get_date_formats()

I would argue that this should be more like a service, because having a protected method on this object would make it impossible to reuse it. system_get_date_formats seems to be a function you might want to call from outside.

Is this a decent middle ground short of making date formats be a service themselves? This patch adds injection of the ConfigFactory. This issue is my first route conversion so I'm still feeling out the new guidelines.

Larry said he'll see this just because it's a conversion issue. Doesn't need "Stalking Crell."

This actually duplicates the efforts of #2003892: Convert date formats to config entities
Can we mark this one as a duplicate?

Then let's do it.