[aur-dev] [PATCH 1/2] Add capability to pin comments above others

[Lukas Fleischer]

On Sat, 28 Nov 2015 at 01:44:26, Mark Weiman wrote:
> Adds functions and credential information to pin comments before others.
> 
> This needs two extra columns (PinnedTS and PinnedUsersID) to the
> PackageComments table.
> 
> Signed-off-by: Mark Weiman <mark.weiman at markzz.com>
> ---
>  web/html/pkgbase.php         |   4 ++
>  web/lib/credentials.inc.php  |   2 +
>  web/lib/pkgbasefuncs.inc.php | 123 ++++++++++++++++++++++++++++++++++++++++++-
>  web/lib/pkgfuncs.inc.php     |  54 +++++++++++++++++++
>  4 files changed, 182 insertions(+), 1 deletion(-)
> 

First of all, thanks for working on this! I planned on implementing this
for a while but never got around to doing it.

The main difference of this series to what I had in mind is that you
allow for pinning multiple comments whereas I planned to have a single
pinned comment (per package base) only. Can you think of a use case
where having several pinned comments is desirable? Can't the maintainer
simply edit the pinned comment and add new information? Would we ever
want to pin a comment from a user other than the current maintainer?

More comments on the implementation below.

> [...]
>  /**
> + * Get all pinned package comment information for a specific package base
> + *
> + * @param int $base_id The package base ID to get comments for
> + * @param int $limit Maximum number of comments to return (0 means unlimited)
> + * @param bool $include_deleted True if deleted comments should be included
> + *
> + * @return array Pinned package comment information for specific package base
> + */
> +function pkgbase_pinned_comments($base_id, $limit, $include_deleted) {
> [...]

I did not have a very detailed look at the second patch in this series
but is this really needed? Can't we simply retrieve all comments by
calling pkgbase_comments() and order them such that pinned comments are
displayed first?

> [...]
> +/**
> + * Display pinned comments before the other comments on a package page
> + *
> + * @param string $base_id The package base ID to add the comment on
> + * @param string $uid The user ID of the individual who pinned the comment
> + * @param string $comment_id The comment id to be pinned
> + *
> + * @return void
> + */
> +function pkgbase_disp_pin_comment($base_id, $uid, $comment_id) {
> +       $dbh = DB::connect();
> +       
> +       $q = "UPDATE PackageComments ";
> +       $q.= "SET PinnedTS = UNIXTIMESTAMP()), PinnedUsersID=" . $uid . " ";
> +       $q.= "WHERE ID = " . intval($comment_id);

After having read the function name and the comment, I would have
expected this function to print a pinned comment but that does not seem
to be what this function does... Anyway, what is the difference between
this function and pkgbase_pin_comment() below? Can't we somehow merge
those functions?

> [...]
>  /**
> + * Determine if the user can pin a specific package comment
> + *
> + * Only the Package Maintainer, Trusted Users, and Developers can pin
> + * comments. This function is used for the backend side of comment pinning.
> + * 
> + * @param string $comment_id The comment ID in the database
> + *
> + * @return bool True if the user can pin the comment, otherwise false
> + */
> +function can_pin_comment($comment_id=0) {
> +    $dbh = DB::connect();
> +    
> +    $q = "SELECT MaintainerUID FROM PackageBases AS pb ";
> +    $q.= "LEFT JOIN PackageComments AS pc ON pb.ID = pc.PackageBaseID ";
> +    $q.= "WHERE pc.ID = " . intval($comment_id);
> +    $result = $dbh->query($q);
> +    
> +    if (!$result) {
> +        return false;
> +    }
> +    
> +    $uid = $result->fetch(PDO::FETCH_COLUMN, 0);
> +    
> +    return has_credential(CRED_COMMENT_PIN, array($uid));
> +}
> +
> +/**
> + * Determine if the user can edit a specific package comment using an array
> + *
> + * Only the Package Maintainer, Trusted Users, and Developers can pin
> + * comments. This function is used for the frontend side of comment pinning.
> + *
> + * @param array $comment All database information relating a specific comment
> + *
> + * @return bool True if the user can edit the comment, otherwise false
> + */
> +function can_pin_comment_array($comment) {
> +    $dbh = DB::connect();
> +    
> +    $q = "SELECT MaintainerUID FROM PackageBases AS pb ";
> +    $q.= "LEFT JOIN PackageComments AS pc ON pb.ID = pc.PackageBaseID ";
> +    $q.= "WHERE pc.ID = " . intval($comment['ID']);
> +    $result = $dbh->query($q);
> +    
> +    if (!$result) {
> +        return false;
> +    }
> +    
> +    $uid = $result->fetch(PDO::FETCH_COLUMN, 0);
> +
> +    return has_credential(CRED_COMMENT_PIN, array($uid));
> +}
> [...]

Isn't this equivalent to

    function can_pin_comment_array($comment) {
        return can_pin_comment($comment['ID']);
    }

...? I also think that the function name is slightly misleading. I would
have expected that can_pin_comment_array() accepts an array of comment
IDs and returns an array of boolean values. Do we need that function at
all? Can't we simply call can_pin_comment($comment['ID']) instead?

Regards,
Lukas