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

[Lukas Fleischer]

On Tue, 21 Jul 2015 at 17:59:33, Marcel Korpel wrote:
> Signed-off-by: Marcel Korpel <marcel.korpel at gmail.com>
> ---
> Changes since v1:
> * Drop any POST method.
> * Drop save_comment() method and use the normal web form for submission.
> 
>  web/lib/aurjson.class.php | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php
> index a272741..a4f78d4 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', 'load-comment'

I'd prefer get-comment (or fetch-comment) instead of load-comment.

>         );
>         private static $exposed_fields = array(
>                 'name', 'name-desc'
> @@ -477,5 +477,29 @@ class AurJSON {
>  
>                 return json_encode($result_array);
>         }
> +
> +       private function load_comment($http_data) {
> +               $comment_id = $http_data['arg'];

Please cast this into an integer value using intval(), even if it is
quoted or converted inside some helper function later.

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

Reduce nesting depth:

    if (!isset($_GET['base_id']) || !isset($_GET['pkgbase_name'])) {
    	return json_encode(false);
    }

> +                       $base_id = $_GET['base_id'];

Cast this into an integer, see above.

> +                       $pkgbase_name = $_GET['pkgbase_name'];
> +
> +                       list($user_id, $comment) = comment_by_id($comment_id);
> +
> +                       if (!has_credential(CRED_COMMENT_EDIT, array($user_id)) || is_null($comment)) {
> +                               $output = false;

Reduce nesting depth. Early return here, no need for an else-block
below.

> +                       } else {
> +                               $no_header = true;

This won't be needed when implementing the idea I mentioned in my reply
to patch 1/3.

> +                               ob_start();
> +                               include('pkg_comment_form.php');
> +                               $html = ob_get_clean();
> +                               $output = array('form' => $html);

We can directly return here when adding early returns for the error
cases. I wonder whether it is a good idea to provide different data
types, depending on whether an error occurred or not.

Would it make sense to use json_results() and json_error() here?

> +                       }
> +               } else {
> +                       $output = false;
> +               }
> +
> +               return json_encode($output);
> +       }
>  }
>  
> -- 
> 2.4.6