Re: [PATCH] Exclude suspended Users from being notified
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
[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
Re: [PATCH] Exclude suspended Users from being notified
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: [PATCH] Exclude suspended Users from being notified
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.
Re: [PATCH] Exclude suspended Users from being notified
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
Re: [PATCH] Exclude suspended Users from being notified
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 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 > > --- > > 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.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
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 > --- > 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.
Re: [PATCH] Exclude suspended Users from being notified
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 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 > --- > 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() >