Part of #1971384: [META] Convert page callbacks to controllers

For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.

Files: 
CommentFileSizeAuthor
#30 1987818-convert-date-format-language-30.patch4.22 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 58,574 pass(es).
[ View ]
#30 1987818-diff-26-30.txt1.93 KBvijaycs85
#31 Localize date formats.png76.56 KBvijaycs85
#31 Edit date format | drupal.dev_.png41.07 KBvijaycs85
#26 drupal-convert_system_date_format_overview_page-1987818-26.patch4.17 KBjlbellido
FAILED: [[SimpleTest]]: [MySQL] 57,414 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#26 interdiff.txt1.91 KBjlbellido
#22 Date and time formats.png34.03 KBvijaycs85
#19 drupal-convert_system_date_format_overview_page-1987818-19.patch4.13 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 57,056 pass(es).
[ View ]
#17 drupal-convert_system_date_format_overview_page-1987818-17.patch5.25 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 57,214 pass(es).
[ View ]
#11 drupal-convert_system_date_format_overview_page-1987818-11.patch4.14 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-convert_system_date_format_overview_page-1987818-11.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#11 interdiff.txt590 bytesdisasm
#8 drupal-convert_system_date_format_overview_page-1987818-8.patch4.14 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] 56,370 pass(es), 1 fail(s), and 20 exception(s).
[ View ]
#8 interdiff.txt1.19 KBdisasm
#6 convert-system_date_format_language_overview_page-1987818-6.patch4.13 KBMattDanger
PASSED: [[SimpleTest]]: [MySQL] 55,759 pass(es).
[ View ]
#2 convert-system_date_format_language_overview_page-1987818-2.patch4.07 KBMattDanger
PASSED: [[SimpleTest]]: [MySQL] 55,940 pass(es).
[ View ]

Comments

Assigned:Unassigned» MattDanger

Status:Active» Needs review
StatusFileSize
new4.07 KB
PASSED: [[SimpleTest]]: [MySQL] 55,940 pass(es).
[ View ]

Status:Needs review» Needs work
Issue tags:-WSCCI-conversion

The last submitted patch, convert-system_date_format_language_overview_page-1987818-2.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+WSCCI-conversion

+++ b/core/modules/system/lib/Drupal/system/LanguageController.phpundefined

I believe this should be in a Controller directory. Otherwise, this looks pretty good to me.

StatusFileSize
new4.13 KB
PASSED: [[SimpleTest]]: [MySQL] 55,759 pass(es).
[ View ]

Moved LanguageController.php to lib/Drupal/system/Controller

Status:Needs review» Needs work

+++ b/core/modules/system/lib/Drupal/system/Controller/LanguageController.phpundefined
@@ -0,0 +1,50 @@
+ * Definition of Drupal\system\Controller\LanguageController.

Should be Contains \Drupal\system\Controller\LanguageController

+++ b/core/modules/system/lib/Drupal/system/Controller/LanguageController.phpundefined
@@ -0,0 +1,50 @@
+   * @see locale_menu()

we remove those from the controllers

