Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling and validation #12

Closed
AidasK opened this issue Apr 12, 2019 · 10 comments
Closed

Error handling and validation #12

AidasK opened this issue Apr 12, 2019 · 10 comments
Assignees
Labels
question Further information is requested

Comments

@AidasK
Copy link

AidasK commented Apr 12, 2019

I have read all the wiki, but error handling and validation is not covered.
Currently I am using this snippet. I don't like it, because it contains two different catch blocks and a check after all of it. The biggest problem is that I can't tell to the user what is wrong with a form if $response->isSuccessful() is not successful, because $response->getErrors() is always empty.

$method = new \Checkout\Models\Payments\TokenSource($request->get('token'));
$payment = new \Checkout\Models\Payments\Payment($method, $order->product_currency);
$payment->amount = bcmul($order->product_price, 100);
$payment->reference = $order->id;
try {
     /** @var Payment $response */
     $response = $checkoutApi->payments()->request($payment);
} catch (CheckoutHttpException $e) {
            throw ValidationException::withMessages(['payment_method' => $e->getErrors() ?: [$e->getMessage()]]);
} catch (CheckoutModelException $e) {
            throw ValidationException::withMessages(['payment_method' => [$e->getMessage()]]);
}
if (!$response->isSuccessful()) {
            throw ValidationException::withMessages(['payment_method' => $response->getErrors() ?: ['Invalid card data']]);
}
$order->status = Order::STATUS_APPROVED;
@aquila-freitas-cko aquila-freitas-cko self-assigned this Apr 23, 2019
@aquila-freitas-cko aquila-freitas-cko added the question Further information is requested label Apr 23, 2019
@aquila-freitas-cko
Copy link
Contributor

aquila-freitas-cko commented Apr 23, 2019

Hi @AidasK ,
In fact, there are some features there are not yet fully documented. I understand your doubt with $response->getErrors(); Let me explain why it is always empty in your case.

Your form is not successful if you get an exception, at that stage the $response object is useless. You will be able to do $e->getErrors() if $e is instance of CheckoutHttpException.

If you disable the http exceptions on the config file, $response->getErrors() will return you the errors without throwing an exception.

Your code would look like this:

$method = new \Checkout\Models\Payments\TokenSource($request->get('token'));
$payment = new \Checkout\Models\Payments\Payment($method, $order->product_currency);
$payment->amount = bcmul($order->product_price, 100);
$payment->reference = $order->id;

/** @var Payment $response **/
$response = $checkoutApi->payments()->request($payment);

if(!$response->isSuccessful()) {
    throw ValidationException::withMessages(['payment_method' => [$response->getErrors()]]);
    /**Obs: you won't have message here.**/
}

$order->status = Order::STATUS_APPROVED;

@AidasK
Copy link
Author

AidasK commented Apr 24, 2019

Thanks for your brief response. It does look cleaner.
My code now looks like this:

        $checkoutApi = new CheckoutApi(
            config('services.checkout.secret_key'),
            config('app.debug'),
            config('services.checkout.public_key'),
            false
        );
        HttpHandler::$throw = false;
        
        $method = new \Checkout\Models\Payments\TokenSource($request->get('token'));
        $payment = new \Checkout\Models\Payments\Payment($method, $order->product_currency);
        $payment->amount = bcmul($order->product_price, 100);
        $payment->reference = \Brand::getId() . '-' . $order->id;
        /** @var Payment $response */
        $response = $checkoutApi->payments()->request($payment);
        if (!$response->isSuccessful()) {
            throw ValidationException::withMessages([
                'payment_method' => $response->getErrors() ?: ['Invalid card data (' . $response->getCode() . ')']
            ]);
        }
        $order->status = Order::STATUS_APPROVED;

It is a pity, that I can not provide config as an array to CheckoutApi class, so it does not look clean

@AidasK AidasK closed this as completed Apr 24, 2019
@AidasK AidasK reopened this Apr 24, 2019
@aquila-freitas-cko
Copy link
Contributor

These are not configurations you want to update all the times, as this will affect how your code is executed with the SDK. You should either edit the default one or specify a custom as fourth parameter.

$checkoutApi = new CheckoutApi(
            config('services.checkout.secret_key'),
            config('app.debug'),
            config('services.checkout.public_key'),
            '/project/var/checkout.ini'
        );

@AidasK
Copy link
Author

AidasK commented Apr 26, 2019

I just don't want any physical files to configure this. It is not a good practice

@aquila-freitas-cko
Copy link
Contributor

aquila-freitas-cko commented Apr 26, 2019

I understand that if you have an automated deployment it is indeed impracticable to have hard files, but passing a long array is also impracticable if someone has to explicitly type the config. I am afraid HttpHandler::$throw = false; will be the only way to set individually this.

@AidasK
Copy link
Author

AidasK commented Apr 26, 2019 via email

@david-fiaty-cko
Copy link
Contributor

david-fiaty-cko commented May 1, 2019

@AidasK, could you please confirm if you are expecting something like this:

$myConfig = array('your config here'); 
$checkoutApi = new CheckoutApi(
            config('services.checkout.secret_key'),
            config('app.debug'),
            config('services.checkout.public_key'),
            $myConfig
        );

If not, please provide a code snippet illustrating your expectations.

@AidasK
Copy link
Author

AidasK commented May 1, 2019

Yes, that would be much better :)

@david-fiaty-cko
Copy link
Contributor

@AidasK, the SDK has been updated to accept a config array in the constructor as described above. We're closing this issue for now, please reopen if needed.

@AidasK
Copy link
Author

AidasK commented May 2, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Development

No branches or pull requests

3 participants