From: Raphaël Barrois <raphael.barrois@polytechnique.org>
Date: Fri, 1 Jul 2011 20:36:51 +0000 (+0200)
Subject: Make group-NL related code safer by using methods of the Newsletter class instead... 
X-Git-Tag: xorg/1.1.3~2^2~5
X-Git-Url: http://git.polytechnique.org/?a=commitdiff_plain;h=e4b8520cb1fbd583010c36a29ac99744a8389d43;p=platal.git

Make group-NL related code safer by using methods of the Newsletter class instead of duplicating the SQL code.

Signed-off-by: Raphaël Barrois <raphael.barrois@polytechnique.org>
---

diff --git a/include/newsletter.inc.php b/include/newsletter.inc.php
index 3359249..2f6ccca 100644
--- a/include/newsletter.inc.php
+++ b/include/newsletter.inc.php
@@ -361,11 +361,26 @@ class NewsLetter
         }
         if (self::maySubscribe($user)) {
             XDB::execute('INSERT IGNORE INTO  newsletter_ins (nlid, uid, last, hash)
-                                      VALUES  ({?}, {?}, 0, hash)',
+                                      VALUES  ({?}, {?}, NULL, hash)',
                          $this->id, $user->id());
         }
     }
 
+    /** Subscribe a batch of users to a newsletter.
+     * This skips 'maySubscribe' test.
+     *
+     * @p $user_ids Array of user IDs to subscribe to the newsletter.
+     */
+    public function bulkSubscribe($user_ids)
+    {
+        // TODO: use a 'bulkMaySubscribe'.
+        XDB::execute('INSERT IGNORE INTO  newsletter_ins (nlid, uid, last, hash)
+                                  SELECT  {?}, a.uid, NULL, NULL
+                                    FROM  accounts AS a
+                                   WHERE  a.uid IN {?}',
+                     $this->id, $user_ids);
+    }
+
     /** Retrieve subscription state of a user
      * @p $user Target user; if null, use current user.
      * @return Boolean: true if the user has subscribed to the NL.
diff --git a/modules/xnetgrp.php b/modules/xnetgrp.php
index 89477ee..9b04afc 100644
--- a/modules/xnetgrp.php
+++ b/modules/xnetgrp.php
@@ -1278,30 +1278,24 @@ class XnetGrpModule extends PLModule
             }
 
             if ($globals->asso('has_nl')) {
+                $nl = NewsLetter::forGroup($globals->asso('shortname');
                 // Updates group's newsletter subscription.
                 if (Post::i('newsletter') == 1) {
-                    XDB::execute('INSERT IGNORE INTO  newsletter_ins (uid, nlid)
-                                              SELECT  {?}, id
-                                                FROM  newsletters
-                                               WHERE  group_id = {?}',
-                                 $user->id(), $globals->asso('id'));
+                    $nl->subscribe($user);
                 } else {
-                    XDB::execute('DELETE  ni
-                                    FROM  newsletter_ins AS ni
-                              INNER JOIN  newsletters    AS n  ON (n.id = ni.nlid)
-                                   WHERE  ni.uid = {?} AND n.group_id = {?}',
-                                 $user->id(), $globals->asso('id'));
+                    $nl->unsubscribe(null, $user->id);
                 }
             }
         }
 
         $res = XDB::rawFetchAllAssoc('SHOW COLUMNS FROM group_members LIKE \'position\'');
         $positions = str_replace(array('enum(', ')', '\''), '', $res[0]['Type']);
-        $nl_registered = XDB::fetchOneCell('SELECT  COUNT(ni.uid)
-                                              FROM  newsletter_ins AS ni
-                                        INNER JOIN  newsletters    AS n  ON (n.id = ni.nlid)
-                                             WHERE  ni.uid = {?} AND n.group_id = {?}',
-                                           $user->id(), $globals->asso('id'));
+        if ($globals->asso('has_nl')) {
+            $nl = NewsLetter::forGroup($globals->asso('shortname'));
+            $nl_registered = $nl->subscriptionState($user);
+        } else {
+            $nl_registered = false;
+        }
 
         $page->assign('user', $user);
         $page->assign('suggest', $this->suggest($user));
diff --git a/modules/xnetnl.php b/modules/xnetnl.php
index a6fb168..b849193 100644
--- a/modules/xnetnl.php
+++ b/modules/xnetnl.php
@@ -57,16 +57,12 @@ class XnetNlModule extends NewsletterModule
         if (Env::has('add_users')) {
             S::assert_xsrf_token();
 
-            XDB::execute('INSERT IGNORE INTO  newsletter_ins (uid, nlid)
-                                      SELECT  g.uid, n.id
-                                        FROM  group_members AS g
-                                  INNER JOIN  newsletters   AS n  ON (n.group_id = g.asso_id)
-                                       WHERE  g.uid IN {?} AND g.asso_id = {?}',
-                         array_keys(Env::v('add_users')), $globals->asso('id'));
+            $nl->bulkSubscribe(array_keys(Env::v('add_users')));
 
             $page->trigSuccess('Ajouts réalisés avec succès.');
         }
 
+        // TODO(x2006barrois): remove raw SQL query.
         $uids = XDB::fetchColumn('SELECT  DISTINCT(g.uid)
                                     FROM  group_members AS g
                                    WHERE  g.asso_id = {?} AND NOT EXISTS (SELECT  ni.*