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 summary block of BO create order page #16207

Merged
merged 34 commits into from Jan 9, 2020

Conversation

@zuk3975
Copy link
Contributor

zuk3975 commented Oct 30, 2019

Questions Answers
Branch? develop
Description? See below (too long)
Type? refacto
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #10583
How to test? See below

Description

  • Implement dynamic rendering of create order page summary block and order creation from cart action.
  • Add missing actions for update product price/quantity in cart block.
  • Add links to buttons for creating new customer/voucher.

Warning:

  • Addresses buttons are missing, due to PR #15300 waiting to be merged.

Still some things left until page is fully migrated

  • Fix iframes/modals loading on create/edit resources buttons
  • There is no validation of customization fields (legacy didn't have one too)

Additional features to do: Carts/Orders list pagination, some kind of back link after viewing order/cart/customer details.

How to test ?

Page should work similarly as legacy. You can access it through http://localhost:8888/admin-dev/index.php/sell/orders/orders/new
To notice:

  1. buttons that were opening modal window (like Add new customer etc.) are now redirecting instead of modal (temporary solution).
  2. Add address and Edit address buttons not yet implemented because waiting for merge of #15300) - ✔️ DONE within a99ac97
  3. Some legacy bugs were fixed: enable to change cart language, currency, adding customized product, showing customized product image.

This change is Reviewable

@zuk3975 zuk3975 requested a review from PrestaShop/prestashop-core-developers as a code owner Oct 30, 2019
@matks matks added the migration label Oct 30, 2019
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Nov 4, 2019

@zuk3975 Did an early review, everything looks good 👍

@zuk3975 zuk3975 force-pushed the zuk3975:m/orders/create-summary branch 2 times, most recently from 73ad247 to d0173e6 Nov 5, 2019
@zuk3975 zuk3975 changed the title [WIP] Migrate summary block of BO create order page Migrate summary block of BO create order page Nov 8, 2019
@matks matks mentioned this pull request Nov 15, 2019
7 of 34 tasks complete
@jolelievre jolelievre force-pushed the zuk3975:m/orders/create-summary branch from e63cf8c to dcbedc6 Jan 7, 2020
jolelievre added 2 commits Jan 7, 2020
@jolelievre

This comment has been minimized.

Copy link
Contributor

jolelievre commented Jan 8, 2020

Hi @matks !
Sadly, I have two bugs with your fix :

  • After creating an order in a currency different from the default one, the product price is in the "default currency", and not in the selected currency (here, DZD42 instead of DZD5,632.46).
    pr16207 product price order
  • When adding two discount in a currency different from the default one, the first added discount is displayed in the default currency.
    pr16207 discount bad currency

The first one seems to be fixed, but couldn't get in time with second one (vouchers), legacy seems to act strange with cartRules of amount type too. There are some insights for further debugging which might be helpful 🤷‍♂ :

  • The discount['value_real'] comes randomly wrong from legacy in GetCartInformationHandler.
  • Might be associated with context as it is seems being counted using CartRule::getContextualValue (setting context currency, lang, cart and customer didin't help tho).
  • Might somehow be associated with cache - Check CartRule::GetContextualValue 1109

I wasn't able to reproduce the discount bug with different currency, it might have been fixed along with another bug However @Robin-Fischer-PS don't hesitate to test it thoroughly

@matks
matks approved these changes Jan 8, 2020
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Jan 8, 2020

@Robin-Fischer-PS the PR is, once again, yours 😄

@matks matks dismissed jolelievre’s stale review Jan 8, 2020

Requested changes applied

@matks matks merged commit 14d1381 into PrestaShop:develop Jan 9, 2020
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Jan 9, 2020

PR merged in order to move forward, remaining bugs will be fixed in develop

@matks matks removed the waiting for QA label Jan 9, 2020
@Progi1984 Progi1984 added this to the 1.7.7.0 milestone Jan 9, 2020
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.