Move join/leave project functionality to members page (#2898)

This PR finishes our recent effort to modernize the project membership
experience started in #2778. The
join/leave functionality has been moved to the project members page,
leaving `subscribeProject.php` as only a place to configure
notifications.

Author: williamjallen

app/GraphQL/Mutations/JoinProject.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\GraphQL\Mutations;
+
+use App\Models\Project;
+use App\Models\User;
+use Exception;
+
+final class JoinProject extends AbstractMutation
+{
+    /**
+     * @param array{
+     *     projectId: int,
+     * } $args
+     *
+     * @throws Exception
+     */
+    protected function mutate(array $args): void
+    {
+        /** @var ?User $user */
+        $user = auth()->user();
+        $project = isset($args['projectId']) ? Project::find((int) $args['projectId']) : null;
+
+        if (
+            $user === null
+            || $project === null
+            || $user->cannot('join', $project)
+        ) {
+            abort(401, 'This action is unauthorized.');
+        }
+
+        $project
+            ->users()
+            ->attach($user->id, [
+                'emailtype' => 0,
+                'emailcategory' => 62,
+                'emailsuccess' => false,
+                'emailmissingsites' => false,
+                'role' => Project::PROJECT_USER,
+            ]);
+    }
+}

app/GraphQL/Mutations/RemoveProjectUser.php

@@ -26,8 +26,14 @@ protected function mutate(array $args): void
             $user === null
             || $project === null
             || $userToRemove === null
-            || $userToRemove->id === $user->id
-            || $user->cannot('removeUser', $project)
+            || (
+                $userToRemove->id === $user->id
+                && $user->cannot('leave', $project)
+            )
+            || (
+                $userToRemove->id !== $user->id
+                && $user->cannot('removeUser', $project)
+            )
             || !$project->users()->where('id', $userToRemove->id)->exists()
         ) {
             abort(401, 'This action is unauthorized.');

app/Http/Controllers/ProjectMembersController.php

@@ -24,6 +24,8 @@ public function members(int $project_id): View
             'user-id' => $user?->id,
             'can-invite-users' => $user?->can('inviteUser', $eloquentProject) ?? false,
             'can-remove-users' => $user?->can('removeUser', $eloquentProject) ?? false,
+            'can-join-project' => $user?->can('join', $eloquentProject) ?? false,
+            'can-leave-project' => $user?->can('leave', $eloquentProject) ?? false,
         ]);
     }
 }

app/Http/Controllers/SubscribeProjectController.php

@@ -68,10 +68,7 @@ public function subscribeProject(): View|RedirectResponse
         }
 
         // If we ask to subscribe
-        @$Subscribe = $_POST['subscribe'];
         @$UpdateSubscription = $_POST['updatesubscription'];
