[aur-dev] [PATCH] Add comment undeletion functionality

Lukas Fleischer lfleischer at archlinux.org
Mon Jan 18 18:16:06 UTC 2016


On Mon, 18 Jan 2016 at 00:00:10, Marcel Korpel wrote:
> Only Developers and Trusted Users can undelete comments.
> 
> Signed-off-by: Marcel Korpel <marcel.korpel at gmail.com>
> ---
>  web/html/css/aurweb.css       |  9 +++++++++
>  web/html/pkgbase.php          |  5 +++++
>  web/lib/credentials.inc.php   |  2 ++
>  web/lib/pkgbasefuncs.inc.php  | 31 ++++++++++++++++++++++---------
>  web/lib/pkgfuncs.inc.php      | 12 ++++++++++++
>  web/template/pkg_comments.php | 11 +++++++++++
>  6 files changed, 61 insertions(+), 9 deletions(-)
> 
> [...]
> diff --git a/web/lib/pkgbasefuncs.inc.php b/web/lib/pkgbasefuncs.inc.php
> index 2b1201d..b0854d2 100644
> --- a/web/lib/pkgbasefuncs.inc.php
> +++ b/web/lib/pkgbasefuncs.inc.php
> @@ -934,7 +934,7 @@ function pkgbase_notify ($base_ids, $action=true) {
>   *
>   * @return array Tuple of success/failure indicator and error message
>   */
> -function pkgbase_delete_comment() {
> +function pkgbase_delete_comment($undelete=false) {

Missing documentation for that new parameter?

> [...]
> +       if ($undelete) {
> +               if (can_undelete_comment()) {
> +                       $q = "UPDATE PackageComments ";
> +                       $q.= "SET DelUsersID = NULL, ";
> +                       $q.= "DelTS = NULL ";
> +                       $q.= "WHERE ID = ".intval($comment_id);
> +                       $dbh->exec($q);
> +                       return array(true, __("Comment has been undeleted."));
> +               } else {
> +                       return array(false, __("You are not allowed to undelete this comment."));
> +               }

Reduce nesting:

    if (!can_undelete_comment()) {
    	return array(false, __("You are not allowed to undelete this comment."));
    }

... and then do not branch for the main logic.

>         } else {
> -               return array(false, __("You are not allowed to delete this comment."));
> +               if (can_delete_comment($comment_id)) {
> +                       $q = "UPDATE PackageComments ";
> +                       $q.= "SET DelUsersID = ".$uid.", ";
> +                       $q.= "DelTS = UNIX_TIMESTAMP() ";
> +                       $q.= "WHERE ID = ".intval($comment_id);
> +                       $dbh->exec($q);
> +                       return array(true, __("Comment has been deleted."));
> +               } else {
> +                       return array(false, __("You are not allowed to delete this comment."));
> +               }

Same here.

>         }
>  }
>  
> diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php
> index c2bbe38..4438fc4 100644
> --- a/web/lib/pkgfuncs.inc.php
> +++ b/web/lib/pkgfuncs.inc.php
> @@ -43,6 +43,18 @@ function can_delete_comment_array($comment) {
>  }
>  
>  /**
> + * Determine if the user can undelete a specific package comment
> + *
> + * Only Trusted Users and Developers can undelete comments.
> + * This function is used for both sides of comment undeletion.
> + *
> + * @return bool True if the user can undelete the comment, otherwise false
> + */
> +function can_undelete_comment() {
> +       return has_credential(CRED_COMMENT_UNDELETE);
> +}

Do we really need a new function for this? How about simply using
has_credential(CRED_COMMENT_UNDELETE) at all call sites instead?

> +
> +/**
>   * Determine if the user can edit a specific package comment
>   *
>   * Only the comment submitter, Trusted Users, and Developers can edit
> diff --git a/web/template/pkg_comments.php b/web/template/pkg_comments.php
> index d05c512..679d571 100644
> --- a/web/template/pkg_comments.php
> +++ b/web/template/pkg_comments.php
> @@ -53,6 +53,17 @@ if (!isset($count)) {
>                 ?>
>                 <h4 id="comment-<?= $row['ID'] ?>"<?php if ($is_deleted): ?> class="comment-deleted"<?php endif; ?>>
>                         <?= $heading ?>
> +                       <?php if ($is_deleted && can_undelete_comment()): ?>
> +                               <form class="undelete-comment-form" method="post" action="<?= htmlspecialchars(get_pkgbase_uri($pkgbase_name), ENT_QUOTES); ?>">
> +                                       <fieldset style="display:inline;">
> +                                               <input type="hidden" name="action" value="do_UndeleteComment" />
> +                                               <input type="hidden" name="comment_id" value="<?= $row['ID'] ?>" />
> +                                               <input type="hidden" name="token" value="<?= htmlspecialchars($_COOKIE['AURSID']) ?>" />
> +                                               <input type="submit" class="undelete-comment" value="<?= __('Undelete') ?>" name="submit" />
> +                                       </fieldset>
> +                               </form>
> +                       <?php endif;?>
> [...]

I wonder why this is not located next to the other comment action icons?
Ideally, there should be a "undelete" icon at the location where we
usually have the delete icon. On IRC, you mentioned that placing it
there helps "prevent erroneously clicking" but I do not think it really
does. In which way is the restore icon different to the delete icon?

Patch looks good otherwise. Thank you for your work!


More information about the aur-dev mailing list