Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

magento/devdocs#: Layout instructions. Adding more information about cURL [GITBUG] #6823

Merged
merged 7 commits into from
Apr 14, 2020

Conversation

DenisSaltanahmedov
Copy link
Contributor

@DenisSaltanahmedov DenisSaltanahmedov commented Mar 10, 2020

Purpose of this pull request

This pull request (PR) adding more information about using cURL in the Magento in the "Use cURL to run the request".
I think the topic should contain the examples, It will be helpful for developers.

Affected DevDocs pages

Links to Magento source code

whatsnew
Added the 'Using cUrl in Magento' section to the Use cURL to run the request topic.

@devops-devdocs
Copy link
Collaborator

An admin must run tests on this PR before it can be merged.

Copy link
Contributor

@rogyar rogyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @DenisSaltanahmedov. Thanks a lot for the great examples. Please, check some minor recommendations below

@@ -28,6 +28,119 @@ Option | Description
`-T` | Transfers the specified local file to the remote URL.
`-X` | Specifies the request method to use when communicating with the HTTP server. The specified request method is used instead of the default GET method.

### Using cUrl in Magento

Magento provides native functionality for cURL instead of using default php cURL. The class ``Magento\Framework\HTTP\Client\Curl`` can be used to work with HTTP protocol using curl library.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't say it's "native functionality". Rather than that, let's mention that

Magento provides its own service-wrapper for using cURL.

@@ -28,6 +28,119 @@ Option | Description
`-T` | Transfers the specified local file to the remote URL.
`-X` | Specifies the request method to use when communicating with the HTTP server. The specified request method is used instead of the default GET method.

### Using cUrl in Magento

Magento provides native functionality for cURL instead of using default php cURL. The class ``Magento\Framework\HTTP\Client\Curl`` can be used to work with HTTP protocol using curl library.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Magento provides native functionality for cURL instead of using default php cURL. The class ``Magento\Framework\HTTP\Client\Curl`` can be used to work with HTTP protocol using curl library.
Magento provides native functionality for cURL instead of using default php cURL. The class ``Magento\Framework\HTTP\Client\Curl`` can be used to work with HTTP protocol using cURL library.

