Convert this page callback to a new-style Controller, using the instructions on http://drupal.org/node/1800686

This may also require moving the theme functions from update.report.inc back to the .module file.

Files: 
CommentFileSizeAuthor
#22 update-1938822-22.patch4.29 KBYesCT
PASSED: [[SimpleTest]]: [MySQL] 55,508 pass(es).
[ View ]
#22 interdiff-20-22.txt847 bytesYesCT
#20 18-20-interdiff.txt2.22 KBalexpott
#20 update-1938822-20.patch4.29 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 55,438 pass(es).
[ View ]
#18 update-1938822-18.patch3.05 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 55,855 pass(es), 57 fail(s), and 3 exception(s).
[ View ]
#18 interdiff.txt1.69 KBtim.plunkett
#15 convert-update-status-1938822-15.patch7.89 KBsandhya.m
PASSED: [[SimpleTest]]: [MySQL] 55,389 pass(es).
[ View ]
#15 11-15-interdiff.txt2.81 KBsandhya.m
#12 convert-update-status-1938822-11.patch8.25 KBsandhya.m
PASSED: [[SimpleTest]]: [MySQL] 55,641 pass(es).
[ View ]
#9 convert-update-status-1938822-9.patch8.19 KBsandhya.m
PASSED: [[SimpleTest]]: [MySQL] 55,994 pass(es).
[ View ]
#4 convert_update_status-1938822-4.patch3.18 KBEllaTheHarpy
FAILED: [[SimpleTest]]: [MySQL] 53,031 pass(es), 57 fail(s), and 3 exception(s).
[ View ]
#4 interdiff-2-4.txt1.41 KBEllaTheHarpy
#2 convert_update_status-1938822-2.patch3.17 KBEllaTheHarpy
FAILED: [[SimpleTest]]: [MySQL] 53,076 pass(es), 57 fail(s), and 3 exception(s).
[ View ]

Comments

Assigned:Unassigned» EllaTheHarpy

Status:Active» Needs review
StatusFileSize
new3.17 KB
FAILED: [[SimpleTest]]: [MySQL] 53,076 pass(es), 57 fail(s), and 3 exception(s).
[ View ]

