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

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 --- 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)) { ?>

<?= $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!