fix: add idempotency check to role assignment API

[kevalyq]

🐛 Bug Fix: Role Assignment Idempotency

Problem

Phase 3 role assignment API (POST /api/v1/users/{id}/roles) did not handle idempotent role assignments. Attempting to assign the same role twice to the same user caused a database unique constraint violation (500 Internal Server Error).

Root Cause

RoleController::store() did not check if the role was already assigned before attempting to insert into model_has_roles table, which has a unique constraint on (tenant_id, role_id, model_id, model_type).

Solution

✅ Added idempotency check in RoleController::store():

• Queries database for existing assignment before insert
• Returns 200 OK with existing assignment if role already assigned
• Prevents DB constraint violation
• True idempotency: duplicate requests return same result without side effects

✅ Activated skipped integration test:

• Test "role assignment is idempotent" now runs and passes
• Verifies duplicate assignment returns 200 OK (not 201 Created)
• Confirms no duplicate roles created

✅ Added edge case test:

• Verifies behavior when trying to assign same role with different temporal parameters
• Returns existing assignment unchanged (idempotent)
• No modifications to existing role

---

🧪 Testing

Test Results

✅ All 279 tests passing (916 assertions)
✅ PHPStan level max: No errors
✅ Pint PSR-12: No issues
✅ REUSE 3.3: 180/180 files compliant

New Tests

• ✅ role assignment is idempotent (previously skipped)
• ✅ idempotency returns existing role with different temporal parameters (new edge case)

---

📝 Changes Summary

Modified Files (2)

• app/Http/Controllers/RoleController.php - Added idempotency check
• tests/Feature/Integration/RbacIntegrationTest.php - Activated skipped test + new test

Lines Changed

• +72 insertions
• -12 deletions
• 84 total lines

---

✅ Acceptance Criteria

• Duplicate role assignment returns 200 OK (not 500)
• Integration test "role assignment is idempotent" passes
• Existing 279 tests still pass
• API behavior documented in response message
• PHPStan type hints added for DB query result

---

🔗 Related

Fixes #163

Discovered in: PR #162 (Permission Naming Bug Fix)

---

📊 Impact

Severity: Medium → Fixed

• ✅ Prevents 500 errors on duplicate requests
• ✅ Enables safe retry logic in frontend
• ✅ Follows REST best practices (idempotent POST)
• ✅ No breaking changes

---

Type: Bug Fix
Priority: Medium
Breaking Changes: None

[github-actions]

💡 Tip: Consider Using Draft PRs

Benefits of opening PRs as drafts initially:

• 💰 Saves CI runtime and Copilot review credits
• 🎯 Automatically sets linked issues to "🚧 In Progress" status
• 🚀 Mark "Ready for review" when done to trigger full CI pipeline

How to convert:

1. Click "Still in progress? Convert to draft" in the sidebar, OR
2. Use gh pr ready when ready for review

This is just a friendly reminder - feel free to continue as is! 😊

[Copilot]

Pull Request Overview

This PR fixes a bug where assigning the same role to a user multiple times caused a database unique constraint violation (500 error). The fix adds an idempotency check that returns the existing role assignment (200 OK) instead of attempting to create a duplicate. The PR also activates a previously skipped integration test and adds a new edge case test for temporal parameters.

Key Changes:

• Added idempotency check in RoleController::store() to prevent duplicate role assignments
• Activated previously skipped integration test for role assignment idempotency
• Added new test case verifying idempotent behavior with different temporal parameters

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
app/Http/Controllers/RoleController.php	Added database query to check for existing role assignments before insertion, returns existing assignment if found
tests/Feature/Integration/RbacIntegrationTest.php	Removed skip marker from idempotency test and added new test case for temporal parameter edge case