Exclude suspended Users from being notified

Message ID 20200704012916.16235-1-kevr.gtalk@gmail.com
State New
Headers show
Series
  • Exclude suspended Users from being notified
Related show

Commit Message

Kevin Morris July 4, 2020, 1:29 a.m. UTC
The existing notify.py script was grabbing entries regardless
of user suspension. This has been modified to only send notifications
to unsuspended users.

This change was written as a solution to
https://bugs.archlinux.org/task/65554.

Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com>
---
 aurweb/scripts/notify.py | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

Comments

Kevin Morris July 4, 2020, 1:40 a.m. UTC | #1
Alright, that's the final patch revision. This change ultimately just
removes suspended users from the sql results in `notify.py`, which excludes
them from all email notifications.

On Fri, Jul 3, 2020 at 6:29 PM Kevin Morris <kevr.gtalk@gmail.com> wrote:

> The existing notify.py script was grabbing entries regardless
> of user suspension. This has been modified to only send notifications
> to unsuspended users.
>
> This change was written as a solution to
> https://bugs.archlinux.org/task/65554.
>
> Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com>
> ---
>  aurweb/scripts/notify.py | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py
> index 5b18a476..223ed61f 100755
> --- a/aurweb/scripts/notify.py
> +++ b/aurweb/scripts/notify.py
> @@ -124,7 +124,7 @@ class ResetKeyNotification(Notification):
>      def __init__(self, conn, uid):
>          cur = conn.execute('SELECT UserName, Email, BackupEmail, ' +
>                             'LangPreference, ResetKey ' +
> -                           'FROM Users WHERE ID = ?', [uid])
> +                           'FROM Users WHERE ID = ? AND Suspended = 0',
> [uid])
>          self._username, self._to, self._backup, self._lang,
> self._resetkey = cur.fetchone()
>          super().__init__()
>
> @@ -171,7 +171,8 @@ class CommentNotification(Notification):
>                             'ON PackageNotifications.UserID = Users.ID
> WHERE ' +
>                             'Users.CommentNotify = 1 AND ' +
>                             'PackageNotifications.UserID != ? AND ' +
> -                           'PackageNotifications.PackageBaseID = ?',
> +                           'PackageNotifications.PackageBaseID = ? AND ' +
> +                           'Users.Suspended = 0' +,
>                             [uid, pkgbase_id])
>          self._recipients = cur.fetchall()
>          cur = conn.execute('SELECT Comments FROM PackageComments WHERE ID
> = ?',
> @@ -218,7 +219,8 @@ class UpdateNotification(Notification):
>                             'ON PackageNotifications.UserID = Users.ID
> WHERE ' +
>                             'Users.UpdateNotify = 1 AND ' +
>                             'PackageNotifications.UserID != ? AND ' +
> -                           'PackageNotifications.PackageBaseID = ?',
> +                           'PackageNotifications.PackageBaseID = ? AND ' +
> +                           'Users.Suspended = 0',
>                             [uid, pkgbase_id])
>          self._recipients = cur.fetchall()
>          super().__init__()
> @@ -264,7 +266,8 @@ class FlagNotification(Notification):
>                             'INNER JOIN PackageBases ' +
>                             'ON PackageBases.MaintainerUID = Users.ID OR '
> +
>                             'PackageBases.ID =
> PackageComaintainers.PackageBaseID ' +
> -                           'WHERE PackageBases.ID = ?', [pkgbase_id])
> +                           'WHERE PackageBases.ID = ? AND ' +
> +                           'Users.Suspended = 0', [pkgbase_id])
>          self._recipients = cur.fetchall()
>          cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE
> ' +
>                             'ID = ?', [pkgbase_id])
> @@ -302,7 +305,8 @@ class OwnershipEventNotification(Notification):
>                             'ON PackageNotifications.UserID = Users.ID
> WHERE ' +
>                             'Users.OwnershipNotify = 1 AND ' +
>                             'PackageNotifications.UserID != ? AND ' +
> -                           'PackageNotifications.PackageBaseID = ?',
> +                           'PackageNotifications.PackageBaseID = ? AND ' +
> +                           'Users.Suspended = 0',
>                             [uid, pkgbase_id])
>          self._recipients = cur.fetchall()
>          cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE
> ' +
> @@ -341,7 +345,7 @@ class ComaintainershipEventNotification(Notification):
>      def __init__(self, conn, uid, pkgbase_id):
>          self._pkgbase = pkgbase_from_id(conn, pkgbase_id)
>          cur = conn.execute('SELECT Email, LangPreference FROM Users ' +
> -                           'WHERE ID = ?', [uid])
> +                           'WHERE ID = ? AND Suspended = 0', [uid])
>          self._to, self._lang = cur.fetchone()
>          super().__init__()
>
> @@ -384,7 +388,8 @@ class DeleteNotification(Notification):
>                             'INNER JOIN PackageNotifications ' +
>                             'ON PackageNotifications.UserID = Users.ID
> WHERE ' +
>                             'PackageNotifications.UserID != ? AND ' +
> -                           'PackageNotifications.PackageBaseID = ?',
> +                           'PackageNotifications.PackageBaseID = ? AND ' +
> +                           'Users.Suspended = 0',
>                             [uid, old_pkgbase_id])
>          self._recipients = cur.fetchall()
>          super().__init__()
> @@ -431,7 +436,8 @@ class RequestOpenNotification(Notification):
>                             'INNER JOIN Users ' +
>                             'ON Users.ID = PackageRequests.UsersID ' +
>                             'OR Users.ID = PackageBases.MaintainerUID ' +
> -                           'WHERE PackageRequests.ID = ?', [reqid])
> +                           'WHERE PackageRequests.ID = ? AND ' +
> +                           'Users.Suspended = 0', [reqid])
>          self._to = aurweb.config.get('options', 'aur_request_ml')
>          self._cc = [row[0] for row in cur.fetchall()]
>          cur = conn.execute('SELECT Comments FROM PackageRequests WHERE ID
> = ?',
> @@ -485,7 +491,8 @@ class RequestCloseNotification(Notification):
>                             'INNER JOIN Users ' +
>                             'ON Users.ID = PackageRequests.UsersID ' +
>                             'OR Users.ID = PackageBases.MaintainerUID ' +
> -                           'WHERE PackageRequests.ID = ?', [reqid])
> +                           'WHERE PackageRequests.ID = ? AND ' +
> +                           'Users.Suspended = 0', [reqid])
>          self._to = aurweb.config.get('options', 'aur_request_ml')
>          self._cc = [row[0] for row in cur.fetchall()]
>          cur = conn.execute('SELECT PackageRequests.ClosureComment, ' +
> @@ -494,7 +501,8 @@ class RequestCloseNotification(Notification):
>                             'FROM PackageRequests ' +
>                             'INNER JOIN RequestTypes ' +
>                             'ON RequestTypes.ID =
> PackageRequests.ReqTypeID ' +
> -                           'WHERE PackageRequests.ID = ?', [reqid])
> +                           'WHERE PackageRequests.ID = ? AND ' +
> +                           'Users.Suspended = 0', [reqid])
>          self._text, self._reqtype, self._pkgbase = cur.fetchone()
>          self._reqid = int(reqid)
>          self._reason = reason
> @@ -541,7 +549,8 @@ class TUVoteReminderNotification(Notification):
>          cur = conn.execute('SELECT Email, LangPreference FROM Users ' +
>                             'WHERE AccountTypeID IN (2, 4) AND ID NOT IN '
> +
>                             '(SELECT UserID FROM TU_Votes ' +
> -                           'WHERE TU_Votes.VoteID = ?)', [vote_id])
> +                           'WHERE TU_Votes.VoteID = ?) AND ' +
> +                           'Users.Suspended = 0', [vote_id])
>          self._recipients = cur.fetchall()
>          super().__init__()
>
> --
> 2.20.1
>
>
Lukas Fleischer July 4, 2020, 12:50 p.m. UTC | #2
On Fri, 03 Jul 2020 at 21:29:16, Kevin Morris wrote:
> The existing notify.py script was grabbing entries regardless
> of user suspension. This has been modified to only send notifications
> to unsuspended users.
> 
> This change was written as a solution to
> https://bugs.archlinux.org/task/65554.
> 
> Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com>
> ---
>  aurweb/scripts/notify.py | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)

