fix: [error handling] better error handling for bookmarks, fixes #188

- show why something failed - actually fail if a field is missing for bookmarks
cerebrate-project · Nov 28, 2024 · cce4115 · cce4115
1 parent d799214
commit cce4115
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 7 deletions.
diff --git a/src/Controller/Component/CRUDComponent.php b/src/Controller/Component/CRUDComponent.php
@@ -1365,7 +1365,7 @@ public function setResponseForController($action, $success, $message, $data = []
                 if (!empty($additionalData['redirect'])) { // If a redirection occurs, we need to make sure the flash message gets displayed
                     $this->Controller->Flash->error($message);
                 }
-                $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, $action, $data, $message, !is_null($errors) ? $errors : $data->getErrors());
+                $this->Controller->ajaxResponsePayload = $this->RestResponse->ajaxFailResponse($this->ObjectAlias, $action, $data, $message, $errors);
             } else {
                 $this->Controller->Flash->error($message);
                 $this->Controller->redirect($this->Controller->referer());

diff --git a/src/Controller/Component/RestResponseComponent.php b/src/Controller/Component/RestResponseComponent.php
@@ -440,10 +440,14 @@ public function ajaxSuccessResponse($ObjectAlias, $action, $entity, $message, $a
         return $this->viewData($response);
     }
 
-    public function ajaxFailResponse($ObjectAlias, $action, $entity, $message, $errors = [], $description = '')
+    public function ajaxFailResponse($ObjectAlias, $action, $entity = null, $message, $errors = [], $description = '')
     {
         $action = $this->__dissectAdminRouting($action);
-        $entity = is_array($entity) ? $entity : $entity->toArray();
+        if (empty($entity)) {
+            $entity = [];
+        } else {
+            $entity = is_array($entity) ? $entity : $entity->toArray();
+        }
         $response = [
             'success' => false,
             'message' => $message,

diff --git a/src/Controller/UserSettingsController.php b/src/Controller/UserSettingsController.php
@@ -243,9 +243,10 @@ public function getMyBookmarks($forSidebar = false)
     public function saveMyBookmark()
     {
         if (!$this->request->is('get')) {
-            $result = $this->UserSettings->saveBookmark($this->ACL->getUser(), $this->request->getData());
+            $errors = null;
+            $result = $this->UserSettings->saveBookmark($this->ACL->getUser(), $this->request->getData(), $errors);
             $success = !empty($result);
-            $message = $success ? __('Bookmark saved') : __('Could not save bookmark');
+            $message = $success ? __('Bookmark saved') : ($errors ?? __('Could not save bookmark'));
             $this->CRUD->setResponseForController('saveBookmark', $success, $message, $result);
             $responsePayload = $this->CRUD->getResponsePayload();
             if (!empty($responsePayload)) {

diff --git a/src/Model/Table/UserSettingsTable.php b/src/Model/Table/UserSettingsTable.php
@@ -95,9 +95,24 @@ public function editSetting($user, $name, $value)
         return $savedData;
     }
 
-    public function saveBookmark($user, $data)
+    public function saveBookmark($user, $data, &$message = null)
     {
         $setting = $this->getSettingByName($user, $this->BOOKMARK_SETTING_NAME);
+        $fieldsToCheck = [
+            'bookmark_label' => __('Label'),
+            'bookmark_name' => __('Name'),
+            'bookmark_url' => __('URL')
+        ];
+        foreach ($fieldsToCheck as $field => $fieldName) {
+            if (empty($data[$field])) {
+                $message = __('Please fill in all fields, {0} missing.', $fieldName);
+                return null;
+            }
+        }
+        if (empty($data['bookmark_label']) || empty($data['bookmark_name']) || empty($data['bookmark_url'])) {
+
+            return null;
+        }
         $bookmarkData = [
             'label' => $data['bookmark_label'],
             'name' => $data['bookmark_name'],
@@ -107,7 +122,7 @@ public function saveBookmark($user, $data)
         if (!empty($restricted_domains)) {
             $restricted_domains = explode(',', $restricted_domains);
             $parsed = parse_url($bookmarkData['url']);
-            if (!in_array($parsed['host'], $restricted_domains)) {
+            if (!empty($parsed['host']) && !in_array($parsed['host'], $restricted_domains)) {
                 return null;
             }
         }