Make server domain pattern block reason mandatory

diff --git a/src/Console/ServerBlock.php b/src/Console/ServerBlock.php
index 9ef6777db73d3cf307514922775522c5084ec26c..65b806fc70f5fc0a653c9b67540247d97c077507 100644 (file)
--- a/src/Console/ServerBlock.php
+++ b/src/Console/ServerBlock.php
@@ -155,12 +155,12 @@ HELP;
         */
        private function addBlockedServer(): int
        {
-               if (count($this->args) < 2 || count($this->args) > 3) {
-                       throw new CommandArgsException('Add needs a domain pattern and optionally a reason.');
+               if (count($this->args) != 3) {
+                       throw new CommandArgsException('Add needs a domain pattern and a reason.');
                }
 
                $pattern = $this->getArgument(1);
-               $reason  = (count($this->args) === 3) ? $this->getArgument(2) : DomainPatternBlocklist::DEFAULT_REASON;
+               $reason  = $this->getArgument(2);
 
                $result = $this->blocklist->addPattern($pattern, $reason);
                if ($result) {

diff --git a/src/Moderation/DomainPatternBlocklist.php b/src/Moderation/DomainPatternBlocklist.php
index c6063468793bca05ecbdd885366d4598681c1480..a57b5f04cbf2437a24ce5c45f02fbe979e822b15 100644 (file)
--- a/src/Moderation/DomainPatternBlocklist.php
+++ b/src/Moderation/DomainPatternBlocklist.php
@@ -31,8 +31,6 @@ use Friendica\Util\Emailer;
 
 class DomainPatternBlocklist
 {
-       const DEFAULT_REASON = 'blocked';
-
        /** @var IManageConfigValues */
        private $config;
 
@@ -73,11 +71,11 @@ class DomainPatternBlocklist
        }
 
        /**
-        * @param string      $pattern
-        * @param string|null $reason
+        * @param string $pattern
+        * @param string $reason
         * @return int 0 if the block list couldn't be saved, 1 if the pattern was added, 2 if it was updated in place
         */
-       public function addPattern(string $pattern, string $reason = null): int
+       public function addPattern(string $pattern, string $reason): int
        {
                $update = false;
 
@@ -86,7 +84,7 @@ class DomainPatternBlocklist
                        if ($blocked['domain'] === $pattern) {
                                $blocklist[] = [
                                        'domain' => $pattern,
-                                       'reason' => $reason ?? self::DEFAULT_REASON,
+                                       'reason' => $reason,
                                ];
 
                                $update = true;
@@ -98,7 +96,7 @@ class DomainPatternBlocklist
                if (!$update) {
                        $blocklist[] = [
                                'domain' => $pattern,
-                               'reason' => $reason ?? self::DEFAULT_REASON,
+                               'reason' => $reason,
                        ];
                }
 
@@ -179,18 +177,11 @@ class DomainPatternBlocklist
 
                $blocklist = [];
                while (($data = fgetcsv($fp, 1000)) !== false) {
-                       $domain = $data[0];
-                       if (count($data) == 0) {
-                               $reason = self::DEFAULT_REASON;
-                       } else {
-                               $reason = $data[1];
-                       }
-
-                       $data = [
-                               'domain' => $domain,
-                               'reason' => $reason
+                       $item = [
+                               'domain' => $data[0],
+                               'reason' => $data[1] ?? '',
                        ];
-                       if (!in_array($data, $blocklist)) {
+                       if (!in_array($item, $blocklist)) {
                                $blocklist[] = $data;
                        }
                }

diff --git a/tests/src/Console/ServerBlockConsoleTest.php b/tests/src/Console/ServerBlockConsoleTest.php
index cdd7efef0e1386cd9de88e9980aa71feb8d9336b..9837fbb8eed4a265564ca1a47573695f675f04ab 100644 (file)
--- a/tests/src/Console/ServerBlockConsoleTest.php
+++ b/tests/src/Console/ServerBlockConsoleTest.php
@@ -93,25 +93,6 @@ CONS;
                self::assertEquals('The domain pattern \'testme.now\' is now blocked. (Reason: \'I like it!\')' . "\n", $txt);
        }
 
-       /**
-        * Test blockedservers add command with the default reason
-        */
-       public function testAddBlockedServerWithDefaultReason()
-       {
-               $this->blocklistMock
-                       ->shouldReceive('addPattern')
-                       ->with('testme.now', DomainPatternBlocklist::DEFAULT_REASON)
-                       ->andReturn(1)
-                       ->once();
-
-               $console = new ServerBlock($this->blocklistMock, $this->consoleArgv);
-               $console->setArgument(0, 'add');
-               $console->setArgument(1, 'testme.now');
-               $txt = $this->dumpExecute($console);
-
-               self::assertEquals('The domain pattern \'testme.now\' is now blocked. (Reason: \'' . DomainPatternBlocklist::DEFAULT_REASON . '\')' . "\n", $txt);
-       }
-
        /**
         * Test blockedservers add command on existed domain
         */
@@ -170,16 +151,16 @@ CONS;
        {
                $this->blocklistMock
                        ->shouldReceive('removePattern')
-                       ->with('not.exiting')
+                       ->with('not.existing')
                        ->andReturn(1)
                        ->once();
 
                $console = new ServerBlock($this->blocklistMock, $this->consoleArgv);
                $console->setArgument(0, 'remove');
-               $console->setArgument(1, 'not.exiting');
+               $console->setArgument(1, 'not.existing');
                $txt = $this->dumpExecute($console);
 
-               self::assertEquals('The domain pattern \'not.exiting\' wasn\'t blocked.' . "\n", $txt);
+               self::assertEquals('The domain pattern \'not.existing\' wasn\'t blocked.' . "\n", $txt);
        }
 
        /**
@@ -191,7 +172,14 @@ CONS;
                $console->setArgument(0, 'add');
                $txt = $this->dumpExecute($console);
 
-               self::assertStringStartsWith('[Warning] Add needs a domain pattern and optionally a reason.', $txt);
+               self::assertStringStartsWith('[Warning] Add needs a domain pattern and a reason.', $txt);
+
+               $console = new ServerBlock($this->blocklistMock, $this->consoleArgv);
+               $console->setArgument(0, 'add');
+               $console->setArgument(1, 'testme.now');
+               $txt = $this->dumpExecute($console);
+
+               self::assertStringStartsWith('[Warning] Add needs a domain pattern and a reason.', $txt);
        }
 
        /**
@@ -201,13 +189,14 @@ CONS;
        {
                $this->blocklistMock
                        ->shouldReceive('addPattern')
-                       ->with('testme.now', DomainPatternBlocklist::DEFAULT_REASON)
+                       ->with('testme.now', 'I like it!')
                        ->andReturn(0)
                        ->once();
 
                $console = new ServerBlock($this->blocklistMock, $this->consoleArgv);
                $console->setArgument(0, 'add');
                $console->setArgument(1, 'testme.now');
+               $console->setArgument(2, 'I like it!');
                $txt = $this->dumpExecute($console);
 
                self::assertEquals('Couldn\'t save \'testme.now\' as blocked domain pattern' . "\n", $txt);