fix(projects): check membership, not access, in addUserToProject()

isUserAssignedToProject() answers "can this user reach the project",
and it returns true for every admin and owner without consulting
zp_relationuserproject at all. Using it as the idempotence check made
addUserToProject() a permanent no-op for exactly those users: an admin
could never be placed on a project team, and the caller was told
"already a member" about someone holding no relation row.

Found while wiring the first consumer — allocating an owner to a
project in PgmPro Resource Allocation reported success, emitted no
toast, and left the team unchanged.

isUserMemberOfProject() is the same query without the role
short-circuit, which is what writing a membership should be gated on.

Adds a regression test pinning the distinction: a repository where
isUserAssignedToProject returns true but isUserMemberOfProject returns
false must still insert. 42 tests / 146 assertions green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GYSPNgLdUwEnxgYqTxMc85

Author: gloriafolaron

app/Domain/Projects/Services/Projects.php

@@ -2379,7 +2379,15 @@ public function addUserToProject(int $userId, int $projectId, string $projectRol
             return false;
         }
 
-        if ($this->projectRepository->isUserAssignedToProject($userId, $projectId)) {
+        // isUserMemberOfProject, NOT isUserAssignedToProject: the latter
+        // answers "can this user reach the project", which is true for
+        // every admin and owner regardless of any relation row. Using it
+        // here would make the method a silent no-op for exactly those
+        // users — an admin could never be added to a project team, and
+        // callers would get false ("already a member") for someone who
+        // isn't on the team at all. Membership is what we're writing, so
+        // membership is what we check.
+        if ($this->projectRepository->isUserMemberOfProject($userId, $projectId)) {
             return false;
         }
 

tests/Unit/app/Domain/Projects/Services/ProjectsServiceTest.php

@@ -706,7 +706,7 @@ public function test_add_user_to_project_inserts_when_not_a_member(): void
     {
         $added = [];
         $repo = $this->makeEmpty(ProjectRepository::class, [
-            'isUserAssignedToProject' => fn () => false,
+            'isUserMemberOfProject' => fn () => false,
             'addProjectRelation' => function ($userId, $projectId, $role) use (&$added) {
                 $added[] = [$userId, $projectId, $role];
             },
@@ -721,6 +721,31 @@ public function test_add_user_to_project_inserts_when_not_a_member(): void
         $this->assertSame([[7, 42, 'contributor']], $added);
     }
 
+    /**
+     * Membership is not access. isUserAssignedToProject() returns true for
+     * every admin and owner whether or not a relation row exists, so using
+     * it as the idempotence check would make this method a permanent
+     * no-op for exactly those users: an admin could never be put on a
+     * project team, and the caller would be told "already a member" about
+     * someone who is not on the team at all.
+     */
+    public function test_add_user_to_project_adds_admin_who_has_access_but_no_membership(): void
+    {
+        $added = [];
+        $repo = $this->makeEmpty(ProjectRepository::class, [
+            // An admin: reaches every project...
+            'isUserAssignedToProject' => fn () => true,
+            // ...but holds no relation row for this one.
+            'isUserMemberOfProject' => fn () => false,
+            'addProjectRelation' => function ($userId, $projectId, $role) use (&$added) {
+                $added[] = [$userId, $projectId, $role];
+            },
+        ]);
+
+        $this->assertTrue($this->makeService($repo)->addUserToProject(7, 42));
+        $this->assertSame([[7, 42, '']], $added, 'access must not be mistaken for membership');
+    }
+
     /**
      * Idempotence guard. zp_relationuserproject has no unique index on
      * (userId, projectId), so a blind insert would duplicate the row and
@@ -730,7 +755,7 @@ public function test_add_user_to_project_is_idempotent_for_existing_member(): vo
     {
         $addCalls = 0;
         $repo = $this->makeEmpty(ProjectRepository::class, [
-            'isUserAssignedToProject' => fn () => true,
+            'isUserMemberOfProject' => fn () => true,
             'addProjectRelation' => function () use (&$addCalls) {
                 $addCalls++;
             },
@@ -750,7 +775,7 @@ public function test_add_user_to_project_rejects_invalid_ids(): void
     {
         $touched = 0;
         $repo = $this->makeEmpty(ProjectRepository::class, [
-            'isUserAssignedToProject' => function () use (&$touched) {
+            'isUserMemberOfProject' => function () use (&$touched) {
                 $touched++;
 
                 return false;