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

Add latest products #237

Merged
merged 19 commits into from Nov 9, 2017
Merged

Add latest products #237

merged 19 commits into from Nov 9, 2017

Conversation

ghost
Copy link

@ghost ghost commented Nov 4, 2017

propose this PR to fetch latest products

the API looks like /shop-api/product-latest/, with 3 parameters channel, locale and limit, where channel is required and limit default value is 4, which matches the latest product counts on the demo shop

don't hesitate to leave comments on what's missing or inappropriate

Copy link
Member

@lchrusciel lchrusciel left a comment

Choose a reason for hiding this comment

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

Awesome!

return $this->viewHandler->handle(View::create($this->productLatestQuery->getLatestProducts(
$request->query->get('channel'),
$request->query->get('locale'),
(int) $request->query->get('limit', 4)
Copy link
Member

Choose a reason for hiding this comment

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

get -> getInt?

@@ -0,0 +1,5 @@
shop_api_product_show_latest:
path: /product-latest/
Copy link
Member

Choose a reason for hiding this comment

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

/products/latest/? WDYT? Not sure if it will not collide with some other route.

Copy link
Author

Choose a reason for hiding this comment

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

I think there is a conflict with /shop-api/products/{code}, any other suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

you are right

use Sylius\ShopApiPlugin\View\ProductListView;
use Webmozart\Assert\Assert;

class ProductLatestViewRepository implements ProductLatestViewRepositoryInterface
Copy link
Member

Choose a reason for hiding this comment

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

Missing final

private $productViewFactory;

/**
* ProductLatestViewRepository constructor.
Copy link
Member

Choose a reason for hiding this comment

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

Redundant

@lchrusciel
Copy link
Member

@sdleiw are you willing to apply this small changes? Just small things, but we can make them in one shot. Also, all the files are missing a blank line at the end of file(it is required due to cleaner github diffs)

@ghost
Copy link
Author

ghost commented Nov 8, 2017

Hi @lchrusciel, thanks for the review, I'll take care of them, is there some kind of Code-Style that you use for the project, or just psr-2

@lchrusciel
Copy link
Member

We have. Mostly Symfony coding standard, PSR's, and our own clean code rules. All of them are put in https://github.com/SyliusLabs/CodingStandard. I just need to check to use them properly.

@lchrusciel lchrusciel merged commit 6b086b3 into Sylius:master Nov 9, 2017
@lchrusciel
Copy link
Member

Thanks Lei for contributing! :)

@ghost ghost deleted the add-latest-products branch November 9, 2017 22:18
@ghost
Copy link
Author

ghost commented Nov 9, 2017

cool, had a lot of fun working on this repo

@lchrusciel
Copy link
Member

I'm glad to hear that :)

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.

1 participant