-
Notifications
You must be signed in to change notification settings - Fork 121
[POS Orders] Render sales channel option as order filter #15878
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
Changes from 6 commits
20c5f1f
251e256
d65cbf1
a7d4a6e
acb9bdc
1346312
4f93bca
de53799
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ final class FilterOrderListViewModel: FilterListViewModel { | |
| let dateRange: OrderDateRangeFilter? | ||
| let product: FilterOrdersByProduct? | ||
| let customer: CustomerFilter? | ||
| let salesChannel: SalesChannelFilter? | ||
|
|
||
| let numberOfActiveFilters: Int | ||
|
|
||
|
|
@@ -21,18 +22,21 @@ final class FilterOrderListViewModel: FilterListViewModel { | |
| dateRange = nil | ||
| product = nil | ||
| customer = nil | ||
| salesChannel = nil | ||
| numberOfActiveFilters = 0 | ||
| } | ||
|
|
||
| init(orderStatus: [OrderStatusEnum]?, | ||
| dateRange: OrderDateRangeFilter?, | ||
| product: FilterOrdersByProduct?, | ||
| customer: CustomerFilter?, | ||
| salesChannel: SalesChannelFilter?, | ||
| numberOfActiveFilters: Int) { | ||
| self.orderStatus = orderStatus | ||
| self.dateRange = dateRange | ||
| self.product = product | ||
| self.customer = customer | ||
| self.salesChannel = salesChannel | ||
| self.numberOfActiveFilters = numberOfActiveFilters | ||
| } | ||
|
|
||
|
|
@@ -50,13 +54,18 @@ final class FilterOrderListViewModel: FilterListViewModel { | |
| if let customer = customer { | ||
| readable.append(customer.description) | ||
| } | ||
|
|
||
| if let salesChannel = salesChannel { | ||
| readable.append(salesChannel.description) | ||
| } | ||
|
|
||
| return readable.joined(separator: ", ") | ||
| } | ||
| } | ||
|
|
||
| let filterActionTitle = Localization.filterActionTitle | ||
|
|
||
| let filterTypeViewModels: [FilterTypeViewModel] | ||
| var filterTypeViewModels: [FilterTypeViewModel] | ||
|
|
||
| let shouldShowHistory: Bool | ||
|
|
||
|
|
@@ -66,6 +75,7 @@ final class FilterOrderListViewModel: FilterListViewModel { | |
| private let dateRangeFilterViewModel: FilterTypeViewModel | ||
| private let productFilterViewModel: FilterTypeViewModel | ||
| private let customerFilterViewModel: FilterTypeViewModel | ||
| private let salesChannelFilterViewModel: FilterTypeViewModel | ||
|
|
||
| private let siteID: Int64 | ||
| private let stores: StoresManager | ||
|
|
@@ -87,25 +97,35 @@ final class FilterOrderListViewModel: FilterListViewModel { | |
| dateRangeFilterViewModel = OrderListFilter.dateRange.createViewModel(filters: filters, allowedStatuses: allowedStatuses) | ||
| productFilterViewModel = OrderListFilter.product(siteID: siteID).createViewModel(filters: filters, allowedStatuses: allowedStatuses) | ||
| customerFilterViewModel = OrderListFilter.customer(siteID: siteID).createViewModel(filters: filters, allowedStatuses: allowedStatuses) | ||
| salesChannelFilterViewModel = OrderListFilter.salesChannel.createViewModel(filters: filters, allowedStatuses: allowedStatuses) | ||
|
|
||
| self.siteID = siteID | ||
| self.stores = stores | ||
| self.analytics = analytics | ||
|
|
||
| shouldShowHistory = featureFlagService.isFeatureFlagEnabled(.filterHistoryOnOrderAndProductLists) | ||
| filterTypeViewModels = [orderStatusFilterViewModel, dateRangeFilterViewModel, customerFilterViewModel, productFilterViewModel] | ||
| filterTypeViewModels = [orderStatusFilterViewModel, | ||
| dateRangeFilterViewModel, | ||
| customerFilterViewModel, | ||
| productFilterViewModel] | ||
|
|
||
| if featureFlagService.isFeatureFlagEnabled(.pointOfSaleOrdersi2) { | ||
| filterTypeViewModels.append(salesChannelFilterViewModel) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is the only reason the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the only reason indeed, changed on 4f93bca |
||
| } | ||
|
|
||
| var criteria: Filters { | ||
| let orderStatus = orderStatusFilterViewModel.selectedValue as? [OrderStatusEnum] ?? nil | ||
| let dateRange = dateRangeFilterViewModel.selectedValue as? OrderDateRangeFilter ?? nil | ||
| let product = productFilterViewModel.selectedValue as? FilterOrdersByProduct ?? nil | ||
| let customer = customerFilterViewModel.selectedValue as? CustomerFilter ?? nil | ||
| let salesChannel = salesChannelFilterViewModel.selectedValue as? SalesChannelFilter ?? nil | ||
| let numberOfActiveFilters = filterTypeViewModels.numberOfActiveFilters | ||
| return Filters(orderStatus: orderStatus, | ||
| dateRange: dateRange, | ||
| product: product, | ||
| customer: customer, | ||
| salesChannel: salesChannel, | ||
| numberOfActiveFilters: numberOfActiveFilters) | ||
| } | ||
|
|
||
|
|
@@ -120,6 +140,7 @@ final class FilterOrderListViewModel: FilterListViewModel { | |
| dateRange: item.dateRangeFilter, | ||
| product: item.productFilter, | ||
| customer: item.customerFilter, | ||
| salesChannel: nil, // TODO: Filter persistence WOOMOB-712 | ||
| numberOfActiveFilters: item.numberOfActiveFilters()) | ||
| } | ||
| continuation.resume(returning: filters) | ||
|
|
@@ -198,6 +219,7 @@ extension FilterOrderListViewModel { | |
| case dateRange | ||
| case product(siteID: Int64) | ||
| case customer(siteID: Int64) | ||
| case salesChannel | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -212,6 +234,8 @@ private extension FilterOrderListViewModel.OrderListFilter { | |
| return Localization.rowTitleProduct | ||
| case .customer: | ||
| return Localization.rowCustomer | ||
| case .salesChannel: | ||
| return Localization.rowSalesChannel | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -235,6 +259,10 @@ extension FilterOrderListViewModel.OrderListFilter { | |
| return FilterTypeViewModel(title: title, | ||
| listSelectorConfig: .customer(siteID: siteID), | ||
| selectedValue: filters.customer) | ||
| case .salesChannel: | ||
| return FilterTypeViewModel(title: title, | ||
| listSelectorConfig: .salesChannel, | ||
| selectedValue: filters.salesChannel) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -281,20 +309,39 @@ extension FilterOrdersByProduct: FilterType { | |
| // MARK: - Constants | ||
| private extension FilterOrderListViewModel { | ||
| enum Localization { | ||
| static let filterActionTitle = NSLocalizedString("Show Orders", comment: "Button title for applying filters to a list of orders.") | ||
| static let filterActionTitle = NSLocalizedString( | ||
| "filterOrderListViewModel.OrderListFilter.filterActionTitle", | ||
| value: "Show Orders", | ||
| comment: "Button title for applying filters to a list of orders.") | ||
| } | ||
| } | ||
|
|
||
| private extension FilterOrderListViewModel.OrderListFilter { | ||
| enum Localization { | ||
| static let rowTitleOrderStatus = NSLocalizedString("Order Status", comment: "Row title for filtering orders by order status.") | ||
| static let rowTitleDateRange = NSLocalizedString("Date Range", comment: "Row title for filtering orders by date range.") | ||
| static let rowTitleProduct = NSLocalizedString("filterOrderListViewModel.OrderListFilter.rowTitleProduct", | ||
| value: "Product", | ||
| comment: "Row title for filtering orders by Product.") | ||
| static let rowCustomer = NSLocalizedString("filterOrderListViewModel.OrderListFilter.rowCustomer", | ||
| value: "Customer", | ||
| comment: "Row title for filtering orders by customer.") | ||
| static let rowTitleOrderStatus = NSLocalizedString( | ||
| "filterOrderListViewModel.OrderListFilter.rowTitleOrderStatus", | ||
| value: "Order Status", | ||
| comment: "Row title for filtering orders by order status.") | ||
|
|
||
| static let rowTitleDateRange = NSLocalizedString( | ||
| "filterOrderListViewModel.OrderListFilter.rowTitleDateRange", | ||
| value: "Date Range", | ||
| comment: "Row title for filtering orders by date range.") | ||
|
Comment on lines
+323
to
+331
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no real benefit to changing existing localized string definitions to use the It incurrs cost/work, because the translation will need to be done again in all the langages even though it's not changed, and I don't know if Glotpress is good at linking up the old version with the new version...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting point, thanks! Since p91TBi-aJl-p2 I've been updating these to adopt the "DNS key thingy" approach every time I had to directly work with them. I'll take that into account moving forward 👍
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, if you're changing a string, at all, it's definitely right to add the reverse DNS key. |
||
|
|
||
| static let rowTitleProduct = NSLocalizedString( | ||
| "filterOrderListViewModel.OrderListFilter.rowTitleProduct", | ||
| value: "Product", | ||
| comment: "Row title for filtering orders by Product.") | ||
|
|
||
| static let rowCustomer = NSLocalizedString( | ||
| "filterOrderListViewModel.OrderListFilter.rowCustomer", | ||
| value: "Customer", | ||
| comment: "Row title for filtering orders by customer.") | ||
|
|
||
| static let rowSalesChannel = NSLocalizedString( | ||
| "filterOrderListViewModel.OrderListFilter.rowSalesChannel", | ||
| value: "Sales Channel", | ||
| comment: "Row title for filtering orders by sales channel.") | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -322,3 +369,20 @@ extension CustomerFilter: FilterType { | |
| /// Whether the filter is set to a non-empty value. | ||
| var isActive: Bool { true } | ||
| } | ||
|
|
||
| extension FilterOrderListViewModel { | ||
| enum SalesChannelFilter: FilterType { | ||
| case pointOfSale | ||
|
|
||
| var description: String { | ||
| switch self { | ||
| case .pointOfSale: | ||
| return "pos" | ||
|
||
| } | ||
| } | ||
|
|
||
| var isActive: Bool { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
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.
You don't need to change it now, but for a partial PR like this, it might be better to use the default. It saves you a bunch of changes in the tests, which you're probably going to have to change again when you implement it properly. The compiler will still help you catch them all when you remove the default in a future PR.
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.
True 😅 , thanks for pointing it out.