fix(federation): settings layout and error handling

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

5 files changed, 61 insertions, 47 deletions

diff --git a/apps/federation/appinfo/routes.php b/apps/federation/appinfo/routes.php
index 7ab9f84edd5..663da827de3 100644
--- a/apps/federation/appinfo/routes.php
+++ b/apps/federation/appinfo/routes.php
@@ -6,18 +6,6 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 return [
-	'routes' => [
-		[
-			'name' => 'Settings#addServer',
-			'url' => '/trusted-servers',
-			'verb' => 'POST'
-		],
-		[
-			'name' => 'Settings#removeServer',
-			'url' => '/trusted-servers/{id}',
-			'verb' => 'DELETE'
-		],
-	],
 	'ocs' => [
 		// old endpoints, only used by Nextcloud and ownCloud
 		[
@@ -43,5 +31,15 @@ return [
 			'url' => '/shared-secret',
 			'verb' => 'POST',
 		],
+		[
+			'name' => 'Settings#addServer',
+			'url' => '/trusted-servers',
+			'verb' => 'POST'
+		],
+		[
+			'name' => 'Settings#removeServer',
+			'url' => '/trusted-servers/{id}',
+			'verb' => 'DELETE'
+		],
 	],
 ];
diff --git a/apps/federation/css/settings-admin.css b/apps/federation/css/settings-admin.css
index 6e6fcc6f488..6fa103e83b2 100644
--- a/apps/federation/css/settings-admin.css
+++ b/apps/federation/css/settings-admin.css
@@ -9,11 +9,13 @@
 
 #listOfTrustedServers li {
 	padding-bottom: 10px;
+	display: flex;
+	align-items: center;
 }
 
 .removeTrustedServer {
 	display: none;
-	vertical-align:middle;
+	vertical-align: middle;
 	padding-inline-start: 10px;
 }
 
@@ -26,20 +28,20 @@
 }
 
 #listOfTrustedServers .icon {
-	cursor: pointer;
 	display: inline-block;
+	cursor: pointer;
 	vertical-align: middle;
 	margin-inline-start: 10px;
 }
 
-#ocFederationAddServer #serverUrl {
-	width: 270px;
-}
-
 .serverUrl-block {
-	max-width: 310px;
 	display: flex;
-	flex-direction: row;
 	align-items: center;
-	justify-content: space-between;
+	flex-direction: row;
+	justify-content: flex-start;
+	gap: 8px;
+}
+
+.serverUrl-block input {
+	width: 270px;
 }
