[aur-dev] [PATCH v3 3/4] aurjson.class.php: Add method load_comment

Lukas Fleischer lfleischer at archlinux.org
Tue Jul 21 20:33:31 UTC 2015


Commit message needs to be changed. Would also be nice to add 1-2
sentences that explain how this is used.

On Tue, 21 Jul 2015 at 21:56:58, Marcel Korpel wrote:
> Signed-off-by: Marcel Korpel <marcel.korpel at gmail.com>
> ---
> Changes since v2:
> * Drop save_comment() method.
> * Rename load_comment() to get_comment_form().
> * Provide JSDoc.
> * Always use the actual parameter $http_data instead of superglobals.
> * Always use intval() on integer parameters.
> * Provide a better error handling interface.
> * Use early returns to be able to use less deep nesting.
> 
>  web/lib/aurjson.class.php | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
> index a272741..6a51597 100644
> --- a/web/lib/aurjson.class.php
> +++ b/web/lib/aurjson.class.php
> @@ -14,7 +14,7 @@ class AurJSON {
>         private $version = 1;
>         private static $exposed_methods = array(
>                 'search', 'info', 'multiinfo', 'msearch', 'suggest',
> -               'suggest-pkgbase'
> +               'suggest-pkgbase', 'get-comment-form'
>         );
>         private static $exposed_fields = array(
>                 'name', 'name-desc'
> @@ -477,5 +477,46 @@ class AurJSON {
>  
>                 return json_encode($result_array);
>         }
> +
> +       /**
> +        * Get the HTML markup of the comment form.
> +        *
> +        * @param string $http_data Query parameters.
> +        *
> +        * @return string The JSON formatted response.
> +        */
> +       private function get_comment_form($http_data) {
> +               $comment_id = intval($http_data['arg']);

Can we move this further down so that $comment_id, $base_id and
$pkgbase_name are obtained (and sanitized) at the "same time"?

> +
> +               if (!isset($http_data['base_id']) && isset($http_data['pkgbase_name'])) {

This check looks wrong. Did you forget to amend the second part of the
condition?

> +                       return json_encode(false);
> +               }
> +
> +               $base_id = intval($http_data['base_id']);
> +               $pkgbase_name = $http_data['pkgbase_name'];
> +
> +               list($user_id, $comment) = comment_by_id($comment_id);
> +
> +               if (!has_credential(CRED_COMMENT_EDIT, array($user_id))) {
> +                       $output = array(
> +                               'success' => 0,
> +                               'error' => __('You do not have the right to edit this comment.'));

Just a minor nit but we usually indent multiline arrays like this:

    $output = array(
    	'success' => 0,
    	'error' => __('You do not have the right to edit this comment.')
    );


> +                       return json_encode($output);
> +               } elseif (is_null($comment)) {
> +                       $output = array(
> +                               'success' => 0,
> +                               'error' => __('Comment does not exist.'));

Same here.

> +                       return json_encode($output);
> +               }
> +
> +               ob_start();
> +               include('pkg_comment_form.php');
> +               $html = ob_get_clean();
> +               $output = array(
> +                       'success' => 1,
> +                       'form' => $html);

Same here.

> +
> +               return json_encode($output);
> +       }
>  }
>  
> -- 
> 2.4.6


More information about the aur-dev mailing list