Only send notifications to project members (#299)

app/Model/Notification.php

@@ -27,6 +27,27 @@ class Notification extends Base
      */
     const TABLE = 'user_has_notifications';
 
+    /**
+     * Get a list of people with notifications enabled
+     *
+     * @access public
+     * @param  integer   $project_id     Project id
+     * @param  array     $exlude_users   List of user_id to exclude
+     * @return array
+     */
+    public function getUsersWithNotification($project_id, array $exclude_users = array())
+    {
+        return $this->db
+            ->table(ProjectPermission::TABLE)
+            ->columns(User::TABLE.'.id', User::TABLE.'.username', User::TABLE.'.name', User::TABLE.'.email')
+            ->join(User::TABLE, 'id', 'user_id')
+            ->eq('project_id', $project_id)
+            ->eq('notifications_enabled', '1')
+            ->neq('email', '')
+            ->notin(User::TABLE.'.id', $exclude_users)
+            ->findAll();
+    }
+
     /**
      * Get the list of users to send the notification for a given project
      *
@@ -42,12 +63,7 @@ class Notification extends Base
             $exclude_users[] = $this->acl->getUserId();
         }
 
-        $users = $this->db->table(User::TABLE)
+        $users = $this->getUsersWithNotification($project_id, $exclude_users);
-                          ->columns('id', 'username', 'name', 'email')
-                          ->eq('notifications_enabled', '1')
-                          ->neq('email', '')
-                          ->notin('id', $exclude_users)
-                          ->findAll();
 
         foreach ($users as $index => $user) {
 

tests/units/NotificationTest.php

@@ -4,14 +4,56 @@ require_once __DIR__.'/Base.php';
 
 use Model\User;
 use Model\Project;
+use Model\ProjectPermission;
 use Model\Notification;
 
 class NotificationTest extends Base
 {
+    public function testGetUsersWithNotification()
+    {
+        $u = new User($this->registry);
+        $p = new Project($this->registry);
+        $n = new Notification($this->registry);
+        $pp = new ProjectPermission($this->registry);
+
+        $this->assertEquals(1, $p->create(array('name' => 'UnitTest1')));
+
+        // Email + Notifications enabled
+        $this->assertTrue($u->create(array('username' => 'user1', 'email' => 'user1@here', 'notifications_enabled' => 1)));
+
+        // No email + Notifications enabled
+        $this->assertTrue($u->create(array('username' => 'user2', 'email' => '', 'notifications_enabled' => 1)));
+
+        // Email + Notifications enabled
+        $this->assertTrue($u->create(array('username' => 'user3', 'email' => 'user3@here', 'notifications_enabled' => 1)));
+
+        // No email + notifications disabled
+        $this->assertTrue($u->create(array('username' => 'user4')));
+
+        // Nobody is member of any projects
+        $this->assertEmpty($pp->getAllowedUsers(1));
+        $this->assertEmpty($n->getUsersWithNotification(1));
+
+        // We allow all users to be member of our projects
+        $this->assertTrue($pp->allowUser(1, 1));
+        $this->assertTrue($pp->allowUser(1, 2));
+        $this->assertTrue($pp->allowUser(1, 3));
+        $this->assertTrue($pp->allowUser(1, 4));
+
+        $this->assertNotEmpty($pp->getAllowedUsers(1));
+        $users = $n->getUsersWithNotification(1);
+
+        $this->assertNotEmpty($users);
+        $this->assertEquals(2, count($users));
+        $this->assertEquals('user1@here', $users[0]['email']);
+        $this->assertEquals('user3@here', $users[1]['email']);
+    }
+
     public function testGetUserList()
     {
         $u = new User($this->registry);
         $p = new Project($this->registry);
+        $pp = new ProjectPermission($this->registry);
         $n = new Notification($this->registry);
 
         $this->assertEquals(1, $p->create(array('name' => 'UnitTest1')));
@@ -29,6 +71,17 @@ class NotificationTest extends Base
         // No email + notifications disabled
         $this->assertTrue($u->create(array('username' => 'user4')));
 
+        // We allow all users to be member of our projects
+        $this->assertTrue($pp->allowUser(1, 1));
+        $this->assertTrue($pp->allowUser(1, 2));
+        $this->assertTrue($pp->allowUser(1, 3));
+        $this->assertTrue($pp->allowUser(1, 4));
+
+        $this->assertTrue($pp->allowUser(2, 1));
+        $this->assertTrue($pp->allowUser(2, 2));
+        $this->assertTrue($pp->allowUser(2, 3));
+        $this->assertTrue($pp->allowUser(2, 4));
+
         $users = $n->getUsersList(1);
         $this->assertNotEmpty($users);
         $this->assertEquals(2, count($users));