Detect when a module fails to upgrade properly and put up an

informative message to help the user know that she needs to get a
newer copy of the module.  Fixes ticket #1189.

4 files changed, 79 insertions, 20 deletions

diff --git a/modules/gallery/controllers/upgrader.php b/modules/gallery/controllers/upgrader.php
index cb940b46..a3cfac48 100644
--- a/modules/gallery/controllers/upgrader.php
+++ b/modules/gallery/controllers/upgrader.php
@@ -39,10 +39,12 @@ class Upgrader_Controller extends Controller {
       }
     }
 
+    $failed = Input::instance()->get("failed");
     $view = new View("upgrader.html");
     $view->can_upgrade = identity::active_user()->admin || $session->get("can_upgrade");
     $view->upgrade_token = $upgrade_token;
     $view->available = module::available();
+    $view->failed = $failed ? explode(",", $failed) : array();
     $view->done = $available_upgrades == 0;
     print $view;
   }
@@ -65,20 +67,26 @@ class Upgrader_Controller extends Controller {
     }
 
     // Then upgrade the rest
+    $failed = array();
     foreach (module::available() as $id => $module) {
       if ($id == "gallery") {
         continue;
       }
 
       if ($module->active && $module->code_version != $module->version) {
-        module::upgrade($id);
+        try {
+          module::upgrade($id);
+        } catch (Exception $e) {
+          // @todo assume it's MODULE_FAILED_TO_UPGRADE for now
+          $failed[] = $id;
+        }
       }
     }
 
     if (php_sapi_name() == "cli") {
       print "Upgrade complete\n";
     } else {
-      url::redirect("upgrader");
+      url::redirect("upgrader?failed=" . join(",", $failed));
     }
   }
 }
diff --git a/modules/gallery/css/upgrader.css b/modules/gallery/css/upgrader.css
index d1b74c31..8610016e 100644
--- a/modules/gallery/css/upgrader.css
+++ b/modules/gallery/css/upgrader.css
@@ -58,6 +58,10 @@ tr.upgradeable td.gallery {
   color: #00d;
 }
 
+tr.failed td {
+  color: red;
+}
+
 p {
   font-size: .9em;
 }
@@ -120,12 +124,28 @@ div#dialog div {
   opacity: 0.5;
 }
 
+.failed {
+  color: red;
+}
+
 pre {
   display: inline;
   margin: 0px;
   padding: 0px;
 }
 
+div#upgrade_button {
+  margin-bottom: 20px;
+}
+
+div#welcome_message {
+  margin-left: 30px;
+}
+
+#logo {
+  margin-left: 14px;
+}
+
 .rtl {
   direction: rtl;
 }
@@ -153,3 +173,11 @@ pre {
 .rtl div#dialog a.close {
   float: left;
 }
