Adjustments for listable rooms after review

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

15 files changed, 66 insertions, 42 deletions

diff --git a/docs/chat.md b/docs/chat.md
index 736a199f1..2bc58d5ab 100644
--- a/docs/chat.md
+++ b/docs/chat.md
@@ -160,9 +160,9 @@ Base endpoint is: `/ocs/v2.php/apps/spreed/api/v1`
 * `call_ended` - Call with {user1}, {user2}, {user3}, {user4} and {user5} (Duration 30:23)
 * `read_only_off` - {actor} unlocked the conversation
 * `read_only` - {actor} locked the conversation
-* `listable_participants` - {actor} made the conversation listable only for participants
+* `listable_none` - {actor} made the conversation listable for nobody
 * `listable_users` - {actor} made the conversation listable for regular users
-* `listable_all` - {actor} made the conversation listable for everone which includes users and guest users
+* `listable_all` - {actor} made the conversation listable for everone which includes users and guests
 * `lobby_timer_reached` - The conversation is now open to everyone
 * `lobby_none` - {actor} opened the conversation to everyone
 * `lobby_non_moderators` - {actor} restricted the conversation to moderators
diff --git a/docs/constants.md b/docs/constants.md
index 8a250c02c..dd4f0e02f 100644
--- a/docs/constants.md
+++ b/docs/constants.md
@@ -14,7 +14,7 @@ title: Constants
 
 ## Listable scope
 * `0` participants only
-* `1` regular users only, excluding guests
+* `1` regular users only, excluding guest app users
 * `2` everyone
 
 ## Participant types
diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php
index 48e9fa1f6..02e1f6b26 100644
--- a/lib/Chat/Parser/SystemMessage.php
+++ b/lib/Chat/Parser/SystemMessage.php
@@ -165,12 +165,12 @@ class SystemMessage {
 			} elseif ($cliIsActor) {
 				$parsedMessage = $this->l->t('An administrator locked the conversation');
 			}
