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 <[email protected]>
> ---
> 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!
