Tidied up adding project hooks.

Projects::addHook() now accepts only 3 parameters: $project_id, $url and an array of optional event types. The `push_events` event is true by default. Projects::updateHook() now accepts only 2 parameters: $project_id and an array of properties to update.
GitLabPHP · Feb 24, 2015 · 41457f0 · 41457f0
1 parent bbbc030
commit 41457f0
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 183 deletions.
diff --git a/lib/Gitlab/Api/Projects.php b/lib/Gitlab/Api/Projects.php
@@ -1,5 +1,7 @@
 <?php namespace Gitlab\Api;
 
+use Gitlab\Exception\RuntimeException;
+
 class Projects extends AbstractApi
 {
     const ORDER_BY = 'created_at';
@@ -205,59 +207,28 @@ public function hook($project_id, $hook_id)
     /**
      * @param int $project_id
      * @param string $url
-     * @param bool $push_events
-     * @param bool $issues_events
-     * @param bool $merge_requests_events
-     * @param bool $tag_push_events
+     * @param array $params
      * @return mixed
      */
-    public function addHook($project_id, $url, $push_events = true, $issues_events = false, $merge_requests_events = false, $tag_push_events = false)
+    public function addHook($project_id, $url, array $params = array())
     {
-        return $this->post($this->getProjectPath($project_id, 'hooks'), array(
-            'url' => $url,
-            'push_events' => $push_events,
-            'issues_events' => $issues_events,
-            'merge_requests_events' => $merge_requests_events,
-            'tag_push_events' => $tag_push_events
-        ));
+        if (empty($params)) {
+            $params = array('push_events' => true);
+        }
+
+        $params['url'] = $url;
+
+        return $this->post($this->getProjectPath($project_id, 'hooks'), $params);
     }
 
     /**
      * @param int $project_id
      * @param int $hook_id
-     * @param string $url
-     * @param bool $push_events
-     * @param bool $issues_events
-     * @param bool $merge_requests_events
-     * @param bool $tag_push_events
+     * @param array $params
      * @return mixed
      */
-    public function updateHook($project_id, $hook_id, $url, $push_events = true, $issues_events = false, $merge_requests_events = false, $tag_push_events = false)
+    public function updateHook($project_id, $hook_id, array $params)
     {
-        if (is_array($url)) {
-            $params = $url;
-        } else {
-            $params = array(
-                'url' => $url
-            );
-
-            if ($push_events) {
-                $params['push_events'] = $push_events;
-            }
-
-            if ($issues_events) {
-                $params['issues_events'] = $issues_events;
-            }
-
-            if ($merge_requests_events) {
-                $params['merge_requests_events'] = $merge_requests_events;
-            }
-
-            if ($tag_push_events) {
-                $params['tag_push_events'] = $tag_push_events;
-            }
-        }
-
         return $this->put($this->getProjectPath($project_id, 'hooks/'.urlencode($hook_id)), $params);
     }
 

diff --git a/lib/Gitlab/Model/Project.php b/lib/Gitlab/Model/Project.php
@@ -229,38 +229,26 @@ public function hook($id)
 
     /**
      * @param string $url
-     * @param bool $push_events
-     * @param bool $issues_events
-     * @param bool $merge_requests_events
+     * @param array $events
      * @return ProjectHook
      */
-    public function addHook($url, $push_events = true, $issues_events = false, $merge_requests_events = false)
+    public function addHook($url, array $events = array())
     {
-        $data = $this->api('projects')->addHook($this->id, $url, $push_events, $issues_events, $merge_requests_events);
+        $data = $this->api('projects')->addHook($this->id, $url, $events);
 
         return ProjectHook::fromArray($this->getClient(), $this, $data);
     }
 
     /**
      * @param int $hook_id
-     * @param string $url
-     * @param bool $push_events
-     * @param bool $issues_events
-     * @param bool $merge_requests_events
-     * @param bool $tag_push_events
+     * @param array $params
      * @return mixed
      */
-    public function updateHook($hook_id, $url, $push_events = true, $issues_events = true, $merge_requests_events = true, $tag_push_events = true)
+    public function updateHook($hook_id, array $params)
     {
         $hook = new ProjectHook($this, $hook_id, $this->getClient());
 
-        return $hook->update(array(
-            'url' => $url,
-            'push_events' => $push_events,
-            'issues_events' => $issues_events,
-            'merge_requests_events' => $merge_requests_events,
-            'tag_push_events' => $tag_push_events
-        ));
+        return $hook->update($params);
     }
 
     /**

diff --git a/lib/Gitlab/Model/ProjectHook.php b/lib/Gitlab/Model/ProjectHook.php
@@ -91,15 +91,7 @@ public function remove()
      */
     public function update(array $params)
     {
-        $params = array_merge(array(
-            'url' => false,
-            'push_events' => false,
-            'issues_events' => false,
-            'merge_requests_events' => false,
-            'tag_push_events' => false
-        ), $params);
-
-        $data = $this->api('projects')->updateHook($this->project->id, $this->id, $params['url'], $params['push_events'], $params['issues_events'], $params['merge_requests_events'], $params['tag_push_events']);
+        $data = $this->api('projects')->updateHook($this->project->id, $this->id, $params);
 
         return static::fromArray($this->getClient(), $this->project, $data);
     }

diff --git a/test/Gitlab/Tests/Api/ProjectsTest.php b/test/Gitlab/Tests/Api/ProjectsTest.php
@@ -328,165 +328,62 @@ public function shouldAddHook()
         $api = $this->getApiMock();
         $api->expects($this->once())
             ->method('post')
-            ->with('projects/1/hooks', array(
-                'url' => 'http://www.example.com',
-                'push_events' => true,
-                'issues_events' => false,
-                'merge_requests_events' => false,
-                'tag_push_events' => false
-            ))
+            ->with('projects/1/hooks', array('url' => 'http://www.example.com', 'push_events' => true, 'issues_events' => true, 'merge_requests_events' => true))
             ->will($this->returnValue($expectedArray))
         ;
 
-        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com'));
-    }
-
-    /**
-     * @test
-     */
-    public function shouldAddHookWithoutPushEvents()
-    {
-        $expectedArray = array('id' => 3, 'name' => 'A new hook', 'url' => 'http://www.example.com');
-
-        $api = $this->getApiMock();
-        $api->expects($this->once())
-            ->method('post')
-            ->with('projects/1/hooks', array(
-                'url' => 'http://www.example.com',
-                'push_events' => false,
-                'issues_events' => false,
-                'merge_requests_events' => false,
-                'tag_push_events' => false
-            ))
-            ->will($this->returnValue($expectedArray))
-        ;
-
-        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com', false));
-    }
-
-    /**
-     * @test
-     */
-    public function shouldAddHookWithIssuesEvents()
-    {
-        $expectedArray = array('id' => 3, 'name' => 'A new hook', 'url' => 'http://www.example.com');
-
-        $api = $this->getApiMock();
-        $api->expects($this->once())
-            ->method('post')
-            ->with('projects/1/hooks', array(
-                'url' => 'http://www.example.com',
-                'push_events' => false,
-                'issues_events' => true,
-                'merge_requests_events' => false,
-                'tag_push_events' => false
-            ))
-            ->will($this->returnValue($expectedArray))
-        ;
-
-        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com', false, true));
+        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com', array('push_events' => true, 'issues_events' => true, 'merge_requests_events' => true)));
     }
 
     /**
      * @test
      */
-    public function shouldAddHookWithMergeRequestEvents()
+    public function shouldAddHookWithOnlyUrl()
     {
         $expectedArray = array('id' => 3, 'name' => 'A new hook', 'url' => 'http://www.example.com');
 
         $api = $this->getApiMock();
         $api->expects($this->once())
             ->method('post')
-            ->with('projects/1/hooks', array(
-                'url' => 'http://www.example.com',
-                'push_events' => false,
-                'issues_events' => false,
-                'merge_requests_events' => true,
-                'tag_push_events' => false
-            ))
+            ->with('projects/1/hooks', array('url' => 'http://www.example.com', 'push_events' => true))
             ->will($this->returnValue($expectedArray))
         ;
 
-        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com', false, false, true));
+        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com'));
     }
 
     /**
      * @test
      */
-    public function shouldAddHookWithTagPushEvents()
+    public function shouldAddHookWithoutPushEvents()
     {
         $expectedArray = array('id' => 3, 'name' => 'A new hook', 'url' => 'http://www.example.com');
 
         $api = $this->getApiMock();
         $api->expects($this->once())
             ->method('post')
-            ->with('projects/1/hooks', array(
-                'url' => 'http://www.example.com',
-                'push_events' => false,
-                'issues_events' => false,
-                'merge_requests_events' => false,
-                'tag_push_events' => true
-            ))
-            ->will($this->returnValue($expectedArray))
-        ;
-
-        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com', false, false, false, true));
-    }
-
-    /**
-     * @test
-     */
-    public function shouldUpdateHookUrlOnly()
-    {
-        $expectedArray = array('id' => 3, 'name' => 'A new hook', 'url' => 'http://www.example.com');
-
-        $api = $this->getApiMock();
-        $api->expects($this->once())
-            ->method('put')
-            ->with('projects/1/hooks/3', array('url' => 'http://www.example-test.com'))
-            ->will($this->returnValue($expectedArray))
-        ;
-
-        $this->assertEquals($expectedArray, $api->updateHook(1, 3, array('url' => 'http://www.example-test.com')));
-    }
-
-    /**
-     * @test
-     */
-    public function shouldUpdateHookWithPushEvents()
-    {
-        $expectedArray = array('id' => 3, 'name' => 'A new hook', 'url' => 'http://www.example.com');
-
-        $api = $this->getApiMock();
-        $api->expects($this->once())
-            ->method('put')
-            ->with('projects/1/hooks/3', array('url' => 'http://www.example-test.com', 'push_events' => true))
+            ->with('projects/1/hooks', array('url' => 'http://www.example.com', 'push_events' => false))
             ->will($this->returnValue($expectedArray))
         ;
 
-        $this->assertEquals($expectedArray, $api->updateHook(1, 3, array('url' => 'http://www.example-test.com', 'push_events' => true)));
+        $this->assertEquals($expectedArray, $api->addHook(1, 'http://www.example.com', array('push_events' => false)));
     }
 
     /**
      * @test
      */
-    public function shouldUpdateHookWithDifferentEvents()
+    public function shouldUpdateHook()
     {
         $expectedArray = array('id' => 3, 'name' => 'A new hook', 'url' => 'http://www.example.com');
 
         $api = $this->getApiMock();
         $api->expects($this->once())
             ->method('put')
-            ->with('projects/1/hooks/3', array(
-                'url' => 'http://www.example-test.com',
-                'issues_events' => true,
-                'merge_requests_events' => true,
-                'tag_push_events' => true
-            ))
+            ->with('projects/1/hooks/3', array('url' => 'http://www.example-test.com', 'push_events' => false))
             ->will($this->returnValue($expectedArray))
         ;
 
-        $this->assertEquals($expectedArray, $api->updateHook(1, 3, 'http://www.example-test.com', false, true, true, true));
+        $this->assertEquals($expectedArray, $api->updateHook(1, 3, array('url' => 'http://www.example-test.com', 'push_events' => false)));
     }
 
     /**