Rename viewDiscussions => viewForum, viewUserList => searchUsers (#2854)

This naming is clearer as to the intended effect. Changes include:

- A migration to rename all permissions
- Updating the seed migration to use the original naming from the start
- Replacing usage of the old names with new names in code
- Throwing warnings when the old names are used.

js/src/admin/AdminApplication.js

@@ -58,7 +58,7 @@ export default class AdminApplication extends Application {
     const required = [];
 
     if (permission === 'startDiscussion' || permission.indexOf('discussion.') === 0) {
-      required.push('viewDiscussions');
+      required.push('viewForum');
     }
     if (permission === 'discussion.delete') {
       required.push('discussion.hide');

js/src/admin/components/PermissionGrid.js

@@ -100,11 +100,11 @@ export default class PermissionGrid extends Component {
     const items = new ItemList();
 
     items.add(
-      'viewDiscussions',
+      'viewForum',
       {
         icon: 'fas fa-eye',
-        label: app.translator.trans('core.admin.permissions.view_discussions_label'),
-        permission: 'viewDiscussions',
+        label: app.translator.trans('core.admin.permissions.view_forum_label'),
+        permission: 'viewForum',
         allowGuest: true,
       },
       100
@@ -121,11 +121,11 @@ export default class PermissionGrid extends Component {
     );
 
     items.add(
-      'viewUserList',
+      'searchUsers',
       {
         icon: 'fas fa-users',
-        label: app.translator.trans('core.admin.permissions.view_user_list_label'),
-        permission: 'viewUserList',
+        label: app.translator.trans('core.admin.permissions.search_users_label'),
+        permission: 'searchUsers',
         allowGuest: true,
       },
       100

locale/core.yml

@@ -189,14 +189,14 @@ core:
       read_heading: Read
       rename_discussions_label: Rename discussions
       reply_to_discussions_label: Reply to discussions
+      search_users_label: Search users
       sign_up_label: Sign up
       start_discussions_label: Start discussions
       title: Permissions
-      view_discussions_label: View discussions
+      view_forum_label: View forum (discussions and users)
       view_hidden_groups_label: View hidden group badges
       view_last_seen_at_label: Always view user last seen time
       view_post_ips_label: View post IP addresses
-      view_user_list_label: View user list
 
     # These translations are used in the dropdown menus on the Permissions page.
     permissions_controls:

migrations/2018_07_21_000100_seed_default_group_permissions.php

@@ -12,12 +12,12 @@ use Illuminate\Database\Schema\Builder;
 
 $rows = [
     // Guests can view the forum
-    ['permission' => 'viewDiscussions', 'group_id' => Group::GUEST_ID],
+    ['permission' => 'viewForum', 'group_id' => Group::GUEST_ID],
 
-    // Members can create and reply to discussions, and view the user list
+    // Members can create and reply to discussions, and search users
     ['permission' => 'startDiscussion', 'group_id' => Group::MEMBER_ID],
     ['permission' => 'discussion.reply', 'group_id' => Group::MEMBER_ID],
-    ['permission' => 'viewUserList', 'group_id' => Group::MEMBER_ID],
+    ['permission' => 'searchUsers', 'group_id' => Group::MEMBER_ID],
 
     // Moderators can edit + delete stuff
     ['permission' => 'discussion.hide', 'group_id' => Group::MODERATOR_ID],

migrations/2021_05_10_000000_rename_permissions.php

@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+use Illuminate\Database\Schema\Builder;
+
+return [
+    'up' => function (Builder $schema) {
+        $db = $schema->getConnection();
+
+        $db->table('group_permission')
+            ->where('permission', 'LIKE', 'viewDiscussions')
+            ->update(['permission' => $db->raw("REPLACE(permission,  'viewDiscussions', 'viewForum')")]);
+
+        $db->table('group_permission')
+            ->where('permission', 'LIKE', 'viewUserList')
+            ->update(['permission' => $db->raw("REPLACE(permission,  'viewUserList', 'searchUsers')")]);
+    },
+
+    'down' => function (Builder $schema) {
+        $db = $schema->getConnection();
+
+        $db->table('group_permission')
+            ->where('permission', 'LIKE', 'viewForum')
+            ->update(['permission' => $db->raw("REPLACE(permission,  'viewForum', 'viewDiscussions')")]);
+
+        $db->table('group_permission')
+            ->where('permission', 'LIKE', 'searchUsers')
+            ->update(['permission' => $db->raw("REPLACE(permission,  'searchUsers', 'viewUserList')")]);
+    }
+];

src/Api/Controller/ListUsersController.php

@@ -75,7 +75,7 @@ class ListUsersController extends AbstractListController
     {
         $actor = RequestUtil::getActor($request);
 
-        $actor->assertCan('viewUserList');
+        $actor->assertCan('searchUsers');
 
         if (! $actor->hasPermission('user.viewLastSeenAt')) {
             // If a user cannot see everyone's last online date, we prevent them from sorting by it

src/Api/Serializer/ForumSerializer.php

@@ -88,9 +88,9 @@ class ForumSerializer extends AbstractSerializer
             'footerHtml' => $this->settings->get('custom_footer'),
             'allowSignUp' => (bool) $this->settings->get('allow_sign_up'),
             'defaultRoute'  => $this->settings->get('default_route'),
-            'canViewDiscussions' => $this->actor->can('viewDiscussions'),
+            'canViewForum' => $this->actor->can('viewForum'),
             'canStartDiscussion' => $this->actor->can('startDiscussion'),
-            'canViewUserList' => $this->actor->can('viewUserList')
+            'canSearchUsers' => $this->actor->can('searchUsers')
         ];
 
         if ($this->actor->can('administrate')) {

src/Discussion/Access/ScopeDiscussionVisibility.php

@@ -20,7 +20,7 @@ class ScopeDiscussionVisibility
      */
     public function __invoke(User $actor, $query)
     {
-        if ($actor->cannot('viewDiscussions')) {
+        if ($actor->cannot('viewForum')) {
             $query->whereRaw('FALSE');
 
             return;

src/User/Access/ScopeUserVisibility.php

@@ -20,7 +20,7 @@ class ScopeUserVisibility
      */
     public function __invoke(User $actor, $query)
     {
-        if ($actor->cannot('viewDiscussions')) {
+        if ($actor->cannot('viewForum')) {
             if ($actor->isGuest()) {
                 $query->whereRaw('FALSE');
             } else {

src/User/User.php

@@ -404,6 +404,15 @@ class User extends AbstractModel
         return false;
     }
 
+    private function checkForDeprecatedPermissions($permission)
+    {
+        foreach (['viewDiscussions', 'viewUserList'] as $deprecated) {
+            if (strpos($permission, $deprecated) !== false) {
+                trigger_error('The `viewDiscussions` and `viewUserList` permissions have been renamed to `viewForum` and `searchUsers` respectively. Please use those instead.', E_USER_DEPRECATED);
+            }
+        }
+    }
+
     /**
      * Get the notification types that should be alerted to this user, according
      * to their preferences.

tests/integration/api/access_tokens/RemembererTest.php

@@ -50,7 +50,7 @@ class RemembererTest extends TestCase
         Carbon::setTestNow();
 
         $data = json_decode($response->getBody(), true);
-        $this->assertFalse($data['data']['attributes']['canViewUserList']);
+        $this->assertFalse($data['data']['attributes']['canSearchUsers']);
     }
 
     /**
@@ -71,7 +71,7 @@ class RemembererTest extends TestCase
         Carbon::setTestNow();
 
         $data = json_decode($response->getBody(), true);
-        $this->assertFalse($data['data']['attributes']['canViewUserList']);
+        $this->assertFalse($data['data']['attributes']['canSearchUsers']);
     }
 
     /**
@@ -92,6 +92,6 @@ class RemembererTest extends TestCase
         Carbon::setTestNow();
 
         $data = json_decode($response->getBody(), true);
-        $this->assertTrue($data['data']['attributes']['canViewUserList']);
+        $this->assertTrue($data['data']['attributes']['canSearchUsers']);
     }
 }

tests/integration/api/authentication/WithApiKeyTest.php

@@ -46,7 +46,7 @@ class WithApiKeyTest extends TestCase
         );
 
         $data = json_decode($response->getBody()->getContents(), true);
-        $this->assertFalse($data['data']['attributes']['canViewUserList']);
+        $this->assertFalse($data['data']['attributes']['canSearchUsers']);
     }
 
     /**
@@ -60,7 +60,7 @@ class WithApiKeyTest extends TestCase
         );
 
         $data = json_decode($response->getBody()->getContents(), true);
-        $this->assertTrue($data['data']['attributes']['canViewUserList']);
+        $this->assertTrue($data['data']['attributes']['canSearchUsers']);
         $this->assertArrayHasKey('adminUrl', $data['data']['attributes']);
 
         $key = ApiKey::where('key', 'mastertoken')->first();
@@ -79,7 +79,7 @@ class WithApiKeyTest extends TestCase
         );
 
         $data = json_decode($response->getBody()->getContents(), true);
-        $this->assertTrue($data['data']['attributes']['canViewUserList']);
+        $this->assertTrue($data['data']['attributes']['canSearchUsers']);
         $this->assertArrayNotHasKey('adminUrl', $data['data']['attributes']);
 
         $key = ApiKey::where('key', 'personaltoken')->first();
@@ -98,7 +98,7 @@ class WithApiKeyTest extends TestCase
         );
 
         $data = json_decode($response->getBody()->getContents(), true);
-        $this->assertTrue($data['data']['attributes']['canViewUserList']);
+        $this->assertTrue($data['data']['attributes']['canSearchUsers']);
         $this->assertArrayNotHasKey('adminUrl', $data['data']['attributes']);
 
         $key = ApiKey::where('key', 'personaltoken')->first();

tests/integration/api/posts/ListTest.php

@@ -42,7 +42,7 @@ class ListTests extends TestCase
 
     private function forbidGuestsFromSeeingForum()
     {
-        $this->database()->table('group_permission')->where('permission', 'viewDiscussions')->where('group_id', 2)->delete();
+        $this->database()->table('group_permission')->where('permission', 'viewForum')->where('group_id', 2)->delete();
     }
 
     /**

tests/integration/api/users/GroupSearchTest.php

@@ -54,7 +54,7 @@ class GroupSearchTest extends TestCase
     {
         $this->prepareDatabase([
             'group_permission' => [
-                ['permission' => 'viewUserList', 'group_id' => 2],
+                ['permission' => 'searchUsers', 'group_id' => 2],
             ],
         ]);
         $response = $this->createRequest(['admin'], 2);
@@ -69,7 +69,7 @@ class GroupSearchTest extends TestCase
     {
         $this->prepareDatabase([
             'group_permission' => [
-                ['permission' => 'viewUserList', 'group_id' => 2],
+                ['permission' => 'searchUsers', 'group_id' => 2],
             ],
         ]);
 
@@ -120,7 +120,7 @@ class GroupSearchTest extends TestCase
     {
         $this->prepareDatabase([
             'group_permission' => [
-                ['permission' => 'viewUserList', 'group_id' => 2],
+                ['permission' => 'searchUsers', 'group_id' => 2],
             ],
         ]);
 
@@ -139,7 +139,7 @@ class GroupSearchTest extends TestCase
     {
         $this->prepareDatabase([
             'group_permission' => [
-                ['permission' => 'viewUserList', 'group_id' => 2],
+                ['permission' => 'searchUsers', 'group_id' => 2],
             ],
         ]);
         $this->createMultipleUsersAndGroups();

tests/integration/api/users/ListTest.php

@@ -50,7 +50,7 @@ class ListTest extends TestCase
     {
         $this->prepareDatabase([
             'group_permission' => [
-                ['permission' => 'viewUserList', 'group_id' => 2],
+                ['permission' => 'searchUsers', 'group_id' => 2],
             ],
         ]);
 
@@ -98,7 +98,7 @@ class ListTest extends TestCase
     {
         $this->prepareDatabase([
             'group_permission' => [
-                ['permission' => 'viewUserList', 'group_id' => 2],
+                ['permission' => 'searchUsers', 'group_id' => 2],
                 ['permission' => 'user.viewLastSeenAt', 'group_id' => 2],
             ],
         ]);
@@ -120,7 +120,7 @@ class ListTest extends TestCase
     {
         $this->prepareDatabase([
             'group_permission' => [
-                ['permission' => 'viewUserList', 'group_id' => 2],
+                ['permission' => 'searchUsers', 'group_id' => 2],
             ],
         ]);
 

tests/integration/api/users/ShowTest.php

@@ -32,12 +32,12 @@ class ShowTest extends TestCase
 
     private function forbidGuestsFromSeeingForum()
     {
-        $this->database()->table('group_permission')->where('permission', 'viewDiscussions')->where('group_id', 2)->delete();
+        $this->database()->table('group_permission')->where('permission', 'viewForum')->where('group_id', 2)->delete();
     }
 
     private function forbidMembersFromSearchingUsers()
     {
-        $this->database()->table('group_permission')->where('permission', 'viewUserList')->where('group_id', 3)->delete();
+        $this->database()->table('group_permission')->where('permission', 'searchUsers')->where('group_id', 3)->delete();
     }
 
     /**

tests/integration/extenders/UserTest.php

@@ -82,7 +82,7 @@ class UserTest extends TestCase
 
         $user = User::find(2);
 
-        $this->assertContains('viewUserList', $user->getPermissions());
+        $this->assertContains('searchUsers', $user->getPermissions());
     }
 
     /**
@@ -100,7 +100,7 @@ class UserTest extends TestCase
 
         $user = User::find(2);
 
-        $this->assertNotContains('viewUserList', $user->getPermissions());
+        $this->assertNotContains('searchUsers', $user->getPermissions());
     }
 
     /**
@@ -114,7 +114,7 @@ class UserTest extends TestCase
 
         $user = User::find(2);
 
-        $this->assertNotContains('viewUserList', $user->getPermissions());
+        $this->assertNotContains('searchUsers', $user->getPermissions());
     }
 
     /**