Convert locale_translation_manual_status to a Controller

As this issue hasn`t been fixed yet we are going to work today with this issue during Code Sprint UA.

That's fine!

Patch attached.

+++ b/core/modules/locale/lib/Drupal/locale/Controller/LocaleController.phpundefined
@@ -0,0 +1,63 @@
+  /**
+   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+   */
+  public function __construct(ModuleHandlerInterface $module_handler) {

The construct method should be documented

+++ b/core/modules/locale/lib/Drupal/locale/Controller/LocaleController.phpundefined
@@ -0,0 +1,63 @@
+   * Manually checks the translation status without the use of cron.
+   */
+  public function check_translation() {

This should have a @return documentation. Let's also use a camelCase method here.

+++ b/core/modules/locale/locale.moduleundefined
@@ -232,10 +232,8 @@ function locale_menu() {
   $items['admin/reports/translations/check'] = array(
     'title' => 'Manual translation update check',
-    'page callback' => 'locale_translation_manual_status',
-    'access arguments' => array('translate interface'),
     'type' => MENU_CALLBACK,
-    'file' => 'locale.pages.inc',
+    'route_name' => 'locale_check_translation',

If you have just a callback you don't need an entry in hook_menu anymore.

Here is the new patch.

+++ b/core/modules/locale/locale.routing.ymlundefined
@@ -4,3 +4,10 @@ locale_settings:
+    _content: 'Drupal\locale\Controller\LocaleController::checkTranslation'

If you just do a redirect response I would say that _controller is the better choice as _content

That's the last bit, I promise :)

And finally (I hope so :))...

Tested on my local and the patch works fine. Ran the forum test case through simpletest module on my local. Attaching a screenshot for more info.
Queuing this issue for re-testing.

#8: locale-change_callback_to_controller-1978928-8.patch queued for re-testing.

reroll!

The failed patch was missing the route_name property. Added it back. Attaching the fixed patch.

Fixing the patch. Updated the include path for couple of components and added route_name property. Attaching the patch and re-queuing it for testing.

Renaming interdiff.patch to interdiff.txt.

Controller was missing a return statement for the batch process. Tests might be failing coz of this. Attaching another patch and requeing for testing.

Patch provided in #26 is working as expected. Please find attachment (Available translation updates | drupal8.png) for better understanding.

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

+++ b/core/modules/locale/lib/Drupal/locale/Controller/LocaleController.phpundefined
@@ -0,0 +1,69 @@
+    return new RedirectResponse(url('admin/reports/translations', array('absolute' => TRUE)));

Need to inject the url generator.

+++ b/core/modules/locale/locale.moduleundefined
@@ -217,10 +217,8 @@ function locale_menu() {
   $items['admin/reports/translations/check'] = array(
     'title' => 'Manual translation update check',
-    'page callback' => 'locale_translation_manual_status',
-    'access arguments' => array('translate interface'),
     'type' => MENU_CALLBACK,
-    'file' => 'locale.pages.inc',
+    'route_name' => 'locale_check_translation',
   );

Let's remove the entire hook menu... a user should never actually land on the page as the controller does a redirect to admin/reports/translations

@alexpott

Thanks for the review, makes sense to remove the menu hook completely..:)

Updating the patch to inject the url generator properly and removing the menu item.

+++ b/core/modules/locale/lib/Drupal/locale/Controller/LocaleController.phpundefined
@@ -0,0 +1,70 @@
+    $url = $this->urlGenerator->generateFromPath('admin/reports/translations', array('absolute' => TRUE));

You're missing $this->urlGenerator

+++ b/core/modules/locale/lib/Drupal/locale/Controller/LocaleController.phpundefined
@@ -0,0 +1,70 @@
+    $url = $this->urlGenerator->generateFromPath('admin/reports/translations', array('absolute' => TRUE));
+    return new RedirectResponse($url);

While rerolling, can you just make this one line? return new RedirectResponse($this->urlGenerator->...

@tim Thanks. Uploading a patch with the above fixes.

@tim Thanks. Uploading a patch with the above fixes.

+++ b/core/modules/locale/lib/Drupal/locale/Controller/LocaleController.phpundefined
@@ -0,0 +1,71 @@
+    $this->urlGenerator = $url_generator;
...
+    return new static(
+      $container->get('module_handler')

Well, you actually need to pass in the generator service.

@tim Thanks for helping out on this. Updated the patch.

This looks fine.

Committed 2b08f2e and pushed to 8.x. Thanks!