From d64750b3eb7de28d82727105b69c213cae5befe7 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Tue, 11 May 2021 15:15:27 -0400 Subject: [PATCH] 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 | 2 +- js/src/admin/components/PermissionGrid.js | 12 +++---- locale/core.yml | 4 +-- ..._000100_seed_default_group_permissions.php | 6 ++-- .../2021_05_10_000000_rename_permissions.php | 36 +++++++++++++++++++ src/Api/Controller/ListUsersController.php | 2 +- src/Api/Serializer/ForumSerializer.php | 4 +-- .../Access/ScopeDiscussionVisibility.php | 2 +- src/User/Access/ScopeUserVisibility.php | 2 +- src/User/User.php | 9 +++++ .../api/access_tokens/RemembererTest.php | 6 ++-- .../api/authentication/WithApiKeyTest.php | 8 ++--- tests/integration/api/posts/ListTest.php | 2 +- .../integration/api/users/GroupSearchTest.php | 8 ++--- tests/integration/api/users/ListTest.php | 6 ++-- tests/integration/api/users/ShowTest.php | 4 +-- tests/integration/extenders/UserTest.php | 6 ++-- 17 files changed, 82 insertions(+), 37 deletions(-) create mode 100644 migrations/2021_05_10_000000_rename_permissions.php diff --git a/js/src/admin/AdminApplication.js b/js/src/admin/AdminApplication.js index 0aee7b0bf..499d1ea03 100644 --- a/js/src/admin/AdminApplication.js +++ b/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'); diff --git a/js/src/admin/components/PermissionGrid.js b/js/src/admin/components/PermissionGrid.js index 28e54d550..5391f309e 100644 --- a/js/src/admin/components/PermissionGrid.js +++ b/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 diff --git a/locale/core.yml b/locale/core.yml index 4185c84fd..cc6c527a0 100644 --- a/locale/core.yml +++ b/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: diff --git a/migrations/2018_07_21_000100_seed_default_group_permissions.php b/migrations/2018_07_21_000100_seed_default_group_permissions.php index a774a3414..ffba5974b 100644 --- a/migrations/2018_07_21_000100_seed_default_group_permissions.php +++ b/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], diff --git a/migrations/2021_05_10_000000_rename_permissions.php b/migrations/2021_05_10_000000_rename_permissions.php new file mode 100644 index 000000000..4cf906ff9 --- /dev/null +++ b/migrations/2021_05_10_000000_rename_permissions.php @@ -0,0 +1,36 @@ + 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')")]); + } +]; diff --git a/src/Api/Controller/ListUsersController.php b/src/Api/Controller/ListUsersController.php index a4c6cd795..1ff8b0274 100644 --- a/src/Api/Controller/ListUsersController.php +++ b/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 diff --git a/src/Api/Serializer/ForumSerializer.php b/src/Api/Serializer/ForumSerializer.php index e0681c60d..a0124fd52 100644 --- a/src/Api/Serializer/ForumSerializer.php +++ b/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')) { diff --git a/src/Discussion/Access/ScopeDiscussionVisibility.php b/src/Discussion/Access/ScopeDiscussionVisibility.php index ca92cf931..9875eb63d 100644 --- a/src/Discussion/Access/ScopeDiscussionVisibility.php +++ b/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; diff --git a/src/User/Access/ScopeUserVisibility.php b/src/User/Access/ScopeUserVisibility.php index 70e3fbe2e..4fb1ef929 100644 --- a/src/User/Access/ScopeUserVisibility.php +++ b/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 { diff --git a/src/User/User.php b/src/User/User.php index 8cc38e1ba..6420a5749 100644 --- a/src/User/User.php +++ b/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. diff --git a/tests/integration/api/access_tokens/RemembererTest.php b/tests/integration/api/access_tokens/RemembererTest.php index 70bf89f71..8d6a5b6be 100644 --- a/tests/integration/api/access_tokens/RemembererTest.php +++ b/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']); } } diff --git a/tests/integration/api/authentication/WithApiKeyTest.php b/tests/integration/api/authentication/WithApiKeyTest.php index 464697e50..1e6986b94 100644 --- a/tests/integration/api/authentication/WithApiKeyTest.php +++ b/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(); diff --git a/tests/integration/api/posts/ListTest.php b/tests/integration/api/posts/ListTest.php index 9986cef0a..6cc4cd97a 100644 --- a/tests/integration/api/posts/ListTest.php +++ b/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(); } /** diff --git a/tests/integration/api/users/GroupSearchTest.php b/tests/integration/api/users/GroupSearchTest.php index 489b50306..5c244595a 100644 --- a/tests/integration/api/users/GroupSearchTest.php +++ b/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(); diff --git a/tests/integration/api/users/ListTest.php b/tests/integration/api/users/ListTest.php index d91760b55..86c458881 100644 --- a/tests/integration/api/users/ListTest.php +++ b/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], ], ]); diff --git a/tests/integration/api/users/ShowTest.php b/tests/integration/api/users/ShowTest.php index 8e6cbb215..48bb19e92 100644 --- a/tests/integration/api/users/ShowTest.php +++ b/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(); } /** diff --git a/tests/integration/extenders/UserTest.php b/tests/integration/extenders/UserTest.php index ef37ec53b..4727c5c42 100644 --- a/tests/integration/extenders/UserTest.php +++ b/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()); } /**