When import fails because we can't parse an ICS file, show it nicely

Summary:
Ref T10747. When we hit an ICS parser error, render it into a log instead of fataling.

(This will be more important in the future with subscription-based URL ICS import.)

Test Plan: {F1875292}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16721

src/__phutil_library_map__.php

@@ -2118,6 +2118,7 @@ phutil_register_library_map(array(
     'PhabricatorCalendarImportEpochLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportEpochLogType.php',
     'PhabricatorCalendarImportFrequencyLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportFrequencyLogType.php',
     'PhabricatorCalendarImportICSFileTransaction' => 'applications/calendar/xaction/PhabricatorCalendarImportICSFileTransaction.php',
+    'PhabricatorCalendarImportICSLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportICSLogType.php',
     'PhabricatorCalendarImportIgnoredNodeLogType' => 'applications/calendar/importlog/PhabricatorCalendarImportIgnoredNodeLogType.php',
     'PhabricatorCalendarImportListController' => 'applications/calendar/controller/PhabricatorCalendarImportListController.php',
     'PhabricatorCalendarImportLog' => 'applications/calendar/storage/PhabricatorCalendarImportLog.php',
@@ -6952,6 +6953,7 @@ phutil_register_library_map(array(
     'PhabricatorCalendarImportEpochLogType' => 'PhabricatorCalendarImportLogType',
     'PhabricatorCalendarImportFrequencyLogType' => 'PhabricatorCalendarImportLogType',
     'PhabricatorCalendarImportICSFileTransaction' => 'PhabricatorCalendarImportTransactionType',
+    'PhabricatorCalendarImportICSLogType' => 'PhabricatorCalendarImportLogType',
     'PhabricatorCalendarImportIgnoredNodeLogType' => 'PhabricatorCalendarImportLogType',
     'PhabricatorCalendarImportListController' => 'PhabricatorCalendarController',
     'PhabricatorCalendarImportLog' => array(

src/applications/calendar/import/PhabricatorCalendarICSImportEngine.php

@@ -62,9 +62,25 @@ final class PhabricatorCalendarICSImportEngine
 
     $data = $file->loadFileData();
 
-    $parser = id(new PhutilICSParser());
+    $parser = new PhutilICSParser();
 
-    $document = $parser->parseICSData($data);
+    try {
+      $document = $parser->parseICSData($data);
+    } catch (PhutilICSParserException $ex) {
+      // TODO: In theory, it would be nice to store these in a fully abstract
+      // form so they can be translated at display time. As-is, we'll store the
+      // error messages in whatever language we were using when the parser
+      // failure occurred.
+
+      $import->newLogMessage(
+        PhabricatorCalendarImportICSLogType::LOGTYPE,
+        array(
+          'ics.code' => $ex->getParserFailureCode(),
+          'ics.message' => $ex->getMessage(),
+        ));
+
+      $document = null;
+    }
 
     return $this->importEventDocument($viewer, $import, $document);
   }

src/applications/calendar/import/PhabricatorCalendarImportEngine.php

@@ -42,24 +42,26 @@ abstract class PhabricatorCalendarImportEngine
   final protected function importEventDocument(
     PhabricatorUser $viewer,
     PhabricatorCalendarImport $import,
-    PhutilCalendarRootNode $root) {
+    PhutilCalendarRootNode $root = null) {
 
     $event_type = PhutilCalendarEventNode::NODETYPE;
 
     $nodes = array();
-    foreach ($root->getChildren() as $document) {
-      foreach ($document->getChildren() as $node) {
-        $node_type = $node->getNodeType();
-        if ($node_type != $event_type) {
-          $import->newLogMessage(
-            PhabricatorCalendarImportIgnoredNodeLogType::LOGTYPE,
-            array(
-              'node.type' => $node_type,
-            ));
-          continue;
-        }
+    if ($root) {
+      foreach ($root->getChildren() as $document) {
+        foreach ($document->getChildren() as $node) {
+          $node_type = $node->getNodeType();
+          if ($node_type != $event_type) {
+            $import->newLogMessage(
+              PhabricatorCalendarImportIgnoredNodeLogType::LOGTYPE,
+              array(
+                'node.type' => $node_type,
+              ));
+            continue;
+          }
 
-        $nodes[] = $node;
+          $nodes[] = $node;
+        }
       }
     }
 

src/applications/calendar/importlog/PhabricatorCalendarImportICSLogType.php

@@ -0,0 +1,36 @@
+<?php
+
+final class PhabricatorCalendarImportICSLogType
+  extends PhabricatorCalendarImportLogType {
+
+  const LOGTYPE = 'ics';
+
+  public function getDisplayType(
+    PhabricatorUser $viewer,
+    PhabricatorCalendarImportLog $log) {
+    return pht('ICS Parse Error');
+  }
+
+  public function getDisplayDescription(
+    PhabricatorUser $viewer,
+    PhabricatorCalendarImportLog $log) {
+    return pht(
+      'Failed to parse ICS file ("%s"): %s',
+      $log->getParameter('ics.code'),
+      $log->getParameter('ics.message'));
+  }
+
+
+  public function getDisplayIcon(
+    PhabricatorUser $viewer,
+    PhabricatorCalendarImportLog $log) {
+    return 'fa-file';
+  }
+
+  public function getDisplayColor(
+    PhabricatorUser $viewer,
+    PhabricatorCalendarImportLog $log) {
+    return 'red';
+  }
+
+}

src/applications/calendar/view/PhabricatorCalendarImportLogView.php

@@ -48,7 +48,7 @@ final class PhabricatorCalendarImportLogView extends AphrontView {
           : null),
         id(new PHUIIconView())->setIcon($icon, $color),
         $name,
-        $description,
+        phutil_escape_html_newlines($description),
         phabricator_datetime($log->getDateCreated(), $viewer),
       );
     }
@@ -70,12 +70,12 @@ final class PhabricatorCalendarImportLogView extends AphrontView {
         ))
       ->setColumnClasses(
         array(
-          null,
-          null,
-          null,
-          'pri',
-          'wide',
-          null,
+          'top',
+          'top',
+          'top',
+          'top pri',
+          'top wide',
+          'top',
         ));
 
     return $table;