-		} elseif ($message === 'listable_participants') {
-			$parsedMessage = $this->l->t('{actor} made the conversation listable for participants only');
+		} elseif ($message === 'listable_none') {
+			$parsedMessage = $this->l->t('{actor} made the conversation not listable');
 			if ($currentUserIsActor) {
-				$parsedMessage = $this->l->t('You made the conversation listable for participants only');
+				$parsedMessage = $this->l->t('You made the conversation not listable');
 			} elseif ($cliIsActor) {
-				$parsedMessage = $this->l->t('An administrator made the conversation listable for participants only');
+				$parsedMessage = $this->l->t('An administrator made the conversation not listable');
 			}
 		} elseif ($message === 'listable_users') {
 			$parsedMessage = $this->l->t('{actor} made the conversation listable for users only');
diff --git a/lib/Chat/SystemMessage/Listener.php b/lib/Chat/SystemMessage/Listener.php
index 3672aa25e..9642fcab8 100644
--- a/lib/Chat/SystemMessage/Listener.php
+++ b/lib/Chat/SystemMessage/Listener.php
@@ -179,15 +179,11 @@ class Listener {
 		$dispatcher->addListener(Room::EVENT_AFTER_LISTABLE_SET, static function (ModifyRoomEvent $event) {
 			$room = $event->getRoom();
 
-			if ($room->getType() === Room::CHANGELOG_CONVERSATION) {
-				return;
-			}
-
 			/** @var self $listener */
 			$listener = \OC::$server->query(self::class);
 
-			if ($event->getNewValue() === Room::LISTABLE_PARTICIPANTS) {
-				$listener->sendSystemMessage($room, 'listable_participants');
+			if ($event->getNewValue() === Room::LISTABLE_NONE) {
+				$listener->sendSystemMessage($room, 'listable_none');
 			} elseif ($event->getNewValue() === Room::LISTABLE_USERS) {
 				$listener->sendSystemMessage($room, 'listable_users');
 			} elseif ($event->getNewValue() === Room::LISTABLE_ALL) {
diff --git a/lib/Command/Room/Create.php b/lib/Command/Room/Create.php
index eaaae0396..f241fe6b8 100644
--- a/lib/Command/Room/Create.php
+++ b/lib/Command/Room/Create.php
@@ -110,7 +110,12 @@ class Create extends Base {
 			return 1;
 		}
 
-		if (!in_array($listable, [null, '0', '1', '2', '3'], true)) {
+		if (!in_array($listable, [
+			null,
+			(string)Room::LISTABLE_NONE,
+			(string)Room::LISTABLE_USERS,
+			(string)Room::LISTABLE_ALL,
+		], true)) {
 			$output->writeln('<error>Invalid value for option "--listable" given.</error>');
 			return 1;
 		}
@@ -167,6 +172,14 @@ class Create extends Base {
 			case 'owner':
 			case 'moderator':
 				return $this->completeParticipantValues($context);
+			case 'readonly':
+				return ['1', '0'];
+			case 'listable':
+				return [
+					(string)Room::LISTABLE_ALL,
+					(string)Room::LISTABLE_USERS,
+					(string)Room::LISTABLE_NONE,
+				];
 		}
 
 		return parent::completeOptionValues($optionName, $context);
diff --git a/lib/Command/Room/Update.php b/lib/Command/Room/Update.php
index af87053e5..037e7d59a 100644
--- a/lib/Command/Room/Update.php
+++ b/lib/Command/Room/Update.php
@@ -104,7 +104,12 @@ class Update extends Base {
 			return 1;
 		}
 
-		if (!in_array($listable, [null, '0', '1', '2', '3'], true)) {
+		if (!in_array($listable, [
+			null,
+			(string)Room::LISTABLE_NONE,
+			(string)Room::LISTABLE_USERS,
+			(string)Room::LISTABLE_ALL,
+		], true)) {
 			$output->writeln('<error>Invalid value for option "--listable" given.</error>');
 			return 1;
 		}
@@ -168,7 +173,11 @@ class Update extends Base {
 			case 'readonly':
 				return ['1', '0'];
 			case 'listable':
-				return ['2', '1', '0'];
+				return [
+					(string)Room::LISTABLE_ALL,
+					(string)Room::LISTABLE_USERS,
+					(string)Room::LISTABLE_NONE,
+				];
 
 			case 'owner':
 				return $this->completeParticipantValues($context);
diff --git a/lib/Manager.php b/lib/Manager.php
index e6efc5345..1005931c3 100644
--- a/lib/Manager.php
+++ b/lib/Manager.php
@@ -758,7 +758,7 @@ class Manager {
 		if ($row === false) {
 			$room = $this->createRoom(Room::CHANGELOG_CONVERSATION, $userId);
 			$room->setReadOnly(Room::READ_ONLY);
-			$room->setListable(Room::LISTABLE_PARTICIPANTS);
+			$room->setListable(Room::LISTABLE_NONE);
 
 			$this->participantService->addUsers($room,[[
 				'actorType' => Attendee::ACTOR_USERS,
@@ -866,7 +866,7 @@ class Manager {
 		}
 
 		// FIXME: also check guest user case
-		if ($room->getListable() === Room::LISTABLE_PARTICIPANTS) {
+		if ($room->getListable() === Room::LISTABLE_NONE) {
 			try {
 				if ($userId === '') {
 					$sessionId = $this->talkSession->getSessionForRoom($room->getToken());
diff --git a/lib/Migration/Version2100Date20201201102528.php b/lib/Migration/Version2100Date20201201102528.php
index b59fcb5d1..b3fd473c0 100644
--- a/lib/Migration/Version2100Date20201201102528.php
+++ b/lib/Migration/Version2100Date20201201102528.php
@@ -53,8 +53,10 @@ class Version2100Date20201201102528 extends SimpleMigrationStep {
 					'default' => 0,
 				]);
 			}
+
+			return $schema;
 		}
 
-		return $schema;
+		return null;
 	}
 }
diff --git a/lib/Room.php b/lib/Room.php
index 7a3cf2885..e371631c5 100644
--- a/lib/Room.php
+++ b/lib/Room.php
@@ -65,10 +65,10 @@ class Room {
 	/**
 	 * Only visible when joined
 	 */
-	public const LISTABLE_PARTICIPANTS = 0;
+	public const LISTABLE_NONE = 0;
 
 	/**
-	 * Searchable by all regular users and moderators, even when not joined, excluding guest users
+	 * Searchable by all regular users and moderators, even when not joined, excluding users from the guest app
 	 */
 	public const LISTABLE_USERS = 1;
 
@@ -818,11 +818,15 @@ class Room {
 			return true;
 		}
 
-		if (!in_array($this->getType(), [self::GROUP_CALL, self::PUBLIC_CALL, self::CHANGELOG_CONVERSATION], true)) {
+		if (!in_array($this->getType(), [self::GROUP_CALL, self::PUBLIC_CALL], true)) {
 			return false;
 		}
 
-		if ($newState < 0 || $newState > 3) {
+		if (!in_array($newState, [
+			Room::LISTABLE_NONE,
+			Room::LISTABLE_USERS,
+			Room::LISTABLE_ALL,
+		], true)) {
 			return false;
 		}
 
diff --git a/src/components/ConversationSettings/ModerationSettings.vue b/src/components/ConversationSettings/ModerationSettings.vue
index 596e67476..85a93f160 100644
--- a/src/components/ConversationSettings/ModerationSettings.vue
+++ b/src/components/ConversationSettings/ModerationSettings.vue
@@ -22,19 +22,19 @@
 <template>
 	<div>
 		<div class="app-settings-subsection">
-			<div id="moderation_settings_listable_users_hint" class="app-settings-section__hint">
+			<div id="moderation_settings_listable_conversation_hint" class="app-settings-section__hint">
 				{{ t('spreed', 'Defines who can find this conversation') }}
 			</div>
 			<div>
-				<label for="moderation_settings_listable_users_conversation_input">{{ t('spreed', 'Listable for') }}</label>
-				<Multiselect id="moderation_settings_listable_users_conversation_input"
+				<label for="moderation_settings_listable_conversation_input">{{ t('spreed', 'Listable for') }}</label>
+				<Multiselect id="moderation_settings_listable_conversation_input"
 					v-model="listable"
 					:options="listableOptions"
 					:placeholder="t('spreed', 'Listable for')"
 					label="label"
 					track-by="value"
 					:disabled="isListableLoading"
-					aria-describedby="moderation_settings_listable_users_conversation_hint"
+					aria-describedby="moderation_settings_listable_conversation_hint"
 					@input="saveListable" />
 			</div>
 		</div>
@@ -115,9 +115,9 @@ import DatetimePicker from '@nextcloud/vue/dist/Components/DatetimePicker'
 import SipSettings from './SipSettings'
 
 const listableOptions = [
-	{ value: 2, label: t('spreed', 'Everyone') },
-	{ value: 1, label: t('spreed', 'Regular users, without guests') },
-	{ value: 0, label: t('spreed', 'Participants only') },
+	{ value: CONVERSATION.LISTABLE.NONE, label: t('spreed', 'None') },
+	{ value: CONVERSATION.LISTABLE.USERS, label: t('spreed', 'Registered users') },
+	{ value: CONVERSATION.LISTABLE.ALL, label: t('spreed', 'Everyone') },
 ]
 
 export default {
diff --git a/src/constants.js b/src/constants.js
index 6c9b9be69..dfb298173 100644
--- a/src/constants.js
+++ b/src/constants.js
@@ -35,7 +35,7 @@ export const CONVERSATION = {
 		READ_ONLY: 1,
 	},
 	LISTABLE: {
-		PARTICIPANTS: 0,
+		NONE: 0,
 		USERS: 1,
 		ALL: 2,
 	},
diff --git a/src/store/conversationsStore.js b/src/store/conversationsStore.js
index cb8b7745c..00a15d4a3 100644
--- a/src/store/conversationsStore.js
+++ b/src/store/conversationsStore.js
@@ -44,7 +44,7 @@ const DUMMY_CONVERSATION = {
 	participantFlags: PARTICIPANT.CALL_FLAG.DISCONNECTED,
 	participantType: PARTICIPANT.TYPE.USER,
 	readOnly: CONVERSATION.STATE.READ_ONLY,
-	listable: CONVERSATION.LISTABLE.PARTICIPANTS,
+	listable: CONVERSATION.LISTABLE.NONE,
 	hasCall: false,
 	canStartCall: false,
 	lobbyState: WEBINAR.LOBBY.NONE,
diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php
index 6ad43e23c..923737e65 100644
--- a/tests/integration/features/bootstrap/FeatureContext.php
+++ b/tests/integration/features/bootstrap/FeatureContext.php
@@ -809,7 +809,7 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	public function userChangesListableScopeOfTheRoom($user, $identifier, $newState, $statusCode, $apiVersion = 'v3') {
 		$this->setCurrentUser($user);
 		if ($newState === 'joined') {
-			$newStateValue = Room::LISTABLE_PARTICIPANTS;
+			$newStateValue = Room::LISTABLE_NONE;
 		} elseif ($newState === 'users') {
 			$newStateValue = Room::LISTABLE_USERS;
 		} else {
diff --git a/tests/php/RoomTest.php b/tests/php/RoomTest.php
index 1b26775c2..5c710c4f4 100644
--- a/tests/php/RoomTest.php
+++ b/tests/php/RoomTest.php
@@ -63,7 +63,7 @@ class RoomTest extends TestCase {
 			1,
 			Room::PUBLIC_CALL,
 			Room::READ_WRITE,
-			Room::LISTABLE_PARTICIPANTS,
+			Room::LISTABLE_NONE,
 			Webinary::LOBBY_NONE,
 			0,
 			null,
diff --git a/tests/php/Signaling/BackendNotifierTest.php b/tests/php/Signaling/BackendNotifierTest.php
index 0468352b6..bfc6e3d6e 100644
--- a/tests/php/Signaling/BackendNotifierTest.php
+++ b/tests/php/Signaling/BackendNotifierTest.php
@@ -253,7 +253,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NONE,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_WRITE,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 				],
@@ -289,7 +289,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NONE,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_WRITE,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 				],
@@ -336,7 +336,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NONE,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_WRITE,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 				],
@@ -360,7 +360,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NONE,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_WRITE,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 				],
@@ -384,7 +384,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NONE,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_WRITE,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 				],
@@ -408,7 +408,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NONE,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_ONLY,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 				],
@@ -455,7 +455,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NON_MODERATORS,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_WRITE,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 				],
@@ -615,7 +615,7 @@ class BackendNotifierTest extends \Test\TestCase {
 					'lobby-state' => Webinary::LOBBY_NONE,
 					'lobby-timer' => null,
 					'read-only' => Room::READ_WRITE,
-					'listable' => Room::LISTABLE_PARTICIPANTS,
+					'listable' => Room::LISTABLE_NONE,
 					'active-since' => null,
 					'sip-enabled' => 0,
 					'foo' => 'bar',