From a7bd1b092fe6b1f46b1b7b652c8c02455c22d594 Mon Sep 17 00:00:00 2001
From: Steve Boyd <emteknetnz@gmail.com>
Date: Tue, 23 Jan 2024 16:54:27 +1300
Subject: [PATCH] ENH Standardise API responses

---
 lang/en.yml                                   | 12 +--
 src/Controllers/LinkFieldController.php       | 73 ++++++++-----------
 .../Controllers/LinkFieldControllerTest.php   |  8 +-
 3 files changed, 36 insertions(+), 57 deletions(-)

diff --git a/lang/en.yml b/lang/en.yml
index ea3ab5e6..37f9f86e 100644
--- a/lang/en.yml
+++ b/lang/en.yml
@@ -1,17 +1,7 @@
 en:
   SilverStripe\LinkField\Controllers\LinkFieldController:
-    BAD_DATA: 'Bad data'
     CREATE_LINK: 'Create link'
-    EMPTY_DATA: 'Empty data'
-    INVALID_ID: 'Invalid ID'
-    INVALID_OWNER: 'Invalid Owner'
-    INVALID_OWNER_CLASS: 'Invalid ownerClass'
-    INVALID_OWNER_ID: 'Invalid ownerID'
-    INVALID_OWNER_RELATION: 'Invalid ownerRelation'
-    INVALID_TOKEN: 'Invalid CSRF token'
-    INVALID_TYPEKEY: 'Invalid typeKey'
-    MENUTITLE: SilverStripe\LinkField\Controllers\LinkFieldController
-    UNAUTHORIZED: Unauthorized
+    MENUTITLE: 'Link fields'
     UPDATE_LINK: 'Update link'
   SilverStripe\LinkField\Form\Traits\AllowedLinkClassesTrait:
     INVALID_TYPECLASS: '"{class}": {typeclass} is not a valid Link Type'
diff --git a/src/Controllers/LinkFieldController.php b/src/Controllers/LinkFieldController.php
index 6b7d8873..d60e610d 100644
--- a/src/Controllers/LinkFieldController.php
+++ b/src/Controllers/LinkFieldController.php
@@ -72,17 +72,17 @@ public function linkForm(): Form
         if ($id) {
             $link = Link::get()->byID($id);
             if (!$link) {
-                $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+                $this->jsonError(404, 'Invalid ID');
             }
             $operation = 'edit';
             if (!$link->canView()) {
-                $this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
+                $this->jsonError(403, 'Unauthorized');
             }
         } else {
             $typeKey = $this->typeKeyFromRequest();
             $link = LinkTypeService::create()->byKey($typeKey);
             if (!$link) {
-                $this->jsonError(404, _t(__CLASS__ . '.INVALID_TYPEKEY', 'Invalid typeKey'));
+                $this->jsonError(404, 'Invalid typeKey');
             }
             $operation = 'create';
         }
@@ -105,17 +105,13 @@ public function linkData(HTTPRequest $request): HTTPResponse
                 $data[$link->ID] = $this->getLinkData($link);
             }
         }
-
-        $response = $this->getResponse();
-        $response->addHeader('Content-type', 'application/json');
-        $response->setBody(json_encode($data));
-        return $response;
+        return $this->jsonSuccess(200, $data);
     }
 
     private function getLinkData(Link $link): array
     {
         if (!$link->canView()) {
-            $this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
+            $this->jsonError(403, 'Unauthorized');
         }
         $data = $link->jsonSerialize();
         $data['canDelete'] = $link->canDelete();
@@ -132,11 +128,11 @@ public function linkDelete(): HTTPResponse
     {
         $link = $this->linkFromRequest();
         if (!$link->canDelete()) {
-            $this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
+            $this->jsonError(403, 'Unauthorized');
         }
         // Check security token on destructive operation
         if (!SecurityToken::inst()->checkRequest($this->getRequest())) {
-            $this->jsonError(400, _t(__CLASS__ . '.INVALID_TOKEN', 'Invalid CSRF token'));
+            $this->jsonError(400, 'Invalid CSRF token');
         }
         // delete() will also delete any published version immediately
         $link->delete();
@@ -149,10 +145,7 @@ public function linkDelete(): HTTPResponse
             $owner->write();
         }
         // Send response
-        $response = $this->getResponse();
-        $response->addHeader('Content-type', 'application/json');
-        $response->setBody(json_encode(['success' => true]));
-        return $response;
+        return $this->jsonSuccess(201);
     }
 
     /**
@@ -172,9 +165,8 @@ public function getLinkForm(): Form
     public function save(array $data, Form $form): HTTPResponse
     {
         if (empty($data)) {
-            $this->jsonError(400, _t(__CLASS__ . '.EMPTY_DATA', 'Empty data'));
+            $this->jsonError(400, 'Empty data');
         }
-
         /** @var Link $link */
         $id = $this->itemIDFromRequest();
         if ($id) {
@@ -182,10 +174,10 @@ public function save(array $data, Form $form): HTTPResponse
             $operation = 'edit';
             $link = Link::get()->byID($id);
             if (!$link) {
-                $this->jsonErorr(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+                $this->jsonErorr(404, 'Invalid ID');
             }
             if (!$link->canEdit()) {
-                $this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
+                $this->jsonError(403, 'Unauthorized');
             }
         } else {
             // Creating a new Link
@@ -193,11 +185,11 @@ public function save(array $data, Form $form): HTTPResponse
             $typeKey = $this->typeKeyFromRequest();
             $className = LinkTypeService::create()->byKey($typeKey) ?? '';
             if (!$className) {
-                $this->jsonError(404, _t(__CLASS__ . '.INVALID_TYPEKEY', 'Invalid typeKey'));
+                $this->jsonError(404, 'Invalid typeKey');
             }
             $link = $className::create();
             if (!$link->canCreate()) {
-                $this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
+                $this->jsonError(403, 'Unauthorized');
             }
         }
 
@@ -208,7 +200,7 @@ public function save(array $data, Form $form): HTTPResponse
         if ((isset($data['ID']) && ((int) $data['ID'] !== $id))
             || isset($data['Sort'])
         ) {
-            $this->jsonError(400, _t(__CLASS__ . '.BAD_DATA', 'Bad data'));
+            $this->jsonError(400, 'Bad data');
         }
 
         // Update DataObject from form data
@@ -272,13 +264,13 @@ public function linkSort()
         $request = $this->getRequest();
         // Check security token
         if (!SecurityToken::inst()->checkRequest($request)) {
-            $this->jsonError(400, _t(__CLASS__ . '.INVALID_TOKEN', 'Invalid CSRF token'));
+            $this->jsonError(400, 'Invalid CSRF token');
         }
         $json = json_decode($request->getBody() ?? '');
         $newLinkIDs = $json?->newLinkIDs;
         // If someone's passing a JSON object or other non-array here, they're doing something wrong
         if (!is_array($newLinkIDs) || empty($newLinkIDs)) {
-            $this->jsonError(400, _t('LinkField.BAD_DATA', 'Bad data'));
+            $this->jsonError(400, 'Bad data');
         }
         // Fetch and validate links
         $links = Link::get()->filter(['ID' => $newLinkIDs])->toArray();
@@ -293,7 +285,7 @@ public function linkSort()
                 $ownerRelation = $link->OwnerRelation;
             }
             if ($link->OwnerID !== $ownerID || $link->OwnerRelation !== $ownerRelation) {
-                $this->jsonError(400, _t('LinkField.BAD_DATA', 'Bad data'));
+                $this->jsonError(400, 'Bad data');
             }
             $linkIDToLink[$link->ID] = $link;
         }
@@ -305,7 +297,7 @@ public function linkSort()
             // There's also corresponding logic in Link::onBeforeWrite() to also have a minimum of 1
             $sort = $i + 1;
             if ($link->Sort !== $sort && !$link->canEdit()) {
-                $this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
+                $this->jsonError(403, 'Unauthorized');
             }
         }
         // Update Sort field on links
@@ -319,10 +311,7 @@ public function linkSort()
             }
         }
         // Send response
-        $response = $this->getResponse();
-        $response->addHeader('Content-type', 'application/json');
-        $response->setBody(json_encode(['success' => true]));
-        return $response;
+        return $this->jsonSuccess(201);
     }
 
     /**
@@ -394,11 +383,11 @@ private function linkFromRequest(): Link
     {
         $itemID = $this->itemIDFromRequest();
         if (!$itemID) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+            $this->jsonError(404, 'Invalid ID');
         }
         $link = Link::get()->byID($itemID);
         if (!$link) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+            $this->jsonError(404, 'Invalid ID');
         }
         return $link;
     }
@@ -410,11 +399,11 @@ private function linksFromRequest(): DataList
     {
         $itemIDs = $this->itemIDsFromRequest();
         if (empty($itemIDs)) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+            $this->jsonError(404, 'Invalid ID');
         }
         $links = Link::get()->byIDs($itemIDs);
         if (!$links->exists()) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+            $this->jsonError(404, 'Invalid ID');
         }
         return $links;
     }
@@ -427,7 +416,7 @@ private function itemIDFromRequest(): int
         $request = $this->getRequest();
         $itemID = (string) $request->param('ItemID');
         if (!ctype_digit($itemID)) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+            $this->jsonError(404, 'Invalid ID');
         }
         return (int) $itemID;
     }
@@ -441,13 +430,13 @@ private function itemIDsFromRequest(): array
         $itemIDs = $request->getVar('itemIDs');
 
         if (!is_array($itemIDs)) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+            $this->jsonError(404, 'Invalid ID');
         }
 
         $idsAsInt = [];
         foreach ($itemIDs as $id) {
             if (!is_int($id) && !ctype_digit($id)) {
-                $this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
+                $this->jsonError(404, 'Invalid ID');
             }
             $idsAsInt[] = (int) $id;
         }
@@ -463,7 +452,7 @@ private function typeKeyFromRequest(): string
         $request = $this->getRequest();
         $typeKey = (string) $request->getVar('typeKey');
         if (strlen($typeKey) === 0 || !preg_match('#^[a-z\-]+$#', $typeKey)) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_TYPEKEY', 'Invalid typeKey'));
+            $this->jsonError(404, 'Invalid typeKey');
         }
         return $typeKey;
     }
@@ -477,11 +466,11 @@ private function ownerFromRequest(): DataObject
         $request = $this->getRequest();
         $ownerID = (int) ($request->getVar('ownerID') ?: $request->postVar('OwnerID'));
         if ($ownerID === 0) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER_ID', 'Invalid ownerID'));
+            $this->jsonError(404, 'Invalid ownerID');
         }
         $ownerClass = $request->getVar('ownerClass') ?: $request->postVar('OwnerClass');
         if (!is_a($ownerClass, DataObject::class, true)) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER_CLASS', 'Invalid ownerClass'));
+            $this->jsonError(404, 'Invalid ownerClass');
         }
         $ownerRelation = $this->ownerRelationFromRequest();
         /** @var DataObject $obj */
@@ -506,7 +495,7 @@ private function ownerFromRequest(): DataObject
                 return $owner;
             }
         }
-        $this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER', 'Invalid Owner'));
+        $this->jsonError(404, 'Invalid Owner');
     }
 
     /**
@@ -518,7 +507,7 @@ private function ownerRelationFromRequest(): string
         $request = $this->getRequest();
         $ownerRelation = $request->getVar('ownerRelation') ?: $request->postVar('OwnerRelation');
         if (!$ownerRelation) {
-            $this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER_RELATION', 'Invalid ownerRelation'));
+            $this->jsonError(404, 'Invalid ownerRelation');
         }
         return $ownerRelation;
     }
diff --git a/tests/php/Controllers/LinkFieldControllerTest.php b/tests/php/Controllers/LinkFieldControllerTest.php
index 9c97883e..1dcfdab7 100644
--- a/tests/php/Controllers/LinkFieldControllerTest.php
+++ b/tests/php/Controllers/LinkFieldControllerTest.php
@@ -529,7 +529,7 @@ public function testLinkDelete(
         $response = $this->mainSession->sendRequest('DELETE', $url, [], $headers);
         $this->assertSame('application/json', $response->getHeader('Content-type'));
         $this->assertSame($expectedCode, $response->getStatusCode());
-        if ($expectedCode !== 200) {
+        if ($expectedCode >= 400) {
             $jsonError = json_decode($response->getBody(), true);
             $this->assertSame($expectedMessage, $jsonError['errors'][0]['value']);
             $this->assertNotNull(TestPhoneLink::get()->byID($fixtureID));
@@ -549,7 +549,7 @@ public function provideLinkDelete(): array
             'Valid' => [
                 'idType' => 'existing',
                 'fail' => '',
-                'expectedCode' => 200,
+                'expectedCode' => 201,
                 'expectedMessage' => '',
             ],
             'Reject fail canDelete()' => [
@@ -622,7 +622,7 @@ public function testLinkSort(
         }
         $response = $this->post($url, null, $headers, null, $body);
         $this->assertSame($expectedCode, $response->getStatusCode());
-        if ($expectedCode !== 200) {
+        if ($expectedCode >= 400) {
             $jsonError = json_decode($response->getBody(), true);
             $this->assertSame($expectedMessage, $jsonError['errors'][0]['value']);
         }
@@ -638,7 +638,7 @@ public function provideLinkSort(): array
             'Success' => [
                 'newTitleOrder' => [4, 2, 3],
                 'fail' => '',
-                'expectedCode' => 200,
+                'expectedCode' => 201,
                 'expectedMessage' => '',
                 'expectedTitles' => [4, 2, 3],
             ],