[aur-dev] [PATCH 1/2] Merge user ids in CommentNotify

[Lukas Fleischer]

On Wed, 17 Jun 2015 at 02:08:19, Marcel Korpel wrote:
> Fixes FS#27687.
> 
> Signed-off-by: Marcel Korpel <marcel.korpel at gmail.com>
> ---
>  web/lib/pkgbasefuncs.inc.php | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/web/lib/pkgbasefuncs.inc.php b/web/lib/pkgbasefuncs.inc.php
> index c8c99eb..e255cb4 100644
> --- a/web/lib/pkgbasefuncs.inc.php
> +++ b/web/lib/pkgbasefuncs.inc.php
> @@ -508,6 +508,24 @@ function pkgbase_delete ($base_ids, $merge_base_id, $via, $grant=false) {
>                 $q.= "WHERE PackageBaseID IN (" . implode(",", $base_ids) . ")";
>                 $dbh->exec($q);
>  
> +               /* Merge comment notifications */
> +               $q = "SELECT UserID FROM CommentNotify ";
> +               $q.= "WHERE PackageBaseID IN (" . implode(",", $base_ids) . ")";
> +               $result = $dbh->query($q);
> +
> +               while ($uid = $result->fetchColumn(0)) {
> +                       /* Check if a user already gets notifications from $merge_base_id */
> +                       $q = "SELECT COUNT(*) FROM CommentNotify WHERE ";
> +                       $q .= "UserID = $uid AND PackageBaseID = " . intval($merge_base_id);
> +
> +                       $result_notif = $dbh->query($q);
> +                       if ($result_notif->fetchColumn() == 0) {
> +                               $q = "INSERT INTO CommentNotify (PackageBaseID, UserID) VALUES (";
> +                               $q.= intval($merge_base_id) . ", $uid)";
> +                               $dbh->exec($q);
> +                       }
> +               }
> +

Looks good at a glance. Two things come into my mind, though:

1. We could greatly reduce the number of SQL queries by using an
   approach similar to what we do in the AUR blacklist updater. Fetch
   the lists of notified users for both packages, compute the set
   difference, then only insert the remaining users. This applies to
   many other parts of the AUR code base, too.

2. Since it won't make it into 4.0.0 anyway, might this be a good
   candidate for being ported to Python? All the package actions
   (disown, delete, merge) could be implemented in a Python script that
   returns an exit code and prints error messages to stderr. We could
   invoke that script from PHP code and fetch the exit status, as well
   as the error message quite easily. When that works fine, it would be
   easy to add a command to our SSH interface that even allows for
   performing package actions via the command line.

Opinions?

>                 /* Merge votes */
>                 foreach ($base_ids as $base_id) {
>                         $q = "UPDATE PackageVotes ";
> -- 
> 2.4.3