Status:Needs review» Needs work

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.php
@@ -0,0 +1,42 @@
+/**
+ * @file
+ * Contains the controller for the update module.

For a class, the @file directive is fairly simplistic. It's just the name of the class, like so:

@file
Contains \Drupal\update\Controller\UpdateController

(Yeah, it's silly, but that's what the coding standards say.)

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.php
@@ -0,0 +1,42 @@
+  function update_status() {

Coding standards for methods are to use lowerCamel. So this should be:

public function updateStatus() {

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.php
@@ -0,0 +1,42 @@
+      module_load_include('inc', 'update', 'update.compare');

Hm, this function. Actually, this can now be an injected service. Let's go ahead and convert that now, for practice. :-)

There's a service called module_handler, which can be injected via create(). Then there's a method on that service called loadInclude() that replaces this function.

Really, the contents of update.compare.inc should get turned into a service. That's a big complex, though, so we can do that in a follow-up.

Status:Needs work» Needs review
StatusFileSize
new1.41 KB
new3.18 KB
FAILED: [[SimpleTest]]: [MySQL] 53,031 pass(es), 57 fail(s), and 3 exception(s).
[ View ]

I made all the changes that Crell recommended in comment #3, except for the last one, which I will work on next.

Status:Needs review» Needs work

I think we can look at #1938296: Convert book_admin_overview and book_render to a new-style Controller again for an example.

--- /dev/null
+++ b/core/modules/book/lib/Drupal/book/BookManager.phpundefined
@@ -0,0 +1,83 @@
+<?php
+/**
+ * @file
+ * Contains \Drupal\book\BookManager.
+ */
+
+namespace Drupal\book;
+
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\Core\Database\Connection;
+use PDO;
+
+/**
+ * Book Manager Service.
+ */
+class BookManager {
+
+  /**
+   * Database Service Object.

etc.

look at this Manager bit for an example.

@Crell any more pointers than that?

---
moving it back to needs work to add the manager bit.

The book issue needed to make a new service. This issue doesn't. It just needs to use an existing service. So the only change would be in the create() method, where you have to reference the service we're injecting, and then replace module_load_include() with the equivalent from that service. There's no additional class like BookManager that would be needed.

ah! ok.

Assigned:EllaTheHarpy» Unassigned

@EllaTheHarpy if you want to do this, just reassign it to yourself when you have time. Otherwise, this is up for grabs.
It's on my list to try, but I dont have time right away either... I'll check back in on this later.

Assigned:Unassigned» sandhya.m
Status:Needs work» Needs review
StatusFileSize
new8.19 KB
PASSED: [[SimpleTest]]: [MySQL] 55,994 pass(es).
[ View ]

Used moduleHandler to use loadInclude() for the inc files in UpdateController.php

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,65 @@
+
+use Drupal\Core\ControllerInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\Core\Extension\ModuleHandler;

let's order them.

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,65 @@
+   * @var \Drupal\Core\Entity\EntityManager
...
+   * @param \Drupal\Core\Extension\ModuleHandler $module_handler

Should be the interface of the module handler :)

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,65 @@
+  public function __construct(ModuleHandler $module_handler) {

Let's move it on top of create

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,65 @@
+      return theme('update_report', array('data' => $data));
...
+      return theme('update_report', array('data' => _update_no_data()));

I guess it's in the scope of this issue to return a build array.

Issue tags:+LONDON_2013_APRIL

Tagging...

Issue tags:-LONDON_2013_APRIL
StatusFileSize
new8.25 KB
PASSED: [[SimpleTest]]: [MySQL] 55,641 pass(es).
[ View ]

Changes suggested by dawehner : done and added to the new patch

Issue tags:+LONDON_2013_APRIL

tagging

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,68 @@
+   * @var \Drupal\Core\Entity\EntityManager

Should be here marked as ModuleHandlerInterface as well.

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,68 @@
+  /**
+   * Implements \Drupal\Core\ControllerInterface::create().
+   *
+   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
+   *   The service container this object should use.
+   */

You can use just {@inheritdoc}

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,68 @@
+   * @return string
+   *   A HTML-formatted string with the update status of projects.

... That's not a string, but a build array which is returned.

StatusFileSize
new2.81 KB
new7.89 KB
PASSED: [[SimpleTest]]: [MySQL] 55,389 pass(es).
[ View ]

Updated the code as per the comments on #14

Status:Needs review» Reviewed & tested by the community

Great!

Status:Reviewed & tested by the community» Fixed

Committed c7d74b3 and pushed to 8.x. Thanks!

@sandhya.m you need to set up your environment to make your patches a little easier to apply... see http://stackoverflow.com/questions/13675782/git-shell-in-windows-patchs-... / or you can use notepad++ to convert to utf-8 and unix line endings.

Status:Fixed» Needs review
StatusFileSize
new1.69 KB
new3.05 KB
FAILED: [[SimpleTest]]: [MySQL] 55,855 pass(es), 57 fail(s), and 3 exception(s).
[ View ]

#1953800: Make the database connection serializable means we cannot yet inject module_handler.

This was rolled back.

Status:Needs review» Needs work

The last submitted patch, update-1938822-18.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.29 KB
PASSED: [[SimpleTest]]: [MySQL] 55,438 pass(es).
[ View ]
new2.22 KB

Hmmm.... it's because update_theme relied on the menu to include update.report.inc :D

So the moduleHandler injection is fine... afaik the serialisation would only happen on a form conversion... when the controller gets added to form_state.

I have no idea how #15 passed... maybe because of the encoding testbot couldn't actually properly apply the patch but it thought it did.... very very weird.

Oh that makes a bit more sense. I just get twitchy when I see module_handler injected since I wasted a couple hours on it.

StatusFileSize
new847 bytes
new4.29 KB
PASSED: [[SimpleTest]]: [MySQL] 55,508 pass(es).
[ View ]

I checked that the @param removed when Implements \... was replaced with {@inheritdoc} was not adding info.. and it was just the same @param that is in core/lib/Drupal/Core/ControllerInterface.php

So no loss of info.

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,65 @@
+ * @file
+ * Contains \Drupal\update\Controller\UpdateController

just missing period.

The summary should be one line of up to 80 characters ending in ".".
http://drupal.org/node/1354#file

+++ b/core/modules/update/lib/Drupal/update/Controller/UpdateController.phpundefined
@@ -0,0 +1,65 @@
+    $build  = array(

extra white space.

This looks rtbc to me. I compared it to what was done in #1938296: Convert book_admin_overview and book_render to a new-style Controller.

but I just changed some stuff, so someone else can rtbc it.

Status:Needs review» Reviewed & tested by the community

Let's do.

Status:Reviewed & tested by the community» Fixed

Committed a90da0d and pushed to 8.x. Thanks!

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