feat: Integration tests for banning/unbanning users

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

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;
 		}
 

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(),
 		];

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('*')

lib/Service/BanService.php

@@ -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.");
 		}
 	}
 

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(

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: 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 banning and unbanning guest account
+    # Scenario: Moderator trying to ban moderator
     #     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 "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 "participant1" joins room "room" with 200 (v4)
+    #     And user "participant1" bans user "participant1" from room "room" with 403 (v1)
+    #         | internalNote | BannedP1 |
+
+    
+
+
+
+