-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Issue Description
Remove redundant /api/ prefix from Laravel route definitions to align with production API URL structure. Currently, internal routing uses /api/v1/* but production API is accessible at https://api.secpal.app/v1/* (no /api/ in URL).
Type: Technical Debt / Refactoring
Justification: We're at v0.x.x - breaking changes allowed!
Current State vs Desired State
Current (Inconsistent):
// routes/api.php
Route::prefix('v1')->group(function () {
Route::get('/users', ...);
});
// .env / Laravel routing
// Internal: /api/v1/users
// Production URL: https://api.secpal.app/v1/users ← No /api/ prefix!Desired (Consistent):
// routes/api.php
Route::prefix('v1')->group(function () {
Route::get('/users', ...);
});
// No prefix needed - access directly at /v1/*
// Production URL: https://api.secpal.app/v1/users ✅ Matches routingWhy This Matters
-
Documentation Confusion:
- PR fix: resolve permission naming conflict and add RBAC integration tests #162 fixed 9 documentation files changing
/api/v1/→/v1/ - Root cause was inconsistent routing prefix
- PR fix: resolve permission naming conflict and add RBAC integration tests #162 fixed 9 documentation files changing
-
Test Confusion:
- Tests use
$this->getJson('/api/v1/users')but production uses/v1/users - Developers need to remember the discrepancy
- Tests use
-
Cognitive Load:
- Internal routing vs external URL mismatch
- "Why do we have
/api/in routes but not in URLs?"
Proposed Solution
Option 1: Remove /api/ Prefix Entirely (Recommended)
Changes:
- Remove
'prefix' => 'api'frombootstrap/app.phporroutes/api.php - Update all tests:
/api/v1/*→/v1/* - Update any hardcoded URLs in code
Pros:
- ✅ Direct match: Route = URL
- ✅ Simpler to understand
- ✅ No mental mapping needed
Cons:
⚠️ Breaking change (but we're v0.x.x!)⚠️ Need to update all tests
Option 2: Keep /api/ but Document Why (Not Recommended)
Changes:
- Add comment in routes explaining discrepancy
- Update
.github/copilot-instructions.mdto document this
Pros:
- ✅ No breaking changes
Cons:
- ❌ Continues confusion
- ❌ Future PRs will repeat same mistake
- ❌ Doesn't solve root cause
Affected Files
Code Changes (~10 files):
routes/api.php- Remove or update prefixbootstrap/app.php- Check forapiprefix configuration- All test files (~20 files) - Update URLs
Documentation (Already Fixed in PR #162):
- ✅
README.md - ✅
CHANGELOG.md - ✅
docs/api/rbac-endpoints.md - ✅
docs/guides/role-management.md - ✅
docs/guides/permission-system.md - ✅
docs/guides/temporal-roles.md - ✅
docs/guides/direct-permissions.md - ✅
docs/rbac-architecture.md - ✅
docs/GUARD_ARCHITECTURE.md
Implementation Plan
Phase 1: Preparation
- Audit all route definitions in
routes/api.php - Audit all test files for
/api/v1/usage - Create list of all affected files
Phase 2: Remove Prefix
- Remove
'prefix' => 'api'from route configuration - Test locally with DDEV
Phase 3: Update Tests
- Bulk replace
/api/v1/→/v1/in all test files - Run full test suite to verify
- Fix any remaining hardcoded URLs
Phase 4: Verification
- All 277+ tests passing
- PHPStan Level Max clean
- Laravel Pint clean
- Preflight checks pass
Breaking Changes
Before:
# Internal Laravel routing
POST /api/v1/users/123/roles
# But production URL was always:
POST https://api.secpal.app/v1/users/123/rolesAfter:
# Internal Laravel routing matches production
POST /v1/users/123/roles
# Production URL unchanged:
POST https://api.secpal.app/v1/users/123/rolesImpact:
- ❌ Old hardcoded URLs in frontend/scripts break (but shouldn't exist - we use env vars!)
- ✅ More consistent and predictable
- ✅ Documentation already updated (PR fix: resolve permission naming conflict and add RBAC integration tests #162)
Alternatives Considered
Alternative 1: Add /api/ to Production URL
Rejected: Subdomain api.secpal.app already indicates it's an API. Adding /api/ prefix is redundant.
Alternative 2: Use Nginx Rewrite
Rejected: Adds complexity. Better to fix at Laravel level.
Alternative 3: Keep Current State
Rejected: Causes ongoing confusion and documentation drift.
Success Criteria
- All routes accessible at
/v1/*(no/api/prefix) - All tests use
/v1/*URLs - Documentation already correct (PR fix: resolve permission naming conflict and add RBAC integration tests #162)
- Zero cognitive overhead for developers
- Production behavior unchanged (URLs already use
/v1/*)
Effort Estimate
Size: Small (2-4 hours)
- 30 min: Remove prefix configuration
- 1 hour: Bulk update tests
- 30 min: Run tests and fix issues
- 1 hour: Review and verification
Milestone
🎯 v0.3.0 - Technical debt cleanup
Labels
type: refactor🔨priority: low🟢area: routing🛣️effort: S(2-4 hours)breaking-change⚠️ (acceptable in v0.x)
Related Issues/PRs
- PR fix: resolve permission naming conflict and add RBAC integration tests #162 - Fixed documentation URLs (symptom of this issue)
- Issue 🔐 Implement RBAC System (Role-Based Access Control) #5 - RBAC System (where docs were corrected)
Note: This is a "nice-to-have" cleanup, not urgent. Can be tackled during a maintenance sprint or when working on routing-related features.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status