From 2b0a1e01aef567e1f5932bcf55c975f136c3f840 Mon Sep 17 00:00:00 2001
From: Holger Schmermbeck <holger@secpal.app>
Date: Sat, 8 Nov 2025 18:02:00 +0100
Subject: [PATCH] fix: resolve Copilot review comments from PR #112

Addresses post-merge Copilot feedback on RBAC Phase 1 code quality:

1. TemporalRoleUser.php:
   - Fix: Changed $fillable array from 'team_id' to 'tenant_id'
   - Reason: Matches Spatie Permission config (team_foreign_key => 'tenant_id')
   - Impact: Ensures mass assignment uses correct column name

2. User.php:
   - Fix: Added type hint 'Builder $query' to where() callback
   - Reason: Improves type safety, IDE support, consistency with applyActiveFilter()
   - Impact: Better developer experience, aligns with PHPStan Level Max

All tests passing (12/12 temporal role tests, 163 total)
---
 CHANGELOG.md                    | 6 ++++++
 app/Models/TemporalRoleUser.php | 2 +-
 app/Models/User.php             | 3 ++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d60565a..39e3362 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **RBAC Phase 1 Post-Merge Fixes** - Addressed Copilot review comments from PR #112
+  - `TemporalRoleUser`: Changed `$fillable` array from `'team_id'` to `'tenant_id'` (matches Spatie Permission config)
+  - `User::roles()`: Added type hint `Builder $query` to where callback for improved type safety and IDE support
+
 ### Added
 
 - **RBAC Phase 1: Temporal Role Foundation** - Time-based role assignments with automatic expiration
diff --git a/app/Models/TemporalRoleUser.php b/app/Models/TemporalRoleUser.php
index 1b742a9..b1c890e 100644
--- a/app/Models/TemporalRoleUser.php
+++ b/app/Models/TemporalRoleUser.php
@@ -47,7 +47,7 @@ class TemporalRoleUser extends MorphPivot
         'role_id',
         'model_type',
         'model_id',
-        'team_id',
+        'tenant_id',
         'valid_from',
         'valid_until',
         'auto_revoke',
diff --git a/app/Models/User.php b/app/Models/User.php
index 42a9ea9..74337aa 100644
--- a/app/Models/User.php
+++ b/app/Models/User.php
@@ -6,6 +6,7 @@
 namespace App\Models;
 
 // use Illuminate\Contracts\Auth\MustVerifyEmail;
+use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Relations\MorphToMany;
 use Illuminate\Foundation\Auth\User as Authenticatable;
@@ -89,7 +90,7 @@ public function roles(): MorphToMany
                 'created_at',
                 'updated_at',
             ])
-            ->where(function ($query) {
+            ->where(function (Builder $query) {
                 // Only return currently active roles using shared filtering logic
                 TemporalRoleUser::applyActiveFilter($query, 'model_has_roles.');
             });