-
Notifications
You must be signed in to change notification settings - Fork 176
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
Return created/updated objects #175
Comments
+1 |
I have created PR for this. This is what should be ( |
This is a pretty serious issues that the Base.php save() method returns void. E.g. the rest api order docs incorrectly state there will be a response. Getting the response with the new order's id is mission critical for our use case, but I don't see a way to access the updated object. Any contributors able to post a workaround until this is resolved? |
@Zelfapp Yeah, as stated in the original message, I have fixed this (along with other improvements to the library) in my fork: MagicLegend@d43bc94 |
@MagicLegend I saw your PR. This library has too many issues for a private app at this point and feels too hacky. It has great potential though. For the sake of anyone else battling this alpha lib for a private app, likely you're better off simply using guzzle to make REST api endpoint requests. E.g. in just few lines of code you can dynamically pass in any endpoint and payload. Again, this is a great lib, but I would not classify as ready for production. use GuzzleHttp\Client;
use GuzzleHttp\Psr7\Request;
/// Guzzle client
$client = new Client();
$request = new Request(
'GET',
"https://$this->shop/admin/api/2022-07/$endpoint",
[
'X-Shopify-Access-Token' => $this->permanentAccessToken,
],
$endpointParams['payload'] ?? null,
);
$response = $client->sendAsync($request)->wait();
$response = $response->getBody(); |
I respectfully disagree, and we have my fork running in production right now. It works fine, it just has some quirks that Shopify is not allocating resources to to solve. I prefer using a library that's providing typing over doing it manually in Guzzle, but both will of course work fine. |
This issue is stale because it has been open for 90 days with no activity. It will be closed if no further action occurs in 14 days. |
Still waiting on Shopify... |
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
I fully agree on this issue. I would like to see PR #185 to be committed. |
On second thought. When you do for instance:
|
@brrrm Well, you don't pass |
the param $updateObject is not about updating an existing entity or creating a new entity. All it does is update the Object with the returned entity from Shopify's response. Or am I misunderstanding what you mean?
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
FYI for anyone still struggling with this, instead of calling just
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days. |
|
Hey folks, thanks for your patience here. We've removed the stalebot from this repo, and I'm going to add this issue to our tracking. Once again, thank you for your patience! |
No worries. You can find an implementation of this issue in my fork: MagicLegend@d43bc94 |
Overview
The API itself will already return the full object if you do a create/update operation. However, the library swallows this response here:
https://github.com/Shopify/shopify-php-api/blob/0556edf274ceb83346533bd22b662d7622e58c66/src/Rest/Base.php#L60-L74
Solution
Adding
return $response->getDecodedBody();
at the end of the function should suffice. Note that thesaveAndUpdate
function would also need to be updated, withreturn $this->save(true);
. Implementation can be found in my fork: MagicLegend@d43bc94Type
Motivation
I would like to verify the saved object myself, without needing to query the API again for data that was already returned in the first place. This also allows immediate access to the ID of the newly created object.
The text was updated successfully, but these errors were encountered: