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

Document CQRS #232

Merged
merged 4 commits into from
Apr 16, 2019
Merged

Document CQRS #232

merged 4 commits into from
Apr 16, 2019

Conversation

sarjon
Copy link
Contributor

@sarjon sarjon commented Mar 18, 2019

I referred to PrestaShop/PrestaShop#9370 and found that explanation well written, so I basically ended up copying @eternoendless work with some minor changes to it. 😈

I have a few questions:

  1. Should I provide code examples of how to create and use command/handler? Or maybe it should be subsection of CQRS?
  2. Is Architecture good section for this article? I was also thinking about Architecture/Migration Guide, but it feels a bit wrong.

@mickaelandrieu
Copy link
Contributor

Nice introduction!

For me the docs should answer some questions of the PrestaShop developers:

  • what is CQRS? (your introduction);
  • am I concerned by this stuff when working on themes or modules? (tutorials, and/or workflows) Imagine you need to explain that in training for instance;
  • what are the pros/cons of this choice for the PrestaShop developers (ie: why we decide to do that and how it helps PrestaShop developers to work with our platform?);

WDYT? /c @eternoendless

@matks
Copy link
Contributor

matks commented Apr 12, 2019

  1. Should I provide code examples of how to create and use command/handler? Or maybe it should be subsection of CQRS?

In another subsection, yes 😉it's 2 subtopics: what is X and how we implement X in PrestaShop. Like "what is Dependency Injection" and "How Symfony implements Dependency Injection with the Symfony Container"

  1. Is Architecture good section for this article? I was also thinking about Architecture/Migration Guide, but it feels a bit wrong.

Looks like the right place to me 👍

CQRS stands for _Command Query Responsibility Segregation_. In brief, the CQRS pattern suggests to separate "write" and "read" models, which it refers to as Commands and Queries.

## Why using CQRS in PrestaShop?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why we need to keep core and why we need to isolate it ?
Something like

"We are currently migrating PrestaShop Back Office to Symfony but the new Controllers, Templates and classes dedicated to the BO are still using the Core classes of PrestaShop used by the previous BO. One day, we will remove this Core too. Until that day, we need to allo the new BO to access these classes and data while keeping a clear border between these 2 areas of code as one is expected to remain and the other one to disappear."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think this commit b908ef4 explains it?

Commands and Queries allow us to isolate the controllers from the data source, which can be later replaced by something else while leaving behind a nice API.

This implementation proposes a "top-down" design, which inverses the classic data-driven design. It starts on a page and the actions performed in it, and then trickles down layer by layer, finishing on the data layer.

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be nice to have a schema here. This schema would compare old and new architecture.

In old architecture we would see the browser sending HTTP requests to a big bunch of code which would contain Controllers calling directly core classes. Like:

browser ==(send HTTP requests)==> controller ==(calls)==> core

In new architecture, we would show the clear separation of the bus

browser ==(send HTTP requests)==> SF controller =(dispatchs Queries and Commands)=> Bus routing =(Handler handles the Command/Query)=> Adapter Handler ==(calls)==> core

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I've tried to do so in b908ef4 commit, could you check?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

matks and others added 2 commits April 15, 2019 11:09
Co-Authored-By: sarjon <jonusas.sarunas@gmail.com>
Co-Authored-By: sarjon <jonusas.sarunas@gmail.com>
@matks
Copy link
Contributor

matks commented Apr 16, 2019

Thank you @sarjon

@matks matks merged commit aa798f3 into PrestaShop:master Apr 16, 2019
@sarjon sarjon deleted the document-cqrs branch April 16, 2019 13:13
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

Successfully merging this pull request may close these issues.

3 participants