Merged into pu, thanks Kevin! From your emails, I assume that all other
patches in this thread are obsolete? I recommend using the -v option of
git-send-email (e.g. -v2, -v3, ...) to mark revisions of the same patch.
Kevin Morris July 4, 2020, 9:15 p.m. UTC | #3
No problem, Lukas! Yeah, basically all my patches except the most recent
one are obsolete.. pretty new to git send-email, wasn't sure about that --
I'll check out -v for future submissions. Thanks for the tip!

On Sat, Jul 4, 2020 at 5:50 AM Lukas Fleischer <lfleischer@archlinux.org>
wrote:

> On Fri, 03 Jul 2020 at 21:29:16, Kevin Morris wrote:
> > The existing notify.py script was grabbing entries regardless
> > of user suspension. This has been modified to only send notifications
> > to unsuspended users.
> >
> > This change was written as a solution to
> > https://bugs.archlinux.org/task/65554.
> >
> > Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com>
> > ---
> >  aurweb/scripts/notify.py | 31 ++++++++++++++++++++-----------
> >  1 file changed, 20 insertions(+), 11 deletions(-)
>
> Merged into pu, thanks Kevin! From your emails, I assume that all other
> patches in this thread are obsolete? I recommend using the -v option of
> git-send-email (e.g. -v2, -v3, ...) to mark revisions of the same patch.
>
Frédéric Mangano-Tarumi July 13, 2020, 2:47 p.m. UTC | #4
Hi Kevin,

