From d87ac4f4ee5a5a965916b8556d4ba1a03efe4af2 Mon Sep 17 00:00:00 2001
From: Dan Feder <dan.feder@civicactions.com>
Date: Thu, 13 Jan 2022 23:54:17 -0500
Subject: [PATCH] Tighten up publish methods in Service

---
 modules/harvest/src/Service.php               | 70 +++++++++----------
 .../harvest/tests/src/Unit/ServiceTest.php    |  2 +-
 2 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/modules/harvest/src/Service.php b/modules/harvest/src/Service.php
index 1c991147ee..3c28a57244 100644
--- a/modules/harvest/src/Service.php
+++ b/modules/harvest/src/Service.php
@@ -224,59 +224,53 @@ private function getLastHarvestRunId(string $id) {
   /**
    * Publish a harvest.
    *
-   * @param string $id
+   * @param string $harvestId
    *   Harvest identifier.
    *
    * @return array
    *   The uuids of the published datasets.
    */
-  public function publish(string $id): array {
+  public function publish(string $harvestId): array {
 
-    $lastRunInfoObj = $this->getLastRunInfoObj($id);
-    if (!isset($lastRunInfoObj->status->extracted_items_ids)) {
+    $lastRunId = $this->getLastHarvestRunId($harvestId);
+    $lastRunInfo = json_decode($this->getHarvestRunInfo($harvestId, $lastRunId));
+    $status = $lastRunInfo->status ?? NULL;
+    if (!isset($status->extracted_items_ids)) {
       return [];
     }
 
-    return $this->publishHelper($id, $lastRunInfoObj->status);
-  }
-
-  /**
-   * Private.
-   */
-  private function getLastRunInfoObj(string $harvestId) {
-    $lastRunId = $this->getLastHarvestRunId($harvestId);
-    $lastRunInfoJsonString = $this->getHarvestRunInfo($harvestId, $lastRunId);
-    return json_decode($lastRunInfoJsonString);
-  }
-
-  /**
-   * Private.
-   */
-  private function publishHelper(string $harvestId, $lastRunStatus): array {
-    $publishedIdentifiers = [];
-
-    foreach ($lastRunStatus->extracted_items_ids as $uuid) {
-      try {
-        if ($this->metastorePublishHelper($lastRunStatus, $uuid)) {
-          $publishedIdentifiers[] = $uuid;
-        }
-      }
-      catch (\Exception $e) {
-        $this->error("Error publishing dataset {$uuid} in harvest {$harvestId}: {$e->getMessage()}");
-      }
+    $published = [];
+    foreach ($status->extracted_items_ids as $datasetId) {
+      // $this->publishHarvestedDataset() will return true if $datasetId
+      // could be successfully published.
+      $published[] = $this->publishHarvestedDataset($status, $datasetId) ? $datasetId : NULL;
     }
 
-    return $publishedIdentifiers;
+    return array_values(array_filter($published));
   }
 
   /**
-   * Private.
+   * Use metastore service to publish a harvested item.
+   *
+   * @param object $runInfoStatus
+   *   Status object with run information.
+   * @param string $datasetId
+   *   ID to DKAN dataset.
+   *
+   * @return bool
+   *   Whether or not publish action was successful.
    */
-  private function metastorePublishHelper($runInfoStatus, string $uuid): bool {
-    return isset($runInfoStatus->load) &&
-      $runInfoStatus->load->{$uuid} &&
-      $runInfoStatus->load->{$uuid} != 'FAILURE' &&
-      $this->metastore->publish('dataset', $uuid);
+  protected function publishHarvestedDataset($runInfoStatus, string $datasetId): bool {
+    try {
+      return isset($runInfoStatus->load) &&
+        $runInfoStatus->load->{$datasetId} &&
+        $runInfoStatus->load->{$datasetId} != 'FAILURE' &&
+        $this->metastore->publish('dataset', $datasetId);
+    }
+    catch (\Exception $e) {
+      $this->error("Error publishing dataset {$datasetId}: {$e->getMessage()}");
+      return FALSE;
+    }
   }
 
   /**
diff --git a/modules/harvest/tests/src/Unit/ServiceTest.php b/modules/harvest/tests/src/Unit/ServiceTest.php
index bf72824f8c..a366528dc4 100644
--- a/modules/harvest/tests/src/Unit/ServiceTest.php
+++ b/modules/harvest/tests/src/Unit/ServiceTest.php
@@ -263,7 +263,7 @@ public function testPublish() {
     $this->assertEquals(['abcd-1003'], $result);
 
     $loggerResult = $logger->getStoredInput('error')[0];
-    $error = 'Error publishing dataset abcd-1002 in harvest 1: FooBar';
+    $error = 'Error publishing dataset abcd-1002: FooBar';
     $this->assertEquals($error, $loggerResult);
 
     $result = $service->publish('2');