-        @$Unsubscribe = $_POST['unsubscribe'];
-        @$Role = $_POST['role'];
         @$EmailType = $_POST['emailtype'];
         if (!isset($_POST['emailmissingsites'])) {
             $EmailMissingSites = 0;
@@ -90,26 +87,7 @@ public function subscribeProject(): View|RedirectResponse
         $LabelEmail->ProjectId = $this->project->Id;
         $LabelEmail->UserId = $user->id;
 
-        if ($Unsubscribe) {
-            DB::delete('DELETE FROM user2project WHERE userid=? AND projectid=?', [$user->id, $this->project->Id]);
-
-            // Remove the claim sites for this project if they are only part of this project
-            DB::delete('
-                DELETE FROM site2user
-                WHERE
-                    userid=?
-                    AND siteid NOT IN (
-                        SELECT build.siteid
-                        FROM build, user2project AS up
-                        WHERE
-                            up.projectid = build.projectid
-                            AND up.userid=?
-                            AND up.role>0
-                        GROUP BY build.siteid
-                    )
-            ', [$user->id, $user->id]);
-            return redirect('/user?note=unsubscribedtoproject');
-        } elseif ($UpdateSubscription) {
+        if ($UpdateSubscription) {
             $emailcategory_update = intval($_POST['emailcategory_update'] ?? 0);
             $emailcategory_configure = intval($_POST['emailcategory_configure'] ?? 0);
             $emailcategory_warning = intval($_POST['emailcategory_warning'] ?? 0);
@@ -122,7 +100,6 @@ public function subscribeProject(): View|RedirectResponse
                 $db->executePrepared('
                 UPDATE user2project
                 SET
-                    role=?,
                     emailtype=?,
                     emailcategory=?,
                     emailmissingsites=?,
@@ -131,32 +108,13 @@ public function subscribeProject(): View|RedirectResponse
                     userid=?
                     AND projectid=?
             ', [
-                    $Role,
                     $EmailType,
                     $EmailCategory,
                     $EmailMissingSites,
                     $EmailSuccess,
                     $user->id,
                     $this->project->Id,
                 ]);
-
-                if ($Role == 0) {
-                    // Remove the claim sites for this project if they are only part of this project
-                    DB::delete('
-                        DELETE FROM site2user
-                        WHERE
-                            userid=?
-                            AND siteid NOT IN (
-                                SELECT build.siteid
-                                FROM build, user2project AS up
-                                WHERE
-                                    up.projectid=build.projectid
-                                    AND up.userid=?
-                                    AND up.role>0
-                                GROUP BY build.siteid
-                            )
-                    ', [$user->id, $user->id]);
-                }
             }
 
             if (isset($_POST['emaillabels'])) {
@@ -166,77 +124,6 @@ public function subscribeProject(): View|RedirectResponse
             }
             // Redirect
             return redirect('/user');
-        } elseif ($Subscribe) {
-            $emailcategory_update = intval($_POST['emailcategory_update'] ?? 0);
-            $emailcategory_configure = intval($_POST['emailcategory_configure'] ?? 0);
-            $emailcategory_warning = intval($_POST['emailcategory_warning'] ?? 0);
-            $emailcategory_error = intval($_POST['emailcategory_error'] ?? 0);
-            $emailcategory_test = intval($_POST['emailcategory_test'] ?? 0);
-            $emailcategory_dynamicanalysis = intval($_POST['emailcategory_dynamicanalysis'] ?? 0);
-
-            $EmailCategory = $emailcategory_update + $emailcategory_configure + $emailcategory_warning + $emailcategory_error + $emailcategory_test + $emailcategory_dynamicanalysis;
-            if (!empty($user2project)) {
-                $db->executePrepared('
-                UPDATE user2project
-                SET
-                    role=?,
-                    emailtype=?,
-                    emailcategory=?.
-                    emailmissingsites=?,
-                    emailsuccess=?
-                WHERE
-                    userid=?
-                    AND projectid=?
-            ', [
-                    $Role,
-                    $EmailType,
-                    $EmailCategory,
-                    $EmailMissingSites,
-                    $EmailSuccess,
-                    $user->id,
-                    $this->project->Id,
-                ]);
-
-                if ($Role == 0) {
-                    // Remove the claim sites for this project if they are only part of this project
-                    DB::delete('
-                        DELETE FROM site2user
-                        WHERE
-                            userid=?
-                            AND siteid NOT IN (
-                                SELECT build.siteid
-                                FROM build, user2project AS up
-                                WHERE
-                                    up.projectid0=build.projectid
-                                    AND up.userid=?
-                                    AND up.role>0
-                                GROUP BY build.siteid
-                            )
-                    ', [$user->id, $user->id]);
-                }
-            } else {
-                $db->executePrepared('
-                INSERT INTO user2project (
-                    role,
-                    userid,
-                    projectid,
-                    emailtype,
-                    emailcategory,
-                    emailsuccess,
-                    emailmissingsites
-                )
-                VALUES (?, ?, ?, ?, ?, ?, ?)
-            ', [
-                    $Role,
-                    $user->id,
-                    $this->project->Id,
-                    $EmailType,
-                    $EmailCategory,
-                    $EmailSuccess,
-                    $EmailMissingSites,
-                ]);
-            }
-            return redirect('/user?note=subscribedtoproject');
         }
 
         $xml .= '<project>';

app/Policies/ProjectPolicy.php

@@ -98,8 +98,7 @@ public function inviteUser(User $currentUser, Project $project): bool
             return false;
         }
 
-        // If an LDAP filter has been specified and LDAP is enabled, CDash controls the entire members list.
-        if ((bool) config('cdash.ldap_enabled') && $project->ldapfilter !== null && $project->ldapfilter !== '') {
+        if ($this->isLdapControlledMembership($project)) {
             return false;
         }
 
@@ -111,13 +110,40 @@ public function revokeInvitation(User $currentUser, Project $project): bool
         return $this->update($currentUser, $project);
     }
 
+    /**
+     * Whether the current user can remove other users or not.  Use leave() to determine whether
+     * the current user can remove themselves from the project.
+     */
     public function removeUser(User $currentUser, Project $project): bool
     {
-        // If an LDAP filter has been specified and LDAP is enabled, CDash controls the entire members list.
-        if ((bool) config('cdash.ldap_enabled') && $project->ldapfilter !== null && $project->ldapfilter !== '') {
+        if ($this->isLdapControlledMembership($project)) {
             return false;
         }
 
         return $this->update($currentUser, $project);
     }
+
+    public function join(User $currentUser, Project $project): bool
+    {
+        if (!$this->view($currentUser, $project)) {
+            return false;
+        }
+
+        if ($this->isLdapControlledMembership($project)) {
+            return false;
+        }
+
+        return !$project->users()->where('id', $currentUser->id)->exists();
+    }
+
+    public function leave(User $currentUser, Project $project): bool
+    {
+        return !$this->isLdapControlledMembership($project) && $project->users()->where('id', $currentUser->id)->exists();
+    }
+
+    private function isLdapControlledMembership(Project $project): bool
+    {
+        // If a LDAP filter has been specified and LDAP is enabled, CDash controls the entire members list.
+        return (bool) config('cdash.ldap_enabled') && $project->ldapfilter !== null && $project->ldapfilter !== '';
+    }
 }

app/cdash/public/subscribeProject.xsl

