Convert dblog_event() to a new style controller

thanks, this looks mostly good, some notices:

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DBLogController.phpundefined
@@ -0,0 +1,112 @@
+use Drupal\Core\Extension\ModuleHandlerInterface;

i think this is not needed right?

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DBLogController.phpundefined
@@ -0,0 +1,112 @@
+class DBLogController implements ControllerInterface {

i believe the class name should be DbLogController see http://drupal.org/node/608152#naming point 3

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DBLogController.phpundefined
@@ -0,0 +1,112 @@
+   *   format expected by drupal_render(); otherwise, an empty string.

the empty string part is no longer true, right?

Please note also that this needs to work with the changes in #1982954: Convert dblog_top() to a controller and #1977254: Convert dblog_overview() to a Controller.

It might be worth waiting until those two have been committed before working any more on this, otherwise we'll end up with a big dependency mess.

lets wait for this #1977254: Convert dblog_overview() to a Controller

Patch changes a few things:

• Test to check for "DBLog event node was displayed" was failing because it was loading event with id 1 which didn't exist, now loads a valid event id.
• It was also failing because the page title wasn't set to "Details" so restored a simple hook_menu() item with the title and route_name.
• Class name case in dblog.routing.yml (was DBLogController rather than DbLogController)

#11: 1987686-dblog_event-controller-11.patch queued for re-testing.

+++ b/core/modules/dblog/dblog.routing.ymlundefined
@@ -4,3 +4,10 @@ dblog_overview:
+  pattern: 'admin/reports/event/{eventId}'

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DbLogController.phpundefined
@@ -191,6 +191,75 @@ public function overview() {
+   * @param int $eventId
...
+  function eventDetails($eventId) {

Just for codestyle reasons let's use $event_id

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DbLogController.phpundefined
index e619fff..29276fc 100644
--- a/core/modules/dblog/lib/Drupal/dblog/Tests/DbLogTest.php

+++ b/core/modules/dblog/lib/Drupal/dblog/Tests/DbLogTest.phpundefined
@@ -187,7 +188,8 @@ private function verifyReports($response = 200) {
+    $wid = db_query('SELECT MIN(wid) FROM {watchdog}')->fetchField();

I am a bit confused about the test changes. Doesn't the test ensure that there are logs created for login in a user etc. so the test below should not use the new generated log entry.

The original test was checking for an event with event_id = 1, however that event didn't exist whenever I tested it. That particular test is to ensure that the event details page is working I think, but this code change probably isn't needed as the logging in of the user would generate an entry.

+++ b/core/modules/dblog/lib/Drupal/dblog/Tests/DbLogTest.php
@@ -64,6 +64,7 @@ function setUp() {
   function testDbLog() {
     // Login the admin user.
     $this->drupalLogin($this->big_user);
+    $this->generateLogEntries(1);
 
     $row_limit = 100;
     $this->verifyRowLimit($row_limit);

Patch reload to change eventId to event_id, and to remove the unneeded call to generateLogEntries().

Cool

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DbLogController.phpundefined
@@ -191,6 +191,75 @@ public function overview() {
+          check_plain($dblog->hostname),

use String::checkPlain()

Needs reroll due to #2008990: Replace theme() with drupal_render() in dblog module - we need to make sure we replace all the calls to theme with drupal_render or a render array...

 curl https://drupal.org/files/1987686-dblog_event-controller-16.patch | git apply --check
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7354  100  7354    0     0   3451      0  0:00:02  0:00:02 --:--:--  4199
error: patch failed: core/modules/dblog/dblog.admin.inc:71
error: core/modules/dblog/dblog.admin.inc: patch does not apply

+++ b/core/modules/dblog/dblog.routing.ymlundefined
@@ -4,3 +4,9 @@
+    _permission: 'access site reports'
\ No newline at end of file

needs a newline here

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DbLogController.phpundefined
@@ -193,7 +194,76 @@
   }
+   ¶
+  /**
...
+    return $build;
+  }
+  ¶

extra spaces should be removed

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DbLogController.phpundefined
@@ -193,7 +194,76 @@
+  function eventDetails($event_id) {

needs public visibility

looks ready to go. Just one small thing:

+++ b/core/modules/dblog/lib/Drupal/dblog/Controller/DbLogController.phpundefined
@@ -195,6 +196,75 @@
+   * @see dblog_menu()

we remove those now, since controllers dont have much to do with menus

thank you!

#23: 1987686-dblog_event-controller-23.patch queued for re-testing.

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

@naveenvalecha
It's not required, but it is helpful to make interdiffs.

interdiff?
For instructions on creating an interdiff, see https://drupal.org/documentation/git/interdiff | Microbranching workflow: http://xjm.drupalgardens.com/blog/interdiffs-how-make-them-and-why-they-...

Keep that in mind for future. :)

Needs a reroll

git applyc https://drupal.org/files/1987686-dblog_event-controller-23.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7742  100  7742    0     0   7912      0 --:--:-- --:--:-- --:--:--  9689
error: patch failed: core/modules/dblog/dblog.admin.inc:68
error: core/modules/dblog/dblog.admin.inc: patch does not apply

+++ b/core/modules/dblog/lib/Drupal/dblog/Tests/DbLogTest.phpundefined
@@ -187,7 +187,8 @@
-    $this->drupalGet('admin/reports/event/1');
+    $wid = db_query('SELECT MIN(wid) FROM {watchdog}')->fetchField();
+    $this->drupalGet('admin/reports/event/' . $wid);

Like @dawehner I don't get why this change is necessary

re rolled against head

This fixes the failures, removes the old code and fixes some small points.

#37: drupal-1987686-37.patch queued for re-testing.

There's actually quite a bit more refactoring that can/should be done here, but this at least gets the page out of the old router so let's take care of that first, as that's the release-blocking part. :-) Thanks all.

Committed/pushed to 8.x, thanks!