+
+.rtl div#welcome_message {
+  padding-right: 30px;
+}
+
+.rtl #logo {
+  padding-right: 12px;
+}
diff --git a/modules/gallery/helpers/module.php b/modules/gallery/helpers/module.php
index e3fb8684..be9c4249 100644
--- a/modules/gallery/helpers/module.php
+++ b/modules/gallery/helpers/module.php
@@ -214,10 +214,10 @@ class module_Core {
   static function upgrade($module_name) {
     $version_before = module::get_version($module_name);
     $installer_class = "{$module_name}_installer";
+    $available = module::available();
     if (method_exists($installer_class, "upgrade")) {
       call_user_func_array(array($installer_class, "upgrade"), array($version_before));
     } else {
-      $available = module::available();
       if (isset($available->$module_name->code_version)) {
         module::set_version($module_name, $available->$module_name->code_version);
       } else {
@@ -234,6 +234,10 @@ class module_Core {
                           "version_before" => $version_before,
                           "version_after" => $version_after)));
     }
+
+    if ($version_after != $available->$module_name->code_version) {
+      throw new Exception("@todo MODULE_FAILED_TO_UPGRADE");
+    }
   }
 
   /**
diff --git a/modules/gallery/views/upgrader.html.php b/modules/gallery/views/upgrader.html.php
index 0ce24ef8..c2d8a552 100644
--- a/modules/gallery/views/upgrader.html.php
+++ b/modules/gallery/views/upgrader.html.php
@@ -10,7 +10,7 @@
   </head>
   <body<? if (locales::is_rtl()) { echo ' class="rtl"'; } ?>>
     <div id="outer">
-      <img src="<?= url::file("modules/gallery/images/gallery.png") ?>" />
+      <img id="logo" src="<?= url::file("modules/gallery/images/gallery.png") ?>" />
       <div id="inner">
         <? if ($can_upgrade): ?>
         <div id="dialog" style="visibility: hidden">
@@ -31,6 +31,12 @@
                     array("url" => html::mark_clean(url::base()))) ?>
             </p>
           </div>
+          <div id="failed" style="display: none">
+            <h1> <?= t("Some modules failed to upgrade!") ?> </h1>
+            <p>
+              <?= t("Failed modules are <span class=\"failed\">highlighted</span>.  Try getting newer versions or <a href=\"%admin_modules\">deactivating those modules</a>.", array("admin_modules" => url::site("admin/modules"))) ?>
+            </p>
+          </div>
         </div>
         <script type="text/javascript">
           $(document).ready(function() {
@@ -41,6 +47,10 @@
             <? if ($done): ?>
             show_done();
             <? endif ?>
+
+            <? if ($failed): ?>
+            show_failed();
+            <? endif ?>
           });
 
           var show_busy = function() {
@@ -55,10 +65,31 @@
             $("#done").show();
             $("#dialog_close_link").show();
           }
+
+          var show_failed = function() {
+            $("#dialog").css("visibility", "visible");
+            $("#failed").show();
+            $("#dialog_close_link").show();
+          }
         </script>
-        <p class="<?= $done ? "muted" : "" ?>">
-          <?= t("Welcome to the Gallery upgrader.  One click and you're done!") ?>
-        </p>
+        <div id="welcome_message">
+          <p class="<?= $done ? "muted" : "" ?>">
+            <?= t("Welcome to the Gallery upgrader.  One click and you're done!") ?>
+          </p>
+        </div>
+
+        <? if ($done): ?>
+        <div id="upgrade_button" class="button muted">
+          <?= t("Upgrade all") ?>
+        </div>
+        <? else: ?>
+        <div id="upgrade_button" class="button button-active">
+          <a id="upgrade_link" href="<?= url::site("upgrader/upgrade") ?>">
+            <?= t("Upgrade all") ?>
+          </a>
+        </div>
+        <? endif ?>
+
         <table>
           <tr class="<?= $done ? "muted" : "" ?>">
             <th class="name"> <?= t("Module name") ?> </th>
@@ -68,7 +99,7 @@
 
           <? foreach ($available as $id => $module): ?>
           <? if ($module->active): ?>
-          <tr class="<?= $module->version == $module->code_version ? "current" : "upgradeable" ?>" >
+          <tr class="<?= $module->version == $module->code_version ? "current" : "upgradeable" ?> <?= in_array($id, $failed) ? "failed" : "" ?>" >
             <td class="name <?= $id ?>">
               <?= t($module->name) ?>
             </td>
@@ -85,18 +116,6 @@
           <? endforeach ?>
         </table>
 
-        <? if ($done): ?>
-        <div class="button muted">
-          <?= t("Upgrade all") ?>
-        </div>
-        <? else: ?>
-        <div class="button button-active">
-          <a id="upgrade_link" href="<?= url::site("upgrader/upgrade") ?>">
-            <?= t("Upgrade all") ?>
-          </a>
-        </div>
-        <? endif ?>
-
         <? if (@$inactive): ?>
         <p class="<?= $done ? "muted" : "" ?>">
           <?= t("The following modules are inactive and don't require an upgrade.") ?>