From 439d07c9de910d9bc6a1f4908e3057ff75885255 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 24 Mar 2026 13:31:58 +0000 Subject: [PATCH] refactor: standardise error response format across API controllers Add HasStandardApiResponses trait with consistent error format: {"success": false, "error": "Human-readable message", "code": "MACHINE_READABLE_CODE"} Applied to EntitlementApiController, EntitlementWebhookController, and WorkspaceController. Updated tests to assert the new format. Fixes #20 Co-Authored-By: Claude Opus 4.6 (1M context) --- Concerns/HasStandardApiResponses.php | 92 +++++++++++++++++++ .../Api/EntitlementWebhookController.php | 51 +++++----- Controllers/EntitlementApiController.php | 86 ++++++----------- Controllers/WorkspaceController.php | 12 +-- tests/Feature/EntitlementApiTest.php | 19 +++- 5 files changed, 170 insertions(+), 90 deletions(-) create mode 100644 Concerns/HasStandardApiResponses.php diff --git a/Concerns/HasStandardApiResponses.php b/Concerns/HasStandardApiResponses.php new file mode 100644 index 0000000..43b6ccf --- /dev/null +++ b/Concerns/HasStandardApiResponses.php @@ -0,0 +1,92 @@ +json(array_merge([ + 'success' => false, + 'error' => $message, + 'code' => $code, + ], $extra), $status); + } + + /** + * Return a standardised success response. + */ + protected function successResponse(array $data = [], int $status = 200): JsonResponse + { + return response()->json(array_merge([ + 'success' => true, + ], $data), $status); + } + + /** + * Return a not found error response. + */ + protected function notFoundResponse(string $resource = 'Resource'): JsonResponse + { + return $this->errorResponse("{$resource} not found", 'NOT_FOUND', 404); + } + + /** + * Return an unauthenticated error response. + */ + protected function unauthenticatedResponse(): JsonResponse + { + return $this->errorResponse('Unauthenticated', 'UNAUTHENTICATED', 401); + } + + /** + * Return an access denied error response. + */ + protected function accessDeniedResponse(string $message = 'Access denied.'): JsonResponse + { + return $this->errorResponse($message, 'ACCESS_DENIED', 403); + } + + /** + * Return a no workspace error response. + */ + protected function noWorkspaceResponse(): JsonResponse + { + return $this->errorResponse( + 'No workspace found. Please select a workspace first.', + 'NO_WORKSPACE', + 404 + ); + } + + /** + * Return a validation / unprocessable entity error response. + */ + protected function unprocessableResponse(string $message, string $code = 'UNPROCESSABLE_ENTITY', array $extra = []): JsonResponse + { + return $this->errorResponse($message, $code, 422, $extra); + } +} diff --git a/Controllers/Api/EntitlementWebhookController.php b/Controllers/Api/EntitlementWebhookController.php index c168ad9..2145125 100644 --- a/Controllers/Api/EntitlementWebhookController.php +++ b/Controllers/Api/EntitlementWebhookController.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Core\Tenant\Controllers\Api; use Core\Rules\SafeWebhookUrl; +use Core\Tenant\Concerns\HasStandardApiResponses; use Core\Tenant\Exceptions\InvalidWebhookUrlException; use Core\Tenant\Models\EntitlementWebhook; use Core\Tenant\Models\EntitlementWebhookDelivery; @@ -25,6 +26,8 @@ use Illuminate\Validation\Rule; */ class EntitlementWebhookController extends Controller { + use HasStandardApiResponses; + public function __construct( protected EntitlementWebhookService $webhookService ) {} @@ -71,16 +74,16 @@ class EntitlementWebhookController extends Controller metadata: $validated['metadata'] ?? [] ); - return response()->json([ + return $this->successResponse([ 'message' => __('Webhook created successfully'), 'webhook' => $webhook, 'secret' => $webhook->secret, // Return secret on creation only ], 201); } catch (InvalidWebhookUrlException $e) { - return response()->json([ - 'message' => $e->getMessage(), - 'error' => 'invalid_webhook_url', - ], 422); + return $this->unprocessableResponse( + $e->getMessage(), + 'INVALID_WEBHOOK_URL' + ); } } @@ -94,7 +97,7 @@ class EntitlementWebhookController extends Controller $webhook->loadCount('deliveries'); $webhook->load(['deliveries' => fn ($q) => $q->latest('created_at')->limit(10)]); - return response()->json([ + return $this->successResponse([ 'webhook' => $webhook, 'available_events' => $this->webhookService->getAvailableEvents(), ]); @@ -120,15 +123,15 @@ class EntitlementWebhookController extends Controller try { $webhook = $this->webhookService->update($webhook, $validated); - return response()->json([ + return $this->successResponse([ 'message' => __('Webhook updated successfully'), 'webhook' => $webhook, ]); } catch (InvalidWebhookUrlException $e) { - return response()->json([ - 'message' => $e->getMessage(), - 'error' => 'invalid_webhook_url', - ], 422); + return $this->unprocessableResponse( + $e->getMessage(), + 'INVALID_WEBHOOK_URL' + ); } } @@ -141,7 +144,7 @@ class EntitlementWebhookController extends Controller $this->webhookService->unregister($webhook); - return response()->json([ + return $this->successResponse([ 'message' => __('Webhook deleted successfully'), ]); } @@ -155,7 +158,7 @@ class EntitlementWebhookController extends Controller $secret = $webhook->regenerateSecret(); - return response()->json([ + return $this->successResponse([ 'message' => __('Secret regenerated successfully'), 'secret' => $secret, ]); @@ -174,18 +177,17 @@ class EntitlementWebhookController extends Controller try { $delivery = $this->webhookService->testWebhook($webhook); - return response()->json([ + return $this->successResponse([ 'message' => $delivery->isSucceeded() ? __('Test webhook sent successfully') : __('Test webhook failed'), 'delivery' => $delivery, ]); } catch (InvalidWebhookUrlException $e) { - return response()->json([ - 'message' => $e->getMessage(), - 'error' => 'invalid_webhook_url', - 'reason' => $e->reason, - ], 422); + return $this->unprocessableResponse( + $e->getMessage(), + 'INVALID_WEBHOOK_URL' + ); } } @@ -198,7 +200,7 @@ class EntitlementWebhookController extends Controller $this->webhookService->resetCircuitBreaker($webhook); - return response()->json([ + return $this->successResponse([ 'message' => __('Webhook re-enabled successfully'), 'webhook' => $webhook->refresh(), ]); @@ -226,14 +228,15 @@ class EntitlementWebhookController extends Controller $this->authorizeWebhook($request, $delivery->webhook); if ($delivery->isSucceeded()) { - return response()->json([ - 'message' => __('Cannot retry a successful delivery'), - ], 422); + return $this->unprocessableResponse( + __('Cannot retry a successful delivery'), + 'DELIVERY_ALREADY_SUCCEEDED' + ); } $delivery = $this->webhookService->retryDelivery($delivery); - return response()->json([ + return $this->successResponse([ 'message' => $delivery->isSucceeded() ? __('Delivery retried successfully') : __('Delivery retry failed'), diff --git a/Controllers/EntitlementApiController.php b/Controllers/EntitlementApiController.php index 8773eab..42afa72 100644 --- a/Controllers/EntitlementApiController.php +++ b/Controllers/EntitlementApiController.php @@ -6,6 +6,7 @@ namespace Core\Tenant\Controllers; use Core\Api\RateLimit\RateLimit; use Core\Front\Controller; +use Core\Tenant\Concerns\HasStandardApiResponses; use Core\Tenant\Models\EntitlementLog; use Core\Tenant\Models\Package; use Core\Tenant\Models\User; @@ -43,6 +44,8 @@ use Illuminate\Support\Str; #[RateLimit(limit: 60, window: 60, key: 'entitlement-api')] class EntitlementApiController extends Controller { + use HasStandardApiResponses; + public function __construct( protected EntitlementService $entitlements ) {} @@ -89,10 +92,11 @@ class EntitlementApiController extends Controller $package = Package::where('code', $validated['product_code'])->first(); if (! $package) { - return response()->json([ - 'success' => false, - 'error' => "Package '{$validated['product_code']}' not found", - ], 404); + return $this->errorResponse( + "Package '{$validated['product_code']}' not found", + 'PACKAGE_NOT_FOUND', + 404 + ); } // Get or create the user's primary workspace @@ -133,8 +137,7 @@ class EntitlementApiController extends Controller ] ); - return response()->json([ - 'success' => true, + return $this->successResponse([ 'entitlement_id' => $workspacePackage->id, 'workspace_id' => $workspace->id, 'workspace_slug' => $workspace->slug, @@ -151,10 +154,7 @@ class EntitlementApiController extends Controller $workspacePackage = WorkspacePackage::find($id); if (! $workspacePackage) { - return response()->json([ - 'success' => false, - 'error' => 'Entitlement not found', - ], 404); + return $this->notFoundResponse('Entitlement'); } $workspace = $workspacePackage->workspace; @@ -170,8 +170,7 @@ class EntitlementApiController extends Controller $this->entitlements->invalidateCache($workspace); - return response()->json([ - 'success' => true, + return $this->successResponse([ 'entitlement_id' => $workspacePackage->id, 'status' => $workspacePackage->fresh()->status, ]); @@ -185,10 +184,7 @@ class EntitlementApiController extends Controller $workspacePackage = WorkspacePackage::find($id); if (! $workspacePackage) { - return response()->json([ - 'success' => false, - 'error' => 'Entitlement not found', - ], 404); + return $this->notFoundResponse('Entitlement'); } $workspace = $workspacePackage->workspace; @@ -203,8 +199,7 @@ class EntitlementApiController extends Controller $this->entitlements->invalidateCache($workspace); - return response()->json([ - 'success' => true, + return $this->successResponse([ 'entitlement_id' => $workspacePackage->id, 'status' => $workspacePackage->fresh()->status, ]); @@ -218,10 +213,7 @@ class EntitlementApiController extends Controller $workspacePackage = WorkspacePackage::find($id); if (! $workspacePackage) { - return response()->json([ - 'success' => false, - 'error' => 'Entitlement not found', - ], 404); + return $this->notFoundResponse('Entitlement'); } $workspace = $workspacePackage->workspace; @@ -237,8 +229,7 @@ class EntitlementApiController extends Controller $this->entitlements->invalidateCache($workspace); - return response()->json([ - 'success' => true, + return $this->successResponse([ 'entitlement_id' => $workspacePackage->id, 'status' => $workspacePackage->fresh()->status, ]); @@ -257,10 +248,7 @@ class EntitlementApiController extends Controller $workspacePackage = WorkspacePackage::find($id); if (! $workspacePackage) { - return response()->json([ - 'success' => false, - 'error' => 'Entitlement not found', - ], 404); + return $this->notFoundResponse('Entitlement'); } $workspace = $workspacePackage->workspace; @@ -291,8 +279,7 @@ class EntitlementApiController extends Controller $this->entitlements->invalidateCache($workspace); - return response()->json([ - 'success' => true, + return $this->successResponse([ 'entitlement_id' => $workspacePackage->id, 'status' => $workspacePackage->fresh()->status, 'expires_at' => $workspacePackage->fresh()->expires_at?->toIso8601String(), @@ -307,14 +294,10 @@ class EntitlementApiController extends Controller $workspacePackage = WorkspacePackage::with(['package', 'workspace'])->find($id); if (! $workspacePackage) { - return response()->json([ - 'success' => false, - 'error' => 'Entitlement not found', - ], 404); + return $this->notFoundResponse('Entitlement'); } - return response()->json([ - 'success' => true, + return $this->successResponse([ 'entitlement' => [ 'id' => $workspacePackage->id, 'workspace_id' => $workspacePackage->workspace_id, @@ -356,22 +339,20 @@ class EntitlementApiController extends Controller $user = User::where('email', $validated['email'])->first(); if (! $user) { - return response()->json([ + return $this->errorResponse('User not found', 'USER_NOT_FOUND', 404, [ 'allowed' => false, - 'reason' => 'User not found', 'feature_code' => $validated['feature'], - ], 404); + ]); } // Get user's primary workspace $workspace = $user->defaultHostWorkspace(); if (! $workspace) { - return response()->json([ + return $this->errorResponse('No workspace found for user', 'NO_WORKSPACE', 404, [ 'allowed' => false, - 'reason' => 'No workspace found for user', 'feature_code' => $validated['feature'], - ], 404); + ]); } // Check entitlement @@ -411,20 +392,14 @@ class EntitlementApiController extends Controller $user = User::where('email', $validated['email'])->first(); if (! $user) { - return response()->json([ - 'success' => false, - 'error' => 'User not found', - ], 404); + return $this->errorResponse('User not found', 'USER_NOT_FOUND', 404); } // Get user's primary workspace $workspace = $user->defaultHostWorkspace(); if (! $workspace) { - return response()->json([ - 'success' => false, - 'error' => 'No workspace found for user', - ], 404); + return $this->errorResponse('No workspace found for user', 'NO_WORKSPACE', 404); } // Record usage @@ -436,8 +411,7 @@ class EntitlementApiController extends Controller $validated['metadata'] ?? null ); - return response()->json([ - 'success' => true, + return $this->successResponse([ 'usage_record_id' => $record->id, 'feature_code' => $validated['feature'], 'quantity' => $validated['quantity'] ?? 1, @@ -500,17 +474,13 @@ class EntitlementApiController extends Controller $user = $request->user(); if (! $user) { - return response()->json([ - 'error' => 'Unauthenticated', - ], 401); + return $this->unauthenticatedResponse(); } $workspace = $user->defaultHostWorkspace(); if (! $workspace) { - return response()->json([ - 'error' => 'No workspace found', - ], 404); + return $this->errorResponse('No workspace found', 'NO_WORKSPACE', 404); } return $this->summary($request, $workspace); diff --git a/Controllers/WorkspaceController.php b/Controllers/WorkspaceController.php index bc63131..5157620 100644 --- a/Controllers/WorkspaceController.php +++ b/Controllers/WorkspaceController.php @@ -5,13 +5,13 @@ declare(strict_types=1); namespace Core\Tenant\Controllers; use Core\Front\Controller; +use Core\Tenant\Concerns\HasStandardApiResponses; use Core\Tenant\Models\User; use Core\Tenant\Models\Workspace; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; -use Mod\Api\Controllers\Concerns\HasApiResponses; use Mod\Api\Controllers\Concerns\ResolvesWorkspace; use Mod\Api\Resources\PaginatedCollection; use Mod\Api\Resources\WorkspaceResource; @@ -24,7 +24,7 @@ use Mod\Api\Resources\WorkspaceResource; */ class WorkspaceController extends Controller { - use HasApiResponses; + use HasStandardApiResponses; use ResolvesWorkspace; /** @@ -219,10 +219,10 @@ class WorkspaceController extends Controller // Prevent deleting user's only workspace $workspaceCount = $user->workspaces()->count(); if ($workspaceCount <= 1) { - return response()->json([ - 'error' => 'cannot_delete', - 'message' => 'You cannot delete your only workspace.', - ], 422); + return $this->unprocessableResponse( + 'You cannot delete your only workspace.', + 'CANNOT_DELETE_LAST_WORKSPACE' + ); } $workspace->delete(); diff --git a/tests/Feature/EntitlementApiTest.php b/tests/Feature/EntitlementApiTest.php index 3988ab1..2b8c528 100644 --- a/tests/Feature/EntitlementApiTest.php +++ b/tests/Feature/EntitlementApiTest.php @@ -118,8 +118,10 @@ describe('Cross-App Entitlement API', function () { $response->assertStatus(404) ->assertJson([ + 'success' => false, + 'error' => 'User not found', + 'code' => 'USER_NOT_FOUND', 'allowed' => false, - 'reason' => 'User not found', ]); }); @@ -131,8 +133,10 @@ describe('Cross-App Entitlement API', function () { $response->assertStatus(404) ->assertJson([ + 'success' => false, + 'error' => 'No workspace found for user', + 'code' => 'NO_WORKSPACE', 'allowed' => false, - 'reason' => 'No workspace found for user', ]); }); @@ -266,6 +270,7 @@ describe('Cross-App Entitlement API', function () { ->assertJson([ 'success' => false, 'error' => 'User not found', + 'code' => 'USER_NOT_FOUND', ]); }); @@ -282,6 +287,7 @@ describe('Cross-App Entitlement API', function () { ->assertJson([ 'success' => false, 'error' => 'No workspace found for user', + 'code' => 'NO_WORKSPACE', ]); }); @@ -356,7 +362,9 @@ describe('Cross-App Entitlement API', function () { $response->assertStatus(404) ->assertJson([ + 'success' => false, 'error' => 'No workspace found', + 'code' => 'NO_WORKSPACE', ]); }); @@ -506,6 +514,7 @@ describe('Blesta Provisioning API', function () { ->assertJson([ 'success' => false, 'error' => "Package 'nonexistent-package' not found", + 'code' => 'PACKAGE_NOT_FOUND', ]); }); @@ -634,6 +643,7 @@ describe('Blesta Provisioning API', function () { ->assertJson([ 'success' => false, 'error' => 'Entitlement not found', + 'code' => 'NOT_FOUND', ]); }); @@ -698,6 +708,7 @@ describe('Blesta Provisioning API', function () { ->assertJson([ 'success' => false, 'error' => 'Entitlement not found', + 'code' => 'NOT_FOUND', ]); }); @@ -782,6 +793,7 @@ describe('Blesta Provisioning API', function () { ->assertJson([ 'success' => false, 'error' => 'Entitlement not found', + 'code' => 'NOT_FOUND', ]); }); @@ -852,6 +864,7 @@ describe('Blesta Provisioning API', function () { ->assertJson([ 'success' => false, 'error' => 'Entitlement not found', + 'code' => 'NOT_FOUND', ]); }); @@ -935,6 +948,7 @@ describe('Blesta Provisioning API', function () { ->assertJson([ 'success' => false, 'error' => 'Entitlement not found', + 'code' => 'NOT_FOUND', ]); }); @@ -1063,6 +1077,7 @@ describe('Error Response Format', function () { ->assertJsonStructure([ 'success', 'error', + 'code', ]); });