+++ b/core/modules/system/lib/Drupal/system/Controller/LanguageController.phpundefined
@@ -0,0 +1,50 @@
+  public function overviewPage() {

misses @return docs

+++ b/core/modules/system/lib/Drupal/system/Controller/LanguageController.phpundefined
@@ -0,0 +1,50 @@
+  ¶
...
+  ¶
...
+  ¶

unneeded whitespace

+++ b/core/modules/system/lib/Drupal/system/Controller/LanguageController.phpundefined
@@ -0,0 +1,50 @@
+    return theme('table', array('header' => $header, 'rows' => $rows));

this should return a render array

StatusFileSize
new1.19 KB
new4.14 KB
FAILED: [[SimpleTest]]: [MySQL] 56,370 pass(es), 1 fail(s), and 20 exception(s).
[ View ]

attached patch fixes comments in #7.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, drupal-convert_system_date_format_overview_page-1987818-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new590 bytes
new4.14 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-convert_system_date_format_overview_page-1987818-11.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Syntax for render array was wrong. Looked it up, and should be good now. Ran failed test locally, and passed without issues.

Status:Needs review» Reviewed & tested by the community

yeah its perfect now, thanks!

Issue tags:+RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Needs a reroll

git ac https://drupal.org/files/drupal-convert_system_date_format_overview_page-1987818-11.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4243  100  4243    0     0   3665      0  0:00:01  0:00:01 --:--:--  4334
error: patch failed: core/modules/system/system.routing.yml:95
error: core/modules/system/system.routing.yml: patch does not apply

Status:Needs work» Needs review
Issue tags:-Needs reroll, -WSCCI-conversion, -RTBC July 1

Status:Needs review» Needs work
Issue tags:+Needs reroll, +WSCCI-conversion, +RTBC July 1

The last submitted patch, drupal-convert_system_date_format_overview_page-1987818-11.patch, failed testing.

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new5.25 KB
PASSED: [[SimpleTest]]: [MySQL] 57,214 pass(es).
[ View ]

Rerolled #11 and removed tag

Status:Needs review» Needs work
Issue tags:+D8MI, +sprint, +Needs reroll

StatusFileSize
new4.13 KB
PASSED: [[SimpleTest]]: [MySQL] 57,056 pass(es).
[ View ]

I have reviewed my reroll #17 and it was wrong:
- It add code that in HEAD is deleted.
- It doesn't delete 'system_date_format_language_overview_page' function from system.admin.inc. This function is deleted in #11.

For fix this , i add a new reroll.

Status:Needs work» Needs review

sending to the testbot by changing status to needs review.

Issue tags:-Needs reroll

StatusFileSize
new34.03 KB

Tested patch in #19 and working as expected

Steps below:

1. Applied patch
2. Enabled language module
3. Added new language
4. Visited admin/config/regional/date-time/locale (I couldn't find a link in UI to go this path though)

Attaching screenshot of the page.

RTBC from my side.

Issue tags:+language-base

Feel free to continue working on this, as per Dries' post this is still possible: http://buytaert.net/drupal-8-api-freeze (see Deprecating Drupal 7 APIs which according to @xjm applies to issues such as this one).

Fix tagging for D8MI.

+++ b/core/modules/system/lib/Drupal/system/Controller/LanguageController.phpundefined
@@ -0,0 +1,51 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Controller\LanguageController.
+ */
+
+namespace Drupal\system\Controller;
+
+/**
+ * Controller for Language handling.
+ */
+class LanguageController {
+
+  /**
+   * Displays edit date format links for each language.
+   *
+   * @return array
+   *   Render array of overview page.
+   */

This is just for date formats. How does this become a "LanguageController"? Sounds very generic.

+++ b/core/modules/system/lib/Drupal/system/Controller/LanguageController.phpundefined
@@ -0,0 +1,51 @@
+      $links['edit'] = array(
+        'title' => t('Edit'),
+        'href' => "admin/config/regional/date-time/locale/$langcode/edit",
+      );
+      $links['reset'] = array(
+        'title' => t('Reset'),
+        'href' => "admin/config/regional/date-time/locale/$langcode/reset",
+      );

May be too much to change paths, but 'locale' is so-so-so wrong!

Also should use string concatenation, not double quotes.

Status:Needs review» Needs work

Issue tags:+API change, +API clean-up

Add API change tags.

Issue tags:+DdaySan2013-sprint
StatusFileSize
new1.91 KB
new4.17 KB
FAILED: [[SimpleTest]]: [MySQL] 57,414 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Added the following updates:

- Renamed "LanguageController" class to "LanguageDateFormatController".
- Changed double quotes to string concatenated.
- Changed paths "admin/config/regional/date-time/locale/$langcode/reset" to "admin/config/regional/date-time/$langcode/reset"

This last change needs work because i think that parent path "admin/config/regional/date-time/locale" should be changed to "admin/config/regional/date-time/" too. This should do changes in Tests , ect.. Should it?

Is this page still doing something useful? Languages are already assigned on the main editing page. What do the edit controllers do? Changing the path here is certainly not good in itself so long as the edit paths are nt reacting to it. I think we should figure out first and foremost if this is not just leftover code that should just be removed.

Status:Needs work» Needs review

needs review just to let the bot have a go at testing it.
As #27 we still need to look and see if we need this at all.

Status:Needs review» Needs work

The last submitted patch, drupal-convert_system_date_format_overview_page-1987818-26.patch, failed testing.

StatusFileSize
new1.93 KB
new4.22 KB
PASSED: [[SimpleTest]]: [MySQL] 58,574 pass(es).
[ View ]

Re-rolling...

Reg #27, we have two pages related to language for dateFormat(see attached #31). Don't see any link for this locale page though.

Status:Needs work» Needs review
StatusFileSize
new41.07 KB
new76.56 KB

Attaching screenshots.

What do these pages even do and how do they relate? #2052193: [META] Date format localisation is a huge mess, conflicts, does not work, regressed claims the combination of UIs is broken and regressed features from Drupal 7 unintentionally.

Status:Needs review» Reviewed & tested by the community

It is a simple conversion.

Yeah, I agree it is best to just get this in for now, but the whole date localization UI / process / backend is a disaster in Drupal 8: #2052193: [META] Date format localisation is a huge mess, conflicts, does not work, regressed

Status:Reviewed & tested by the community» Fixed

Looks good.

Committed and pushed to 8.x. Thanks!

Issue tags:-sprint

All right, now let's move onto the heap of issues in #2052193: [META] Date format localisation is a huge mess, conflicts, does not work, regressed and fix the bugs.

Automatically closed -- issue fixed for 2 weeks with no activity.