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

Naming #28

Open
trickreich opened this issue Dec 10, 2018 · 8 comments
Open

Naming #28

trickreich opened this issue Dec 10, 2018 · 8 comments

Comments

@trickreich
Copy link
Collaborator

I'm discovering the code and in my opinion the usage is a bit confusing.

For example:
$allProducts = $apiClient->product()->getAll()

The part ->product() is a bit misleading in my opinion.
Maybe introduce gateways for each api endpoint?
Then also the extension point for custom api`s would exists?

@trickreich
Copy link
Collaborator Author

FYI: How I've implemented my client library actually

https://github.com/sulu/SuluSyliusConsumerBundle/blob/master/Gateway/CartGateway.php

@Nyholm
Copy link
Member

Nyholm commented Dec 10, 2018

Isnt that what Im trying todo?

->product() gives you the ProductApi(Gateway) where all the /api/v1/product/ endpoints are.

@trickreich
Copy link
Collaborator Author

Yes.. maybe you are right.
I'm trying to think in Symfony DI.
Would I create one service to retrieve all the gateways or would I create one service per gateway?

@trickreich
Copy link
Collaborator Author

trickreich commented Dec 10, 2018

When I'm adding my custom api to this library I would need to overwrite the SyliusClient class as well.
Just only to add a method to retrieve my CustomApi / CustomGateway.

@Nyholm
Copy link
Member

Nyholm commented Dec 10, 2018

Would I create one service to retrieve all the gateways or would I create one service per gateway?

You would create one service for SyliusClient.

When I'm adding my custom api to this library I would need to overwrite the SyliusClient class as well.

Yes. Im not 100% sure this is the best way. But earlier today I did #27

@trickreich
Copy link
Collaborator Author

trickreich commented Dec 10, 2018

You would create one service for SyliusClient.

In your current structure, for sure. But when I think about doctrine where you have a repository for each entity, maybe this should be the way to go here also.

@Nyholm
Copy link
Member

Nyholm commented Dec 10, 2018

You are correct. That makes sense.

An idea: you could register the productApi as a service by using the SyliusClient as a service Factory.

Btw, While considering a solution to this, we should also consider how we do customer authentication.

@trickreich
Copy link
Collaborator Author

Btw, While considering a solution to this, we should also consider how we do customer authentication.

Hmm.. I'm not sure what this bundle could provide for this?
You are talking about an endpoint on Sylius site?

I've implemented one in our plugin:
https://github.com/sulu/SuluSyliusProducerPlugin/blob/master/src/Controller/AuthenticationController.php

I've then written a Symfony Authenticator, but this shouldn't be part of this bundle, right?
https://github.com/sulu/SuluSyliusConsumerBundle/blob/master/Security/SyliusAuthenticator.php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants