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

[Woo POS] Bump minimum required WC version to 9.6, replace local products filters with downloadable parameter, page size 25 #14843

Merged
merged 15 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Networking/Networking/Remote/ProductsRemote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol {
ParameterKey.orderBy: OrderKey.name.value,
ParameterKey.order: Order.ascending.value,
ParameterKey.productStatus: POSConstants.productStatus,
ParameterKey.downloadable: String(false),
]
let request = JetpackRequest(wooApiVersion: .mark3,
method: .get,
Expand Down Expand Up @@ -637,6 +638,7 @@ public extension ProductsRemote {
static let before = "before"
static let after = "after"
static let extendedInfo = "extended_info"
static let downloadable = "downloadable"
}

private enum ParameterValues {
Expand All @@ -648,7 +650,7 @@ public extension ProductsRemote {

private extension ProductsRemote {
enum POSConstants {
static let productsPerPage = "100"
static let productsPerPage = "25"
static let productType = "simple"
static let productStatus = "publish"
}
Expand Down
13 changes: 13 additions & 0 deletions Networking/NetworkingTests/Remote/ProductsRemoteTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,19 @@ final class ProductsRemoteTests: XCTestCase {
}
}

func test_loadProductsForPointOfSale_sets_correct_parameters() async throws {
// Given
let remote = ProductsRemote(network: network)

// When
_ = try? await remote.loadProductsForPointOfSale(for: sampleSiteID, productTypes: [.simple, .variable], pageNumber: 1)

// Then
let queryParametersDictionary = try XCTUnwrap(network.queryParametersDictionary)
XCTAssertEqual(queryParametersDictionary["downloadable"] as? String, String(false))
XCTAssertEqual(queryParametersDictionary["include_types"] as? String, "simple,variable")
}

func test_loadProductsForPointOfSale_when_page_has_no_products_then_loads_expected_products() async throws {
// Given
let remote = ProductsRemote(network: network)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,117 +243,6 @@
]
}
},
{
"id": 34,
"name": "Downloadable Long Sleeve Tee",
"slug": "downloadable-long-sleeve-tee",
"permalink": "https://example.com/product/downloadable-long-sleeve-tee/",
"date_created": "2018-01-26T21:49:46",
"date_created_gmt": "2018-01-26T21:49:46",
"date_modified": "2018-09-19T17:46:38",
"date_modified_gmt": "2018-09-19T17:46:38",
"type": "simple",
"status": "publish",
"featured": false,
"catalog_visibility": "visible",
"description": "<p>Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. Aenean ultricies mi vitae est. Mauris placerat eleifend leo.</p>\n",
"short_description": "",
"sku": "",
"price": "2517",
"regular_price": "2517",
"sale_price": "",
"date_on_sale_from": null,
"date_on_sale_from_gmt": null,
"date_on_sale_to": null,
"date_on_sale_to_gmt": null,
"price_html": "<span class=\"woocommerce-Price-amount amount\"><span class=\"woocommerce-Price-currencySymbol\">&#36;</span>2,517.00</span>",
"on_sale": false,
"purchasable": true,
"total_sales": 3,
"virtual": false,
"downloadable": true,
"downloads": [],
"download_limit": -1,
"download_expiry": -1,
"external_url": "",
"button_text": "",
"tax_status": "taxable",
"tax_class": "",
"manage_stock": false,
"stock_quantity": null,
"stock_status": "instock",
"backorders": "no",
"backorders_allowed": false,
"backordered": false,
"sold_individually": false,
"weight": "",
"dimensions": {
"length": "",
"width": "",
"height": ""
},
"shipping_required": true,
"shipping_taxable": true,
"shipping_class": "",
"shipping_class_id": 0,
"reviews_allowed": true,
"average_rating": "0.00",
"rating_count": 0,
"related_ids": [
36,
37,
35
],
"upsell_ids": [],
"cross_sell_ids": [],
"parent_id": 0,
"purchase_note": "",
"categories": [
{
"id": 17,
"name": "Tshirts",
"slug": "tshirts"
}
],
"tags": [],
"images": [
{
"id": 15,
"date_created": "2018-01-26T21:49:45",
"date_created_gmt": "2018-01-26T21:49:45",
"date_modified": "2018-01-26T21:49:45",
"date_modified_gmt": "2018-01-26T21:49:45",
"src": "https://i2.wp.com/thuy-nonjtpk.mystagingwebsite.com/wp-content/uploads/2018/01/long-sleeve-tee.jpg?fit=801%2C801&ssl=1",
"name": "Long Sleeve Tee",
"alt": ""
}
],
"attributes": [],
"default_attributes": [],
"variations": [],
"grouped_products": [],
"menu_order": 0,
"meta_data": [
{
"id": 101,
"key": "_customize_changeset_uuid",
"value": "e96101a5-dc4f-4cd7-8f38-30db941b1a97"
}
],
"jetpack_publicize_connections": [],
"_links": {
"self": [
{
"href": "https://example.com/wp-json/wc/v3/products/34"
}
],
"collection": [
{
"href": "https://example.com/wp-json/wc/v3/products"
}
]
}
},
{
"id": 33,
"name": "Private Hoodie",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,16 @@ private extension POSEligibilityChecker {
return
}

let wcPluginMinimumVersion = wcPluginMinimumVersion

let action = SystemStatusAction.fetchSystemPlugin(siteID: siteID, systemPluginName: Constants.wcPluginName) { wcPlugin in
guard let wcPlugin = wcPlugin, wcPlugin.active else {
promise(.success(false))
return
}

let isSupported = VersionHelpers.isVersionSupported(version: wcPlugin.version,
minimumRequired: Constants.wcPluginMinimumVersion)
minimumRequired: wcPluginMinimumVersion)
promise(.success(isSupported))
}
self.stores.dispatch(action)
Expand Down Expand Up @@ -116,11 +118,19 @@ private extension POSEligibilityChecker {
return false
}
}

