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

New Feature/Enhancement requests/ideas #18

Open
vienthuong opened this issue Nov 29, 2021 · 7 comments
Open

New Feature/Enhancement requests/ideas #18

vienthuong opened this issue Nov 29, 2021 · 7 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@vienthuong
Copy link
Owner

We've released the SDK for a while and as Shopware core is growing really fast as well, the SDK might get a bit out of date APIs, or lack features, using deprecated APIs, etc...

So with this thread, I want to gather the ideas/solutions from the community or even improve PR to make the SDK better. Any comments/suggestions are welcomed and highly appreciated 🤝

For bug issues, please create a new one so I can keep the track of them :)

@vienthuong vienthuong added the help wanted Extra attention is needed label Nov 29, 2021
@vienthuong vienthuong self-assigned this Nov 29, 2021
@solverat
Copy link
Contributor

Hey @vienthuong. It would be nice to have something like toArray() in Entity classes.

$product = new ProductEntity();
$product->active = true;

$productRepository->create($product->toArray(), $context); 

... which only should return populated fields. I've tried this by using the jsonSerialize method, but this method of course returns every available property populated with a null value if not set.

@vienthuong
Copy link
Owner Author

vienthuong commented Dec 20, 2021

Hi @solverat
I got your intention here, if the payload is relatively small I'd just put it into the payload

$productRepository->create([
    'active' => true, // or $product->active
], $context); 

Or

$productRepository->create(array_filter($product->jsonSerialize()), $context); 

Will it work in your case? Filtering every null property out seems not a correct approach. For example when you want to set a property back to null then you need to define that property as null.

But feel free to create a PR or just a draft solution then I will take a look if it could be a good idea to add the method, thanks

@solverat
Copy link
Contributor

Maybe another one :):

https://shopware.stoplight.io/docs/admin-api/ZG9jOjEyNjI1Mzkw-media-handling#upload-the-resource-directly

Uploading asset binaries via repository (?) would be nice. In some cases, URLs are not reachable from outside.

@vienthuong
Copy link
Owner Author

vienthuong commented Dec 21, 2021

@solverat
Hi, sounds like a good idea, I created a new issue here #23
But honestly, I don't 100% get your expectation so can you write a bit more specific in the new ticket? Like why we need it and why it's not possible currently with the SDK :)
Thanks for contribution

@jakub-groncki
Copy link

Hi @vienthuong,

On my team we use psalm and sometimes it gets a little bit frustrating that all of the properties in entities are nullable/set to null by default. This either generates a lot of null-checking or disabling null checks in psalm altogether. I reckon that "fixing" or adjusting this wouldn't be a breaking change, right? It's a slight annoyance, especially when you don't normally expect a null value in Shopware:

// CurrencyEntity/SDK:

public ?string $symbol = null;


// CurrencyEntity/Shopware:

/**
 * @var string
 */
protected $symbol;

@vienthuong
Copy link
Owner Author

Hi @jakub-groncki
Sorry for the late response.
Change the nullable to non nullable is breaking change unfortunately. By default it's all nullable so we can set all the properties with value or null without fatal error. I already considered it in the generator script but for some reason I commented it out so I am not sure that add it back is a safe solution.

image

https://github.com/vienthuong/shopware-php-sdk/blob/master/script/src/CodeGenerator.php

I'd recommend ignore these psalm rule using some regex pattern if possible. Or feel free to create a MR to update the CodeGenerator if you find it useful :)

@jakub-groncki
Copy link

Hi @vienthuong,

I was thinking about the same thing when it comes to the code generator. I think I'm going to fork this repo and try it out.

We consider psalm rules very ad hoc in this context so having a solid nullability correspondence with the actual Shopware API would be great. I think I might give it a go :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants