Fix bad unique constraints in Mysql table user_has_notifications

6 files changed, 30 insertions, 19 deletions

diff --git a/ChangeLog b/ChangeLog
index 5fded0d4..0da552ed 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,7 @@ New features:
 
 Improvements:
 
+* Improve notification configuration form
 * Handle state in OAuth2 client
 * Allow to use the original template in overridden templates
 * Unification of the project header
@@ -30,6 +31,7 @@ Improvements:
 
 Bug fixes:
 
+* Fix bad unique constraints in Mysql table user_has_notifications
 * Force integer type for aggregated metrics (Burndown chart concat values instead of summing)
 * Fixes cycle time calculation when the start date is defined in the future
 * Access allowed to any tasks from the shared public board by changing the URL parameters
diff --git a/app/Model/UserNotification.php b/app/Model/UserNotification.php
index e8a967ac..7795da2e 100644
--- a/app/Model/UserNotification.php
+++ b/app/Model/UserNotification.php
@@ -117,23 +117,20 @@ class UserNotification extends Base
      */
     public function saveSettings($user_id, array $values)
     {
-        $this->db->startTransaction();
+        $types = empty($values['notification_types']) ? array() : array_keys($values['notification_types']);
 
-        if (isset($values['notifications_enabled']) && $values['notifications_enabled'] == 1) {
+        if (! empty($types)) {
             $this->enableNotification($user_id);
-
-            $filter = empty($values['notifications_filter']) ? UserNotificationFilter::FILTER_BOTH : $values['notifications_filter'];
-            $projects = empty($values['notification_projects']) ? array() : array_keys($values['notification_projects']);
-            $types = empty($values['notification_types']) ? array() : array_keys($values['notification_types']);
-
-            $this->userNotificationFilter->saveFilter($user_id, $filter);
-            $this->userNotificationFilter->saveSelectedProjects($user_id, $projects);
-            $this->userNotificationType->saveSelectedTypes($user_id, $types);
         } else {
             $this->disableNotification($user_id);
         }
 
-        $this->db->closeTransaction();
+        $filter = empty($values['notifications_filter']) ? UserNotificationFilter::FILTER_BOTH : $values['notifications_filter'];
+        $project_ids = empty($values['notification_projects']) ? array() : array_keys($values['notification_projects']);
+
+        $this->userNotificationFilter->saveFilter($user_id, $filter);
+        $this->userNotificationFilter->saveSelectedProjects($user_id, $project_ids);
+        $this->userNotificationType->saveSelectedTypes($user_id, $types);
     }
 
     /**
diff --git a/app/Model/UserNotificationFilter.php b/app/Model/UserNotificationFilter.php
index d4afd278..780ddfc7 100644
--- a/app/Model/UserNotificationFilter.php
+++ b/app/Model/UserNotificationFilter.php
@@ -61,10 +61,11 @@ class UserNotificationFilter extends Base
      * @access public
      * @param  integer  $user_id
      * @param  string   $filter
+     * @return boolean
      */
     public function saveFilter($user_id, $filter)
     {
-        $this->db->table(User::TABLE)->eq('id', $user_id)->update(array(
+        return $this->db->table(User::TABLE)->eq('id', $user_id)->update(array(
             'notifications_filter' => $filter,
         ));
     }
@@ -87,17 +88,21 @@ class UserNotificationFilter extends Base
      * @access public
      * @param  integer    $user_id
      * @param  integer[]  $project_ids
+     * @return boolean
      */
     public function saveSelectedProjects($user_id, array $project_ids)
     {
+        $results = array();
         $this->db->table(self::PROJECT_TABLE)->eq('user_id', $user_id)->remove();
 
         foreach ($project_ids as $project_id) {
-            $this->db->table(self::PROJECT_TABLE)->insert(array(
+            $results[] = $this->db->table(self::PROJECT_TABLE)->insert(array(
                 'user_id' => $user_id,
                 'project_id' => $project_id,
             ));
         }
+
+        return !in_array(false, $results, true);
     }
 
     /**
diff --git a/app/Schema/Mysql.php b/app/Schema/Mysql.php
index ccb5a9cf..a041b3dc 100644
--- a/app/Schema/Mysql.php
+++ b/app/Schema/Mysql.php
@@ -6,7 +6,16 @@ use PDO;
 use Kanboard\Core\Security\Token;
 use Kanboard\Core\Security\Role;
 
-const VERSION = 109;
+const VERSION = 110;
+
+function version_110(PDO $pdo)
+{
+    $pdo->exec("ALTER TABLE user_has_notifications DROP FOREIGN KEY `user_has_notifications_ibfk_1`");
+    $pdo->exec("ALTER TABLE user_has_notifications DROP FOREIGN KEY `user_has_notifications_ibfk_2`");
+    $pdo->exec("DROP INDEX `project_id` ON user_has_notifications");
+    $pdo->exec("ALTER TABLE user_has_notifications DROP KEY `user_id`");
+    $pdo->exec("CREATE UNIQUE INDEX `user_has_notifications_unique_idx` ON `user_has_notifications` (`user_id`, `project_id`)");
+}
 
 function version_109(PDO $pdo)
 {
diff --git a/app/Template/user/notifications.php b/app/Template/user/notifications.php
index 2a5c8152..6e1a0004 100644
--- a/app/Template/user/notifications.php
+++ b/app/Template/user/notifications.php
@@ -3,11 +3,8 @@
 </div>
 
 <form method="post" action="<?= $this->url->href('user', 'notifications', array('user_id' => $user['id'])) ?>" autocomplete="off">
-
     <?= $this->form->csrf() ?>
-    <?= $this->form->checkbox('notifications_enabled', t('Enable notifications'), '1', $notifications['notifications_enabled'] == 1) ?><br>
 
-    <hr>
     <h4><?= t('Notification methods:') ?></h4>
     <?= $this->form->checkboxes('notification_types', $types, $notifications) ?>
 
diff --git a/tests/units/Model/UserNotificationFilterTest.php b/tests/units/Model/UserNotificationFilterTest.php
index 0b5f1d98..924f0883 100644
--- a/tests/units/Model/UserNotificationFilterTest.php
+++ b/tests/units/Model/UserNotificationFilterTest.php
@@ -26,10 +26,11 @@ class UserNotificationFilterTest extends Base
 
         $this->assertEquals(1, $p->create(array('name' => 'UnitTest1')));
         $this->assertEquals(2, $p->create(array('name' => 'UnitTest2')));
+        $this->assertEquals(3, $p->create(array('name' => 'UnitTest3')));
 
         $this->assertEmpty($nf->getSelectedProjects(1));
-        $nf->saveSelectedProjects(1, array(1, 2));
-        $this->assertEquals(array(1, 2), $nf->getSelectedProjects(1));
+        $this->assertTrue($nf->saveSelectedProjects(1, array(1, 2, 3)));
+        $this->assertEquals(array(1, 2, 3), $nf->getSelectedProjects(1));
     }
 
     public function testSaveUserFilter()