[PATCH v2] Exclude suspended Users from being notified

2020-07-15 Thread Kevin Morris
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 
---

The issue was in OwnershipEventNotification: `Users.Suspended = 0` was
included in the bottom-most SQL fetch, which is not a fetch of the
recipients. That part of the stanza was removed, and `Users.Suspended = 0`
is only ever used when fetching recipients for notifications.

 aurweb/scripts/notify.py | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py
index 5b18a476..644a4b1f 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 ' +
 

Re: [PATCH] Exclude suspended Users from being notified

2020-07-15 Thread Kevin Morris
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  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 
> 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.gt...@gmail.com
> Personal Phone: (415) 583-9687
>
> Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery,
> Javascript, SQL, Redux
>


-- 
Kevin Morris
Software Developer

Personal Inquiries: kevr.gt...@gmail.com
Personal Phone: (415) 583-9687

Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery,
Javascript, SQL, Redux


Re: [PATCH] Exclude suspended Users from being notified

2020-07-15 Thread Kevin Morris
[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 
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.gt...@gmail.com
Personal Phone: (415) 583-9687

Technologies: C, C++, Python, Django, Ruby, Rails, ReactJS, jQuery,
Javascript, SQL, Redux