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

Open
wants to merge 11 commits into
base: trunk
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion Networking/Networking/Remote/ProductVariationsRemote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,6 @@ public extension ProductVariationsRemote {

private extension ProductVariationsRemote {
enum POSConstants {
static let variationsPerPage: Int = 100
static let variationsPerPage: Int = 25
}
}
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
10 changes: 0 additions & 10 deletions Yosemite/Yosemite/PointOfSale/PointOfSaleItemService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ public final class PointOfSaleItemService: PointOfSaleItemServiceProtocol {
}

let eligibilityCriteria: [(Product) -> Bool] = [
isNotVirtual,
isNotDownloadable,
hasPrice
]
let filteredProducts = filterProducts(products: products, using: eligibilityCriteria)
Expand Down Expand Up @@ -127,14 +125,6 @@ private extension PointOfSaleItemService {
}
}

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

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

func hasPrice(product: Product) -> Bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on pdfdoF-62l-p2#comment-7285, this one can be removed as well 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in b192a0c.

!product.price.isEmpty
}
Expand Down
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"]

// 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
Loading