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.

Sign up for GitHub

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

Jump to bottom

Migrate "Orders > Shopping Carts" page #31934

Closed
wants to merge 13 commits into from
Closed

Migrate "Orders > Shopping Carts" page #31934

wants to merge 13 commits into from

Conversation

boherm
Copy link
Member

@boherm boherm commented Mar 27, 2023

Questions Answers
Branch? develop
Description? "Orders > Carts" page migration
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
How to test? Before, please build admin new-theme with make admin-new-theme !
First, active Carts index feature flag, and then navigate into Orders -> Carts page.
You can filter data, delete not already ordered carts one by one or by bulk actions.
Multistore mode is supported, and in that case we add Shop column into grid (only for all / group context).
Fixed ticket? Fixes #10584
Related PRs
Sponsor company

Warning:
For now new Kpi use legacy to retrieve data in all already migrated pages!

@boherm boherm requested a review from a team as a code owner March 27, 2023 09:52
@prestonBot prestonBot added develop Branch Improvement Type: Improvement labels Mar 27, 2023
@boherm boherm changed the title #10584 migrate bo orders shopping carts page Migrate "Orders > Shopping Carts" page Mar 27, 2023
@boherm boherm force-pushed the #10584-migrate-bo-orders-shopping-carts-page branch from b46a275 to 3c9e720 Compare March 27, 2023 14:33
zuk3975
zuk3975 requested changes Mar 27, 2023
View reviewed changes
Copy link
Contributor

@zuk3975 zuk3975 left a comment

Choose a reason for hiding this comment

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

Very nice work, I added some comments, that shouldn't take too much time.
Also this PR is missing tests (behats for command handlers and ideally some integration tests for CartController grid action)

@boherm
Copy link
Member Author

boherm commented Mar 27, 2023

Thanks for your review @zuk3975!
I will fix my PR soon.

@boherm boherm force-pushed the #10584-migrate-bo-orders-shopping-carts-page branch from 39a9245 to c8c1459 Compare March 29, 2023 08:23
@boherm boherm requested review from FabienPapet and zuk3975 and removed request for FabienPapet and zuk3975 March 29, 2023 09:04
@boherm
Copy link
Member Author

boherm commented Mar 29, 2023

I work now on testing ;)

@boherm boherm force-pushed the #10584-migrate-bo-orders-shopping-carts-page branch 2 times, most recently from 76fdd80 to 2f70429 Compare March 30, 2023 07:32
@boherm boherm requested a review from FabienPapet April 3, 2023 09:03
FabienPapet
FabienPapet requested changes Apr 3, 2023
View reviewed changes
src/PrestaShopBundle/Resources/config/services/core/form/choice_provider.yml Show resolved Hide resolved
src/PrestaShopBundle/Resources/config/services/core/grid/accessibility_checker.yml Outdated Show resolved Hide resolved
src/Adapter/Cart/Repository/CartRepository.php Outdated Show resolved Hide resolved
src/Adapter/Kpi/AverageOrderValueKpi.php Outdated Show resolved Hide resolved
src/Adapter/Kpi/AverageOrderValueKpi.php Outdated Show resolved Hide resolved
src/Core/Domain/Cart/CartStatusType.php Outdated Show resolved Hide resolved
src/Core/Grid/Action/Row/AccessibilityChecker/DeleteCartAccessibilityChecker.php Outdated Show resolved Hide resolved
src/PrestaShopBundle/Resources/config/services/adapter/kpi.yml Outdated Show resolved Hide resolved
src/PrestaShopBundle/Resources/config/services/core/domain/cart.yml Outdated Show resolved Hide resolved
@boherm boherm force-pushed the #10584-migrate-bo-orders-shopping-carts-page branch from 7eeaa13 to ca58fc9 Compare April 4, 2023 14:15
FabienPapet
FabienPapet previously approved these changes Apr 4, 2023
View reviewed changes
zuk3975
zuk3975 previously requested changes May 12, 2023
View reviewed changes
Copy link
Contributor

@zuk3975 zuk3975 left a comment

Choose a reason for hiding this comment

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

just this one https://github.com/PrestaShop/PrestaShop/pull/31934/files#r1192088594
(sorry for noticing late) and its ok for me ✔️

the other one (https://github.com/PrestaShop/PrestaShop/pull/31934/files#r1192094627) could be improved in dedicated PR, not blocking

@boherm boherm force-pushed the #10584-migrate-bo-orders-shopping-carts-page branch from 5e0bc4b to 9593650 Compare May 22, 2023 13:57
@boherm boherm requested a review from zuk3975 May 22, 2023 13:58
FabienPapet
FabienPapet reviewed May 22, 2023
View reviewed changes
src/Core/Domain/Cart/CartStatus.php Outdated
/**
* Cart status
*/
class CartStatus
Copy link
Member

Choose a reason for hiding this comment

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

can be an enum

FabienPapet
FabienPapet reviewed May 22, 2023
View reviewed changes
src/Core/Form/ChoiceProvider/CartStatusesChoiceProvider.php
/**
* Class CartStatusesChoiceProvider is responsible for providing cart status choices in cart grid filters.
*/
final class CartStatusesChoiceProvider implements FormChoiceProviderInterface
Copy link
Member

Choose a reason for hiding this comment

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

I know there is a discussion about FormChoiceProvider being relevant for simple use case like this, I think this one could simply be inside a form type. WDYT ?

ping @jolelievre

@MatShir MatShir mentioned this pull request Jul 6, 2023
Open
boherm added 12 commits July 11, 2023 18:35
@boherm
Add new feature flag to enable/disable new Carts page in bo
e1b8f3f
@boherm
Improve BadgeColumn for Grids
c9e37af 
- Add alignement system in column
- Add `badge_type_field` to use column in grid to select badge type directly and no only color (like `color_field` for instance)
@boherm
Improve used KPI
db56d65 
- Use DI instead of `Context`
- Add purple color
- Create kpi_row factory for Carts page

kpi
@boherm
Update CartController
f0bd38f 
- Add index action page
- Add solo/bulk delete actions
- Add export into csv
@boherm
Add grid and query builder for Carts page
2620e60 
Add cart JS

grid

query

grid
@boherm
Add CQRS command to solo/bulk delete carts
36f8bd9
@boherm
Fix kpi injections
c1c07d6
@boherm
Fix delete actions and CartRepository
e1c5b0c 
fix repository
@boherm
Move private function in CartGridDefinitionFactory
4f2e5a5
@boherm
Improve carts grid
b80cce1 
- Add new column for display & search status
- Fix AbandonedCartKpi to go directly into abandoned cart

fix grid improvement
@boherm
Add behat tests for deleting carts
935d4aa
@boherm
Apply suggestions from code review
0588b91
@boherm boherm force-pushed the #10584-migrate-bo-orders-shopping-carts-page branch from 9593650 to 098103a Compare July 11, 2023 17:23
@boherm boherm force-pushed the #10584-migrate-bo-orders-shopping-carts-page branch from 098103a to 62c14da Compare July 11, 2023 20:22
@boherm boherm dismissed zuk3975’s stale review July 11, 2023 20:46

Ready for review! :)

@boherm boherm requested a review from FabienPapet July 11, 2023 20:46
@MatShir
Copy link
Contributor

MatShir commented Aug 10, 2023

Might fix this issue #31385. Check it as a bonus on the QA, not a blocker for QA :)

@boherm
Copy link
Member Author

boherm commented Aug 23, 2023

Can I close my PR? Stuck since a lot of time.

@matks
Copy link
Contributor

matks commented Aug 23, 2023

Can I close my PR? Stuck since a lot of time.

No, you must keep it open UNTIL THE END OF TIMES!

@boherm boherm closed this Aug 23, 2023
@boherm boherm mentioned this pull request Jan 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zuk3975 zuk3975 Awaiting requested review from zuk3975

@FabienPapet FabienPapet Awaiting requested review from FabienPapet

Assignees
No one assigned
Labels
develop Branch Improvement Type: Improvement
Projects
PR Dashboard
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate "Orders > Shopping Carts" page
6 participants
@boherm @MatShir @matks @FabienPapet @zuk3975 @prestonBot