From 873bd85000c3970bc20d4c5fafe61fdbd0ceb10f Mon Sep 17 00:00:00 2001 From: Shiki Date: Fri, 23 Aug 2019 12:24:32 -0600 Subject: [PATCH 01/14] Include posts with remote in auto-upload --- .../Services/PostAutoUploadInteractor.swift | 3 +-- .../PostAutoUploadInteractorTests.swift | 19 ++++++++++--------- ...stCoordinatorFailedPostsFetcherTests.swift | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/WordPress/Classes/Services/PostAutoUploadInteractor.swift b/WordPress/Classes/Services/PostAutoUploadInteractor.swift index 568b22b871b3..5201a6fe684b 100644 --- a/WordPress/Classes/Services/PostAutoUploadInteractor.swift +++ b/WordPress/Classes/Services/PostAutoUploadInteractor.swift @@ -26,8 +26,7 @@ final class PostAutoUploadInteractor { func autoUploadAction(for post: AbstractPost) -> AutoUploadAction { guard post.isFailed, let status = post.status, - PostAutoUploadInteractor.allowedStatuses.contains(status), - !post.hasRemote() else { + PostAutoUploadInteractor.allowedStatuses.contains(status) else { return .nothing } diff --git a/WordPress/WordPressTest/Services/PostAutoUploadInteractorTests.swift b/WordPress/WordPressTest/Services/PostAutoUploadInteractorTests.swift index 837436dfb578..e57bb020a544 100644 --- a/WordPress/WordPressTest/Services/PostAutoUploadInteractorTests.swift +++ b/WordPress/WordPressTest/Services/PostAutoUploadInteractorTests.swift @@ -31,10 +31,12 @@ class PostCoordinatorUploadActionUseCaseTests: XCTestCase { createPost(.publish, confirmedAutoUpload: true): .upload, // Published local drafts with no confirmation will be remote auto-saved createPost(.publish): .autoSave, - // Draft and published posts with remote are currently unsupported. This will be - // fixed soon. - createPost(.draft, hasRemote: true): .nothing, - createPost(.publish, hasRemote: true): .nothing, + // Posts with remote that do not have confirmation will be remote auto-saved + createPost(.draft, hasRemote: true): .autoSave, + createPost(.publish, hasRemote: true): .autoSave, + // Posts with remote that have confirmation will be automatically uploaded + createPost(.draft, hasRemote: true, confirmedAutoUpload: true): .upload, + createPost(.publish, hasRemote: true, confirmedAutoUpload: true): .upload, // Other statuses are currently ignored createPost(.publishPrivate, confirmedAutoUpload: true): .nothing, createPost(.scheduled, confirmedAutoUpload: true): .nothing, @@ -67,7 +69,7 @@ class PostCoordinatorUploadActionUseCaseTests: XCTestCase { } /// Test which auto-uploaded post types can be canceled by the user - func testCancelAutoUploadMethodAppliesToLocalDraftsAndConfirmedUploads() { + func testCancelAutoUploadMethodAppliesToDraftsAndConfirmedUploads() { // Arrange let postsAndExpectedCancelableResult: [Post: Bool] = [ // Local drafts are automatically uploaded and do not need to be canceled. We consider @@ -79,10 +81,9 @@ class PostCoordinatorUploadActionUseCaseTests: XCTestCase { // auto-saved is considered safe and do not need to be canceled. It should also happen // in the background without any interaction from the user. createPost(.publish): false, - // Draft and published posts with remote are currently unsupported. These should - // allow cancelation. This will be fixed soon. - createPost(.draft, hasRemote: true, confirmedAutoUpload: true): false, - createPost(.publish, hasRemote: true, confirmedAutoUpload: true): false, + // Confirmed draft and published posts with remote will be automatically uploaded. + createPost(.draft, hasRemote: true, confirmedAutoUpload: true): true, + createPost(.publish, hasRemote: true, confirmedAutoUpload: true): true, // Posts with remote that have no confirmation will be remote auto-saved. createPost(.draft, hasRemote: true): false, createPost(.publish, hasRemote: true): false, diff --git a/WordPress/WordPressTest/Services/PostCoordinatorFailedPostsFetcherTests.swift b/WordPress/WordPressTest/Services/PostCoordinatorFailedPostsFetcherTests.swift index 0e629a5a787b..6f850b03e765 100644 --- a/WordPress/WordPressTest/Services/PostCoordinatorFailedPostsFetcherTests.swift +++ b/WordPress/WordPressTest/Services/PostCoordinatorFailedPostsFetcherTests.swift @@ -29,11 +29,11 @@ class PostCoordinatorFailedPostsFetcherTests: XCTestCase { createPost(status: .draft), createPost(status: .draft), createPost(status: .draft), - createPost(status: .publish) - ] - let unexpectedPosts = [ + createPost(status: .publish), createPost(status: .draft, hasRemote: true), createPost(status: .publish, hasRemote: true), + ] + let unexpectedPosts = [ createPost(status: .publishPrivate), createPost(status: .publishPrivate, hasRemote: true), createPost(status: .scheduled), From dd8d07a7f4d87f814a83f7b4dbe63f05331b6326 Mon Sep 17 00:00:00 2001 From: Shiki Date: Fri, 30 Aug 2019 10:06:26 -0600 Subject: [PATCH 02/14] Use a different message for auto-uploaded drafts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of just “Upload failed”, we’ll show “Draft will be uploaded…” instead. --- .../Post/PostCardStatusViewModel.swift | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift index ec75b51e6918..4a22ced8133b 100644 --- a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift @@ -56,15 +56,27 @@ class PostCardStatusViewModel: NSObject { if MediaCoordinator.shared.isUploadingMedia(for: post) { return NSLocalizedString("Uploading media...", comment: "Message displayed on a post's card while the post is uploading media") } else if post.isFailed { - if canCancelAutoUpload { - return StatusMessages.postWillBePublished + let defaultFailedMessage = StatusMessages.uploadFailed + guard let postStatus = post.status else { + return defaultFailedMessage } if post.wasAutoUploadCancelled { return StatusMessages.localChanges } - return StatusMessages.uploadFailed + if autoUploadInteractor.autoUploadAction(for: post) != .upload { + return defaultFailedMessage + } + + switch postStatus { + case .draft: + return StatusMessages.draftWillBeUploaded + case .publish: + return StatusMessages.postWillBePublished + default: + return defaultFailedMessage + } } else if post.remoteStatus == .pushing { return NSLocalizedString("Uploading post...", comment: "Message displayed on a post's card when the post has failed to upload") } else { @@ -98,7 +110,8 @@ class PostCardStatusViewModel: NSObject { } if post.isFailed { - return (canCancelAutoUpload || post.wasAutoUploadCancelled) ? .warning : .error + let autoUploadAction = autoUploadInteractor.autoUploadAction(for: post) + return (autoUploadAction == .upload || post.wasAutoUploadCancelled) ? .warning : .error } switch status { @@ -225,5 +238,7 @@ class PostCardStatusViewModel: NSObject { static let postWillBePublished = NSLocalizedString("Post will be published next time your device is online", comment: "Message shown in the posts list when a post is scheduled for publishing") static let localChanges = NSLocalizedString("Local changes", comment: "A status label for a post that only exists on the user's iOS device, and has not yet been published to their blog.") + static let draftWillBeUploaded = NSLocalizedString("Draft will be uploaded next time your device is online", + comment: "Message shown in post list when a draft is scheduled to be automatically uploaded.") } } From 21dd3efea25412738efecb26f583ccf4a879f594 Mon Sep 17 00:00:00 2001 From: Shiki Date: Fri, 30 Aug 2019 20:40:21 -0600 Subject: [PATCH 03/14] Update PostCardStatusViewModelTests --- .../Views/PostCardStatusViewModelTests.swift | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift b/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift index feba1c4d3a9e..a315d64df7ef 100644 --- a/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift +++ b/WordPress/WordPressTest/ViewRelated/Post/Views/PostCardStatusViewModelTests.swift @@ -34,6 +34,16 @@ class PostCardStatusViewModelTests: XCTestCase { PostBuilder(context).drafted().with(remoteStatus: .failed).build(), ButtonGroups(primary: [.edit, .publish, .trash], secondary: []) ), + ( + "Draft with remote and confirmed local changes", + PostBuilder(context).drafted().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build(), + ButtonGroups(primary: [.edit, .cancelAutoUpload, .more], secondary: [.publish, .trash]) + ), + ( + "Draft with remote and canceled local changes", + PostBuilder(context).drafted().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().cancelledAutoUpload().build(), + ButtonGroups(primary: [.edit, .publish, .trash], secondary: []) + ), ( "Local published draft with confirmed auto-upload", PostBuilder(context).published().with(remoteStatus: .failed).confirmedAutoUpload().build(), @@ -48,7 +58,17 @@ class PostCardStatusViewModelTests: XCTestCase { "Published post", PostBuilder(context).published().withRemote().build(), ButtonGroups(primary: [.edit, .view, .more], secondary: [.stats, .moveToDraft, .trash]) - ) + ), + ( + "Published post with local confirmed changes", + PostBuilder(context).published().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build(), + ButtonGroups(primary: [.edit, .cancelAutoUpload, .more], secondary: [.stats, .moveToDraft, .trash]) + ), + ( + "Published post with canceled local changes", + PostBuilder(context).published().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build(), + ButtonGroups(primary: [.edit, .cancelAutoUpload, .more], secondary: [.stats, .moveToDraft, .trash]) + ), ] // Act and Assert From ab130573ae52b171db7c34d07bb311aa2073e57a Mon Sep 17 00:00:00 2001 From: Shiki Date: Fri, 30 Aug 2019 20:40:44 -0600 Subject: [PATCH 04/14] Update PostCardCellTests for draft and published --- .../WordPressTest/PostCardCellTests.swift | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/WordPress/WordPressTest/PostCardCellTests.swift b/WordPress/WordPressTest/PostCardCellTests.swift index 1329936bc26b..3c09b153729c 100644 --- a/WordPress/WordPressTest/PostCardCellTests.swift +++ b/WordPress/WordPressTest/PostCardCellTests.swift @@ -14,6 +14,7 @@ class PostCardCellTests: XCTestCase { private var postActionSheetDelegateMock: PostActionSheetDelegateMock! override func setUp() { + super.setUp() contextManager = TestContextManager() context = contextManager.newDerivedContext() @@ -264,15 +265,20 @@ class PostCardCellTests: XCTestCase { } func testShowsWarningMessageForFailedPublishedPosts() { - // Given - let post = PostBuilder().published().with(remoteStatus: .failed).confirmedAutoUpload().build() + // Arrange + let posts = [ + PostBuilder().published().with(remoteStatus: .failed).confirmedAutoUpload().build(), + PostBuilder().published().with(remoteStatus: .failed).withRemote().confirmedAutoUpload().build(), + ] - // When - postCell.configure(with: post) + posts.forEach { post in + // Act + postCell.configure(with: post) - // Then - XCTAssertEqual(postCell.statusLabel.text, StatusMessages.postWillBePublished) - XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) + // Assert + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.postWillBePublished) + XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) + } } func testShowsCancelButtonForUserConfirmedFailedPublishedPosts() { @@ -326,6 +332,23 @@ class PostCardCellTests: XCTestCase { XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) } + func testShowsWillBeUploadedMessageForDrafts() { + // Arrange + let posts = [ + PostBuilder(context).drafted().with(remoteStatus: .failed).confirmedAutoUpload().build(), + PostBuilder(context).drafted().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build() + ] + + posts.forEach { post in + // Act + postCell.configure(with: post) + + // Assert + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.draftWillBeUploaded) + XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) + } + } + private func postCellFromNib() -> PostCardCell { let bundle = Bundle(for: PostCardCell.self) guard let postCell = bundle.loadNibNamed("PostCardCell", owner: nil)?.first as? PostCardCell else { From 4ce88f3e7a55c9b67bc100420c51f9dcfd85f09b Mon Sep 17 00:00:00 2001 From: Shiki Date: Sat, 31 Aug 2019 08:04:07 -0600 Subject: [PATCH 05/14] =?UTF-8?q?Change=20local=20draft=20to=20just=20show?= =?UTF-8?q?=20=E2=80=9CLocal=20changes=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The message “Draft will not be uploaded” is probably not necessary for it since it’s not cancelaeable anyway. --- .../Classes/ViewRelated/Post/PostCardStatusViewModel.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift index 4a22ced8133b..92c4397869e0 100644 --- a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift @@ -71,7 +71,7 @@ class PostCardStatusViewModel: NSObject { switch postStatus { case .draft: - return StatusMessages.draftWillBeUploaded + return canCancelAutoUpload ? StatusMessages.draftWillBeUploaded : StatusMessages.localChanges case .publish: return StatusMessages.postWillBePublished default: From e564ff306df16f273ef96dcfcd54ead4d48e43c0 Mon Sep 17 00:00:00 2001 From: Shiki Date: Sat, 31 Aug 2019 08:07:23 -0600 Subject: [PATCH 06/14] PostNoticeViewModel: Promote autoUploadInteractor --- WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift index cded17dd64d3..3c094707a3cf 100644 --- a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift @@ -7,6 +7,7 @@ enum PostNoticeUserInfoKey { struct PostNoticeViewModel { private let post: AbstractPost private let postCoordinator: PostCoordinator + private let autoUploadInteractor = PostAutoUploadInteractor() init(post: AbstractPost, postCoordinator: PostCoordinator = PostCoordinator.shared) { self.post = post @@ -201,8 +202,7 @@ struct PostNoticeViewModel { } private var failureAction: FailureAction { - let interactor = PostAutoUploadInteractor() - return interactor.canCancelAutoUpload(of: post) ? .cancel : .retry + return autoUploadInteractor.canCancelAutoUpload(of: post) ? .cancel : .retry } // MARK: - Action Handlers From 4b1485ed14375c2d26f8139611e52b1519bf1d45 Mon Sep 17 00:00:00 2001 From: Shiki Date: Sat, 31 Aug 2019 08:17:55 -0600 Subject: [PATCH 07/14] PostNoticeViewModel: Move failure titles to enum --- .../Classes/ViewRelated/Post/PostNoticeViewModel.swift | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift index 3c094707a3cf..16cbee4598ee 100644 --- a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift @@ -104,16 +104,15 @@ struct PostNoticeViewModel { } } - // TODO Move strings to FailureTitles enum private var failureTitle: String { if post.status == .publish { return FailureTitles.postWillBePublished } if post is Page { - return NSLocalizedString("Page failed to upload", comment: "Title of notification displayed when a page has failed to upload.") + return FailureTitles.pageFailedToUpload } else { - return NSLocalizedString("Post failed to upload", comment: "Title of notification displayed when a post has failed to upload.") + return FailureTitles.postFailedToUpload } } @@ -247,6 +246,10 @@ struct PostNoticeViewModel { enum FailureTitles { static let postWillBePublished = NSLocalizedString("Post will be published the next time your device is online", comment: "Text displayed in notice after a post if published while offline.") + static let pageFailedToUpload = NSLocalizedString("Page failed to upload", + comment: "Title of notification displayed when a page has failed to upload.") + static let postFailedToUpload = NSLocalizedString("Post failed to upload", + comment: "Title of notification displayed when a post has failed to upload.") } enum FailureActionTitles { From 73a947c89aa39c410df2cc573d9fd5c0fd3a3eb2 Mon Sep 17 00:00:00 2001 From: Shiki Date: Sat, 31 Aug 2019 08:55:22 -0600 Subject: [PATCH 08/14] =?UTF-8?q?Show=20=E2=80=9CDraft=20will=20be=20uploa?= =?UTF-8?q?ded=E2=80=A6=E2=80=9D=20notice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will affect drafts with remote only. --- .../Post/PostNoticeViewModel.swift | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift index 16cbee4598ee..4c52e7533663 100644 --- a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift @@ -105,14 +105,28 @@ struct PostNoticeViewModel { } private var failureTitle: String { - if post.status == .publish { - return FailureTitles.postWillBePublished + var defaultTitle: String { + if post is Page { + return FailureTitles.pageFailedToUpload + } else { + return FailureTitles.postFailedToUpload + } } - if post is Page { - return FailureTitles.pageFailedToUpload - } else { - return FailureTitles.postFailedToUpload + guard let postStatus = post.status, + autoUploadInteractor.autoUploadAction(for: post) == .upload, + autoUploadInteractor.canCancelAutoUpload(of: post) else { + + return defaultTitle + } + + switch postStatus { + case .draft: + return FailureTitles.draftWillBeUploaded + case .publish: + return FailureTitles.postWillBePublished + default: + return defaultTitle } } @@ -246,6 +260,8 @@ struct PostNoticeViewModel { enum FailureTitles { static let postWillBePublished = NSLocalizedString("Post will be published the next time your device is online", comment: "Text displayed in notice after a post if published while offline.") + static let draftWillBeUploaded = NSLocalizedString("Draft will be uploaded next time your device is online", + comment: "Text displayed in notice after the app fails to upload a draft.") static let pageFailedToUpload = NSLocalizedString("Page failed to upload", comment: "Title of notification displayed when a page has failed to upload.") static let postFailedToUpload = NSLocalizedString("Post failed to upload", From 2bc133d29189c6575dcd1b3d2d676f78d23a4307 Mon Sep 17 00:00:00 2001 From: Shiki Date: Sat, 31 Aug 2019 10:00:36 -0600 Subject: [PATCH 09/14] PostCardCell: Add test for local draft message --- .../WordPressTest/PostCardCellTests.swift | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/WordPress/WordPressTest/PostCardCellTests.swift b/WordPress/WordPressTest/PostCardCellTests.swift index 3c09b153729c..2fed5f61308a 100644 --- a/WordPress/WordPressTest/PostCardCellTests.swift +++ b/WordPress/WordPressTest/PostCardCellTests.swift @@ -332,21 +332,28 @@ class PostCardCellTests: XCTestCase { XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) } - func testShowsWillBeUploadedMessageForDrafts() { - // Arrange - let posts = [ - PostBuilder(context).drafted().with(remoteStatus: .failed).confirmedAutoUpload().build(), - PostBuilder(context).drafted().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build() - ] + func testShowsWillBeUploadedMessageForDraftsWithRemote() { + // Given + let post = PostBuilder(context).drafted().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build() - posts.forEach { post in - // Act - postCell.configure(with: post) + // When + postCell.configure(with: post) - // Assert - XCTAssertEqual(postCell.statusLabel.text, StatusMessages.draftWillBeUploaded) - XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) - } + // Then + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.draftWillBeUploaded) + XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) + } + + func testShowsLocalChangesMessageForLocalDrafts() { + // Given + let post = PostBuilder(context).drafted().with(remoteStatus: .failed).build() + + // When + postCell.configure(with: post) + + // Then + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.localChanges) + XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) } private func postCellFromNib() -> PostCardCell { From 9b47445d58d798a5932409e9347a9fa6574fdea1 Mon Sep 17 00:00:00 2001 From: Shiki Date: Wed, 4 Sep 2019 07:00:04 -0600 Subject: [PATCH 10/14] =?UTF-8?q?Show=20=E2=80=9CChanges=20will=20be=20upl?= =?UTF-8?q?oaded=E2=80=9D=20in=20Post=20List?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will be shown for posts with remote that have local changes and are confirmed to be automatically uploaded when the device is back online. This also reverts 4ce88f3. Now, we will show “Draft will be uploaded…” for local drafts instead of ”Local changes” only. This was discussed with Chris and it seems much clearer for the user. --- .../Post/PostCardStatusViewModel.swift | 6 ++- .../WordPressTest/PostCardCellTests.swift | 41 +++++++++++-------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift index 92c4397869e0..2c495d60b4cc 100644 --- a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift @@ -71,9 +71,9 @@ class PostCardStatusViewModel: NSObject { switch postStatus { case .draft: - return canCancelAutoUpload ? StatusMessages.draftWillBeUploaded : StatusMessages.localChanges + return canCancelAutoUpload ? StatusMessages.changesWillBeUploaded : StatusMessages.draftWillBeUploaded case .publish: - return StatusMessages.postWillBePublished + return post.hasRemote() ? StatusMessages.changesWillBeUploaded : StatusMessages.postWillBePublished default: return defaultFailedMessage } @@ -240,5 +240,7 @@ class PostCardStatusViewModel: NSObject { static let localChanges = NSLocalizedString("Local changes", comment: "A status label for a post that only exists on the user's iOS device, and has not yet been published to their blog.") static let draftWillBeUploaded = NSLocalizedString("Draft will be uploaded next time your device is online", comment: "Message shown in post list when a draft is scheduled to be automatically uploaded.") + static let changesWillBeUploaded = NSLocalizedString("Changes will be uploaded next time your device is online", + comment: "Message shown in post list when a post's local changes will be automatically uploaded when the device is online.") } } diff --git a/WordPress/WordPressTest/PostCardCellTests.swift b/WordPress/WordPressTest/PostCardCellTests.swift index 2fed5f61308a..46611ac32b0f 100644 --- a/WordPress/WordPressTest/PostCardCellTests.swift +++ b/WordPress/WordPressTest/PostCardCellTests.swift @@ -264,21 +264,28 @@ class PostCardCellTests: XCTestCase { XCTAssertFalse(postCell.separatorLabel.isHidden) } - func testShowsWarningMessageForFailedPublishedPosts() { - // Arrange - let posts = [ - PostBuilder().published().with(remoteStatus: .failed).confirmedAutoUpload().build(), - PostBuilder().published().with(remoteStatus: .failed).withRemote().confirmedAutoUpload().build(), - ] + func testShowsChangesWillBeUploadedWarningForFailedPublishedPostsWithRemote() { + // Given + let post = PostBuilder(context).published().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build() - posts.forEach { post in - // Act - postCell.configure(with: post) + // When + postCell.configure(with: post) - // Assert - XCTAssertEqual(postCell.statusLabel.text, StatusMessages.postWillBePublished) - XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) - } + // Then + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.changesWillBeUploaded) + XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) + } + + func testShowsPostWillBePublishedWarningForLocallyPublishedPosts() { + // Given + let post = PostBuilder(context).published().with(remoteStatus: .failed).confirmedAutoUpload().build() + + // When + postCell.configure(with: post) + + // Then + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.postWillBePublished) + XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) } func testShowsCancelButtonForUserConfirmedFailedPublishedPosts() { @@ -332,7 +339,7 @@ class PostCardCellTests: XCTestCase { XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) } - func testShowsWillBeUploadedMessageForDraftsWithRemote() { + func testShowsChangesWillBeUploadedMessageForDraftsWithRemote() { // Given let post = PostBuilder(context).drafted().withRemote().with(remoteStatus: .failed).confirmedAutoUpload().build() @@ -340,11 +347,11 @@ class PostCardCellTests: XCTestCase { postCell.configure(with: post) // Then - XCTAssertEqual(postCell.statusLabel.text, StatusMessages.draftWillBeUploaded) + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.changesWillBeUploaded) XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) } - func testShowsLocalChangesMessageForLocalDrafts() { + func testShowsDraftWillBeUploadedMessageForLocalDrafts() { // Given let post = PostBuilder(context).drafted().with(remoteStatus: .failed).build() @@ -352,7 +359,7 @@ class PostCardCellTests: XCTestCase { postCell.configure(with: post) // Then - XCTAssertEqual(postCell.statusLabel.text, StatusMessages.localChanges) + XCTAssertEqual(postCell.statusLabel.text, StatusMessages.draftWillBeUploaded) XCTAssertEqual(postCell.statusLabel.textColor, UIColor.warning) } From 88b62b5bbf42a3df85ab9573815d72b987d12407 Mon Sep 17 00:00:00 2001 From: Shiki Date: Wed, 4 Sep 2019 09:06:17 -0600 Subject: [PATCH 11/14] =?UTF-8?q?Show=20=E2=80=9CDraft=20will=20be=20uploa?= =?UTF-8?q?ded=E2=80=9D=20in=20notice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit And for posts with remote, always use “Changes will be uploaded…” as the notice message. --- .../Post/PostCardStatusViewModel.swift | 7 ++- .../Post/PostNoticeViewModel.swift | 10 +++- .../Post/Utils/PostNoticeViewModelTests.swift | 58 ++++++++++++++----- 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift index 2c495d60b4cc..c48c7dc06a50 100644 --- a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift @@ -68,12 +68,15 @@ class PostCardStatusViewModel: NSObject { if autoUploadInteractor.autoUploadAction(for: post) != .upload { return defaultFailedMessage } + if post.hasRemote() { + return StatusMessages.changesWillBeUploaded + } switch postStatus { case .draft: - return canCancelAutoUpload ? StatusMessages.changesWillBeUploaded : StatusMessages.draftWillBeUploaded + return StatusMessages.draftWillBeUploaded case .publish: - return post.hasRemote() ? StatusMessages.changesWillBeUploaded : StatusMessages.postWillBePublished + return StatusMessages.postWillBePublished default: return defaultFailedMessage } diff --git a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift index 4c52e7533663..1af6735abbc7 100644 --- a/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostNoticeViewModel.swift @@ -114,12 +114,14 @@ struct PostNoticeViewModel { } guard let postStatus = post.status, - autoUploadInteractor.autoUploadAction(for: post) == .upload, - autoUploadInteractor.canCancelAutoUpload(of: post) else { - + autoUploadInteractor.autoUploadAction(for: post) == .upload else { return defaultTitle } + if post.hasRemote() { + return FailureTitles.changesWillBeUploaded + } + switch postStatus { case .draft: return FailureTitles.draftWillBeUploaded @@ -266,6 +268,8 @@ struct PostNoticeViewModel { comment: "Title of notification displayed when a page has failed to upload.") static let postFailedToUpload = NSLocalizedString("Post failed to upload", comment: "Title of notification displayed when a post has failed to upload.") + static let changesWillBeUploaded = NSLocalizedString("Changes will be uploaded next time your device is online", + comment: "Text displayed in notice after the app fails to upload a post.") } enum FailureActionTitles { diff --git a/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift b/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift index 4025a609918d..190411cf4842 100644 --- a/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift +++ b/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift @@ -4,6 +4,9 @@ import Nimble @testable import WordPress +private typealias FailureActionTitles = PostNoticeViewModel.FailureActionTitles +private typealias FailureTitles = PostNoticeViewModel.FailureTitles + class PostNoticeViewModelTests: XCTestCase { private var contextManager: TestContextManager! private var context: NSManagedObjectContext! @@ -22,22 +25,49 @@ class PostNoticeViewModelTests: XCTestCase { super.tearDown() } - func testCreatesNoticeWithFailureMessageForFailedPublishedPosts() { - // Given - let post = PostBuilder(context) - .published() - .with(title: "Darby Ritchie") - .with(remoteStatus: .failed) - .confirmedAutoUpload() - .build() + func testNoticesToBeShownAfterFailingToUploadPosts() { + struct Expectation { + let scenario: String + let post: Post + let title: String + let actionTitle: String + } - // When - let notice = PostNoticeViewModel(post: post).notice + // Arrange + let expectations: [Expectation] = [ + Expectation( + scenario: "Local draft", + post: PostBuilder(context).with(title: "molestiae").with(remoteStatus: .failed).drafted().build(), + title: FailureTitles.draftWillBeUploaded, + actionTitle: FailureActionTitles.retry + ), + Expectation( + scenario: "Local published draft with confirmed auto-upload", + post: PostBuilder(context).with(title: "dolores").published().with(remoteStatus: .failed).confirmedAutoUpload().build(), + title: FailureTitles.postWillBePublished, + actionTitle: FailureActionTitles.cancel + ), + ] - // Then - expect(notice.title).to(equal(PostNoticeViewModel.FailureTitles.postWillBePublished)) - expect(notice.message).to(equal(post.postTitle)) - expect(notice.actionTitle).to(equal(PostNoticeViewModel.FailureActionTitles.cancel)) + expectations.forEach { expectation in + // Act + let notice = PostNoticeViewModel(post: expectation.post).notice + + // Assert + expect({ + guard notice.title == expectation.title else { + return .failed(reason: "Scenario “\(expectation.scenario)” failed. Expected notice.title to equal ”\(expectation.title)”. Actual is ”\(notice.title).") + } + guard notice.actionTitle == expectation.actionTitle else { + return .failed(reason: "Scenario ”\(expectation.scenario)” failed. Expected notice.actionTitle to equal ”\(expectation.actionTitle)”. Actual is ”\(String(describing: notice.actionTitle)).") + } + guard notice.message == expectation.post.postTitle else { + return .failed(reason: "Scenario ”\(expectation.scenario)” failed. Expected notice.message to equal ”\(String(describing: expectation.post.postTitle))”. Actual is ”\(String(describing: notice.message)).") + } + + return .succeeded + }).to(succeed()) + } } func testFailedPublishedPostsCancelButtonWillCancelAutoUpload() { From 2c6380a68b583dafe3eba8bb7cccaa69790bbf5b Mon Sep 17 00:00:00 2001 From: Shiki Date: Wed, 4 Sep 2019 09:27:37 -0600 Subject: [PATCH 12/14] Add additional tests in PostNoticeViewModelTests --- WordPress/WordPressTest/PostBuilder.swift | 5 ++ .../Post/Utils/PostNoticeViewModelTests.swift | 51 ++++++++++++++++--- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/WordPress/WordPressTest/PostBuilder.swift b/WordPress/WordPressTest/PostBuilder.swift index c7235fd13ae9..727137660758 100644 --- a/WordPress/WordPressTest/PostBuilder.swift +++ b/WordPress/WordPressTest/PostBuilder.swift @@ -42,6 +42,11 @@ class PostBuilder { return self } + func with(status: BasePost.Status) -> PostBuilder { + post.status = status + return self + } + func with(title: String) -> PostBuilder { post.postTitle = title return self diff --git a/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift b/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift index 190411cf4842..a0da7644330d 100644 --- a/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift +++ b/WordPress/WordPressTest/ViewRelated/Post/Utils/PostNoticeViewModelTests.swift @@ -37,16 +37,40 @@ class PostNoticeViewModelTests: XCTestCase { let expectations: [Expectation] = [ Expectation( scenario: "Local draft", - post: PostBuilder(context).with(title: "molestiae").with(remoteStatus: .failed).drafted().build(), + post: createPost(.draft), title: FailureTitles.draftWillBeUploaded, actionTitle: FailureActionTitles.retry ), Expectation( - scenario: "Local published draft with confirmed auto-upload", - post: PostBuilder(context).with(title: "dolores").published().with(remoteStatus: .failed).confirmedAutoUpload().build(), + scenario: "Draft with confirmed local changes", + post: createPost(.draft, hasRemote: true), + title: FailureTitles.changesWillBeUploaded, + actionTitle: FailureActionTitles.cancel + ), + Expectation( + scenario: "Local published draft", + post: createPost(.publish), title: FailureTitles.postWillBePublished, actionTitle: FailureActionTitles.cancel ), + Expectation( + scenario: "Published post with confirmed local changes", + post: createPost(.publish, hasRemote: true), + title: FailureTitles.changesWillBeUploaded, + actionTitle: FailureActionTitles.cancel + ), + Expectation( + scenario: "Currently unsupported: Locally scheduled post", + post: createPost(.scheduled), + title: FailureTitles.postFailedToUpload, + actionTitle: FailureActionTitles.retry + ), + Expectation( + scenario: "Currently unsupported: Scheduled post with confirmed local changes", + post: createPost(.scheduled, hasRemote: true), + title: FailureTitles.postFailedToUpload, + actionTitle: FailureActionTitles.retry + ), ] expectations.forEach { expectation in @@ -56,13 +80,13 @@ class PostNoticeViewModelTests: XCTestCase { // Assert expect({ guard notice.title == expectation.title else { - return .failed(reason: "Scenario “\(expectation.scenario)” failed. Expected notice.title to equal ”\(expectation.title)”. Actual is ”\(notice.title).") + return .failed(reason: "Scenario “\(expectation.scenario)” failed. Expected notice.title to equal ”\(expectation.title)”. Actual is ”\(notice.title)”.") } guard notice.actionTitle == expectation.actionTitle else { - return .failed(reason: "Scenario ”\(expectation.scenario)” failed. Expected notice.actionTitle to equal ”\(expectation.actionTitle)”. Actual is ”\(String(describing: notice.actionTitle)).") + return .failed(reason: "Scenario ”\(expectation.scenario)” failed. Expected notice.actionTitle to equal ”\(expectation.actionTitle)”. Actual is ”\(String(describing: notice.actionTitle))”.") } guard notice.message == expectation.post.postTitle else { - return .failed(reason: "Scenario ”\(expectation.scenario)” failed. Expected notice.message to equal ”\(String(describing: expectation.post.postTitle))”. Actual is ”\(String(describing: notice.message)).") + return .failed(reason: "Scenario ”\(expectation.scenario)” failed. Expected notice.message to equal ”\(String(describing: expectation.post.postTitle))”. Actual is ”\(String(describing: notice.message))”.") } return .succeeded @@ -90,6 +114,21 @@ class PostNoticeViewModelTests: XCTestCase { expect(postCoordinator.cancelAutoUploadOfInvocations).to(equal(1)) } + private func createPost(_ status: BasePost.Status, hasRemote: Bool = false) -> Post { + var builder = PostBuilder(context) + .with(title: UUID().uuidString) + .with(status: status) + .with(remoteStatus: .failed) + + if hasRemote { + builder = builder.withRemote() + } + + builder = builder.confirmedAutoUpload() + + return builder.build() + } + private final class MockPostCoordinator: PostCoordinator { private(set) var cancelAutoUploadOfInvocations: Int = 0 From adb140b380549637a0aa7cf957ef186ad5f02428 Mon Sep 17 00:00:00 2001 From: Shiki Date: Mon, 9 Sep 2019 10:16:51 -0600 Subject: [PATCH 13/14] Add PostCardStatusViewModel.generateFailedStatus() This is just a helper method to make things a bit readable. It should only be used by `private var status: String?` --- .../Post/PostCardStatusViewModel.swift | 58 +++++++++++-------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift index c48c7dc06a50..fbb39499ddeb 100644 --- a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift @@ -56,30 +56,7 @@ class PostCardStatusViewModel: NSObject { if MediaCoordinator.shared.isUploadingMedia(for: post) { return NSLocalizedString("Uploading media...", comment: "Message displayed on a post's card while the post is uploading media") } else if post.isFailed { - let defaultFailedMessage = StatusMessages.uploadFailed - guard let postStatus = post.status else { - return defaultFailedMessage - } - - if post.wasAutoUploadCancelled { - return StatusMessages.localChanges - } - - if autoUploadInteractor.autoUploadAction(for: post) != .upload { - return defaultFailedMessage - } - if post.hasRemote() { - return StatusMessages.changesWillBeUploaded - } - - switch postStatus { - case .draft: - return StatusMessages.draftWillBeUploaded - case .publish: - return StatusMessages.postWillBePublished - default: - return defaultFailedMessage - } + return generateFailedStatusMessage() } else if post.remoteStatus == .pushing { return NSLocalizedString("Uploading post...", comment: "Message displayed on a post's card when the post has failed to upload") } else { @@ -232,6 +209,39 @@ class PostCardStatusViewModel: NSObject { return [status, sticky].filter { !$0.isEmpty }.joined(separator: separator) } + /// Determine what the failed status message should be and return it. + /// + /// This is a helper method for `status`. + private func generateFailedStatusMessage() -> String { + assert(post.remoteStatus == .failed, "This should only be used if the remoteStatus is .failed") + + let defaultFailedMessage = StatusMessages.uploadFailed + + guard post.remoteStatus == .failed, let postStatus = post.status else { + return defaultFailedMessage + } + + if post.wasAutoUploadCancelled { + return StatusMessages.localChanges + } + + if autoUploadInteractor.autoUploadAction(for: post) != .upload { + return defaultFailedMessage + } + if post.hasRemote() { + return StatusMessages.changesWillBeUploaded + } + + switch postStatus { + case .draft: + return StatusMessages.draftWillBeUploaded + case .publish: + return StatusMessages.postWillBePublished + default: + return defaultFailedMessage + } + } + private enum Constants { static let stickyLabel = NSLocalizedString("Sticky", comment: "Label text that defines a post marked as sticky") } From 77369515f0c31cd21b0455390a24a80b6c6e4e9e Mon Sep 17 00:00:00 2001 From: Shiki Date: Mon, 9 Sep 2019 16:24:35 -0600 Subject: [PATCH 14/14] Remove unreliable post.remoteStatus assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should have been a `post.isFailed` check but it’s better to just rely on the `guard` instead. --- .../Classes/ViewRelated/Post/PostCardStatusViewModel.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift index fbb39499ddeb..ee4f3d6738c1 100644 --- a/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift +++ b/WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift @@ -213,11 +213,9 @@ class PostCardStatusViewModel: NSObject { /// /// This is a helper method for `status`. private func generateFailedStatusMessage() -> String { - assert(post.remoteStatus == .failed, "This should only be used if the remoteStatus is .failed") - let defaultFailedMessage = StatusMessages.uploadFailed - guard post.remoteStatus == .failed, let postStatus = post.status else { + guard post.isFailed, let postStatus = post.status else { return defaultFailedMessage }