private var wcPluginMinimumVersion: String {
guard featureFlagService.isFeatureFlagEnabled(.variableProductsInPointOfSale) else {
return Constants.legacyWcPluginMinimumVersion
}
return Constants.wcPluginMinimumVersion
}
}

private extension POSEligibilityChecker {
enum Constants {
static let wcPluginName = "WooCommerce"
static let wcPluginMinimumVersion = "6.6.0"
static let wcPluginMinimumVersion = "9.6.0-beta"
static let legacyWcPluginMinimumVersion = "6.6.0"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ final class MockFeatureFlagService: FeatureFlagService {
var tapToPayEducation: Bool
var receiptsForPOS: Bool
var hideSitesInStorePicker: Bool
var isVariableProductsInPOSEnabled: Bool

init(isInboxOn: Bool = false,
isShowInboxCTAEnabled: Bool = false,
Expand All @@ -52,7 +53,8 @@ final class MockFeatureFlagService: FeatureFlagService {
isSendReceiptAfterPaymentEnabled: Bool = false,
tapToPayEducation: Bool = false,
receiptsForPOS: Bool = false,
hideSitesInStorePicker: Bool = false) {
hideSitesInStorePicker: Bool = false,
isVariableProductsInPOSEnabled: Bool = false) {
self.isInboxOn = isInboxOn
self.isShowInboxCTAEnabled = isShowInboxCTAEnabled
self.isUpdateOrderOptimisticallyOn = isUpdateOrderOptimisticallyOn
Expand All @@ -78,6 +80,7 @@ final class MockFeatureFlagService: FeatureFlagService {
self.tapToPayEducation = tapToPayEducation
self.receiptsForPOS = receiptsForPOS
self.hideSitesInStorePicker = hideSitesInStorePicker
self.isVariableProductsInPOSEnabled = isVariableProductsInPOSEnabled
}

func isFeatureFlagEnabled(_ featureFlag: FeatureFlag) -> Bool {
Expand Down Expand Up @@ -132,6 +135,8 @@ final class MockFeatureFlagService: FeatureFlagService {
return receiptsForPOS
case .hideSitesInStorePicker:
return hideSitesInStorePicker
case .variableProductsInPointOfSale:
return isVariableProductsInPOSEnabled
default:
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,14 @@ final class POSEligibilityCheckerTests: XCTestCase {
XCTAssertFalse(isEligible)
}

func test_is_eligible_when_WooCommerce_version_is_below_6_6_then_returns_false() throws {
func test_is_eligible_when_WooCommerce_version_is_below_9_6_then_returns_false() throws {
// Given
let featureFlagService = MockFeatureFlagService(isPointOfSaleEnabled: true)
let featureFlagService = MockFeatureFlagService(isPointOfSaleEnabled: true,
isVariableProductsInPOSEnabled: true)
setupCountry(country: .us)

// Unsupported WooCommerce version
setupWooCommerceVersion("6.5.0")
setupWooCommerceVersion("9.5.2")

// When
let checker = POSEligibilityChecker(userInterfaceIdiom: .pad,
Expand All @@ -164,9 +165,53 @@ final class POSEligibilityCheckerTests: XCTestCase {
XCTAssertFalse(isEligible)
}

func test_is_eligible_when_WooCommerce_version_is_above_6_6_then_returns_true() throws {
func test_is_eligible_when_WooCommerce_version_is_at_least_9_6_then_returns_true() throws {
// Given
let featureFlagService = MockFeatureFlagService(isPointOfSaleEnabled: true)
let featureFlagService = MockFeatureFlagService(isPointOfSaleEnabled: true,
isVariableProductsInPOSEnabled: true)
setupCountry(country: .us)
accountWhitelistedInBackend(true)

// Supported WooCommerce version
setupWooCommerceVersion("9.6.0-beta1")

// When
let checker = POSEligibilityChecker(userInterfaceIdiom: .pad,
siteSettings: siteSettings,
currencySettings: Fixtures.usdCurrencySettings,
stores: stores,
featureFlagService: featureFlagService)
checker.isEligible.assign(to: &$isEligible)

// Then
XCTAssertTrue(isEligible)
}

func test_is_eligible_when_WooCommerce_version_is_below_6_6_and_variable_products_disabled_then_returns_false() throws {
// Given
let featureFlagService = MockFeatureFlagService(isPointOfSaleEnabled: true,
isVariableProductsInPOSEnabled: false)
setupCountry(country: .us)

// Unsupported WooCommerce version
setupWooCommerceVersion("6.5.2")

// When
let checker = POSEligibilityChecker(userInterfaceIdiom: .pad,
siteSettings: siteSettings,
currencySettings: Fixtures.usdCurrencySettings,
stores: stores,
featureFlagService: featureFlagService)
checker.isEligible.assign(to: &$isEligible)

// Then
XCTAssertFalse(isEligible)
}

func test_is_eligible_when_WooCommerce_version_is_at_least_6_6_and_variable_products_disabled_then_returns_true() throws {
// Given
let featureFlagService = MockFeatureFlagService(isPointOfSaleEnabled: true,
isVariableProductsInPOSEnabled: false)
setupCountry(country: .us)
accountWhitelistedInBackend(true)

Expand Down Expand Up @@ -199,7 +244,7 @@ private extension POSEligibilityCheckerTests {
siteSettings.refresh()
}

func setupWooCommerceVersion(_ version: String = "6.6.0") {
func setupWooCommerceVersion(_ version: String = "9.6.0-beta") {
stores.whenReceivingAction(ofType: SystemStatusAction.self) { action in
switch action {
case .fetchSystemPlugin(_, _, let completion):
Expand Down
29 changes: 1 addition & 28 deletions Yosemite/Yosemite/PointOfSale/PointOfSaleItemService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,7 @@ public final class PointOfSaleItemService: PointOfSaleItemServiceProtocol {
return .init(items: [], hasMorePages: false)
}

let eligibilityCriteria: [(Product) -> Bool] = [
isNotVirtual,
isNotDownloadable,
hasPrice
]
let filteredProducts = filterProducts(products: products, using: eligibilityCriteria)

return .init(items: mapProductsToPOSItems(products: filteredProducts), hasMorePages: pagedProducts.hasMorePages)
return .init(items: mapProductsToPOSItems(products: products), hasMorePages: pagedProducts.hasMorePages)
}

public func providePointOfSaleVariationItems(for parentProduct: POSVariableParentProduct, pageNumber: Int) async throws -> PagedItems<POSItem> {
Expand Down Expand Up @@ -123,23 +116,3 @@ public final class PointOfSaleItemService: PointOfSaleItemServiceProtocol {
}
}
}

private extension PointOfSaleItemService {
func filterProducts(products: [Product], using criteria: [(Product) -> Bool]) -> [Product] {
return products.filter { product in
criteria.allSatisfy { $0(product) }
}
}

func isNotVirtual(product: Product) -> Bool {
!product.virtual
}

func isNotDownloadable(product: Product) -> Bool {
!product.downloadable
}

func hasPrice(product: Product) -> Bool {
!product.price.isEmpty
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,24 @@ final class PointOfSaleItemServiceTests: XCTestCase {

func test_PointOfSaleItemServiceProtocol_when_eligibility_criteria_applies_then_returns_correct_number_of_items() async throws {
// Given
let expectedNumberOfItems = 2
let expectedItemNames = ["Dymo LabelWriter 4XL", "Private Hoodie"]
let expectedItemNames = ["Dymo LabelWriter 4XL", "Virtual Polo", "Private Hoodie", "Hoodie with Zipper without price"]

// When
network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all-for-eligibility-criteria")
let pagedItems = try await itemProvider.providePointOfSaleItems()

// Then
let expectedItems = pagedItems.items
XCTAssertEqual(expectedItems.count, expectedNumberOfItems)

guard case .simpleProduct(let firstEligibleSimpleProduct) = expectedItems.first,
case .simpleProduct(let secondEligibleSimpleProduct) = expectedItems.last else {
return XCTFail("Expected \(expectedNumberOfItems) eligible items. Got \(expectedItems.count) instead.")
let items = pagedItems.items
XCTAssertEqual(items.count, expectedItemNames.count)

let itemNames: [String] = items.compactMap {
guard case let .simpleProduct(simpleProduct) = $0 else {
XCTFail("Expected simple product.")
return nil
}
return simpleProduct.name
}
XCTAssertEqual(firstEligibleSimpleProduct.name, expectedItemNames.first)
XCTAssertEqual(secondEligibleSimpleProduct.name, expectedItemNames.last)
XCTAssertEqual(itemNames, expectedItemNames)
}

// MARK: - Query Parameters
Expand Down