wip2

application/controllers/ApiV1ContactgroupsController.php

@@ -46,10 +46,9 @@ public function indexAction(): void
         $responseCode = 200;
         $db = Database::get();
         $identifier = $request->getParam('identifier');
-        // TODO: Remove rawurldecode(). Only added to test, bcz phpstorm's http client encodes the params
-        $queryString = rawurldecode(Url::fromRequest()->getQueryString());
+
         $filter = FilterProcessor::assembleFilter(
-            QueryString::fromString($queryString)
+            QueryString::fromString(Url::fromRequest()->getQueryString())
                 ->on(
                     QueryString::ON_CONDITION,
                     function (Filter\Condition $condition) {
@@ -140,34 +139,27 @@ function (Filter\Condition $condition) {
 
                 $this->assertValidData($data);
 
+                if ($this->getContactgroupId($data['id']) !== null) {
+                    throw new HttpException('422', 'Contactgroup already exists');
+                }
+
                 $db->beginTransaction();
 
                 if ($identifier === null) {
-                    if ($this->getContactgroupId($data['id']) !== null) {
-                        throw new HttpException('422', 'Contactgroup already exists');
-                    }
-
                     $this->addContactgroup($data);
-
-                    $identifier = $data['id'];
                 } else {
                     $contactgroupId = $this->getContactgroupId($identifier);
                     if ($contactgroupId === null) {
                         $this->httpNotFound('Contactgroup not found');
                     }
 
-                    if ($identifier === $data['id']) {
-                        throw new HttpException('422', 'Contactgroup already exists');
-                    }
-
-                    $identifier = $data['id'];
                     $this->removeContactgroup($contactgroupId);
                     $this->addContactgroup($data);
                 }
 
                 $db->commitTransaction();
 
-                $this->getResponse()->setHeader('Location', self::ENDPOINT . '/' . $identifier);
+                $this->getResponse()->setHeader('Location', self::ENDPOINT . '/' . $data['id']);
                 $responseCode = 201;
 
                 break;
@@ -188,11 +180,9 @@ function (Filter\Condition $condition) {
 
                 $contactgroupId = $this->getContactgroupId($identifier);
                 if ($contactgroupId !== null) {
-                    $db->update('contactgroup', [
-                        'name'  => $data['name'],
-                    ], ['id = ?' => $contactgroupId]);
+                    $db->update('contactgroup', ['name' => $data['name']], ['id = ?' => $contactgroupId]);
 
-                    $db->delete('contactgroup_member', ['contactgroup_id = ?' => $identifier]);
+                    $db->delete('contactgroup_member', ['contactgroup_id = ?' => $contactgroupId]);
 
                     if (! empty($data['users'])) {
                         $this->addUsers($contactgroupId, $data['users']);
@@ -309,13 +299,12 @@ private function getContactgroupId(string $identifier): ?int
      */
     private function addContactgroup(array $data): void
     {
-        $db = Database::get();
-        $db->insert('contactgroup', [
+        Database::get()->insert('contactgroup', [
             'name'              => $data['name'],
             'external_uuid'     => $data['id']
         ]);
 
-        $id = $db->lastInsertId();
+        $id = Database::get()->lastInsertId();
 
         if (! empty($data['users'])) {
             $this->addUsers($id, $data['users']);
@@ -349,10 +338,8 @@ private function addUsers(int $contactgroupId, array $users): void
      */
     private function removeContactgroup(int $id): void
     {
-        $db = Database::get();
-
-        $db->delete('contactgroup_member', ['contactgroup_id = ?' => $id]);
-        $db->delete('contactgroup', ['id = ?' => $id]);
+        Database::get()->delete('contactgroup_member', ['contactgroup_id = ?' => $id]);
+        Database::get()->delete('contactgroup', ['id = ?' => $id]);
     }
 
     /**
@@ -365,7 +352,7 @@ private function removeContactgroup(int $id): void
     private function assertValidData(array $data): void
     {
         if (! isset($data['id'], $data['name'])) {
-            $this->httpBadRequest('missing required fields');
+            $this->httpBadRequest('fields id and name are required');
         }
     }
 }

application/controllers/ApiV1ContactsController.php

@@ -46,10 +46,9 @@ public function indexAction(): void
         $responseCode = 200;
         $db = Database::get();
         $identifier = $request->getParam('identifier');
-        // TODO: Remove rawurldecode(). Only added to test, bcz phpstorm's http client encodes the params
-        $queryString = rawurldecode(Url::fromRequest()->getQueryString());
+
         $filter = FilterProcessor::assembleFilter(
-            QueryString::fromString($queryString)
+            QueryString::fromString(Url::fromRequest()->getQueryString())
                 ->on(
                     QueryString::ON_CONDITION,
                     function (Filter\Condition $condition) {
@@ -162,34 +161,27 @@ function (Filter\Condition $condition) {
 
                 $this->assertValidData($data);
 
+                if ($this->getContactId($data['id']) !== null) {
+                    throw new HttpException('422', 'Contact already exists');
+                }
+
                 $db->beginTransaction();
 
                 if ($identifier === null) {
-                    if ($this->getContactId($data['id']) !== null) {
-                        throw new HttpException('422', 'Contact already exists');
-                    }
-
                     $this->addContact($data);
-                    $identifier = $data['id'];
                 } else {
                     $contactId = $this->getContactId($identifier);
                     if ($contactId === null) {
                         $this->httpNotFound('Contact not found');
                     }
 
-                    if ($identifier === $data['id']) {
-                        throw new HttpException('422', 'Contact already exists');
-                    }
-
                     $this->removeContact($contactId);
                     $this->addContact($data);
-
-                    $identifier = $data['id'];
                 }
 
                 $db->commitTransaction();
 
-                $this->getResponse()->setHeader('Location', self::ENDPOINT . '/' . $identifier);
+                $this->getResponse()->setHeader('Location', self::ENDPOINT . '/' . $data['id']);
                 $responseCode = 201;
 
                 break;
@@ -210,6 +202,10 @@ function (Filter\Condition $condition) {
 
                 $contactId = $this->getContactId($identifier);
                 if ($contactId !== null) {
+                    if (isset($data['username'])) {
+                        $this->assertUniqueUsername($data['username']);
+                    }
+
                     $db->update('contact', [
                         'full_name'             => $data['full_name'],
                         'username'              => $data['username'] ?? null,
@@ -382,20 +378,18 @@ protected function getContactId(string $identifier): ?int
      */
     private function addContact(array $data): void
     {
-        $db = Database::get();
-
         if (isset($data['username'])) {
             $this->assertUniqueUsername($data['username']);
         }
 
-        $db->insert('contact', [
+        Database::get()->insert('contact', [
             'full_name'             => $data['full_name'],
             'username'              => $data['username'] ?? null,
             'default_channel_id'    => $this->getChannelId($data['default_channel']),
             'external_uuid'         => $data['id']
         ]);
 
-        $contactId = $db->lastInsertId();
+        $contactId = Database::get()->lastInsertId();
 
         if (! empty($data['addresses'])) {
             $this->addAddresses($contactId, $data['addresses']);
@@ -443,12 +437,15 @@ private function assertAddressTypesExist(array $addressTypes): void
         $types = Database::get()->fetchCol(
             (new Select())
                 ->from('available_channel_type')
-                ->columns(1)
+                ->columns('type')
                 ->where(['type IN (?)' => $addressTypes])
         );
 
         if (count($types) !== count($addressTypes)) {
-            $this->httpBadRequest('An undefined address type given');
+            $this->httpBadRequest(sprintf(
+                    'undefined address type %s given',
+                    implode(', ', array_diff($addressTypes, $types))
+            ));
         }
     }
 
@@ -485,7 +482,6 @@ private function addAddresses(int $contactId, array $addresses): void
         $this->assertAddressTypesExist(array_keys($addresses));
 
         foreach ($addresses as $type => $address) {
-            //TODO: Check if type exists, db allows any type
             Database::get()->insert('contact_address', [
                 'contact_id'    => $contactId,
                 'type'          => $type,
@@ -503,11 +499,9 @@ private function addAddresses(int $contactId, array $addresses): void
      */
     private function removeContact(int $id): void
     {
-        $db = Database::get();
-
-        $db->delete('contactgroup_member', ['contact_id = ?' => $id]);
-        $db->delete('contact_address', ['contact_id = ?' =>$id]);
-        $db->delete('contact', ['id = ?' => $id]);
+        Database::get()->delete('contactgroup_member', ['contact_id = ?' => $id]);
+        Database::get()->delete('contact_address', ['contact_id = ?' =>$id]);
+        Database::get()->delete('contact', ['id = ?' => $id]);
     }
 
     /**
@@ -522,7 +516,7 @@ private function removeContact(int $id): void
     private function assertValidData(array $data): void
     {
         if (! isset($data['id'], $data['full_name'], $data['default_channel'])) {
-            $this->httpBadRequest('missing required fields');
+            $this->httpBadRequest('fields id, full_name and default_channel are required');
         }
     }
 }