@@ -47,68 +47,13 @@
 <form name="form1" enctype="multipart/form-data" method="post" action="">
   <div id="wizard">
       <ul>
-          <li>
-            <a class="cdash-link" href="#fragment-1"><span>Select your role in this project</span></a></li>
           <li>
             <a class="cdash-link" href="#fragment-3"><span>Email Notifications</span></a></li>
           <li>
             <a class="cdash-link" href="#fragment-4"><span>Email Category</span></a></li>
           <li>
             <a class="cdash-link" href="#fragment-5"><span>Email Labels</span></a></li>
       </ul>
-    <div id="fragment-1" class="tab_content" >
-      <div class="tab_help"></div>
-        <table width="800" >
-          <tr>
-            <td></td>
-            <td><input type="radio" onchange="saveChanges();" name="role" value="0" checked="checked">
-            <xsl:if test="/cdash/role=0">
-            <xsl:attribute name="checked"></xsl:attribute>
-            </xsl:if>
-            </input>
-             Normal user <i>(you are working on or using this project)</i></td>
-          </tr>
-           <tr>
-            <td></td>
-            <td><input type="radio" onchange="saveChanges();" name="role" value="1">
-             <xsl:if test="/cdash/role=1">
-            <xsl:attribute name="checked"></xsl:attribute>
-            </xsl:if>
-            </input>
-             Site maintainer <i>(you are responsible for machines that are submitting builds for this project)</i></td>
-          </tr>
-          <xsl:if test="/cdash/role>1">
-           <tr>
-            <td></td>
-            <td ><b>Warning: if you change to a normal or maintainer role you won't be able to go back.</b> </td>
-            </tr>
-          <tr>
-            <td></td>
-            <td ><input type="radio" onchange="saveChanges();" name="role" value="2" checked="checked">
-            <xsl:if test="/cdash/role=2">
-            <xsl:attribute name="checked"></xsl:attribute>
-            </xsl:if>
-            </input>
-             Project Administrator <i>(You are administering the project)</i></td>
-          </tr>
-          </xsl:if>
-          <xsl:if test="/cdash/role>2">
-           <tr>
-            <td></td>
-            <td ><input type="radio" onchange="saveChanges();" name="role" value="3">
-             <xsl:if test="/cdash/role=3">
-            <xsl:attribute name="checked"></xsl:attribute>
-            </xsl:if>
-            </input>
-              Project Super Administrator<i>(You have full control of this project)</i></td>
-          </tr>
-          </xsl:if>
-           <tr>
-            <td></td>
-            <td bgcolor="#FFFFFF"></td>
-          </tr>
-        </table>
-    </div>
     <div id="fragment-3" class="tab_content" >
       <div class="tab_help"></div>
         <table width="800" >
@@ -312,18 +257,12 @@
   <div style="width:900px;margin-left:auto;margin-right:auto;text-align:right;">
   <table width="100%" border="0">
   <tr>
-    <td style="text-align:left;" ><input type="submit" onclick="return confirm('Are you sure you want to unsubscribe?')" name="unsubscribe" value="Unsubscribe"/></td>
     <td><span id="changesmade" style="color:red;display:none;">*Changes need to be updated </span>
     <input type="submit" onclick="SubmitForm()" name="updatesubscription" value="Update Subscription"/></td>
    </tr>
   </table>
   </div>
   </xsl:if>
-  <xsl:if test="/cdash/edit=0">
-   <div style="width:900px;margin-left:auto;margin-right:auto;text-align:right;"><br/>
-  <input type="submit" name="subscribe" value="Subscribe"/>
-  </div>
-</xsl:if>
 
 </form>
 </td>

app/cdash/tests/CMakeLists.txt

@@ -302,6 +302,8 @@ add_feature_test(/Feature/Traits/UpdatesSiteInformationTest)
 
 add_feature_test(/Feature/GraphQL/Mutations/ChangeGlobalRoleTest)
 
+add_feature_test(/Feature/GraphQL/Mutations/JoinProjectTest)
+
 # Needs RUN_SERIAL to verify that no unexpected invitations are created
 add_feature_test(/Feature/GraphQL/Mutations/CreateGlobalInvitationTest)
 set_property(TEST /Feature/GraphQL/Mutations/CreateGlobalInvitationTest APPEND PROPERTY RUN_SERIAL TRUE)

app/cdash/tests/test_email.php

@@ -43,11 +43,14 @@ public function testRegisterUser()
             return;
         }
 
-        $this->actingAs(['email' => 'user1@kw', 'password' => 'user1'])
-            ->connect($this->url . "/subscribeProject.php?projectid={$this->project}");
-        $this->setField('credentials[0]', 'user1kw');
-        $this->setField('emailsuccess', '1');
-        $this->clickSubmitByName('subscribe');
+        $user->projects()->attach($this->project, [
+            'role' => 0,
+            'emailtype' => 1,
+            'emailcategory' => 126,
+            'emailsuccess' => 1,
+            'emailmissingsites' => 0,
+        ]);
+
         if (!$this->checkLog($this->logfilename)) {
             $this->fail('Errors logged');
         }