feat: Integration tests for banning/unbanning users

Signed-off-by: skalidindi53 <s.teja2004@gmail.com>

6 files changed, 118 insertions, 36 deletions

diff --git a/lib/Migration/Version20000Date20240621150333.php b/lib/Migration/Version20000Date20240621150333.php
index 70bf2a931..ba7addbef 100644
--- a/lib/Migration/Version20000Date20240621150333.php
+++ b/lib/Migration/Version20000Date20240621150333.php
@@ -32,11 +32,11 @@ class Version20000Date20240621150333 extends SimpleMigrationStep {
 				'notnull' => true,
 				'length' => 20,
 			]);
-			$table->addColumn('actor_id', Types::STRING, [
+			$table->addColumn('actor_type', Types::STRING, [
 				'notnull' => true,
 				'length' => 64,
 			]);
-			$table->addColumn('actor_type', Types::STRING, [
+			$table->addColumn('actor_id', Types::STRING, [
 				'notnull' => true,
 				'length' => 64,
 			]);
@@ -44,11 +44,11 @@ class Version20000Date20240621150333 extends SimpleMigrationStep {
 				'notnull' => true,
 				'unsigned' => true,
 			]);
-			$table->addColumn('banned_id', Types::STRING, [
+			$table->addColumn('banned_type', Types::STRING, [
 				'length' => 64,
 				'notnull' => true,
 			]);
-			$table->addColumn('banned_type', Types::STRING, [
+			$table->addColumn('banned_id', Types::STRING, [
 				'length' => 64,
 				'notnull' => true,
 			]);
@@ -60,8 +60,8 @@ class Version20000Date20240621150333 extends SimpleMigrationStep {
 			]);
 
 			$table->setPrimaryKey(['id']);
-			// $table->addUniqueIndex(['user_id', 'room_id'], 'talk_ban_user_room'); //A user should not be banned from the same room more than once
-			// $table->addIndex(['banned_at'], 'talk_ban_banned_at');
+			$table->addUniqueIndex(['banned_type', 'banned_id', 'room_id'], 'talk_bans_unique_actor_room'); //A user should not be banned from the same room more than once
+			$table->addIndex(['room_id']);
 			return $schema;
 		}
 
diff --git a/lib/Model/Ban.php b/lib/Model/Ban.php
index 6e9caee2b..455aee353 100644
--- a/lib/Model/Ban.php
+++ b/lib/Model/Ban.php
@@ -13,37 +13,37 @@ use OCP\AppFramework\Db\Entity;
 /**
  * @method void setId(int $id)
  * @method int getId()
- * @method void setActorId(string $actorId)
- * @method string getActorId()
  * @method void setActorType(string $actorType)
  * @method string getActorType()
+ * @method void setActorId(string $actorId)
+ * @method string getActorId()
  * @method void setRoomId(int $roomId)
  * @method int getRoomId()
- * @method void setBannedId(string $bannedId)
- * @method string getBannedId()
  * @method void setBannedType(string $bannedType)
  * @method string getBannedType()
+ * @method void setBannedId(string $bannedId)
+ * @method string getBannedId()
  * @method void setBannedTime(\DateTime $bannedTime)
  * @method \DateTime getBannedTime()
  * @method void setInternalNote(string $internalNote)
  * @method string getInternalNote()
  */
 class Ban extends Entity implements \JsonSerializable {
-	protected string $actorId = '';
 	protected string $actorType = '';
+	protected string $actorId = '';
 	protected int $roomId = 0;
-	protected string $bannedId = '';
 	protected string $bannedType = '';
+	protected string $bannedId = '';
 	protected ?\DateTime $bannedTime = null;
 	protected ?string $internalNote = null;
 
 	public function __construct() {
 		$this->addType('id', 'int');
-		$this->addType('actorId', 'string');
 		$this->addType('actorType', 'string');
+		$this->addType('actorId', 'string');
 		$this->addType('roomId', 'int');
-		$this->addType('bannedId', 'string');
 		$this->addType('bannedType', 'string');
+		$this->addType('bannedId', 'string');
 		$this->addType('bannedTime', 'datetime');
 		$this->addType('internalNote', 'string');
 	}
@@ -51,11 +51,11 @@ class Ban extends Entity implements \JsonSerializable {
 	public function jsonSerialize(): array {
 		return [
 			'id' => $this->getId(),
-			'actorId' => $this->getActorId(),
 			'actorType' => $this->getActorType(),
+			'actorId' => $this->getActorId(),
 			'roomId' => $this->getRoomId(),
-			'bannedId' => $this->getBannedId(),
 			'bannedType' => $this->getBannedType(),
+			'bannedId' => $this->getBannedId(),
 			'bannedTime' => $this->getBannedTime() ? $this->getBannedTime()->getTimestamp() : null,
 			'internalNote' => $this->getInternalNote(),
 		];
diff --git a/lib/Model/BanMapper.php b/lib/Model/BanMapper.php
index 2de2d9b94..c3ba2b0df 100644
--- a/lib/Model/BanMapper.php
+++ b/lib/Model/BanMapper.php
@@ -10,7 +10,6 @@ namespace OCA\Talk\Model;
 
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\QBMapper;
-use OCP\AppFramework\Db\TTransactional;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 
@@ -21,7 +20,6 @@ use OCP\IDBConnection;
  * @template-extends QBMapper<Ban>
  */
 class BanMapper extends QBMapper {
-	use TTransactional;
 
 	public function __construct(IDBConnection $db) {
 		parent::__construct($db, 'talk_bans', Ban::class);
@@ -45,11 +43,15 @@ class BanMapper extends QBMapper {
 		$query = $this->db->getQueryBuilder();
 		$query->select('*')
 			->from($this->getTableName())
-			->where($query->expr()->eq('room_id', $query->createNamedParameter($roomId, IQueryBuilder::PARAM_INT)));
+			->where($query->expr()->eq('room_id', $query->createNamedParameter($roomId, IQueryBuilder::PARAM_INT)))
+			->orderBy('id', 'ASC');
 
 		return $this->findEntities($query);
 	}
 
+	/**
+	 * @throws DoesNotExistException
+	 */
 	public function findByBanId(int $banId): Ban {
 		$query = $this->db->getQueryBuilder();
 		$query->select('*')
diff --git a/lib/Service/BanService.php b/lib/Service/BanService.php
index 4dd4cb41d..3b8210228 100644
--- a/lib/Service/BanService.php
+++ b/lib/Service/BanService.php
@@ -15,7 +15,7 @@ use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Utility\ITimeFactory;
 
 class BanService {
-	
+
 	public function __construct(
 		protected BanMapper $banMapper,
 		protected ITimeFactory $timeFactory,
@@ -26,12 +26,20 @@ class BanService {
 	 * Validate the ban data.
 	 */
 	private function validateBanData(string $actorId, string $actorType, int $roomId, string $bannedId, string $bannedType, ?DateTime $bannedTime, ?string $internalNote): void {
-		if (empty($actorId) || empty($actorType) || empty($roomId) || empty($bannedId) || empty($bannedType)) {
-			throw new \InvalidArgumentException("Invalid ban data provided.");
+		if (empty($bannedId)) {
+			throw new \InvalidArgumentException("invalid_bannedId.");
+		}
+
+		if (empty($bannedType)) {
+			throw new \InvalidArgumentException("invalid_bannedType.");
 		}
 
+		if (empty($internalNote)) {
+			throw new \InvalidArgumentException("invalid_internalNote.");
+		}
+		
 		if ($bannedTime !== null && !$bannedTime instanceof DateTime) {
-			throw new \InvalidArgumentException("Invalid date format for bannedTime.");
+			throw new \InvalidArgumentException("invalid_bannedTime.");
 		}
 	}
 
diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php
index 7ab2dc83a..90abcbead 100644
--- a/tests/integration/features/bootstrap/FeatureContext.php
+++ b/tests/integration/features/bootstrap/FeatureContext.php
@@ -1217,6 +1217,15 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	 */
 	public function userJoinsRoomWithNamedSession(string $user, string $identifier, int $statusCode, string $apiVersion, string $sessionName, ?TableNode $formData = null): void {
 		$this->setCurrentUser($user, $identifier);
+
+		$userBanId = self::$userToBanId[self::$identifierToToken[$identifier]]['users'][$user] ?? null;
+		$guestBanId = self::$userToBanId[self::$identifierToToken[$identifier]]['guests'][$user] ?? null;
+
+		if ($userBanId !== null || $guestBanId !== null) {
+			//User is banned
+			return;
+		}
+
 		$this->sendRequest(
 			'POST', '/apps/spreed/api/' . $apiVersion . '/room/' . self::$identifierToToken[$identifier] . '/participants/active',
 			$formData
@@ -1478,6 +1487,8 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 			}
 		}
 
+		$actorType .= 's';
+
 		$this->setCurrentUser($user);
 		$body = [
 			'actorType' => $actorType,
@@ -1519,11 +1530,11 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		}
 
 		$bans = $this->getDataFromResponse($this->response);
+		foreach ($bans as &$key) {
+			unset($key['id'], $key['roomId'], $key['bannedTime']);
+		}
 
-		var_dump($bans);
-		var_dump($tableNode->getColumnsHash());
-
-		// FIXME compare the 2 arrays
+		Assert::assertEquals($tableNode->getColumnsHash(), $bans);
 	}
 
 	/**
@@ -1545,7 +1556,11 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 				$actorType = 'federated_user';
 			}
 		}
+
+		$actorType .= 's';
+
 		$banId = self::$userToBanId[self::$identifierToToken[$identifier]][$actorType][$actorId];
+		unset(self::$userToBanId[self::$identifierToToken[$identifier]][$actorType][$actorId]);
 
 		$this->setCurrentUser($user);
 		$this->sendRequest(
diff --git a/tests/integration/features/conversation-1/ban.feature b/tests/integration/features/conversation-1/ban.feature
index 397db630e..688fbe4b6 100644
--- a/tests/integration/features/conversation-1/ban.feature
+++ b/tests/integration/features/conversation-1/ban.feature
@@ -13,23 +13,80 @@ Feature: conversation/ban
         And user "participant2" joins room "room" with 200 (v4)
         And user "participant3" joins room "room" with 200 (v4)
         And user "participant1" bans user "participant2" from room "room" with 200 (v1)
-            | internalNote | BannedP1 |
-        And user "participant1" bans user "participant3" from room "room" with 200 (v1)
             | internalNote | BannedP2 |
+        And user "participant1" bans user "participant3" from room "room" with 200 (v1)
+            | internalNote | BannedP3 |
         And user "participant1" sees the following bans in room "room" with 200 (v1)
           | actorType | actorId      | bannedType | bannedId     | internalNote |
-          | users     | participant1 | users      | participant2 | BannedP1     |
-          | users     | participant1 | users      | participant3 | BannedP2     |
+          | users     | participant1 | users      | participant2 | BannedP2     |
+          | users     | participant1 | users      | participant3 | BannedP3     |
         And user "participant2" joins room "room" with 403 (v4)
         And user "participant3" joins room "room" with 403 (v4)
         And user "participant1" unbans user "participant2" from room "room" with 200 (v1)
         And user "participant1" unbans user "participant3" from room "room" with 200 (v1)
+        And user "participant2" joins room "room" with 200 (v4)
+        And user "participant3" joins room "room" with 200 (v4)
 
-    # Scenario: Moderator banning and unbanning guest account
+    Scenario: Users trying to ban moderator
+        Given user "participant1" creates room "room" (v4)
+        | roomType | 3 |
+        | roomName | room |
+        And user "participant2" joins room "room" with 200 (v4)
+        And user "participant3" joins room "room" with 200 (v4)
+        And user "participant2" bans user "participant1" from room "room" with 403 (v1)
+            | internalNote | BannedP1 |
+        And user "participant3" bans user "participant1" from room "room" with 403 (v1)
+            | internalNote | BannedP1 |
+
+    Scenario: Users trying to ban other users
+        Given user "participant1" creates room "room" (v4)
+        | roomType | 3 |
+        | roomName | room |
+        And user "participant2" joins room "room" with 200 (v4)
+        And user "participant3" joins room "room" with 200 (v4)
+        And user "participant2" bans user "participant3" from room "room" with 403 (v1)
+            | internalNote | BannedP3 |
+        And user "participant3" bans user "participant2" from room "room" with 403 (v1)
+            | internalNote | BannedP2 |
+
+    Scenario: User trying to ban themselves
+        Given user "participant1" creates room "room" (v4)
+        | roomType | 3 |
+        | roomName | room |
+        And user "participant1" joins room "room" with 200 (v4)
+        And user "participant2" joins room "room" with 200 (v4)
+        And user "participant2" bans user "participant2" from room "room" with 403 (v1)
+            | internalNote | BannedP2 |
+
+    Scenario: Moderator trying to ban an invalid user
+        Given user "participant1" creates room "room" (v4)
+        | roomType | 3 |
+        | roomName | room |
+        And user "participant2" joins room "room" with 200 (v4)
+        And user "participant1" bans user "participant3" from room "room" with 200 (v1)
+            | internalNote | BannedInvalid |
+
+    # Scenario: Moderator trying to ban moderator
+    #     Given user "participant1" creates room "room" (v4)
+    #     | roomType | 3 |
+    #     | roomName | room |
+    #     And user "participant1" joins room "room" with 200 (v4)
+    #     And user "participant2" joins room "room" with 200 (v4)
+    #     And user "participant1" adds user "participant2" to room "room" with 200 (v4)
+    #     And user "participant1" promotes "participant2" in room "room" with 200 (v4)
+    #     And user "participant1" bans user "participant2" from room "room" with 403 (v1)
+    #         | internalNote | BannedP2 |
+
+    # Scenario: Moderator trying to ban themselves
     #     Given user "participant1" creates room "room" (v4)
     #     | roomType | 3 |
     #     | roomName | room |
-    #     And user "user-guest@example.com" joins room "room" with 200 (v4)
-    #     And user "participant1" bans user "user-guest@example.com" from room "room" with 200 (v1)
-    #         | internalNote | BannedG1 |
-    #     And user "participant1" unbans user "user-guest@exmaple.com" from room "room" with 200 (v1)
+    #     And user "participant1" joins room "room" with 200 (v4)
+    #     And user "participant1" bans user "participant1" from room "room" with 403 (v1)
+    #         | internalNote | BannedP1 |
+
+    
+
+
+
+