| Message ID | 20200704012916.16235-1-kevr.gtalk@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Exclude suspended Users from being notified | expand |
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 > >
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.
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. >
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,
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.
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?
[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? >
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 >
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__()
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(-)