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

[Core][Cart] Cart locale update #5915

Merged
merged 14 commits into from
Aug 31, 2016

Conversation

Arminek
Copy link
Contributor

@Arminek Arminek commented Aug 29, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Related tickets #5813
License MIT

I want to be able to see order currency on the order summary page

Background:
Given the store operates on a single channel in "France"
Copy link
Member

Choose a reason for hiding this comment

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

Didn't we change this steps to meet a default shop configuration?

Copy link
Contributor Author

@Arminek Arminek Aug 29, 2016

Choose a reason for hiding this comment

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

Yes we did but right now this feature is in progress. Or it has been already done.

@Arminek Arminek changed the title [Core][Cart] Cart locale update [POC][RFC][Core][Cart] Cart locale update Aug 29, 2016
@pjedrzejewski pjedrzejewski added BC Break PRs introducing BC breaks (do not even try to merge). New Feature labels Aug 30, 2016
@Arminek Arminek force-pushed the cart-locale-update branch from 591672e to 01d5c83 Compare August 30, 2016 14:59
@Arminek Arminek changed the title [POC][RFC][Core][Cart] Cart locale update [Core][Cart] Cart locale update Aug 30, 2016
@@ -222,7 +247,8 @@ protected function getDefinedElements()
*/
private function getCountryNameOrDefault($code)
{
$countryName = null === $code ? 'Select' : Intl::getRegionBundle()->getCountryNames('en')[$code];
$displayLocale = $this->localeContext->getLocaleCode();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not depend on context here, the locale should be passed as an argument.

@Arminek Arminek force-pushed the cart-locale-update branch from 01d5c83 to 9270a36 Compare August 31, 2016 08:29
$this->cartManager->persist($cart);
$this->cartManager->flush();
} catch (CartNotFoundException $exception) {
throw new HandleException(self::class, 'Sylius was unable to found cart.', $exception);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the "found" be in infinitive form?

@Arminek Arminek force-pushed the cart-locale-update branch 2 times, most recently from 25f9034 to 31a7ebe Compare August 31, 2016 08:44
</service>
</services>

</container>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing blank line

@Arminek Arminek force-pushed the cart-locale-update branch from 31a7ebe to 1b06b2c Compare August 31, 2016 09:54
@michalmarcinkowski michalmarcinkowski merged commit a28ef4b into Sylius:master Aug 31, 2016
@michalmarcinkowski
Copy link
Contributor

Great job Arek! 👍

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break PRs introducing BC breaks (do not even try to merge).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants