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 order product management #16150

Merged
merged 42 commits into from Jan 10, 2020

Conversation

@sarjon
Copy link
Contributor

sarjon commented Oct 28, 2019

Questions Answers
Branch? develop
Description? Migrates product management in Order view page.
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? #15826 #16512
How to test? Not yet. => Rebuild assets

This change is Reviewable

@sarjon

This comment has been minimized.

Copy link
Contributor Author

sarjon commented Oct 29, 2019

When editing product in Order, user can enter new price, but every time I enter price it gets duplicated to another input. Does anyone know how it should work? Or is it expected behavior?

These fields :
image

Ping @MatShir

@MatShir

This comment has been minimized.

Copy link

MatShir commented Oct 29, 2019

@sarjon the expected behavior should be as on the product page to edit the price:

  • If the user edit an amount in the input without taxe it will insert the amount plus the product taxes in the input with taxe.

  • If the user edit an amount in the input with taxe it will insert the amount less the product taxes in the input without taxe.

The specs will be updated ;)

@sarjon sarjon force-pushed the sarjon:m/orders/products-manage branch from 1102465 to 878aa94 Oct 30, 2019
@colinegin colinegin added the High label Nov 4, 2019
MatShir added a commit to PrestaShop/prestashop-specs that referenced this pull request Nov 5, 2019
@MatShir

This comment has been minimized.

Copy link

MatShir commented Nov 6, 2019

Don't forget guys,
The product list displays depending on the client group setting.

If the client group is flagged with tax included:

  • The column "base price" has "tax included" next to it.
  • The column "Total" has "tax included" next to it.

If the client group is flagged with tax excluded:

  • The column "base price" has "tax excluded" next to it.
  • The column "Total" has "tax excluded" next to it.
  • In the summary appears sum of Taxes

The only problem with the legacy, if one of the client group has tax excluded, it displays for all the product lists in tab excluded even for the order of customer group with tax included.

@matks do you think we could fix it?

@MatShir

This comment has been minimized.

Copy link

MatShir commented Nov 6, 2019

Pagination everywhere 😆
The ideal:
To keep the page useable is to add pagination on the product list if it goes beyond the 8 products. Otherwise, the merchant will have to scroll anytime if he wants to interact with the modules or the order tabs.
Pagination page commande

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Nov 7, 2019

Don't forget guys,
The product list displays depending on the client group setting.

If the client group is flagged with tax included:

  • The column "base price" has "tax included" next to it.
  • The column "Total" has "tax included" next to it.

If the client group is flagged with tax excluded:

  • The column "base price" has "tax excluded" next to it.
  • The column "Total" has "tax excluded" next to it.
  • In the summary appears sum of Taxes

The only problem with the legacy, if one of the client group has tax excluded, it displays for all the product lists in tab excluded even for the order of customer group with tax included.

@matks do you think we could fix it?

This will be fixed in #16319

@atomiix atomiix self-assigned this Nov 13, 2019
@atomiix atomiix force-pushed the sarjon:m/orders/products-manage branch from 309fb34 to b29e5db Nov 19, 2019
@Progi1984 Progi1984 closed this Nov 22, 2019
@Progi1984 Progi1984 reopened this Nov 22, 2019
@Progi1984 Progi1984 force-pushed the sarjon:m/orders/products-manage branch 2 times, most recently from 0b8132c to 156094e Nov 22, 2019
@Progi1984 Progi1984 marked this pull request as ready for review Nov 22, 2019
@Progi1984 Progi1984 requested a review from PrestaShop/prestashop-core-developers as a code owner Nov 22, 2019
@Progi1984 Progi1984 removed the WIP label Nov 22, 2019
@Progi1984 Progi1984 assigned Progi1984 and unassigned atomiix Nov 22, 2019
@Progi1984 Progi1984 added this to the 1.7.7.0 milestone Nov 22, 2019
@Progi1984 Progi1984 force-pushed the sarjon:m/orders/products-manage branch from cd256c4 to 25b3e55 Nov 22, 2019
@Progi1984 Progi1984 requested review from matks and zuk3975 Nov 22, 2019
@matthieu-rolland matthieu-rolland force-pushed the sarjon:m/orders/products-manage branch 2 times, most recently from 8872c38 to 2653f73 Jan 9, 2020
Copy link
Contributor

jolelievre left a comment

Nice work all!

@matthieu-rolland matthieu-rolland force-pushed the sarjon:m/orders/products-manage branch from 2653f73 to ac7299f Jan 10, 2020
@jolelievre jolelievre merged commit 1590acb into PrestaShop:develop Jan 10, 2020
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
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.