diff --git a/apps/federation/js/settings-admin.js b/apps/federation/js/settings-admin.js
index 2487ed993ac..c2b9e206ad8 100644
--- a/apps/federation/js/settings-admin.js
+++ b/apps/federation/js/settings-admin.js
@@ -51,9 +51,6 @@
         });
 
         $inpServerUrl.on("change keyup", function (e) {
-
-            console.log("typing away");
-
             var url = $(this).val();
 
             // toggle add-button visibility based on input length
@@ -79,11 +76,11 @@
         OC.msg.startSaving('#ocFederationAddServer .msg');
 
         $.post(
-            OC.generateUrl('/apps/federation/trusted-servers'),
+            OC.getRootPath() + '/ocs/v2.php/apps/federation/trusted-servers',
             {
                 url: url
             }
-        ).done(function (data) {
+        ).done(function({data}) {
             $("#serverUrl").attr('value', '');
             $("#listOfTrustedServers").prepend(
                 $('<li>')
@@ -95,13 +92,13 @@
             OC.msg.finishedSuccess('#ocFederationAddServer .msg', data.message);
         })
         .fail(function (jqXHR) {
-            OC.msg.finishedError('#ocFederationAddServer .msg', JSON.parse(jqXHR.responseText).message);
+            OC.msg.finishedError('#ocFederationAddServer .msg', JSON.parse(jqXHR.responseText).data.message);
         });
     };
 
     function removeServer( id ) {
         $.ajax({
-            url: OC.generateUrl('/apps/federation/trusted-servers/' + id),
+            url: OC.getRootPath() + '/ocs/v2.php/apps/federation/trusted-servers/' + id,
             type: 'DELETE',
             success: function(response) {
                 $("#ocFederationSettings").find("#" + id).remove();
diff --git a/apps/federation/lib/Controller/SettingsController.php b/apps/federation/lib/Controller/SettingsController.php
index 663dda8916a..3da38e34fb8 100644
--- a/apps/federation/lib/Controller/SettingsController.php
+++ b/apps/federation/lib/Controller/SettingsController.php
@@ -9,14 +9,16 @@ namespace OCA\Federation\Controller;
 
 use OCA\Federation\Settings\Admin;
 use OCA\Federation\TrustedServers;
-use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
 use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\Http\JSONResponse;
+use OCP\AppFramework\OCSController;
 use OCP\HintException;
 use OCP\IL10N;
 use OCP\IRequest;
 
-class SettingsController extends Controller {
+class SettingsController extends OCSController {
 	public function __construct(
 		string $AppName,
 		IRequest $request,
@@ -29,18 +31,30 @@ class SettingsController extends Controller {
 
 	/**
 	 * Add server to the list of trusted Nextclouds.
-	 *
-	 * @throws HintException
 	 */
 	#[AuthorizedAdminSetting(settings: Admin::class)]
-	public function addServer(string $url): DataResponse {
-		$this->checkServer(trim($url));
-		$id = $this->trustedServers->addServer(trim($url));
+	public function addServer(string $url): JSONResponse {
+		try {
+			$this->checkServer(trim($url));
+		} catch (HintException $e) {
+			return new JSONResponse([
+				'message' => 'error',
+				'data' => [
+					'message' => $e->getMessage(),
+					'hint' => $e->getHint(),
+				],
+			], $e->getCode());
+		}
 
-		return new DataResponse([
-			'url' => $url,
-			'id' => $id,
-			'message' => $this->l->t('Added to the list of trusted servers')
+		// Add the server to the list of trusted servers, all is well
+		$id = $this->trustedServers->addServer(trim($url));
+		return new JSONResponse([
+			'message' => 'ok',
+			'data' => [
+				'url' => $url,
+				'id' => $id,
+				'message' => $this->l->t('Added to the list of trusted servers')
+			],
 		]);
 	}
 
@@ -48,9 +62,12 @@ class SettingsController extends Controller {
 	 * Add server to the list of trusted Nextclouds.
 	 */
 	#[AuthorizedAdminSetting(settings: Admin::class)]
-	public function removeServer(int $id): DataResponse {
+	public function removeServer(int $id): JSONResponse {
 		$this->trustedServers->removeServer($id);
-		return new DataResponse();
+		return new JSONResponse([
+			'message' => 'ok',
+			'data' => ['id' => $id],
+		]);
 	}
 
 	/**
@@ -63,13 +80,13 @@ class SettingsController extends Controller {
 		if ($this->trustedServers->isTrustedServer($url) === true) {
 			$message = 'Server is already in the list of trusted servers.';
 			$hint = $this->l->t('Server is already in the list of trusted servers.');
-			throw new HintException($message, $hint);
+			throw new HintException($message, $hint, Http::STATUS_CONFLICT);
 		}
 
 		if ($this->trustedServers->isNextcloudServer($url) === false) {
 			$message = 'No server to federate with found';
 			$hint = $this->l->t('No server to federate with found');
-			throw new HintException($message, $hint);
+			throw new HintException($message, $hint, Http::STATUS_NOT_FOUND);
 		}
 
 		return true;
diff --git a/apps/federation/templates/settings-admin.php b/apps/federation/templates/settings-admin.php
index dabd341ce72..175598c9c30 100644
--- a/apps/federation/templates/settings-admin.php
+++ b/apps/federation/templates/settings-admin.php
@@ -35,8 +35,9 @@ style('federation', 'settings-admin')
 			</li>
 		<?php } ?>
 	</ul>
-	<p id="ocFederationAddServer">
-		<button id="ocFederationAddServerButton" class=""><?php p($l->t('+ Add trusted server')); ?></button>
+
+	<div id="ocFederationAddServer">
+		<button id="ocFederationAddServerButton"><?php p($l->t('+ Add trusted server')); ?></button>
 		<div class="serverUrl hidden">
 			<div class="serverUrl-block">
 				<label for="serverUrl"><?php p($l->t('Trusted server')); ?></label>
@@ -45,6 +46,5 @@ style('federation', 'settings-admin')
 			</div>
 			<span class="msg"></span>
 		</div>
-	</p>
-
+	</div>
 </div>