[aur-dev] [PATCH] send emails when delteing packages
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- web/lib/pkgfuncs.inc.php | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 43 insertions(+), 0 deletions(-) diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php index 3e89fa3..b6d4d08 100644 --- a/web/lib/pkgfuncs.inc.php +++ b/web/lib/pkgfuncs.inc.php @@ -710,6 +710,49 @@ function pkg_delete ($atype, $ids, $mergepkgid, $dbh=NULL) { } if ($mergepkgid) { + $mergepkgname = pkgname_from_id($mergepkgid, $dbh); + } + + # Send email notifications + foreach ($ids as $pkgid) { + $q = 'SELECT CommentNotify.*, Users.Email '; + $q.= 'FROM CommentNotify, Users '; + $q.= 'WHERE Users.ID = CommentNotify.UserID '; + $q.= 'AND CommentNotify.UserID != ' . uid_from_sid($_COOKIE['AURSID']) . ' '; + $q.= 'AND CommentNotify.PkgID = ' . intval($pkgid); + $result = db_query($q, $dbh); + $bcc = array(); + + if (mysql_num_rows($result)) { + while ($row = mysql_fetch_assoc($result)) { + array_push($bcc, $row['Email']); + } + + $q = 'SELECT Packages.* '; + $q.= 'FROM Packages '; + $q.= 'WHERE Packages.ID = ' . intval($pkgid); + $result = db_query($q, $dbh); + $row = mysql_fetch_assoc($result); + + # TODO: native language emails for users, based on their prefs + # Simply making these strings translatable won't work, users would be + # getting emails in the language that the user who posted the comment was in + $body = ""; + if ($mergepkgid) { + $body .= username_from_sid($_COOKIE['AURSID']) . " merged \"".$row["Name"]."\" into \"$mergepkgname\".\n\n"; + $body .= "\n\n---\nYou will no longer receive notifications about this package, please go to https://aur.archlinux.org/packages.php?ID=".$mergepkgid." and click the Notify button if you wish to recieve them again."; + } else { + $body .= username_from_sid($_COOKIE['AURSID']) . " deleted \"".$row["Name"]."\".\n\n"; + $body .= "\n\n---\nYou will no longer receive notifications about this package."; + } + $body = wordwrap($body, 70); + $bcc = implode(', ', $bcc); + $headers = "Bcc: $bcc\nReply-to: nobody@archlinux.org\nFrom: aur-notify@archlinux.org\nX-Mailer: AUR\n"; + @mail(' ', "AUR Package deleted: " . $row['Name'], $body, $headers); + } + } + + if ($mergepkgid) { /* Merge comments */ $q = "UPDATE PackageComments "; $q.= "SET PackageID = " . intval($mergepkgid) . " "; -- 1.7.6.1
On Thu, Sep 08, 2011 at 10:45:33AM +0200, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- web/lib/pkgfuncs.inc.php | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 43 insertions(+), 0 deletions(-)
As I already stated in another thread, I'm not really sure if we want this and if this is the Arch way of informing users. How will we handle mass-deletes in future if users expect mail notifications on package removal (we currently run a simple DELETE query on the database)? Keep in mind that this might be confusing if a package is moved to the binary repositories, also. Trusted Users and/or developers should at least drop one more comment before moving a package in this case. +/-0 from me. Any other opinions (I'll review the patch anyway)?
diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php index 3e89fa3..b6d4d08 100644 --- a/web/lib/pkgfuncs.inc.php +++ b/web/lib/pkgfuncs.inc.php @@ -710,6 +710,49 @@ function pkg_delete ($atype, $ids, $mergepkgid, $dbh=NULL) { }
if ($mergepkgid) { + $mergepkgname = pkgname_from_id($mergepkgid, $dbh); + } + + # Send email notifications + foreach ($ids as $pkgid) { + $q = 'SELECT CommentNotify.*, Users.Email '; + $q.= 'FROM CommentNotify, Users '; + $q.= 'WHERE Users.ID = CommentNotify.UserID '; + $q.= 'AND CommentNotify.UserID != ' . uid_from_sid($_COOKIE['AURSID']) . ' '; + $q.= 'AND CommentNotify.PkgID = ' . intval($pkgid);
No real need to convert to an int here. We already sanitized the "$ids" array before.
+ $result = db_query($q, $dbh); + $bcc = array(); + + if (mysql_num_rows($result)) {
Uh, we should really get rid of all mysql_*() invocations and write wrappers for fetching results as soon as possible...
+ while ($row = mysql_fetch_assoc($result)) { + array_push($bcc, $row['Email']); + } + + $q = 'SELECT Packages.* '; + $q.= 'FROM Packages '; + $q.= 'WHERE Packages.ID = ' . intval($pkgid); + $result = db_query($q, $dbh); + $row = mysql_fetch_assoc($result);
Any reason not to use pkgname_from_id() here? :)
+ + # TODO: native language emails for users, based on their prefs + # Simply making these strings translatable won't work, users would be + # getting emails in the language that the user who posted the comment was in + $body = ""; + if ($mergepkgid) { + $body .= username_from_sid($_COOKIE['AURSID']) . " merged \"".$row["Name"]."\" into \"$mergepkgname\".\n\n"; + $body .= "\n\n---\nYou will no longer receive notifications about this package, please go to https://aur.archlinux.org/packages.php?ID=".$mergepkgid." and click the Notify button if you wish to recieve them again."; + } else { + $body .= username_from_sid($_COOKIE['AURSID']) . " deleted \"".$row["Name"]."\".\n\n"; + $body .= "\n\n---\nYou will no longer receive notifications about this package."; + } + $body = wordwrap($body, 70); + $bcc = implode(', ', $bcc); + $headers = "Bcc: $bcc\nReply-to: nobody@archlinux.org\nFrom: aur-notify@archlinux.org\nX-Mailer: AUR\n"; + @mail(' ', "AUR Package deleted: " . $row['Name'], $body, $headers);
So this means we will send a whole bunch of notifications if, say, forty-two packages are deleted at once? I do not really insist on bundling mails since we will not mass-delete packages very often. We should also ensure that none of the fields included in the headers might contain a newline in any case. I don't want sigurd to become a spam catapult. Having a look at valid_email(), which is used to validate mail addresses during the register process, it doesn't seem like there's any possibility to inject a line break here, though.
+ } + } + + if ($mergepkgid) { /* Merge comments */ $q = "UPDATE PackageComments "; $q.= "SET PackageID = " . intval($mergepkgid) . " "; -- 1.7.6.1
Thanks!
On Thu, Sep 8, 2011 at 6:54 AM, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
On Thu, Sep 08, 2011 at 10:45:33AM +0200, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- web/lib/pkgfuncs.inc.php | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 43 insertions(+), 0 deletions(-)
As I already stated in another thread, I'm not really sure if we want this and if this is the Arch way of informing users.
How will we handle mass-deletes in future if users expect mail notifications on package removal (we currently run a simple DELETE query on the database)? Keep in mind that this might be confusing if a package is moved to the binary repositories, also. Trusted Users and/or developers should at least drop one more comment before moving a package in this case.
+/-0 from me. Any other opinions (I'll review the patch anyway)?
diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php index 3e89fa3..b6d4d08 100644 --- a/web/lib/pkgfuncs.inc.php +++ b/web/lib/pkgfuncs.inc.php @@ -710,6 +710,49 @@ function pkg_delete ($atype, $ids, $mergepkgid, $dbh=NULL) { }
if ($mergepkgid) { + $mergepkgname = pkgname_from_id($mergepkgid, $dbh); + } + + # Send email notifications + foreach ($ids as $pkgid) { + $q = 'SELECT CommentNotify.*, Users.Email '; + $q.= 'FROM CommentNotify, Users '; + $q.= 'WHERE Users.ID = CommentNotify.UserID '; + $q.= 'AND CommentNotify.UserID != ' . uid_from_sid($_COOKIE['AURSID']) . ' '; + $q.= 'AND CommentNotify.PkgID = ' . intval($pkgid);
No real need to convert to an int here. We already sanitized the "$ids" array before.
+ $result = db_query($q, $dbh); + $bcc = array(); + + if (mysql_num_rows($result)) {
Uh, we should really get rid of all mysql_*() invocations and write wrappers for fetching results as soon as possible... Not to mention num_rows is a bad idea anyway here. Loop your results and then check that array for size > 0 rather than having to ask the database for a number you don't actually care about (the only thing cared about is the existence of at least one row).
+ while ($row = mysql_fetch_assoc($result)) { + array_push($bcc, $row['Email']); + } + + $q = 'SELECT Packages.* '; + $q.= 'FROM Packages '; + $q.= 'WHERE Packages.ID = ' . intval($pkgid); + $result = db_query($q, $dbh); + $row = mysql_fetch_assoc($result);
Any reason not to use pkgname_from_id() here? :)
+ + # TODO: native language emails for users, based on their prefs + # Simply making these strings translatable won't work, users would be + # getting emails in the language that the user who posted the comment was in + $body = ""; + if ($mergepkgid) { + $body .= username_from_sid($_COOKIE['AURSID']) . " merged \"".$row["Name"]."\" into \"$mergepkgname\".\n\n"; + $body .= "\n\n---\nYou will no longer receive notifications about this package, please go to https://aur.archlinux.org/packages.php?ID=".$mergepkgid." and click the Notify button if you wish to recieve them again."; + } else { + $body .= username_from_sid($_COOKIE['AURSID']) . " deleted \"".$row["Name"]."\".\n\n"; + $body .= "\n\n---\nYou will no longer receive notifications about this package."; + } + $body = wordwrap($body, 70); + $bcc = implode(', ', $bcc); + $headers = "Bcc: $bcc\nReply-to: nobody@archlinux.org\nFrom: aur-notify@archlinux.org\nX-Mailer: AUR\n"; + @mail(' ', "AUR Package deleted: " . $row['Name'], $body, $headers);
So this means we will send a whole bunch of notifications if, say, forty-two packages are deleted at once? I do not really insist on bundling mails since we will not mass-delete packages very often.
We should also ensure that none of the fields included in the headers might contain a newline in any case. I don't want sigurd to become a spam catapult. Having a look at valid_email(), which is used to validate mail addresses during the register process, it doesn't seem like there's any possibility to inject a line break here, though.
+ } + } + + if ($mergepkgid) { /* Merge comments */ $q = "UPDATE PackageComments "; $q.= "SET PackageID = " . intval($mergepkgid) . " "; -- 1.7.6.1
Thanks!
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- web/lib/pkgfuncs.inc.php | 38 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 38 insertions(+), 0 deletions(-) diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php index 3e89fa3..2a83306 100644 --- a/web/lib/pkgfuncs.inc.php +++ b/web/lib/pkgfuncs.inc.php @@ -710,6 +710,44 @@ function pkg_delete ($atype, $ids, $mergepkgid, $dbh=NULL) { } if ($mergepkgid) { + $mergepkgname = pkgname_from_id($mergepkgid, $dbh); + } + + # Send email notifications + foreach ($ids as $pkgid) { + $q = 'SELECT CommentNotify.*, Users.Email '; + $q.= 'FROM CommentNotify, Users '; + $q.= 'WHERE Users.ID = CommentNotify.UserID '; + $q.= 'AND CommentNotify.UserID != ' . uid_from_sid($_COOKIE['AURSID']) . ' '; + $q.= 'AND CommentNotify.PkgID = ' . $pkgid; + $result = db_query($q, $dbh); + $bcc = array(); + + while ($row = mysql_fetch_assoc($result)) { + array_push($bcc, $row['Email']); + } + if (!empty($bcc)) { + $pkgname = pkgname_from_id($pkgid); + + # TODO: native language emails for users, based on their prefs + # Simply making these strings translatable won't work, users would be + # getting emails in the language that the user who posted the comment was in + $body = ""; + if ($mergepkgid) { + $body .= username_from_sid($_COOKIE['AURSID']) . " merged \"".$pkgname."\" into \"$mergepkgname\".\n\n"; + $body .= "You will no longer receive notifications about this package, please go to https://aur.archlinux.org/packages.php?ID=".$mergepkgid." and click the Notify button if you wish to recieve them again."; + } else { + $body .= username_from_sid($_COOKIE['AURSID']) . " deleted \"".$pkgname."\".\n\n"; + $body .= "You will no longer receive notifications about this package."; + } + $body = wordwrap($body, 70); + $bcc = implode(', ', $bcc); + $headers = "Bcc: $bcc\nReply-to: nobody@archlinux.org\nFrom: aur-notify@archlinux.org\nX-Mailer: AUR\n"; + @mail(' ', "AUR Package deleted: " . $pkgname, $body, $headers); + } + } + + if ($mergepkgid) { /* Merge comments */ $q = "UPDATE PackageComments "; $q.= "SET PackageID = " . intval($mergepkgid) . " "; -- 1.7.6.1
On Wed, Sep 14, 2011 at 02:57:27PM +0200, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- web/lib/pkgfuncs.inc.php | 38 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php index 3e89fa3..2a83306 100644 --- a/web/lib/pkgfuncs.inc.php +++ b/web/lib/pkgfuncs.inc.php @@ -710,6 +710,44 @@ function pkg_delete ($atype, $ids, $mergepkgid, $dbh=NULL) { }
if ($mergepkgid) { + $mergepkgname = pkgname_from_id($mergepkgid, $dbh); + } + + # Send email notifications + foreach ($ids as $pkgid) { + $q = 'SELECT CommentNotify.*, Users.Email '; + $q.= 'FROM CommentNotify, Users '; + $q.= 'WHERE Users.ID = CommentNotify.UserID '; + $q.= 'AND CommentNotify.UserID != ' . uid_from_sid($_COOKIE['AURSID']) . ' '; + $q.= 'AND CommentNotify.PkgID = ' . $pkgid; + $result = db_query($q, $dbh); + $bcc = array(); + + while ($row = mysql_fetch_assoc($result)) { + array_push($bcc, $row['Email']); + } + if (!empty($bcc)) { + $pkgname = pkgname_from_id($pkgid); + + # TODO: native language emails for users, based on their prefs + # Simply making these strings translatable won't work, users would be + # getting emails in the language that the user who posted the comment was in + $body = ""; + if ($mergepkgid) { + $body .= username_from_sid($_COOKIE['AURSID']) . " merged \"".$pkgname."\" into \"$mergepkgname\".\n\n"; + $body .= "You will no longer receive notifications about this package, please go to https://aur.archlinux.org/packages.php?ID=".$mergepkgid." and click the Notify button if you wish to recieve them again."; + } else { + $body .= username_from_sid($_COOKIE['AURSID']) . " deleted \"".$pkgname."\".\n\n"; + $body .= "You will no longer receive notifications about this package."; + } + $body = wordwrap($body, 70); + $bcc = implode(', ', $bcc); + $headers = "Bcc: $bcc\nReply-to: nobody@archlinux.org\nFrom: aur-notify@archlinux.org\nX-Mailer: AUR\n"; + @mail(' ', "AUR Package deleted: " . $pkgname, $body, $headers); + } + } + + if ($mergepkgid) { /* Merge comments */ $q = "UPDATE PackageComments "; $q.= "SET PackageID = " . intval($mergepkgid) . " ";
Looks good now. I'm currently working on refactoring package deletion anyway (create a separate deletion page with the possibility to select a reason, like "Moved to the binary repositories." etc.) and will push your patch alongside that patch series. Thanks!
participants (3)
-
Dan McGee
-
Florian Pritz
-
Lukas Fleischer