firebed / aade-mydata

Interface for ΑΑΔΕ myDATA invoicing REST API. It handles all the boilerplate code for sending, cancelling and requesting invoices.
https://docs.invoicemaker.gr/getting-started
MIT License
50 stars 20 forks source link

Some considerations... #4

Closed ghost closed 10 months ago

ghost commented 1 year ago

Thank you for taking the time to write and publish your library, it can be quite useful to some devs.

I've got some considerations:

It would be helpful, if you could backport your library for PHP 7.4 and if guzzlehttp is not installed, then fallback to plain curl functions. That way, your library would be usable by a lot more projects!

Thank you.

firebed commented 1 year ago

Thank you for your feedback and for taking the time to consider using my library. I understand that requiring PHP 8.1 and guzzlehttp may limit its use in some environments, and I appreciate your suggestions for making it more widely compatible.

I will definitely consider backporting the library to support PHP 7.4 and adding a fallback to the curl functions if guzzlehttp is not installed. Thank you for bringing this to my attention and for your suggestion. I appreciate your willingness to provide constructive feedback.

lowv-developer commented 1 year ago

Hi @LauraTaylorUK.

I do have problems with Guzzle too. specially in local environment.

cURL error 60: SSL certificate problem: unable to get local issuer certificate (see https://curl.haxx.se/libcurl/c/libcurl-errors.html)

There is open issue on https://github.com/guzzle/guzzle/issues/1935 will some interesting feedback.

firebed commented 1 year ago

This looks like a descent solution. $client = new \GuzzleHttp\Client(['verify' => false]);

I will add a static method for changing the verify parameter, something like: MyDataRequest::verifyClient($verify)

However, since I haven't encountered this error and am unable to test it, would you mind trying it yourself and seeing if it resolves the issue?

lowv-developer commented 1 year ago

I already did but no luck. I added 'verify' => false in vendor/firebed/aade-mydata/src/Http/MyDataRequest.php

private function client(): Client
    {
       return new Client([
            'headers' => [
               'aade-user-id'              => self::$user_id,
               'Ocp-Apim-Subscription-Key' => self::$subscription_key,
               'Content-Type'              => "text/xml"
           ],
           'verify' => false
       ]);
   }

I even changed the default value in vendor/guzzlehttp/guzzle/src/Client.php to false but this error comes up

Symfony\Component\HttpFoundation\Response::setContent(): Argument #1 ($content) must be of type ?string, Firebed\AadeMyData\Models\RequestedBookInfo given

Edit: Stripe, Mailchimp, Flare... all use Guzzle Edit2: I already had Mailchimp installed but I had no error with Guzzle (maybe i havent used it yet) idk

lowv-developer commented 1 year ago

Actually it did worked!

I should return a Response instead of the $response object in controller (silly mistake!)

changed this

$request = new RequestMyIncome();
$response = $request->handle($dateFrom, $dateTo);

return $response;

to this

$request = new RequestMyIncome();
$response = $request->handle($dateFrom, $dateTo);

return response()->json($response);

so curl issue in local env solved! notice that it is not recommended in production to do verify = false

firebed commented 1 year ago

This looks like a descent solution. $client = new \GuzzleHttp\Client(['verify' => false]);

I will add a static method for changing the verify parameter, something like: MyDataRequest::verifyClient($verify)

However, since I haven't encountered this error and am unable to test it, would you mind trying it yourself and seeing if it resolves the issue?

That's great to hear! In that case, I will proceed with this approach and let the developers decide whether or not to verify it.

ghost commented 10 months ago

Nobody cares, closing.