[aur-dev] [PATCH v2] Implement capability to pin comments above others

Johannes Löthberg johannes at kyriasis.com
Sat Dec 5 20:01:32 UTC 2015


>Adds functions and credential information to pin comments before others.
>
>This needs two extra columns (PinnedTS and PinnedUsersID) to the
>PackageComments table.
>
>Implements FS#10863
>
>Signed-off-by: Mark Weiman <mark.weiman at markzz.com>

For starters, you've used spaces instead of tabs everywhere for indentation, so
you're going to have to fix that, and won't comment on every instance of it, and it
seems you forgot to add the details of the new SQL schema to schema/ and upgrading/

<snip>

>   * @param bool $include_deleted True if deleted comments should be included
>+ * @param bool $show_only_pinned True when only pinned comments are to be included
>   *
>   * @return array All package comment information for a specific package base
>   */
>-function pkgbase_comments($base_id, $limit, $include_deleted) {
>+function pkgbase_comments($base_id, $limit, $include_deleted, $show_only_pinned=false) {

$show_only_pinned is misleading since this function doesn't show anything at all.
Something like $only_pinned would be a more accurate name.

<snip>

>  /**
>+ * Pin a package comment
>+ *
>+ * @return array Tuple of success/failure indicator and error message
>+ */
>+function pkgbase_pin_comment() {
>+       $uid = uid_from_sid($_COOKIE["AURSID"]);
>+       if (!$uid) {
>+               return array(false, __("You must be logged in before you can edit package information."));
>+       }
>+

<snip>

Both here and below you should check that the user has the right credentials,
otherwise anyone that's logged in could send a POST request to pin any comment.

Should probably set a max number of pinnable comments as well.

<snip>

>index 21ce16f..8f1fb9f 100644
>--- a/web/template/pkg_comments.php
>+++ b/web/template/pkg_comments.php
>@@ -6,13 +6,20 @@ if (isset($row['BaseID'])) {
>  	/* On a package base details page. */
>  	$base_id = $row['ID'];
>  }
>-$include_deleted = has_credential(CRED_COMMENT_VIEW_DELETED);
>-$count = pkgbase_comments_count($base_id, $include_deleted);
>+if (!isset($count)) {
>+	$count = pkgbase_comments_count($base_id, $include_deleted);
>+}
>  ?>
>  <div id="news">
>  	<h3>
>-		<a href="<?= htmlentities(get_pkgbase_uri($pkgbase_name), ENT_QUOTES) . '?' . mkurl('comments=all') ?>" title="<?= __('View all comments' , $count) ?> (<?= $count ?>)"><?= __('Latest Comments') ?></a>
>-		<span class="arrow"></span>
>+       <?php if (!isset($comments)): ?>
>+    	    <?php $comments = $pinned ?>
>+       		<a href="<?= htmlentities(get_pkgbase_uri($pkgbase_name), ENT_QUOTES) . '?' . mkurl('comments=all') ?>" title="<?= __('View all comments' , $count) ?> (<?= $count ?>)"><?= __('Pinned Comments') ?></a>
>+               <span class="arrow"></span>
>+               <?php else: ?>
>+                <a href="<?= htmlentities(get_pkgbase_uri($pkgbase_name), ENT_QUOTES) . '?' . mkurl('comments=all') ?>" title="<?= __('View all comments' , $count) ?> (<?= $count ?>)"><?= __('Latest Comments') ?></a>
>+               <span class="arrow"></span>
>+               <?php endif; ?>
>  	</h3>

The indentation is really all over the place here.

<snip>

>+                               alt="<?= __('Unin comment') ?>" title="<?= __('Pin comment') ?>" name="submit" value="1" />

"Unin comment" and "Pin comment" instead of "Unpin comment"

-- 
Sincerely,
  Johannes Löthberg
  PGP Key ID: 0x50FB9B273A9D0BB5
  https://theos.kyriasis.com/~kyrias/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1768 bytes
Desc: not available
URL: <https://lists.archlinux.org/pipermail/aur-dev/attachments/20151205/033145d6/attachment.asc>


More information about the aur-dev mailing list