-
Notifications
You must be signed in to change notification settings - Fork 110
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
Added custom statuses in Order Filters #6390
Added custom statuses in Order Filters #6390
Conversation
…us Filters, for mantaining the data about each status.
…s fetched from API
…ng status filters fetched from API, we reset them
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Case 1
7. Re-launch the app. The old filters should be still selected.
Somehow the order status filters are reset after relaunching the app or pulling to refresh 🤔 (Feel free to try the test store "fun testing")
simulator.mov
WooCommerce/Classes/ViewRelated/Orders/OrdersRootViewController.swift
Outdated
Show resolved
Hide resolved
WooCommerce/Classes/ViewRelated/Orders/OrdersRootViewController.swift
Outdated
Show resolved
Hide resolved
WooCommerce/Classes/ViewRelated/Orders/Order Filters/FilterOrderListViewModel.swift
Show resolved
Hide resolved
...s/ViewRelated/Orders/Order Filters/Order Status Filter/OrderStatusFilterViewController.swift
Outdated
Show resolved
Hide resolved
Thanks for discovering the issue! I fixed it here: 8d966cd Ready for another test 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @pmusolino! With test case 2, I ran into a different issue: when syncing the orders with the previously selected filter that was deleted in core, the API threw an error Invalid parameter(s): status
most likely because we're passing the deleted status in the /orders
request. The orders tab then got stuck on the error state with the top banner and the status filter is still on:
Maybe we want to also call resetFiltersIfAnyStatusFilterIsNoMoreExisting
after syncing order statuses like after viewModel.syncOrderStatuses()
completes? 🤔
Steps to repro:
- Set an order to a custom status
- On the orders tab, tap Filter
- Tap "Order Status" and select the custom status in the first step and/or any other status
- Close the app
- Launch the app
- Go to the orders tab --> an error top banner appeared, and the only way to get out of this state is to reset the filters
...s/ViewRelated/Orders/Order Filters/Order Status Filter/OrderStatusFilterViewController.swift
Outdated
Show resolved
Hide resolved
…'trunk' of https://github.com/woocommerce/woocommerce-ios into issue/5498-add-custom-statuses-in-order-filters
Discovered the bug. In reality two bugs: there was a missing Can you take another test now? Thanks for your patience Jaclyn 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly a request on the assertionFailure
call, otherwise LGTM! 🚢
some minor issues that aren't blocking:
- In case 2, the top banner "We couldn't load your data" still appears and stays. It disappears after pulling down to refresh
- Maybe we can use the name of the status from the API response? Since the Order Status Manager plugin allows renaming a core status while we localize the core statuses in the app, there might be a mismatch on the status name which might be confusing to merchants with custom naming:
core | app |
---|---|
WooCommerce/Classes/ViewRelated/Orders/OrdersRootViewController.swift
Outdated
Show resolved
Hide resolved
TIL that is possible to rename the existing filters. I created a separate issue here #6433 👍 |
Closes #5498
Description
During the implementation of Order Filters, @jaclync suggested implementing also the support for custom statuses, that could be added to the website using an external plugin to WooCommerce. This is a functionality that was super requested by our users.
So, in this PR for Hack Week, I implemented a way to display all the statuses that we fetch in the Order List, including the custom statuses created from the web. So, from now, merchants will be able to filter orders also by one or more custom statuses.
Testing instructions
Case 1
Case 2
Screenshots
RELEASE-NOTES.txt
if necessary.