diff options
author | Joas Schilling <coding@schilljs.com> | 2024-07-10 12:36:55 +0200 |
---|---|---|
committer | Joas Schilling <coding@schilljs.com> | 2024-07-10 13:00:53 +0200 |
commit | 9eb9aa2caade90b26c9d9fb29af30f5cc33d495c (patch) | |
tree | 938947fa14fddf98f1c43fe423a04bd725db36bc | |
parent | a24bb4c1884163eeba7a834698dcfce9e45db108 (diff) |
fix(ban): Simplify validation and document known errors
Signed-off-by: Joas Schilling <coding@schilljs.com>
-rw-r--r-- | lib/Controller/BanController.php | 12 | ||||
-rw-r--r-- | lib/Service/BanService.php | 31 | ||||
-rw-r--r-- | openapi-full.json | 6 | ||||
-rw-r--r-- | openapi.json | 6 | ||||
-rw-r--r-- | src/types/openapi/openapi-full.ts | 3 | ||||
-rw-r--r-- | src/types/openapi/openapi.ts | 3 |
6 files changed, 30 insertions, 31 deletions
diff --git a/lib/Controller/BanController.php b/lib/Controller/BanController.php index bc709824a..8c246ca14 100644 --- a/lib/Controller/BanController.php +++ b/lib/Controller/BanController.php @@ -16,6 +16,7 @@ use OCA\Talk\Service\BanService; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; /** @@ -25,7 +26,8 @@ class BanController extends AEnvironmentAwareController { public function __construct( string $appName, IRequest $request, - protected BanService $banService + protected BanService $banService, + protected ITimeFactory $timeFactory, ) { parent::__construct($appName, $request); } @@ -39,7 +41,7 @@ class BanController extends AEnvironmentAwareController { * @psalm-param Attendee::ACTOR_*|'ip' $actorType Type of actor to ban, or `ip` when banning a clients remote address * @param string $actorId Actor ID or the IP address or range in case of type `ip` * @param string $internalNote Optional internal note - * @return DataResponse<Http::STATUS_OK, TalkBan, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: string}, array{}> + * @return DataResponse<Http::STATUS_OK, TalkBan, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'bannedActor'|'internalNote'}, array{}> * * 200: Ban successfully * 400: Actor information is invalid @@ -58,14 +60,16 @@ class BanController extends AEnvironmentAwareController { $this->room->getId(), $actorId, $actorType, - null, + $this->timeFactory->getDateTime(), $internalNote ); return new DataResponse($ban->jsonSerialize(), Http::STATUS_OK); } catch (\InvalidArgumentException $e) { + /** @var 'bannedActor'|'internalNote' $message */ + $message = $e->getMessage(); return new DataResponse([ - 'error' => $e->getMessage(), + 'error' => $message, ], Http::STATUS_BAD_REQUEST); } } diff --git a/lib/Service/BanService.php b/lib/Service/BanService.php index 96f67b97c..d176f620b 100644 --- a/lib/Service/BanService.php +++ b/lib/Service/BanService.php @@ -12,42 +12,27 @@ use DateTime; use OCA\Talk\Model\Ban; use OCA\Talk\Model\BanMapper; use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Utility\ITimeFactory; class BanService { public function __construct( protected BanMapper $banMapper, - protected ITimeFactory $timeFactory, ) { } /** - * Validate the ban data. + * Create a new ban + * + * @throws \InvalidArgumentException */ - private function validateBanData(string $actorId, string $actorType, int $roomId, string $bannedId, string $bannedType, ?DateTime $bannedTime, ?string $internalNote): void { - if (empty($bannedId)) { - throw new \InvalidArgumentException("invalid_bannedId."); - } - - if (empty($bannedType)) { - throw new \InvalidArgumentException("invalid_bannedType."); + public function createBan(string $actorId, string $actorType, int $roomId, string $bannedId, string $bannedType, DateTime $bannedTime, string $internalNote): Ban { + if (empty($bannedId) || empty($bannedType)) { + throw new \InvalidArgumentException('bannedActor'); } if (empty($internalNote)) { - throw new \InvalidArgumentException("invalid_internalNote."); - } - - if ($bannedTime !== null && !$bannedTime instanceof DateTime) { - throw new \InvalidArgumentException("invalid_bannedTime."); + throw new \InvalidArgumentException('internalNote'); } - } - - /** - * Create a new ban - */ - public function createBan(string $actorId, string $actorType, int $roomId, string $bannedId, string $bannedType, ?DateTime $bannedTime, ?string $internalNote): Ban { - $this->validateBanData($actorId, $actorType, $roomId, $bannedId, $bannedType, $bannedTime, $internalNote); $ban = new Ban(); $ban->setActorId($actorId); @@ -55,7 +40,7 @@ class BanService { $ban->setRoomId($roomId); $ban->setBannedId($bannedId); $ban->setBannedType($bannedType); - $ban->setBannedTime($bannedTime ?? new \DateTime()); + $ban->setBannedTime($bannedTime); $ban->setInternalNote($internalNote); return $this->banMapper->insert($ban); diff --git a/openapi-full.json b/openapi-full.json index 9b1bb467b..f9f21adc8 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -2099,7 +2099,11 @@ ], "properties": { "error": { - "type": "string" + "type": "string", + "enum": [ + "bannedActor", + "internalNote" + ] } } } diff --git a/openapi.json b/openapi.json index 19bb7a71d..d70cb5769 100644 --- a/openapi.json +++ b/openapi.json @@ -1986,7 +1986,11 @@ ], "properties": { "error": { - "type": "string" + "type": "string", + "enum": [ + "bannedActor", + "internalNote" + ] } } } diff --git a/src/types/openapi/openapi-full.ts b/src/types/openapi/openapi-full.ts index a1d38bd4d..43e07580a 100644 --- a/src/types/openapi/openapi-full.ts +++ b/src/types/openapi/openapi-full.ts @@ -2428,7 +2428,8 @@ export interface operations { ocs: { meta: components["schemas"]["OCSMeta"]; data: { - error: string; + /** @enum {string} */ + error: "bannedActor" | "internalNote"; }; }; }; diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index f521d639b..1487919d2 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1913,7 +1913,8 @@ export interface operations { ocs: { meta: components["schemas"]["OCSMeta"]; data: { - error: string; + /** @enum {string} */ + error: "bannedActor" | "internalNote"; }; }; }; |