-
Notifications
You must be signed in to change notification settings - Fork 89
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
Adding a Cookbook #514
Adding a Cookbook #514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Thank you very much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @CoderMaggie, would you like to take a look, once you have a free moment?
Adding information about Serialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey mamazu,
I have just read your cookbook and it looks good to me. I have added some comments and thoughts.
Even if they sound a little bit rude, they are not supposed to be. 😓
I haven't much experience with Sylius perhaps some comments are really newbish.
doc/Cookbook.md
Outdated
|
||
## How does the Shop Api work | ||
<img src="Workflow.png" alt="Apis Workflow" /> | ||
The general approach that Shop Api takes is that every request is validated by the `CommandProvider` and then converted into a command by this class. Then the command is either handled directly in the `Controller` and passed to the `ViewRepository` which returns a view or by a `MessageHandler`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to add here some links to the different variants. I think the MessageHandler
is part of the Messenger framework, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes in the MessageHandler section there is a link to the symfony configuration of this tool.
@CSchulz Thanks for your feedback, having no knowledge about the api is the best thing, if you are reading through documentation. I have improved on your suggestions. Could you have a look at it again. |
Yes I will look into it at the weekend. |
Hey Max, it is probably the best description atm, so I think it would be great to add it to main repo. Rebase should fix the issue. Also, can you add a link to docs from the main README? |
Thanks to Łukasz Co-Authored-By: Łukasz Chruściel <lchrusciel@gmail.com>
… the possibility to extend the api with payment methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have found some more questions, sorry for the late reply.
I wouldn't say, that hostname resolving is a best practice, just sensible
default for the plugin.
My point is that this example is full of errors, it would be better to
follow one of the Core examples.
…On Tue, Sep 10, 2019, 14:26 mamazu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In doc/Cookbook.md
<#514 (comment)>:
> +use Sylius\Component\Channel\Context\ChannelContextInterface;
+
+class RequestAttributeChannelContext implements ChannelContextInterface
+{
+ private $channelRepository;
+ private $requestStack;
+
+ public function __construct(ChannelRepositoryInterface $channelRepository, RequestStack $requestStack)
+ {
+ $this->channelRepository = $channelRepository;
+ $this->requestStack = $requestStack;
+ }
+
+ public function getChannel(): ChannelInterface
+ {
+ $request = $this->requestStack->getCurrentRequest();
The reason why this case is documented is that in the past the routes all
had a channel attribute so this is more a documentation on how to maintain
backwards compatibiltiy rather than best practice. Best practice is to use
the hostname to resolve the channel.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#514?email_source=notifications&email_token=AAGUSW45WTO5D4MQ3VRKRLTQI5DZLA5CNFSM4ILOKZSKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEFUPWQ#discussion_r322588834>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGUSWY3OW7V4BB2BWIDXM3QI5DZLANCNFSM4ILOKZSA>
.
|
True, Sylius Core handles it that way so this plugin should allow suit for the hostname resolution. |
In the default implementation of the Sylius Solution, there is already an api implemented that provides a lot of features (which is called the Admin Api). The Admin Api is more geared towards integrating other closed systems like a warehouse management system or similar. The reason behind that is that for the Admin Api you need to have to [exchange tokens to authenticate](https://docs.sylius.com/en/latest/cookbook/api/api.html) a new client for usage (which is based on oauth). On the other side in the Shop Api everyone can log into Sylius who has an account, no token exchange necessary. The Shop Api uses the [JWT](https://github.com/lexik/LexikJWTAuthenticationBundle) to authenticate its users. | ||
|
||
## How does the Shop Api work | ||
<img src="Workflow.png" alt="Apis Workflow" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering why the request isn't mentioned in the workflow, perhaps you can also add it to the beginning.
This is the last iteration of the cookbook before 1.0 |
### Command - Handler Structure | ||
Command handling has multiple parts to it. When dispatching a command, the `MessageBus` looks for a handler that has an `__invoke` method with the parameter type matching the type of the command that was dispatched. Before and after the handler is executed, there is a way for a "Middleware" to be executed (see below). The CommandHandler itself, however, does not return anything (this is not a technical limitation that is just the convention we chose in Shop Api). | ||
|
||
All Middlewares which are executed are defined under the `framework.messenger` bundle: [config.yml](https://github.com/Sylius/ShopApiPlugin/blob/fc25f36274e6add118f5b575a44db81bcc47b2e5/src/Resources/config/app/config.yml#L17) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All Middlewares which are executed are defined under the `framework.messenger` bundle: [config.yml](https://github.com/Sylius/ShopApiPlugin/blob/fc25f36274e6add118f5b575a44db81bcc47b2e5/src/Resources/config/app/config.yml#L17) | |
All Middlewares which are executed are defined under the `framework.messenger` bundle: [config.yml](https://github.com/Sylius/ShopApiPlugin/blob/master/src/Resources/config/app/config.yml#L17) |
Thanks, @mamazu! 🥇 |
Well Shop Api got documentation now. Feel free to comment on my not very documentation-like writing style.