Skip to content

Commit e31de65

Browse files
selanthiraiyanselanthiraiyan
committed
Merge branch 'trunk' into feat/15127-store-id-app-events
2 parents 186df92 + bfafb75 commit e31de65

21 files changed

+508
-143
lines changed

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- [*] Background image upload: Fix missing error notice in iPhones [https://github.com/woocommerce/woocommerce-ios/pull/15117]
1212
- [*] Background image upload: Show a notice when the user leaves product details while uploads are pending [https://github.com/woocommerce/woocommerce-ios/pull/15134]
1313
- [*] Filters applied in product selector no longer affect the main product list screen. [https://github.com/woocommerce/woocommerce-ios/pull/14764]
14+
- [**] Product Images: Update error handling [https://github.com/woocommerce/woocommerce-ios/pull/15105]
1415

1516
21.7
1617
-----

‎WooCommerce.xcworkspace/xcshareddata/swiftpm/Package.resolved‎

Lines changed: 2 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎WooCommerce/Classes/Analytics/WooAnalyticsStat.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,7 @@ enum WooAnalyticsStat: String {
822822
case productImageSettingsAddImagesSourceTapped = "product_image_settings_add_images_source_tapped"
823823
case productImageSettingsDeleteImageButtonTapped = "product_image_settings_delete_image_button_tapped"
824824
case productImageUploadFailed = "product_image_upload_failed"
825+
case productImageUploadRetryButtonTapped = "product_image_upload_retry_button_tapped"
825826
case savingProductAfterBackgroundImageUploadSuccess = "saving_product_after_background_image_upload_success"
826827
case savingProductAfterBackgroundImageUploadFailed = "saving_product_after_background_image_upload_failed"
827828
case failureSavingProductAfterImageUploadNoticeShown = "failure_saving_product_after_image_upload_notice_shown"

‎WooCommerce/Classes/ServiceLocator/ProductImageUploader.swift‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import protocol Yosemite.StoresManager
88
struct ProductImageUploadErrorInfo {
99
let siteID: Int64
1010
let productOrVariationID: ProductOrVariationID
11-
let productImageStatuses: [ProductImageStatus]
1211
let error: ProductImageUploaderError
1312
}
1413

@@ -113,6 +112,7 @@ final class ProductImageUploader: ProductImageUploaderProtocol {
113112
private let errorsSubject: PassthroughSubject<ProductImageUploadErrorInfo, Never> = .init()
114113
private var statusUpdatesExcludedProductKeys: Set<Key> = []
115114
private var statusUpdatesSubscriptions: Set<AnyCancellable> = []
115+
private var imageUploadSubscriptions: Set<AnyCancellable> = []
116116

117117
private var actionHandlersByProduct: [Key: ProductImageActionHandler] = [:]
118118
private var imagesSaverByProduct: [Key: ProductImagesSaver] = [:]
@@ -136,6 +136,7 @@ final class ProductImageUploader: ProductImageUploaderProtocol {
136136
actionHandler = ProductImageActionHandler(siteID: key.siteID, productID: key.productOrVariationID, imageStatuses: originalStatuses, stores: stores)
137137
actionHandlersByProduct[key] = actionHandler
138138
observeStatusUpdates(key: key, actionHandler: actionHandler)
139+
observeImageUploads(key: key, actionHandler: actionHandler)
139140
}
140141

141142
return actionHandler
@@ -186,7 +187,7 @@ final class ProductImageUploader: ProductImageUploaderProtocol {
186187

187188
if let productImagesSaver, productImagesSaver.imageStatusesToSave.isNotEmpty {
188189
// If there are images scheduled to be saved, there are no unsaved changes if the image statuses to save match the latest image statuses.
189-
return handler.productImageStatuses != productImagesSaver.imageStatusesToSave
190+
return handler.productImageStatuses.images != productImagesSaver.imageStatusesToSave.images
190191
} else {
191192
if handler.productImageStatuses.hasPendingUpload {
192193
return true
@@ -231,7 +232,6 @@ final class ProductImageUploader: ProductImageUploaderProtocol {
231232
if case let .failure(error) = result {
232233
self.errorsSubject.send(.init(siteID: key.siteID,
233234
productOrVariationID: key.productOrVariationID,
234-
productImageStatuses: handler.productImageStatuses,
235235
error: .failedSavingProductAfterImageUpload(error: error)))
236236
}
237237
self.updateProductIDOfImagesUploadedUsingLocalProductID(siteID: key.siteID,
@@ -243,6 +243,7 @@ final class ProductImageUploader: ProductImageUploaderProtocol {
243243
func reset() {
244244
statusUpdatesExcludedProductKeys = []
245245
statusUpdatesSubscriptions = []
246+
imageUploadSubscriptions = []
246247
activeUploadsPublisher = []
247248

248249
actionHandlersByProduct = [:]
@@ -266,7 +267,7 @@ private extension ProductImageUploader {
266267
}
267268

268269
func observeStatusUpdates(key: Key, actionHandler: ProductImageActionHandler) {
269-
let observationToken = actionHandler.addUpdateObserver(self) { [weak self] (productImageStatuses, error) in
270+
let observationToken = actionHandler.addUpdateObserver(self) { [weak self] productImageStatuses in
270271
guard let self = self else { return }
271272

272273
if !activeUploadsPublisher.contains(key), productImageStatuses.hasPendingUpload {
@@ -276,16 +277,24 @@ private extension ProductImageUploader {
276277
/// remove the key from active uploads
277278
removeProductFromActiveUploads(key: key)
278279
}
280+
}
281+
statusUpdatesSubscriptions.insert(observationToken)
282+
}
279283

280-
if let error = error, statusUpdatesExcludedProductKeys.contains(key) == false {
281-
removeProductFromActiveUploads(key: key)
282-
errorsSubject.send(.init(siteID: key.siteID,
283-
productOrVariationID: key.productOrVariationID,
284-
productImageStatuses: productImageStatuses,
285-
error: .failedUploadingImage(error: error)))
284+
func observeImageUploads(key: Key, actionHandler: ProductImageActionHandler) {
285+
let observationToken = actionHandler.addAssetUploadObserver(self) { [weak self] asset, result in
286+
guard let self else { return }
287+
288+
if case .failure(let error) = result {
289+
let infoError = ProductImageUploadErrorInfo(siteID: key.siteID,
290+
productOrVariationID: key.productOrVariationID,
291+
error: .failedUploadingImage(asset: asset, error: error))
292+
if statusUpdatesExcludedProductKeys.contains(key) == false {
293+
errorsSubject.send(infoError)
294+
}
286295
}
287296
}
288-
statusUpdatesSubscriptions.insert(observationToken)
297+
imageUploadSubscriptions.insert(observationToken)
289298
}
290299

291300
func removeProductFromActiveUploads(key: Key) {
@@ -309,7 +318,7 @@ enum ProductImageUploaderError: Error {
309318
case noActionHandlerFound
310319
case noRemoteProductIDFound
311320
case failedSavingProductAfterImageUpload(error: Error)
312-
case failedUploadingImage(error: Error)
321+
case failedUploadingImage(asset: ProductImageAssetType, error: Error)
313322
}
314323

315324
private enum Localization {

‎WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/Collection View Cells/FailedProductImageCollectionViewCell.swift‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import UIKit
2+
3+
final class FailedProductImageCollectionViewCell: UICollectionViewCell {
4+
5+
@IBOutlet var imageView: UIImageView!
6+
@IBOutlet var effectView: UIVisualEffectView!
7+
@IBOutlet var errorImageView: UIImageView!
8+
9+
override func awakeFromNib() {
10+
super.awakeFromNib()
11+
configureBackground()
12+
configureImageView()
13+
configureErrorImageView()
14+
configureCellAppearance()
15+
}
16+
}
17+
18+
/// Private Methods
19+
///
20+
private extension FailedProductImageCollectionViewCell {
21+
func configureBackground() {
22+
applyGrayBackgroundStyle()
23+
}
24+
25+
func configureImageView() {
26+
imageView.contentMode = Settings.imageContentMode
27+
imageView.clipsToBounds = Settings.clipToBounds
28+
}
29+
30+
func configureBlurView() {
31+
let blurEffect = UIBlurEffect(style: .extraLight)
32+
effectView.effect = blurEffect
33+
}
34+
35+
func configureErrorImageView() {
36+
errorImageView.image = UIImage(systemName: "arrow.counterclockwise")
37+
errorImageView.tintColor = .white
38+
}
39+
40+
func configureCellAppearance() {
41+
contentView.layer.cornerRadius = Constants.cornerRadius
42+
contentView.layer.borderWidth = Constants.borderWidth
43+
contentView.layer.borderColor = Colors.borderColor.cgColor
44+
contentView.layer.masksToBounds = Settings.maskToBounds
45+
}
46+
}
47+
48+
/// Constants
49+
///
50+
private extension FailedProductImageCollectionViewCell {
51+
enum Constants {
52+
static let cornerRadius = CGFloat(2.0)
53+
static let borderWidth = CGFloat(0.5)
54+
}
55+
56+
enum Colors {
57+
static let borderColor = UIColor.systemColor(.systemGray4)
58+
}
59+
60+
enum Settings {
61+
static let clipToBounds = true
62+
static let imageContentMode = ContentMode.center
63+
static let maskToBounds = true
64+
}
65+
}

‎WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/Collection View Cells/FailedProductImageCollectionViewCell.xib‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<document type="com.apple.InterfaceBuilder3.CocoaTouch.XIB" version="3.0" toolsVersion="23504" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES">
3+
<device id="retina6_12" orientation="portrait" appearance="light"/>
4+
<dependencies>
5+
<deployment identifier="iOS"/>
6+
<plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="23506"/>
7+
<capability name="collection view cell content view" minToolsVersion="11.0"/>
8+
<capability name="documents saved in the Xcode 8 format" minToolsVersion="8.0"/>
9+
</dependencies>
10+
<objects>
11+
<placeholder placeholderIdentifier="IBFilesOwner" id="-1" userLabel="File's Owner"/>
12+
<placeholder placeholderIdentifier="IBFirstResponder" id="-2" customClass="UIResponder"/>
13+
<collectionViewCell opaque="NO" clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="center" id="6bQ-pQ-pPf" customClass="FailedProductImageCollectionViewCell" customModule="WooCommerce" customModuleProvider="target">
14+
<rect key="frame" x="0.0" y="0.0" width="229" height="199"/>
15+
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMaxY="YES"/>
16+
<collectionViewCellContentView key="contentView" opaque="NO" clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="center" insetsLayoutMarginsFromSafeArea="NO" id="Ugl-WK-kVN">
17+
<rect key="frame" x="0.0" y="0.0" width="229" height="199"/>
18+
<autoresizingMask key="autoresizingMask"/>
19+
<subviews>
20+
<imageView clipsSubviews="YES" userInteractionEnabled="NO" contentMode="scaleAspectFit" horizontalHuggingPriority="251" verticalHuggingPriority="251" translatesAutoresizingMaskIntoConstraints="NO" id="Wxb-SP-8eM">
21+
<rect key="frame" x="0.0" y="0.0" width="229" height="199"/>
22+
</imageView>
23+
<visualEffectView opaque="NO" contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="AfW-fi-jBR">
24+
<rect key="frame" x="0.0" y="0.0" width="229" height="199"/>
25+
<view key="contentView" opaque="NO" clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="center" insetsLayoutMarginsFromSafeArea="NO" id="Wvy-yO-E8M">
26+
<rect key="frame" x="0.0" y="0.0" width="229" height="199"/>
27+
<autoresizingMask key="autoresizingMask" widthSizable="YES" heightSizable="YES"/>
28+
</view>
29+
<blurEffect style="regular"/>
30+
</visualEffectView>
31+
<imageView clipsSubviews="YES" userInteractionEnabled="NO" contentMode="scaleAspectFit" horizontalHuggingPriority="251" verticalHuggingPriority="251" translatesAutoresizingMaskIntoConstraints="NO" id="imw-fJ-OtJ">
32+
<rect key="frame" x="76.666666666666657" y="61.666666666666664" width="75.666666666666657" height="75.666666666666686"/>
33+
<constraints>
34+
<constraint firstAttribute="width" secondItem="imw-fJ-OtJ" secondAttribute="height" id="XhB-KK-7Vn"/>
35+
</constraints>
36+
</imageView>
37+
</subviews>
38+
<constraints>
39+
<constraint firstItem="imw-fJ-OtJ" firstAttribute="centerX" secondItem="Ugl-WK-kVN" secondAttribute="centerX" id="2Wj-uU-uUQ"/>
40+
<constraint firstItem="imw-fJ-OtJ" firstAttribute="width" secondItem="Ugl-WK-kVN" secondAttribute="width" multiplier="0.33" id="5fK-xi-gEI"/>
41+
<constraint firstAttribute="trailing" secondItem="Wxb-SP-8eM" secondAttribute="trailing" id="9e4-04-4yc"/>
42+
<constraint firstItem="AfW-fi-jBR" firstAttribute="leading" secondItem="Wxb-SP-8eM" secondAttribute="leading" id="AzL-LI-Aks"/>
43+
<constraint firstItem="Wxb-SP-8eM" firstAttribute="leading" secondItem="Ugl-WK-kVN" secondAttribute="leading" id="CGG-Ge-V3j"/>
44+
<constraint firstItem="AfW-fi-jBR" firstAttribute="trailing" secondItem="Wxb-SP-8eM" secondAttribute="trailing" id="CVQ-Q1-TBb"/>
45+
<constraint firstItem="Wxb-SP-8eM" firstAttribute="top" secondItem="Ugl-WK-kVN" secondAttribute="top" id="Q7R-qf-3Ix"/>
46+
<constraint firstItem="AfW-fi-jBR" firstAttribute="top" secondItem="Wxb-SP-8eM" secondAttribute="top" id="cLt-eH-Ufh"/>
47+
<constraint firstAttribute="bottom" secondItem="Wxb-SP-8eM" secondAttribute="bottom" id="jtN-Ew-3dt"/>
48+
<constraint firstItem="AfW-fi-jBR" firstAttribute="bottom" secondItem="Wxb-SP-8eM" secondAttribute="bottom" id="lKc-lT-nny"/>
49+
<constraint firstItem="imw-fJ-OtJ" firstAttribute="centerY" secondItem="Ugl-WK-kVN" secondAttribute="centerY" id="mxu-uB-gwv"/>
50+
</constraints>
51+
</collectionViewCellContentView>
52+
<size key="customSize" width="229" height="199"/>
53+
<connections>
54+
<outlet property="effectView" destination="AfW-fi-jBR" id="0AI-1b-8sf"/>
55+
<outlet property="errorImageView" destination="imw-fJ-OtJ" id="5ri-Tl-vxU"/>
56+
<outlet property="imageView" destination="Wxb-SP-8eM" id="M2h-nc-rPC"/>
57+
</connections>
58+
<point key="canvasLocation" x="-70.992366412213741" y="72.183098591549296"/>
59+
</collectionViewCell>
60+
</objects>
61+
</document>

‎WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesCollectionViewDataSource.swift‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ private extension ProductImagesCollectionViewDataSource {
6060
case .uiImage(let image, _, _):
6161
configureUploadingImageCell(cell, image: image)
6262
}
63+
case let .uploadFailure(asset, _):
64+
switch asset {
65+
case .phAsset(let asset):
66+
configureFailedImageCell(cell, asset: asset)
67+
case .uiImage(let image, _, _):
68+
configureFailedImageCell(cell, image: image)
69+
}
6370
}
6471
}
6572

@@ -109,6 +116,29 @@ private extension ProductImagesCollectionViewDataSource {
109116
cell.imageView.contentMode = .scaleAspectFit
110117
cell.imageView.image = image
111118
}
119+
120+
func configureFailedImageCell(_ cell: UICollectionViewCell, asset: PHAsset) {
121+
guard let cell = cell as? FailedProductImageCollectionViewCell else {
122+
fatalError()
123+
}
124+
125+
cell.imageView.contentMode = .center
126+
cell.imageView.image = .productsTabProductCellPlaceholderImage
127+
128+
productUIImageLoader.requestImage(asset: asset, targetSize: cell.bounds.size) { [weak cell] image in
129+
cell?.imageView.contentMode = .scaleAspectFit
130+
cell?.imageView.image = image
131+
}
132+
}
133+
134+
func configureFailedImageCell(_ cell: UICollectionViewCell, image: UIImage) {
135+
guard let cell = cell as? FailedProductImageCollectionViewCell else {
136+
fatalError()
137+
}
138+
139+
cell.imageView.contentMode = .scaleAspectFit
140+
cell.imageView.image = image
141+
}
112142
}
113143

114144
enum ProductImagesItem {
@@ -124,6 +154,8 @@ enum ProductImagesItem {
124154
return ProductImageCollectionViewCell.reuseIdentifier
125155
case .uploading:
126156
return InProgressProductImageCollectionViewCell.reuseIdentifier
157+
case .uploadFailure:
158+
return FailedProductImageCollectionViewCell.reuseIdentifier
127159
}
128160
case .addImage:
129161
return AddProductImageCollectionViewCell.reuseIdentifier

‎WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesHeaderTableViewCell.swift‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ final class ProductImagesHeaderTableViewCell: UITableViewCell {
1717
///
1818
var onImageSelected: ((ProductImage?, IndexPath?) -> Void)?
1919

20+
/// Closure to be executed when a failed upload is tapped
21+
///
22+
var onFailedUploadSelected: ((_ asset: ProductImageAssetType, _ error: Error) -> Void)?
23+
2024
/// Closure to be executed when add image cell is tapped
2125
///
2226
var onAddImage: (() -> Void)?
@@ -70,8 +74,10 @@ extension ProductImagesHeaderTableViewCell: UICollectionViewDelegate {
7074
switch status {
7175
case .remote(let image):
7276
onImageSelected?(image, indexPath)
73-
default:
74-
break
77+
case .uploading:
78+
onImageSelected?(nil, indexPath)
79+
case let .uploadFailure(asset, error):
80+
onFailedUploadSelected?(asset, error)
7581
}
7682
case .addImage:
7783
onAddImage?()

‎WooCommerce/Classes/ViewRelated/Products/Cells/Product Images/ProductImagesHeaderViewModel.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ extension ProductImagesHeaderViewModel {
5454
let cells = [
5555
ProductImageCollectionViewCell.self,
5656
InProgressProductImageCollectionViewCell.self,
57+
FailedProductImageCollectionViewCell.self,
5758
AddProductImageCollectionViewCell.self,
5859
ExtendedAddProductImageCollectionViewCell.self
5960
]

‎WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormTableViewDataSource.swift‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ final class ProductFormTableViewDataSource: NSObject {
2323
private var onNameChange: ((_ name: String?) -> Void)?
2424
private var onStatusChange: ((_ isEnabled: Bool) -> Void)?
2525
private var onAddImage: (() -> Void)?
26+
private var onFailedImageUpload: ((ProductImageAssetType, Error) -> Void)?
2627

2728
private let productImageStatuses: [ProductImageStatus]
2829
private let productUIImageLoader: ProductUIImageLoader
@@ -41,10 +42,14 @@ final class ProductFormTableViewDataSource: NSObject {
4142
super.init()
4243
}
4344

44-
func configureActions(onNameChange: ((_ name: String?) -> Void)?, onStatusChange: ((_ isEnabled: Bool) -> Void)?, onAddImage: @escaping () -> Void) {
45+
func configureActions(onNameChange: ((_ name: String?) -> Void)?,
46+
onStatusChange: ((_ isEnabled: Bool) -> Void)?,
47+
onAddImage: @escaping () -> Void,
48+
onFailedImageUpload: @escaping (ProductImageAssetType, Error) -> Void) {
4549
self.onNameChange = onNameChange
4650
self.onStatusChange = onStatusChange
4751
self.onAddImage = onAddImage
52+
self.onFailedImageUpload = onFailedImageUpload
4853
}
4954
}
5055

@@ -175,6 +180,9 @@ private extension ProductFormTableViewDataSource {
175180
cell.onAddImage = { [weak self] in
176181
self?.onAddImage?()
177182
}
183+
cell.onFailedUploadSelected = { [weak self] (asset, error) in
184+
self?.onFailedImageUpload?(asset, error)
185+
}
178186
}
179187

180188
func configureName(cell: UITableViewCell, name: String?, isEditable: Bool, productStatus: ProductStatus) {

0 commit comments

Comments
 (0)