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

Migrate product component for Orders page #15982

Merged
merged 27 commits into from Oct 29, 2019

Conversation

@RaimondasSapola
Copy link
Contributor

RaimondasSapola commented Oct 16, 2019

Questions Answers
Branch? develop
Description? Migration of order create page cart component search products and add to cart
Type? refacto
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? #10583
How to test? Create order cart block should be migrated and should have the same functionality as legacy

This change is Reviewable

*/
private function addCustomizationFields(Request $request): ?int
{
$customizationFields = $request->request->get(self::PRODUCT_CUSTOMIZATION_FIELDS_REQUEST_KEY);

This comment has been minimized.

Copy link
@RaimondasSapola

RaimondasSapola Oct 18, 2019

Author Contributor

Hey @matks i need your guidance :) For add product to cart component in BO order creation i have to add product customization fields via ajax. They can be added by user in product page and can be file or text type. Im thinking of creating a separate service for validating and resolving data of these fields and use command to actually add them to database with legacy functions. Other idea would be to use form with collection type fields. Although form imo would look cleaner, i am not sure if it is going to work. Maybe you have some suggestions or insights about that?

This comment has been minimized.

Copy link
@matks

matks Oct 29, 2019

Contributor

I think I answered that 🤔 maybe in gitter ?

@RaimondasSapola RaimondasSapola force-pushed the RaimondasSapola:M/orders/PoroductsComponent branch from d76bbdd to 8ffe716 Oct 18, 2019
@matks matks added the migration label Oct 21, 2019
@zuk3975 zuk3975 force-pushed the RaimondasSapola:M/orders/PoroductsComponent branch 2 times, most recently from 97a27b0 to ef4a4e2 Oct 21, 2019
@RaimondasSapola RaimondasSapola changed the title [WIP] M/orders/poroducts component migration M/orders/products component migration Oct 24, 2019
@RaimondasSapola RaimondasSapola marked this pull request as ready for review Oct 24, 2019
@RaimondasSapola RaimondasSapola requested a review from PrestaShop/prestashop-core-developers as a code owner Oct 24, 2019
class: PrestaShop\PrestaShop\Adapter\Order\QueryHandler\SearchProductsForOrderCreationHandler
arguments:
- '@prestashop.adapter.legacy.context'
- '@prestashop.core.localization.locale.repository'

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 24, 2019

Contributor

You should use prestashop.core.localization.locale.context_locale here

This comment has been minimized.

Copy link
@zuk3975

zuk3975 Oct 25, 2019

Contributor

maybe you know what is going on with it? I mean that Locale implements LocaleInterface which has no methods.

This comment has been minimized.

Copy link
@matks

matks Oct 29, 2019

Contributor

I'm sorry, I did not get the question

class: PrestaShop\PrestaShop\Adapter\Product\QueryHandler\SearchProductsHandler
arguments:
- '@prestashop.adapter.legacy.context'
- '@prestashop.core.localization.locale.repository'

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 24, 2019

Contributor

You should use prestashop.core.localization.locale.context_locale here

- ~
- '@serializer.name_converter.camel_case_to_snake_case'

# Snake case serializer definition

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 24, 2019

Contributor

Nice!

This comment has been minimized.

Copy link
@zuk3975

zuk3975 Oct 24, 2019

Contributor

Not sure if its needed though 😄 more here #15977 (comment)

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 25, 2019

Contributor

Well I had a similar problem in one of my current PR actually.. We need to define a convention do we return json objects as snake case or camel case?

This comment has been minimized.

Copy link
@zuk3975

zuk3975 Oct 25, 2019

Contributor

I vote for camelCase 🥇

@zuk3975 zuk3975 force-pushed the RaimondasSapola:M/orders/PoroductsComponent branch from 7eac76e to ed1948d Oct 29, 2019
@matks
matks approved these changes Oct 29, 2019
@matks matks changed the title M/orders/products component migration Migrate product component for Orders page Oct 29, 2019
@sarjon sarjon merged commit b2f4a35 into PrestaShop:develop Oct 29, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@sarjon

This comment has been minimized.

Copy link
Contributor

sarjon commented Oct 29, 2019

Thanks @zuk3975

As discussed with @matks QA will happen on whole page after migrating remaining components.

@Progi1984 Progi1984 added this to the 1.7.7.0 milestone Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.