Kevin Morris [2020-07-03 18:29:16 -0700]
> diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py
> index 5b18a476..223ed61f 100755
> --- a/aurweb/scripts/notify.py
> +++ b/aurweb/scripts/notify.py
> @@ -171,7 +171,8 @@ class CommentNotification(Notification):
>                             'ON PackageNotifications.UserID = Users.ID WHERE ' +
>                             'Users.CommentNotify = 1 AND ' +
>                             'PackageNotifications.UserID != ? AND ' +
> -                           'PackageNotifications.PackageBaseID = ?',
> +                           'PackageNotifications.PackageBaseID = ? AND ' +
> +                           'Users.Suspended = 0' +,

In that last line, `+,` is not a valid Python syntax. The test suite is
screaming.

Since you’re a new contributor, you might want to read `test/README.md`
and `CONTRIBUTING.md` to prevent these little errors.

Regards,
Lukas Fleischer July 14, 2020, 10:25 p.m. UTC | #5
On Mon, 13 Jul 2020 at 10:47:03, Frédéric Mangano-Tarumi wrote:
> In that last line, `+,` is not a valid Python syntax. The test suite is
> screaming.

Thanks for the pointer Frédéric, I amended the patch.
Lukas Fleischer July 14, 2020, 10:33 p.m. UTC | #6
On Tue, 14 Jul 2020 at 18:25:04, Lukas Fleischer wrote:
> On Mon, 13 Jul 2020 at 10:47:03, Frédéric Mangano-Tarumi wrote:
> > In that last line, `+,` is not a valid Python syntax. The test suite is
> > screaming.
> 
> Thanks for the pointer Frédéric, I amended the patch.

Seems like some tests are still failing, even after fixing the typo.
Kevin, could you please have another look?
Kevin Morris July 15, 2020, 3:40 p.m. UTC | #7
[Re-post to aur-dev]

Yeah. Was planning to, had a bit too much on my plate to get started when I
saw Frederic's first message. I have a bit of an issue at this point -- on
my dev system, I don't run arch on my current dev machine, and
test/README.md seems to hint that this is an issue (dep: libalpm). I've got
an arch VM setup, so I'll test it out in there. I'll reply with findings in
some hours today.

On Tue, Jul 14, 2020 at 3:33 PM Lukas Fleischer <lfleischer@archlinux.org>
wrote:

> On Tue, 14 Jul 2020 at 18:25:04, Lukas Fleischer wrote:
> > On Mon, 13 Jul 2020 at 10:47:03, Frédéric Mangano-Tarumi wrote:
> > > In that last line, `+,` is not a valid Python syntax. The test suite is
> > > screaming.
> >
> > Thanks for the pointer Frédéric, I amended the patch.
>
> Seems like some tests are still failing, even after fixing the typo.
> Kevin, could you please have another look?
>
Kevin Morris July 15, 2020, 4:59 p.m. UTC | #8
Okay -- I'm going to need some time to study/research how these tests work
before I can confidently figure out what's going wrong. I'm on it, not sure
how long it'll take me.

On Wed, Jul 15, 2020 at 8:40 AM Kevin Morris <kevr.gtalk@gmail.com> wrote:

> [Re-post to aur-dev]
>
> Yeah. Was planning to, had a bit too much on my plate to get started when
> I saw Frederic's first message. I have a bit of an issue at this point --
> on my dev system, I don't run arch on my current dev machine, and
> test/README.md seems to hint that this is an issue (dep: libalpm). I've got
> an arch VM setup, so I'll test it out in there. I'll reply with findings in
> some hours today.
>
> On Tue, Jul 14, 2020 at 3:33 PM Lukas Fleischer <lfleischer@archlinux.org>
> wrote:
>
>> On Tue, 14 Jul 2020 at 18:25:04, Lukas Fleischer wrote:
>> > On Mon, 13 Jul 2020 at 10:47:03, Frédéric Mangano-Tarumi wrote:
>> > > In that last line, `+,` is not a valid Python syntax. The test suite
>> is
>> > > screaming.
>> >
>> > Thanks for the pointer Frédéric, I amended the patch.
>>
>> Seems like some tests are still failing, even after fixing the typo.
>> Kevin, could you please have another look?
>>
>
>
> --
> Kevin Morris
> Software Developer
>
> Personal Inquiries: kevr.gtalk@gmail.com
> Personal Phone: (415) 583-9687
>
> Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery,
> Javascript, SQL, Redux
>

Patch

diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py
index 5b18a476..223ed61f 100755
--- a/aurweb/scripts/notify.py
+++ b/aurweb/scripts/notify.py
@@ -124,7 +124,7 @@  class ResetKeyNotification(Notification):
     def __init__(self, conn, uid):
         cur = conn.execute('SELECT UserName, Email, BackupEmail, ' +
                            'LangPreference, ResetKey ' +
-                           'FROM Users WHERE ID = ?', [uid])
+                           'FROM Users WHERE ID = ? AND Suspended = 0', [uid])
         self._username, self._to, self._backup, self._lang, self._resetkey = cur.fetchone()
         super().__init__()
 
@@ -171,7 +171,8 @@  class CommentNotification(Notification):
                            'ON PackageNotifications.UserID = Users.ID WHERE ' +
                            'Users.CommentNotify = 1 AND ' +
                            'PackageNotifications.UserID != ? AND ' +
-                           'PackageNotifications.PackageBaseID = ?',
+                           'PackageNotifications.PackageBaseID = ? AND ' +
+                           'Users.Suspended = 0' +,
                            [uid, pkgbase_id])
         self._recipients = cur.fetchall()
         cur = conn.execute('SELECT Comments FROM PackageComments WHERE ID = ?',
@@ -218,7 +219,8 @@  class UpdateNotification(Notification):
                            'ON PackageNotifications.UserID = Users.ID WHERE ' +
                            'Users.UpdateNotify = 1 AND ' +
                            'PackageNotifications.UserID != ? AND ' +
-                           'PackageNotifications.PackageBaseID = ?',
+                           'PackageNotifications.PackageBaseID = ? AND ' +
+                           'Users.Suspended = 0',
                            [uid, pkgbase_id])
         self._recipients = cur.fetchall()
         super().__init__()
@@ -264,7 +266,8 @@  class FlagNotification(Notification):
                            'INNER JOIN PackageBases ' +
                            'ON PackageBases.MaintainerUID = Users.ID OR ' +
                            'PackageBases.ID = PackageComaintainers.PackageBaseID ' +
-                           'WHERE PackageBases.ID = ?', [pkgbase_id])
+                           'WHERE PackageBases.ID = ? AND ' +
+                           'Users.Suspended = 0', [pkgbase_id])
         self._recipients = cur.fetchall()
         cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' +
                            'ID = ?', [pkgbase_id])
@@ -302,7 +305,8 @@  class OwnershipEventNotification(Notification):
                            'ON PackageNotifications.UserID = Users.ID WHERE ' +
                            'Users.OwnershipNotify = 1 AND ' +
                            'PackageNotifications.UserID != ? AND ' +
-                           'PackageNotifications.PackageBaseID = ?',
+                           'PackageNotifications.PackageBaseID = ? AND ' +
+                           'Users.Suspended = 0',
                            [uid, pkgbase_id])
         self._recipients = cur.fetchall()
         cur = conn.execute('SELECT FlaggerComment FROM PackageBases WHERE ' +
@@ -341,7 +345,7 @@  class ComaintainershipEventNotification(Notification):
     def __init__(self, conn, uid, pkgbase_id):
         self._pkgbase = pkgbase_from_id(conn, pkgbase_id)
         cur = conn.execute('SELECT Email, LangPreference FROM Users ' +
-                           'WHERE ID = ?', [uid])
+                           'WHERE ID = ? AND Suspended = 0', [uid])
         self._to, self._lang = cur.fetchone()
         super().__init__()
 
@@ -384,7 +388,8 @@  class DeleteNotification(Notification):
                            'INNER JOIN PackageNotifications ' +
                            'ON PackageNotifications.UserID = Users.ID WHERE ' +
                            'PackageNotifications.UserID != ? AND ' +
-                           'PackageNotifications.PackageBaseID = ?',
+                           'PackageNotifications.PackageBaseID = ? AND ' +
+                           'Users.Suspended = 0',
                            [uid, old_pkgbase_id])
         self._recipients = cur.fetchall()
         super().__init__()
@@ -431,7 +436,8 @@  class RequestOpenNotification(Notification):
                            'INNER JOIN Users ' +
                            'ON Users.ID = PackageRequests.UsersID ' +
                            'OR Users.ID = PackageBases.MaintainerUID ' +
-                           'WHERE PackageRequests.ID = ?', [reqid])
+                           'WHERE PackageRequests.ID = ? AND ' +
+                           'Users.Suspended = 0', [reqid])
         self._to = aurweb.config.get('options', 'aur_request_ml')
         self._cc = [row[0] for row in cur.fetchall()]
         cur = conn.execute('SELECT Comments FROM PackageRequests WHERE ID = ?',
@@ -485,7 +491,8 @@  class RequestCloseNotification(Notification):
                            'INNER JOIN Users ' +
                            'ON Users.ID = PackageRequests.UsersID ' +
                            'OR Users.ID = PackageBases.MaintainerUID ' +
-                           'WHERE PackageRequests.ID = ?', [reqid])
+                           'WHERE PackageRequests.ID = ? AND ' +
+                           'Users.Suspended = 0', [reqid])
         self._to = aurweb.config.get('options', 'aur_request_ml')
         self._cc = [row[0] for row in cur.fetchall()]
         cur = conn.execute('SELECT PackageRequests.ClosureComment, ' +
@@ -494,7 +501,8 @@  class RequestCloseNotification(Notification):
                            'FROM PackageRequests ' +
                            'INNER JOIN RequestTypes ' +
                            'ON RequestTypes.ID = PackageRequests.ReqTypeID ' +
-                           'WHERE PackageRequests.ID = ?', [reqid])
+                           'WHERE PackageRequests.ID = ? AND ' +
+                           'Users.Suspended = 0', [reqid])
         self._text, self._reqtype, self._pkgbase = cur.fetchone()
         self._reqid = int(reqid)
         self._reason = reason
@@ -541,7 +549,8 @@  class TUVoteReminderNotification(Notification):
         cur = conn.execute('SELECT Email, LangPreference FROM Users ' +
                            'WHERE AccountTypeID IN (2, 4) AND ID NOT IN ' +
                            '(SELECT UserID FROM TU_Votes ' +
-                           'WHERE TU_Votes.VoteID = ?)', [vote_id])
+                           'WHERE TU_Votes.VoteID = ?) AND ' +
+                           'Users.Suspended = 0', [vote_id])
         self._recipients = cur.fetchall()
         super().__init__()