public function __construct(
Magento\Framework\HTTP\Client\Curl $curl
) {
$this->_curl = $curl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove the underscore from the property name, so we have $this->curl. PSR-2 is really strict about that:

Property names SHOULD NOT be prefixed with a single underscore to indicate protected or private visibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This recommendation also applies to all other examples

$result = $this->_curl->getBody();
```

Where the ``$url`` is the endpoint url, ``$params`` an array, the extra parameters can be added in the url.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may extend the $params argument description. Instead of saying that it's just an "array" I would say that it's an array of data that is being sent via the POST request.


#### Set curl header using setHeaders method

The ``setHeaders`` method accepts a parameter as an array.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, reverse the statements here:

Suggested change
The ``setHeaders`` method accepts a parameter as an array.
The ``setHeaders`` method accepts an array as a parameter.


It is equivalent to setting CURLOPT_HTTPHEADER value

```text
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```text
```php

It is equivalent to setting CURLOPT_HTTPHEADER value

```text
“Authorization : “. “Basic “.base64_encode($userName.”:”.$password)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Authorization : “. “Basic “.base64_encode($userName.”:”.$password)
"Authorization : " . "Basic " . base64_encode($userName . ":" . $password)


#### Set curl option using setOption method

The ``setOption`` method accepts two parameters. The curl option name and the curl option value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, make sure that the cURL spelling is the same for the entire doc.

Suggested change
The ``setOption`` method accepts two parameters. The curl option name and the curl option value.
The ``setOption`` method accepts two parameters. The cURL option name and the curl option value.


#### Set curl option using setOptions method

The ``setOptions`` method accepts a parameter as an array.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The ``setOptions`` method accepts a parameter as an array.
The ``setOptions`` method accepts an array as a parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at this one, please


#### Set curl cookies using setCookies method

The ``setCookies`` method accepts a parameter as an array.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The ``setCookies`` method accepts a parameter as an array.
The ``setCookies`` method accepts an array as a parameter.

@rogyar rogyar added 2.3.x Magento 2.3 related changes Major Update Significant original updates to existing content labels Mar 10, 2020
@DenisSaltanahmedov
Copy link
Contributor Author

Hi @rogyar, thanks for your help.
I have fixed the recommendations.


#### Set curl option using setOption method

The ``setOption`` method accepts two parameters. The curl option name and the cURL option value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The ``setOption`` method accepts two parameters. The curl option name and the cURL option value.
The ``setOption`` method accepts two parameters. The cURL option name and the cURL option value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, make sure that we use the same spelling for the entire document. So we don't have Curl, curl, cURL.

Thank you.

$result = $this->curl->getBody();
```

Where the ``$url`` is the endpoint url, ``$params`` is an array of data that is being sent via the POST request, the extra parameters can be added in the url.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since URL is an abbreviation, I would suggest using the upper case everywhere. The same suggestion applies to other places of this document.

Suggested change
Where the ``$url`` is the endpoint url, ``$params`` is an array of data that is being sent via the POST request, the extra parameters can be added in the url.
Where the ``$url`` is the endpoint URL, ``$params`` is an array of data that is being sent via the POST request, the extra parameters can be added in the URL.

It is equivalent to setting CURLOPT_HTTPHEADER value

```php
"Authorization : ". "Basic ".base64_encode($userName.":".$password)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, make sure you follow PSR-1 in your examples

Suggested change
"Authorization : ". "Basic ".base64_encode($userName.":".$password)
"Authorization : " . "Basic " . base64_encode($userName . ":" . $password)

@DenisSaltanahmedov
Copy link
Contributor Author

Hi @rogyar, thanks for your review.
I have fixed the recommendations.

@rogyar
Copy link
Contributor

rogyar commented Mar 15, 2020

Hi @DenisSaltanahmedov. Thanks a lot. The only thing left here is the following:

https://github.com/magento/devdocs/pull/6823/files#diff-31d74dc11c9cb7d5599974a83deefd61R128

@DenisSaltanahmedov
Copy link
Contributor Author

Hi @rogyar, fixed.
Thanks.

Copy link
Contributor

@dobooth dobooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Denis, for this nice update. So far, it is a good, generic cURL how-to guide. I would like to see some Magento-specific examples, if possible. What situations would Magento users use cURL?

@DenisSaltanahmedov
Copy link
Contributor Author

HI @dobooth.
The main reason to use the cURL is requests to some external service.
for example, we can use the "Magento\Marketplace\Model\Partners" class.
Could you clarify, what will be the bast place where we can add the Magento example.
Maybe at the end of document as an example section.

@dobooth
Copy link
Contributor

dobooth commented Mar 17, 2020

Yes, I use it frequently. I was hoping there there were some common Magento tasks that we could use as the actual example code. That would help change this topic from "How to use cURL" to "How to use cURL with Magento". How are you currently using cURL in your Magento work?

@dobooth
Copy link
Contributor

dobooth commented Mar 18, 2020

@DenisSaltanahmedov
Copy link
Contributor Author

Hi @dobooth thank you for your help.
I added the example.

@dobooth dobooth changed the title magento/devdocs#: Layout instructions. Adding more information about cURL magento/devdocs#: Layout instructions. Adding more information about cURL [GITBUG] Mar 19, 2020
@dobooth
Copy link
Contributor

dobooth commented Mar 19, 2020

Just a note that we have a strange bug in our repo that is causing some PRs to not update. We will merge as soon as we get it solved.

@DenisSaltanahmedov
Copy link
Contributor Author

Hi @dobooth, any updates about the issue? Thanks.

@jeff-matthews
Copy link
Contributor

@DenisSaltanahmedov, the Update branch button isn't working. We've opened a ticket with GitHub support to fix it. If we can't get support to help us by next week, we'll start using workarounds to merge all affected PRs.

@DenisSaltanahmedov
Copy link
Contributor Author

Hi, if we still have the problem with merge in the PR, I can recreate the changes to new PR from the current master branch. Thanks.

@dobooth
Copy link
Contributor

dobooth commented Apr 14, 2020

running tests

@dobooth dobooth merged commit ebdc0ca into magento:master Apr 14, 2020
@ghost
Copy link

ghost commented Apr 14, 2020

Hi @DenisSaltanahmedov, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2.3.x Magento 2.3 related changes Major Update Significant original updates to existing content Partner: Atwix partners-contribution PR created by Magento partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants