[PATCH] Remove PackageNotifications when User is suspended
This path will only trigger when suspending a user through an account page; if the database is managed manually or through another script, PackageNotifications will also need manual modifications if intended. This change was written as a solution to https://bugs.archlinux.org/task/65554. Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com> --- web/lib/acctfuncs.inc.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index d238c0e0..fb7cca8c 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -403,6 +403,10 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$BE="",$H="",$P=" $q.= " WHERE ID = ".intval($UID); $result = $dbh->exec($q); + // Clean up PackageNotifications for $UID as well. + $q = "DELETE FROM PackageNotifications WHERE UserID = ".intval($UID); + $result = $dbh->exec($q); + if (isset($ssh_keys) && count($ssh_keys) > 0) { $ssh_key_result = account_set_ssh_keys($UID, $ssh_keys, $ssh_fingerprints); } else { -- 2.20.1
Updated patch inc which only modifies `scripts/notify.py` and preserves notification records. On Fri, Jul 3, 2020 at 12:02 PM Kevin Morris <kevr.gtalk@gmail.com> wrote:
This path will only trigger when suspending a user through an account page; if the database is managed manually or through another script, PackageNotifications will also need manual modifications if intended.
This change was written as a solution to https://bugs.archlinux.org/task/65554.
Signed-off-by: Kevin Morris <kevr.gtalk@gmail.com> --- web/lib/acctfuncs.inc.php | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index d238c0e0..fb7cca8c 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -403,6 +403,10 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$BE="",$H="",$P=" $q.= " WHERE ID = ".intval($UID); $result = $dbh->exec($q);
+ // Clean up PackageNotifications for $UID as well. + $q = "DELETE FROM PackageNotifications WHERE UserID = ".intval($UID); + $result = $dbh->exec($q); + if (isset($ssh_keys) && count($ssh_keys) > 0) { $ssh_key_result = account_set_ssh_keys($UID, $ssh_keys, $ssh_fingerprints); } else { -- 2.20.1
-- 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
The existing notify.py script was grabbing entries regardless of user suspension. This has been modified to only send notifications to unsuspended users, except for a Trusted User Vote Notification. 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 | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py index 5b18a476..94ca1936 100755 --- a/aurweb/scripts/notify.py +++ b/aurweb/scripts/notify.py @@ -170,6 +170,7 @@ class CommentNotification(Notification): 'FROM Users INNER JOIN PackageNotifications ' + 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.CommentNotify = 1 AND ' + + 'Users.Suspended = 0 AND ' + 'PackageNotifications.UserID != ? AND ' + 'PackageNotifications.PackageBaseID = ?', [uid, pkgbase_id]) @@ -217,6 +218,7 @@ class UpdateNotification(Notification): 'INNER JOIN PackageNotifications ' + 'ON PackageNotifications.UserID = Users.ID WHERE ' + 'Users.UpdateNotify = 1 AND ' + + 'Users.Suspended = 0 AND ' + 'PackageNotifications.UserID != ? AND ' + 'PackageNotifications.PackageBaseID = ?', [uid, pkgbase_id]) @@ -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 -- 2.20.1
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
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
-- 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
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.
-- 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
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, -- fmang
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?
-- 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
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
-- 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
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> --- 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 ' + '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, ' + @@ -541,7 +548,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 Wed, 15 Jul 2020 at 14:45:54, 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> ---
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(-)
Thanks for identifying the problem and sending an updated patch! Replaced the previous one in pu with this version.
participants (3)
-
Frédéric Mangano-Tarumi
-
Kevin Morris
-
Lukas Fleischer