From 8b72a3351d09b8b294278ddcd5bb710e78e46655 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 26 May 2026 14:35:21 +1200 Subject: [PATCH 1/3] Run swift-format on touched Swift files --- Sources/WordPressData/Swift/Blog+Post.swift | 12 +- .../DiscussionSettingsViewController.swift | 202 ++++++++++++------ .../Post/PostListEditorPresenter.swift | 31 ++- .../PostDiscussionSettingsView.swift | 4 +- 4 files changed, 178 insertions(+), 71 deletions(-) diff --git a/Sources/WordPressData/Swift/Blog+Post.swift b/Sources/WordPressData/Swift/Blog+Post.swift index aa3892b66e76..ae6eee309e9f 100644 --- a/Sources/WordPressData/Swift/Blog+Post.swift +++ b/Sources/WordPressData/Swift/Blog+Post.swift @@ -38,7 +38,12 @@ extension Blog { @objc(lookupLocalPostWithForeignID:inContext:) public func lookupLocalPost(withForeignID foreignID: UUID, in context: NSManagedObjectContext) -> AbstractPost? { let request = NSFetchRequest(entityName: NSStringFromClass(AbstractPost.self)) - request.predicate = NSPredicate(format: "blog = %@ AND original = NULL AND (postID = NULL OR postID <= 0) AND \(#keyPath(AbstractPost.foreignID)) == %@", self, foreignID as NSUUID) + request.predicate = NSPredicate( + format: + "blog = %@ AND original = NULL AND (postID = NULL OR postID <= 0) AND \(#keyPath(AbstractPost.foreignID)) == %@", + self, + foreignID as NSUUID + ) request.fetchLimit = 1 return (try? context.fetch(request))?.first } @@ -61,8 +66,9 @@ extension Blog { post.foreignID = UUID() if let categoryID = settings?.defaultCategoryID, - categoryID != PostCategory.uncategorized, - let category = try? PostCategory.lookup(withBlogID: objectID, categoryID: categoryID, in: context) { + categoryID != PostCategory.uncategorized, + let category = try? PostCategory.lookup(withBlogID: objectID, categoryID: categoryID, in: context) + { post.addCategoriesObject(category) } diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift index c63459cc567a..6dad73d04989 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift @@ -52,12 +52,16 @@ open class DiscussionSettingsViewController: UITableViewController { // MARK: - Persistance! private func refreshSettings() { let service = BlogService(coreDataStack: ContextManager.shared) - service.syncSettings(for: blog, success: { [weak self] in - self?.tableView.reloadData() - DDLogInfo("Reloaded Settings") - }, failure: { (error: Error) in - DDLogError("Error while sync'ing blog settings: \(error)") - }) + service.syncSettings( + for: blog, + success: { [weak self] in + self?.tableView.reloadData() + DDLogInfo("Reloaded Settings") + }, + failure: { (error: Error) in + DDLogError("Error while sync'ing blog settings: \(error)") + } + ) } private func setNeedsChangeSettings() { @@ -74,11 +78,15 @@ open class DiscussionSettingsViewController: UITableViewController { navigationItem.rightBarButtonItem = .activityIndicator let service = BlogService(coreDataStack: ContextManager.shared) - service.updateSettings(for: blog, success: { [weak self] in - self?.didFinishChangingSettings(nil) - }, failure: { [weak self] error -> Void in - self?.didFinishChangingSettings(error) - }) + service.updateSettings( + for: blog, + success: { [weak self] in + self?.didFinishChangingSettings(nil) + }, + failure: { [weak self] error -> Void in + self?.didFinishChangingSettings(error) + } + ) } private func didFinishChangingSettings(_ error: Error?) { @@ -90,7 +98,11 @@ open class DiscussionSettingsViewController: UITableViewController { } if let error { DDLogError("Error while persisting settings: \(error)") - let alert = UIAlertController(title: Strings.errorTitle, message: error.localizedDescription, preferredStyle: .alert) + let alert = UIAlertController( + title: Strings.errorTitle, + message: error.localizedDescription, + preferredStyle: .alert + ) alert.addAction(.init(title: SharedStrings.Button.ok, style: .default, handler: nil)) present(alert, animated: true) } @@ -98,11 +110,11 @@ open class DiscussionSettingsViewController: UITableViewController { // MARK: - UITableViewDataSoutce Methods open override func numberOfSections(in tableView: UITableView) -> Int { - return sections.count + sections.count } open override func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { - return sections[section].rows.count + sections[section].rows.count } open override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { @@ -120,14 +132,15 @@ open class DiscussionSettingsViewController: UITableViewController { } open override func tableView(_ tableView: UITableView, titleForHeaderInSection section: Int) -> String? { - return sections[section].headerText + sections[section].headerText } open override func tableView(_ tableView: UITableView, titleForFooterInSection section: Int) -> String? { - return sections[section].footerText + sections[section].footerText } - open override func tableView(_ tableView: UITableView, willDisplayFooterView view: UIView, forSection section: Int) { + open override func tableView(_ tableView: UITableView, willDisplayFooterView view: UIView, forSection section: Int) + { WPStyleGuide.configureTableViewSectionFooter(view) } @@ -138,7 +151,7 @@ open class DiscussionSettingsViewController: UITableViewController { // MARK: - Cell Setup Helpers private func rowAtIndexPath(_ indexPath: IndexPath) -> Row { - return sections[indexPath.section].rows[indexPath.row] + sections[indexPath.section].rows[indexPath.row] } private func cellForRow(_ row: Row, tableView: UITableView) -> UITableViewCell { @@ -215,9 +228,15 @@ open class DiscussionSettingsViewController: UITableViewController { pickerViewController.switchVisible = true pickerViewController.switchOn = settings.commentsCloseAutomatically pickerViewController.switchText = NSLocalizedString("Automatically Close", comment: "Discussion Settings") - pickerViewController.selectionText = NSLocalizedString("Close after", comment: "Close comments after a given number of days") + pickerViewController.selectionText = NSLocalizedString( + "Close after", + comment: "Close comments after a given number of days" + ) pickerViewController.selectionFormat = NSLocalizedString("%d days", comment: "Number of days") - pickerViewController.pickerHint = NSLocalizedString("Automatically close comments on content after a certain number of days.", comment: "Discussion Settings: Comments Auto-close") + pickerViewController.pickerHint = NSLocalizedString( + "Automatically close comments on content after a certain number of days.", + comment: "Discussion Settings: Comments Auto-close" + ) pickerViewController.pickerFormat = NSLocalizedString("%d days", comment: "Number of days") pickerViewController.pickerMinimumValue = commentsAutocloseMinimumValue pickerViewController.pickerMaximumValue = commentsAutocloseMaximumValue @@ -271,7 +290,10 @@ open class DiscussionSettingsViewController: UITableViewController { pickerViewController.switchOn = settings.commentsPagingEnabled pickerViewController.switchText = NSLocalizedString("Paging", comment: "Discussion Settings") pickerViewController.selectionText = NSLocalizedString("Comments per page", comment: "A label title.") - pickerViewController.pickerHint = NSLocalizedString("Break comment threads into multiple pages.", comment: "Text snippet summarizing what comment paging does.") + pickerViewController.pickerHint = NSLocalizedString( + "Break comment threads into multiple pages.", + comment: "Text snippet summarizing what comment paging does." + ) pickerViewController.pickerMinimumValue = commentsPagingMinimumValue pickerViewController.pickerMaximumValue = commentsPagingMaximumValue pickerViewController.pickerSelectedValue = settings.commentsPageSize as? Int @@ -307,7 +329,10 @@ open class DiscussionSettingsViewController: UITableViewController { pickerViewController.title = NSLocalizedString("Links in comments", comment: "Comments Paging") pickerViewController.switchVisible = false pickerViewController.selectionText = NSLocalizedString("Links in comments", comment: "A label title") - pickerViewController.pickerHint = NSLocalizedString("Require manual approval for comments that include more than this number of links.", comment: "An explaination of a setting.") + pickerViewController.pickerHint = NSLocalizedString( + "Require manual approval for comments that include more than this number of links.", + comment: "An explaination of a setting." + ) pickerViewController.pickerMinimumValue = commentsLinksMinimumValue pickerViewController.pickerMaximumValue = commentsLinksMaximumValue pickerViewController.pickerSelectedValue = settings.commentsMaximumLinks as? Int @@ -322,9 +347,18 @@ open class DiscussionSettingsViewController: UITableViewController { let moderationKeys = settings.commentsModerationKeys let settingsViewController = SettingsListEditorViewController(collection: moderationKeys) settingsViewController.title = NSLocalizedString("Hold for Moderation", comment: "Moderation Keys Title") - settingsViewController.insertTitle = NSLocalizedString("New Moderation Word", comment: "Moderation Keyword Insertion Title") - settingsViewController.editTitle = NSLocalizedString("Edit Moderation Word", comment: "Moderation Keyword Edition Title") - settingsViewController.footerText = NSLocalizedString("When a comment contains any of these words in its content, name, URL, e-mail or IP, it will be held in the moderation queue. You can enter partial words, so \"press\" will match \"WordPress\".", comment: "Text rendered at the bottom of the Discussion Moderation Keys editor") + settingsViewController.insertTitle = NSLocalizedString( + "New Moderation Word", + comment: "Moderation Keyword Insertion Title" + ) + settingsViewController.editTitle = NSLocalizedString( + "Edit Moderation Word", + comment: "Moderation Keyword Edition Title" + ) + settingsViewController.footerText = NSLocalizedString( + "When a comment contains any of these words in its content, name, URL, e-mail or IP, it will be held in the moderation queue. You can enter partial words, so \"press\" will match \"WordPress\".", + comment: "Text rendered at the bottom of the Discussion Moderation Keys editor" + ) settingsViewController.onChange = { [weak self] (updated: Set) in self?.settings.commentsModerationKeys = updated self?.didChangeSetting("comments_hold_for_moderation", value: updated.count as Any) @@ -336,9 +370,18 @@ open class DiscussionSettingsViewController: UITableViewController { let blocklistKeys = settings.commentsBlocklistKeys let settingsViewController = SettingsListEditorViewController(collection: blocklistKeys) settingsViewController.title = NSLocalizedString("Blocklist", comment: "Blocklist Title") - settingsViewController.insertTitle = NSLocalizedString("New Blocklist Word", comment: "Blocklist Keyword Insertion Title") - settingsViewController.editTitle = NSLocalizedString("Edit Blocklist Word", comment: "Blocklist Keyword Edition Title") - settingsViewController.footerText = NSLocalizedString("When a comment contains any of these words in its content, name, URL, e-mail, or IP, it will be marked as spam. You can enter partial words, so \"press\" will match \"WordPress\".", comment: "Text rendered at the bottom of the Discussion Blocklist Keys editor") + settingsViewController.insertTitle = NSLocalizedString( + "New Blocklist Word", + comment: "Blocklist Keyword Insertion Title" + ) + settingsViewController.editTitle = NSLocalizedString( + "Edit Blocklist Word", + comment: "Blocklist Keyword Edition Title" + ) + settingsViewController.footerText = NSLocalizedString( + "When a comment contains any of these words in its content, name, URL, e-mail, or IP, it will be marked as spam. You can enter partial words, so \"press\" will match \"WordPress\".", + comment: "Text rendered at the bottom of the Discussion Blocklist Keys editor" + ) settingsViewController.onChange = { [weak self] (updated: Set) in self?.settings.commentsBlocklistKeys = updated self?.didChangeSetting("comments_block_list", value: updated.count as Any) @@ -353,33 +396,42 @@ open class DiscussionSettingsViewController: UITableViewController { // MARK: - Computed Properties private var sections: [Section] { - return [postsSection, commentsSection, otherSection] + [postsSection, commentsSection, otherSection] } private var postsSection: Section { let headerText = NSLocalizedString("Defaults for New Posts", comment: "Discussion Settings: Posts Section") - let footerText = NSLocalizedString("You can override these settings for individual posts.", comment: "Discussion Settings: Footer Text") + let footerText = NSLocalizedString( + "You can override these settings for individual posts.", + comment: "Discussion Settings: Footer Text" + ) let rows = [ - Row(style: .switch, + Row( + style: .switch, title: NSLocalizedString("Allow Comments", comment: "Settings: Comments Enabled"), boolValue: self.settings.commentsAllowed, handler: { [weak self] in self?.pressedCommentsAllowed($0) - }), + } + ), - Row(style: .switch, + Row( + style: .switch, title: NSLocalizedString("Send Pingbacks", comment: "Settings: Sending Pingbacks"), boolValue: self.settings.pingbackOutboundEnabled, handler: { [weak self] in self?.pressedPingbacksOutbound($0) - }), + } + ), - Row(style: .switch, + Row( + style: .switch, title: NSLocalizedString("Receive Pingbacks", comment: "Settings: Receiving Pingbacks"), boolValue: self.settings.pingbackInboundEnabled, handler: { [weak self] in self?.pressedPingbacksInbound($0) - }) + } + ) ] return Section(headerText: headerText, footerText: footerText, rows: rows) @@ -388,61 +440,77 @@ open class DiscussionSettingsViewController: UITableViewController { private var commentsSection: Section { let headerText = NSLocalizedString("Comments", comment: "Settings: Comment Sections") let rows = [ - Row(style: .switch, + Row( + style: .switch, title: NSLocalizedString("Require name and email", comment: "Settings: Comments Approval settings"), boolValue: self.settings.commentsRequireNameAndEmail, handler: { [weak self] in self?.pressedRequireNameAndEmail($0) - }), + } + ), - Row(style: .switch, + Row( + style: .switch, title: NSLocalizedString("Require users to log in", comment: "Settings: Comments Approval settings"), boolValue: self.settings.commentsRequireRegistration, handler: { [weak self] in self?.pressedRequireRegistration($0) - }), + } + ), - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Close Commenting", comment: "Settings: Close comments after X period"), details: self.detailsForCloseCommenting, handler: { [weak self] in self?.pressedCloseCommenting($0) - }), + } + ), - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Sort By", comment: "Settings: Comments Sort Order"), details: self.detailsForSortBy, handler: { [weak self] in self?.pressedSortBy($0) - }), + } + ), - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Threading", comment: "Settings: Comments Threading preferences"), details: self.detailsForThreading, handler: { [weak self] in self?.pressedThreading($0) - }), + } + ), - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Paging", comment: "Settings: Comments Paging preferences"), details: self.detailsForPaging, handler: { [weak self] in self?.pressedPaging($0) - }), + } + ), - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Automatically Approve", comment: "Settings: Comments Approval settings"), details: self.detailsForAutomaticallyApprove, handler: { [weak self] in self?.pressedAutomaticallyApprove($0) - }), + } + ), - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Links in comments", comment: "Settings: Comments Approval settings"), details: self.detailsForLinksInComments, handler: { [weak self] in self?.pressedLinksInComments($0) - }), + } + ) ] return Section(headerText: headerText, rows: rows) @@ -450,13 +518,17 @@ open class DiscussionSettingsViewController: UITableViewController { private var otherSection: Section { let rows = [ - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Hold for Moderation", comment: "Settings: Comments Moderation"), - handler: self.pressedModeration), + handler: self.pressedModeration + ), - Row(style: .value1, + Row( + style: .value1, title: NSLocalizedString("Blocklist", comment: "Settings: Comments Blocklist"), - handler: self.pressedBlocklist) + handler: self.pressedBlocklist + ) ] return Section(rows: rows) @@ -474,7 +546,7 @@ open class DiscussionSettingsViewController: UITableViewController { } private var detailsForSortBy: String { - return settings.commentsSorting.description + settings.commentsSorting.description } private var detailsForThreading: String { @@ -537,7 +609,13 @@ open class DiscussionSettingsViewController: UITableViewController { let handler: Handler? var boolValue: Bool? - init(style: Style, title: String? = nil, details: String? = nil, boolValue: Bool? = nil, handler: Handler? = nil) { + init( + style: Style, + title: String? = nil, + details: String? = nil, + boolValue: Bool? = nil, + handler: Handler? = nil + ) { self.style = style self.title = title self.details = details @@ -558,7 +636,7 @@ open class DiscussionSettingsViewController: UITableViewController { // MARK: - Computed Properties private var settings: BlogSettings { - return blog.settings! + blog.settings! } // MARK: - Typealiases @@ -576,5 +654,9 @@ open class DiscussionSettingsViewController: UITableViewController { } private enum Strings { - static let errorTitle = NSLocalizedString("discussionSettings.saveErrorTitle", value: "Failed to save settings", comment: "Error tilte") + static let errorTitle = NSLocalizedString( + "discussionSettings.saveErrorTitle", + value: "Failed to save settings", + comment: "Error tilte" + ) } diff --git a/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift b/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift index bd6560cb209e..4eec134cc604 100644 --- a/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift +++ b/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift @@ -15,7 +15,11 @@ protocol EditorAnalyticsProperties: AnyObject { /// Analytics are also tracked. struct PostListEditorPresenter { - static func handle(post: Post, in postListViewController: EditorPresenterViewController, entryPoint: PostEditorEntryPoint = .unknown) { + static func handle( + post: Post, + in postListViewController: EditorPresenterViewController, + entryPoint: PostEditorEntryPoint = .unknown + ) { // Return early if a post is still uploading when the editor's requested. guard !PostCoordinator.shared.isUpdating(post) else { return // It's clear from the UI that the cells are not interactive @@ -23,8 +27,9 @@ struct PostListEditorPresenter { // No editing posts until the conflict has been resolved. if let error = PostCoordinator.shared.syncError(for: post.getOriginal()), - let saveError = error as? PostRepository.PostSaveError, - case .conflict(let latest) = saveError { + let saveError = error as? PostRepository.PostSaveError, + case .conflict(let latest) = saveError + { let post = post.getOriginal() PostCoordinator.shared.showResolveConflictView(post: post, remoteRevision: latest, source: .postList) return @@ -43,10 +48,18 @@ struct PostListEditorPresenter { openEditor(with: newPost, in: postListViewController) - WPAppAnalytics.track(.postListDuplicateAction, properties: postListViewController.propertiesForAnalytics(), post: post) + WPAppAnalytics.track( + .postListDuplicateAction, + properties: postListViewController.propertiesForAnalytics(), + post: post + ) } - private static func openEditor(with post: Post, in postListViewController: EditorPresenterViewController, entryPoint: PostEditorEntryPoint = .unknown) { + private static func openEditor( + with post: Post, + in postListViewController: EditorPresenterViewController, + entryPoint: PostEditorEntryPoint = .unknown + ) { /// This is a workaround for the lack of vie wapperance callbacks send /// by `EditPostViewController` due to its weird setup. NotificationCenter.default.post(name: .postListEditorPresenterWillShowEditor, object: nil) @@ -62,6 +75,10 @@ struct PostListEditorPresenter { } extension Foundation.Notification.Name { - static let postListEditorPresenterWillShowEditor = Foundation.Notification.Name("org.automattic.postListEditorPresenterWillShowEditor") - static let postListEditorPresenterDidHideEditor = Foundation.Notification.Name("org.automattic.postListEditorPresenterDidHideEditor") + static let postListEditorPresenterWillShowEditor = Foundation.Notification.Name( + "org.automattic.postListEditorPresenterWillShowEditor" + ) + static let postListEditorPresenterDidHideEditor = Foundation.Notification.Name( + "org.automattic.postListEditorPresenterDidHideEditor" + ) } diff --git a/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift b/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift index 9133e11bdabc..25bbd5b51426 100644 --- a/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift +++ b/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift @@ -69,7 +69,9 @@ private enum Strings { comment: "Footer text when comments are disabled" ) - static let pingbacksLearnMoreURL = URL(string: "https://wordpress.org/documentation/article/trackbacks-and-pingbacks/")! + static let pingbacksLearnMoreURL = URL( + string: "https://wordpress.org/documentation/article/trackbacks-and-pingbacks/" + )! static let learnMorePingbacksText = NSLocalizedString( "postDiscussion.pingbacks.learnMore.text", From 8bdab3a55c021246e42f735710290593d3ca571c Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 26 May 2026 18:24:56 +1200 Subject: [PATCH 2/3] Make post discussion settings nullable and seed new posts Model comment and pingback toggles as optional so an unset value is distinct from off. New posts seed these from the site's discussion defaults when known, the toggle is hidden when unknown, and the value round-trips through create/update without forcing a default. --- Sources/WordPressData/Swift/Blog+Post.swift | 9 + .../WordPressData/Swift/BlogSettings.swift | 4 +- .../CustomPostSettingsViewModelTests.swift | 10 +- .../Features/Posts/PostSettingsTests.swift | 211 +++++++++++++++++- WordPress/Classes/Services/BlogService.m | 26 +-- .../DiscussionSettingsViewController.swift | 14 +- .../CustomPostTypes/CustomPostTabView.swift | 2 + .../Post/PostListEditorPresenter.swift | 3 +- .../PostDiscussionSettingsView.swift | 50 +++-- .../Post/PostSettings/PostSettings.swift | 36 ++- .../Post/PostSettings/PostSettingsView.swift | 2 +- .../PostSettingsViewModelProtocol.swift | 2 +- 12 files changed, 299 insertions(+), 70 deletions(-) diff --git a/Sources/WordPressData/Swift/Blog+Post.swift b/Sources/WordPressData/Swift/Blog+Post.swift index ae6eee309e9f..1f5bf50719b2 100644 --- a/Sources/WordPressData/Swift/Blog+Post.swift +++ b/Sources/WordPressData/Swift/Blog+Post.swift @@ -1,5 +1,6 @@ import CoreData import Foundation +import WordPressKit // MARK: - Lookup posts @@ -73,6 +74,14 @@ extension Blog { } post.postFormat = settings?.defaultPostFormat + + if let allowComments = settings?.commentsAllowed?.boolValue { + post.commentsStatus = (allowComments ? RemotePostDiscussionState.open : .closed).rawValue + } + if let allowPings = settings?.pingbackInboundEnabled?.boolValue { + post.pingsStatus = (allowPings ? RemotePostDiscussionState.open : .closed).rawValue + } + post.postType = Post.typeDefaultIdentifier if let userID, let author = getAuthorWith(id: userID) { diff --git a/Sources/WordPressData/Swift/BlogSettings.swift b/Sources/WordPressData/Swift/BlogSettings.swift index e540413141ac..2c127b805c7e 100644 --- a/Sources/WordPressData/Swift/BlogSettings.swift +++ b/Sources/WordPressData/Swift/BlogSettings.swift @@ -75,7 +75,7 @@ open class BlogSettings: NSManagedObject { /// Represents whether comments are allowed, or not. /// - @NSManaged public var commentsAllowed: Bool + @NSManaged public var commentsAllowed: NSNumber? /// Contains a list of words, space separated, that would cause a comment to be automatically blocklisted. /// @@ -138,7 +138,7 @@ open class BlogSettings: NSManagedObject { /// If set to true, 3rd party sites will be allowed to post pingbacks. /// - @NSManaged public var pingbackInboundEnabled: Bool + @NSManaged public var pingbackInboundEnabled: NSNumber? /// When Outbound Pingbacks are enabled, 3rd party sites that get linked will be notified. /// diff --git a/Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift b/Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift index 01d715aae1c3..2e5aa2358a7a 100644 --- a/Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift +++ b/Tests/KeystoneTests/Tests/Features/Posts/CustomPostSettingsViewModelTests.swift @@ -195,7 +195,11 @@ struct CustomPostSettingsViewModelTests { // MARK: - Test Helpers -private func makePostWithDisabledConnection(status: PostStatus = .publish) throws -> AnyPostWithEditContext { +private func makePostWithDisabledConnection( + status: PostStatus = .publish, + commentStatus: PostCommentStatus? = .open, + pingStatus: PostPingStatus? = .open +) throws -> AnyPostWithEditContext { // Mirrors the real server response observed when a published post has a // connection that was already shared (server returns enabled: false). let json = #""" @@ -225,8 +229,8 @@ private func makePostWithDisabledConnection(status: PostStatus = .publish) throw author: nil, excerpt: nil, featuredMedia: nil, - commentStatus: .open, - pingStatus: .open, + commentStatus: commentStatus, + pingStatus: pingStatus, format: nil, meta: nil, sticky: nil, diff --git a/Tests/KeystoneTests/Tests/Features/Posts/PostSettingsTests.swift b/Tests/KeystoneTests/Tests/Features/Posts/PostSettingsTests.swift index aa55c4312613..3a76183a7b5a 100644 --- a/Tests/KeystoneTests/Tests/Features/Posts/PostSettingsTests.swift +++ b/Tests/KeystoneTests/Tests/Features/Posts/PostSettingsTests.swift @@ -2,6 +2,7 @@ import Testing import Foundation import CoreData import JetpackSocial +import SwiftUI import WordPressAPIInternal @testable import WordPress @testable import WordPressData @@ -1053,14 +1054,14 @@ struct PostSettingsTests { func testDefaultsInheritsClosedDiscussion() { let context = ContextManager.forTesting().mainContext let blog = BlogBuilder(context).with(siteName: "Test").build() - blog.settings?.commentsAllowed = false - blog.settings?.pingbackInboundEnabled = false + blog.settings?.commentsAllowed = NSNumber(value: false) + blog.settings?.pingbackInboundEnabled = NSNumber(value: false) let settings = PostSettings.defaults(from: blog) let params = settings.makeCreateParameters(taxonomies: []) - #expect(!settings.allowComments) - #expect(!settings.allowPings) + #expect(settings.allowComments == false) + #expect(settings.allowPings == false) #expect(params.commentStatus == .closed) #expect(params.pingStatus == .closed) } @@ -1069,17 +1070,198 @@ struct PostSettingsTests { func testDefaultsInheritsOpenDiscussion() { let context = ContextManager.forTesting().mainContext let blog = BlogBuilder(context).with(siteName: "Test").build() - blog.settings?.commentsAllowed = true - blog.settings?.pingbackInboundEnabled = true + blog.settings?.commentsAllowed = NSNumber(value: true) + blog.settings?.pingbackInboundEnabled = NSNumber(value: true) let settings = PostSettings.defaults(from: blog) let params = settings.makeCreateParameters(taxonomies: []) - #expect(settings.allowComments) - #expect(settings.allowPings) + #expect(settings.allowComments == true) + #expect(settings.allowPings == true) #expect(params.commentStatus == .open) #expect(params.pingStatus == .open) } + + // MARK: - PostSettings discussion tri-state + + @Test("New AbstractPost with unset comment status yields nil allowComments") + func testInitFromNewPostHasUnknownDiscussion() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).build() + let post = PostBuilder(context, blog: blog).build() + post.commentsStatus = nil + post.pingsStatus = nil + + let settings = PostSettings(from: post) + + #expect(settings.allowComments == nil) + #expect(settings.allowPings == nil) + } + + @Test("Existing AbstractPost maps stored comment status to non-nil") + func testInitFromExistingPostHasKnownDiscussion() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).build() + let post = PostBuilder(context, blog: blog).build() + post.commentsStatus = "closed" + post.pingsStatus = "open" + + let settings = PostSettings(from: post) + + #expect(settings.allowComments == false) + #expect(settings.allowPings == true) + } + + @Test("REST post with unset comment status yields nil discussion settings") + func testInitFromRestPostHasUnknownDiscussion() { + let post = makeRemotePost(commentStatus: nil, pingStatus: nil) + + let settings = PostSettings(from: post) + + #expect(settings.allowComments == nil) + #expect(settings.allowPings == nil) + } + + @Test("makeCreateParameters omits comment status when unknown") + func testMakeCreateParametersOmitsUnknownDiscussion() { + var settings = PostSettings() + settings.allowComments = nil + settings.allowPings = nil + + let params = settings.makeCreateParameters() + + #expect(params.commentStatus == nil) + #expect(params.pingStatus == nil) + } + + @Test("makeUpdateParameters omits comment/ping status when unknown") + func testMakeUpdateParametersOmitsUnknownDiscussion() { + let post = makeRemotePost() + var settings = PostSettings(from: post) + settings.allowComments = nil + settings.allowPings = nil + + let params = settings.makeUpdateParameters(from: post) + + #expect(params.commentStatus == nil) + #expect(params.pingStatus == nil) + } + + @Test("apply leaves stored comment/ping status untouched when unknown") + func testApplyLeavesDiscussionUntouchedWhenUnknown() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).build() + let post = PostBuilder(context, blog: blog).build() + post.commentsStatus = "closed" + post.pingsStatus = "closed" + + var settings = PostSettings(from: post) + settings.allowComments = nil + settings.allowPings = nil + settings.apply(to: post) + + #expect(post.commentsStatus == "closed") + #expect(post.pingsStatus == "closed") + } + + // MARK: - Blog.createPost() discussion seeding + + @Test("createPost seeds comment/ping status from blog discussion defaults") + func testCreatePostSeedsDiscussionDefaults() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).with(siteName: "Test").build() + blog.settings?.commentsAllowed = NSNumber(value: false) + blog.settings?.pingbackInboundEnabled = NSNumber(value: true) + + let post = blog.createPost() + + #expect(post.commentsStatus == "closed") + #expect(post.pingsStatus == "open") + } + + // MARK: - Unreadable site discussion defaults + + @Test("defaults yields nil discussion when site defaults are unreadable") + func testDefaultsUnknownWhenSiteDefaultsUnreadable() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).with(siteName: "Test").build() + blog.settings?.commentsAllowed = nil + blog.settings?.pingbackInboundEnabled = nil + + let settings = PostSettings.defaults(from: blog) + + #expect(settings.allowComments == nil) + #expect(settings.allowPings == nil) + } + + @Test("createPost leaves comment status unset when site defaults are unreadable") + func testCreatePostNoSeedWhenUnreadable() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).with(siteName: "Test").build() + blog.settings?.commentsAllowed = nil + blog.settings?.pingbackInboundEnabled = nil + + let post = blog.createPost() + + #expect(post.commentsStatus == nil) + #expect(post.pingsStatus == nil) + } + + // MARK: - Discussion row visibility gate (CMM-2077) + + @Test("Discussion row is hidden for a new post with unknown discussion defaults") + func testDiscussionRowHiddenWhenDefaultsUnknown() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).build() + let post = PostBuilder(context, blog: blog).build() + post.commentsStatus = nil + post.pingsStatus = nil + + let viewModel = PostSettingsViewModel(post: post) + + #expect(viewModel.settings.allowComments == nil) + #expect(!viewModel.visibleMoreOptions.contains(.discussion)) + } + + @Test("Discussion row is shown when the post has a known comment status") + func testDiscussionRowShownWhenStatusKnown() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).build() + let post = PostBuilder(context, blog: blog).build() + post.commentsStatus = "open" + post.pingsStatus = "open" + + let viewModel = PostSettingsViewModel(post: post) + + #expect(viewModel.settings.allowComments == true) + #expect(viewModel.visibleMoreOptions.contains(.discussion)) + } + + @Test("Discussion row is shown when only the ping status is known") + func testDiscussionRowShownWhenOnlyPingStatusKnown() { + let context = ContextManager.forTesting().mainContext + let blog = BlogBuilder(context).build() + let post = PostBuilder(context, blog: blog).build() + post.commentsStatus = nil + post.pingsStatus = "open" + + let viewModel = PostSettingsViewModel(post: post) + + #expect(viewModel.settings.allowComments == nil) + #expect(viewModel.settings.allowPings == true) + #expect(viewModel.visibleMoreOptions.contains(.discussion)) + } + + @Test("Discussion view only shows sections for known settings") + func testDiscussionViewSectionVisibility() { + let commentsOnlyView = makeDiscussionView(allowComments: true, allowPings: nil) + #expect(commentsOnlyView.showsCommentsSection) + #expect(!commentsOnlyView.showsPingsSection) + + let pingsOnlyView = makeDiscussionView(allowComments: nil, allowPings: true) + #expect(!pingsOnlyView.showsCommentsSection) + #expect(pingsOnlyView.showsPingsSection) + } } // MARK: - Test Helpers @@ -1090,12 +1272,21 @@ private extension SiteTaxonomy { } } +private func makeDiscussionView(allowComments: Bool?, allowPings: Bool?) -> PostDiscussionSettingsView { + var settings = PostSettings() + settings.allowComments = allowComments + settings.allowPings = allowPings + return PostDiscussionSettingsView(postSettings: .constant(settings)) +} + private func makeRemotePost( tags: [TermId]? = nil, categories: [TermId]? = nil, featuredMedia: MediaId? = nil, format: PostFormat? = nil, meta: PostMeta? = nil, + commentStatus: PostCommentStatus? = .open, + pingStatus: PostPingStatus? = .open, additionalFields: WpAdditionalFields? = nil ) -> AnyPostWithEditContext { AnyPostWithEditContext( @@ -1117,8 +1308,8 @@ private func makeRemotePost( author: nil, excerpt: nil, featuredMedia: featuredMedia, - commentStatus: .open, - pingStatus: .open, + commentStatus: commentStatus, + pingStatus: pingStatus, format: format, meta: meta, sticky: nil, diff --git a/WordPress/Classes/Services/BlogService.m b/WordPress/Classes/Services/BlogService.m index b86012ec1a3c..158d91fba187 100644 --- a/WordPress/Classes/Services/BlogService.m +++ b/WordPress/Classes/Services/BlogService.m @@ -750,11 +750,11 @@ - (void)updateSettings:(BlogSettings *)settings withRemoteSettings:(RemoteBlogSe { NSParameterAssert(settings); NSParameterAssert(remoteSettings); - + // Transformables NSSet *separatedBlocklistKeys = [remoteSettings.commentsBlocklistKeys uniqueStringComponentsSeparatedByNewline]; NSSet *separatedModerationKeys = [remoteSettings.commentsModerationKeys uniqueStringComponentsSeparatedByNewline]; - + // General settings.name = remoteSettings.name; settings.tagline = remoteSettings.tagline; @@ -763,7 +763,7 @@ - (void)updateSettings:(BlogSettings *)settings withRemoteSettings:(RemoteBlogSe settings.iconMediaID = remoteSettings.iconMediaID; settings.gmtOffset = remoteSettings.gmtOffset; settings.timezoneString = remoteSettings.timezoneString; - + // Writing settings.defaultCategoryID = remoteSettings.defaultCategoryID ?: settings.defaultCategoryID; settings.defaultPostFormat = remoteSettings.defaultPostFormat ?: settings.defaultPostFormat; @@ -773,28 +773,28 @@ - (void)updateSettings:(BlogSettings *)settings withRemoteSettings:(RemoteBlogSe settings.postsPerPage = remoteSettings.postsPerPage; // Discussion - settings.commentsAllowed = [remoteSettings.commentsAllowed boolValue]; + settings.commentsAllowed = remoteSettings.commentsAllowed; settings.commentsBlocklistKeys = separatedBlocklistKeys; settings.commentsCloseAutomatically = [remoteSettings.commentsCloseAutomatically boolValue]; settings.commentsCloseAutomaticallyAfterDays = remoteSettings.commentsCloseAutomaticallyAfterDays; settings.commentsFromKnownUsersAllowlisted = [remoteSettings.commentsFromKnownUsersAllowlisted boolValue]; - + settings.commentsMaximumLinks = remoteSettings.commentsMaximumLinks; settings.commentsModerationKeys = separatedModerationKeys; - + settings.commentsPagingEnabled = [remoteSettings.commentsPagingEnabled boolValue]; settings.commentsPageSize = remoteSettings.commentsPageSize; - + settings.commentsRequireManualModeration = [remoteSettings.commentsRequireManualModeration boolValue]; settings.commentsRequireNameAndEmail = [remoteSettings.commentsRequireNameAndEmail boolValue]; settings.commentsRequireRegistration = [remoteSettings.commentsRequireRegistration boolValue]; - + settings.commentsSortOrderAscending = remoteSettings.commentsSortOrderAscending; - + settings.commentsThreadingDepth = remoteSettings.commentsThreadingDepth; settings.commentsThreadingEnabled = [remoteSettings.commentsThreadingEnabled boolValue]; - - settings.pingbackInboundEnabled = [remoteSettings.pingbackInboundEnabled boolValue]; + + settings.pingbackInboundEnabled = remoteSettings.pingbackInboundEnabled; settings.pingbackOutboundEnabled = [remoteSettings.pingbackOutboundEnabled boolValue]; // Related Posts @@ -843,7 +843,7 @@ - (RemoteBlogSettings *)remoteSettingFromSettings:(BlogSettings *)settings remoteSettings.postsPerPage = settings.postsPerPage; // Discussion - remoteSettings.commentsAllowed = @(settings.commentsAllowed); + remoteSettings.commentsAllowed = settings.commentsAllowed; remoteSettings.commentsBlocklistKeys = joinedBlocklistKeys; remoteSettings.commentsCloseAutomatically = @(settings.commentsCloseAutomatically); remoteSettings.commentsCloseAutomaticallyAfterDays = settings.commentsCloseAutomaticallyAfterDays; @@ -864,7 +864,7 @@ - (RemoteBlogSettings *)remoteSettingFromSettings:(BlogSettings *)settings remoteSettings.commentsThreadingDepth = settings.commentsThreadingDepth; remoteSettings.commentsThreadingEnabled = @(settings.commentsThreadingEnabled); - remoteSettings.pingbackInboundEnabled = @(settings.pingbackInboundEnabled); + remoteSettings.pingbackInboundEnabled = settings.pingbackInboundEnabled; remoteSettings.pingbackOutboundEnabled = @(settings.pingbackOutboundEnabled); // AMP diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift index 6dad73d04989..5029fbc16f53 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift @@ -139,7 +139,11 @@ open class DiscussionSettingsViewController: UITableViewController { sections[section].footerText } - open override func tableView(_ tableView: UITableView, willDisplayFooterView view: UIView, forSection section: Int) + open override func tableView( + _ tableView: UITableView, + willDisplayFooterView view: UIView, + forSection section: Int + ) // swiftlint:disable:next opening_brace { WPStyleGuide.configureTableViewSectionFooter(view) } @@ -187,7 +191,7 @@ open class DiscussionSettingsViewController: UITableViewController { return } didChangeSetting("allow_comments", value: enabled as Any) - settings.commentsAllowed = enabled + settings.commentsAllowed = NSNumber(value: enabled) } private func pressedPingbacksInbound(_ payload: AnyObject?) { @@ -195,7 +199,7 @@ open class DiscussionSettingsViewController: UITableViewController { return } didChangeSetting("receive_pingbacks", value: enabled as Any) - settings.pingbackInboundEnabled = enabled + settings.pingbackInboundEnabled = NSNumber(value: enabled) } private func pressedPingbacksOutbound(_ payload: AnyObject?) { @@ -409,7 +413,7 @@ open class DiscussionSettingsViewController: UITableViewController { Row( style: .switch, title: NSLocalizedString("Allow Comments", comment: "Settings: Comments Enabled"), - boolValue: self.settings.commentsAllowed, + boolValue: self.settings.commentsAllowed?.boolValue ?? false, handler: { [weak self] in self?.pressedCommentsAllowed($0) } @@ -427,7 +431,7 @@ open class DiscussionSettingsViewController: UITableViewController { Row( style: .switch, title: NSLocalizedString("Receive Pingbacks", comment: "Settings: Receiving Pingbacks"), - boolValue: self.settings.pingbackInboundEnabled, + boolValue: self.settings.pingbackInboundEnabled?.boolValue ?? false, handler: { [weak self] in self?.pressedPingbacksInbound($0) } diff --git a/WordPress/Classes/ViewRelated/CustomPostTypes/CustomPostTabView.swift b/WordPress/Classes/ViewRelated/CustomPostTypes/CustomPostTabView.swift index d776e07a0751..0ddd66a2afc4 100644 --- a/WordPress/Classes/ViewRelated/CustomPostTypes/CustomPostTabView.swift +++ b/WordPress/Classes/ViewRelated/CustomPostTypes/CustomPostTabView.swift @@ -254,6 +254,8 @@ struct CustomPostTabView: View { // "no format" state rather than snapshotting today's blog default. settings.postFormat = post.format?.id } + settings.allowComments = post.commentStatus.map { $0 == .open } + settings.allowPings = post.pingStatus.map { $0 == .open } let content = EditorContent( title: post.title?.raw ?? "", content: post.content.raw ?? "" diff --git a/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift b/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift index 4eec134cc604..7eb061c84c69 100644 --- a/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift +++ b/WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift @@ -39,12 +39,13 @@ struct PostListEditorPresenter { } static func handleCopy(post: Post, in postListViewController: EditorPresenterViewController) { - // Copy Post let newPost = post.blog.createDraftPost() newPost.postTitle = post.postTitle newPost.content = post.content newPost.categories = post.categories newPost.postFormat = post.postFormat + newPost.commentsStatus = post.commentsStatus + newPost.pingsStatus = post.pingsStatus openEditor(with: newPost, in: postListViewController) diff --git a/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift b/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift index 25bbd5b51426..2f0f6d6a40c5 100644 --- a/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift +++ b/WordPress/Classes/ViewRelated/Post/PostSettings/PostDiscussionSettingsView.swift @@ -5,32 +5,54 @@ struct PostDiscussionSettingsView: View { var body: some View { Form { - // Comments Section - Section { - Toggle(Strings.allowCommentsLabel, isOn: $postSettings.allowComments) + if showsCommentsSection { + Section { + Toggle( + Strings.allowCommentsLabel, + isOn: Binding( + get: { postSettings.allowComments ?? false }, + set: { postSettings.allowComments = $0 } + ) + ) .accessibilityIdentifier("post_discussion_allow_comments_toggle") - } footer: { - Text(commentsFooterText) + } footer: { + Text(commentsFooterText) + } } - // Pingbacks Section - Section { - Toggle(Strings.allowPingsLabel, isOn: $postSettings.allowPings) + if showsPingsSection { + Section { + Toggle( + Strings.allowPingsLabel, + isOn: Binding( + get: { postSettings.allowPings ?? false }, + set: { postSettings.allowPings = $0 } + ) + ) .accessibilityIdentifier("post_discussion_allow_pings_toggle") - } footer: { - Link(destination: Strings.pingbacksLearnMoreURL) { - (Text(Strings.learnMorePingbacksText) + Text(" ") + Text(Image(systemName: "link"))) - .font(.footnote) + } footer: { + Link(destination: Strings.pingbacksLearnMoreURL) { + (Text(Strings.learnMorePingbacksText) + Text(" ") + Text(Image(systemName: "link"))) + .font(.footnote) + } + .accessibilityIdentifier("post_discussion_pingbacks_learn_more_button") } - .accessibilityIdentifier("post_discussion_pingbacks_learn_more_button") } } .navigationTitle(Strings.discussionTitle) .navigationBarTitleDisplayMode(.inline) } + var showsCommentsSection: Bool { + postSettings.allowComments != nil + } + + var showsPingsSection: Bool { + postSettings.allowPings != nil + } + private var commentsFooterText: String { - if postSettings.allowComments { + if postSettings.allowComments == true { return Strings.commentsEnabledFooter } else { return Strings.commentsDisabledFooter diff --git a/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettings.swift b/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettings.swift index 5ca33275468c..f38d8e11e21f 100644 --- a/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettings.swift +++ b/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettings.swift @@ -69,8 +69,8 @@ struct PostSettings: Hashable { /// Publicize draft state used for change detection and REST parameter encoding. /// When available, `connectionsByID` carries the full post-level connection state. var socialSharingDraft: PostSocialSharingDraft? - var allowComments = true - var allowPings = true + var allowComments: Bool? + var allowPings: Bool? // MARK: - Page-specific var parentPageID: Int? @@ -123,8 +123,8 @@ struct PostSettings: Hashable { } ) sharing = PostSocialSharingSettings.make(for: post) - allowComments = post.allowComments - allowPings = post.allowPings + allowComments = post.commentsStatus.map { $0 != RemotePostDiscussionState.closed.rawValue } + allowPings = post.pingsStatus.map { $0 != RemotePostDiscussionState.closed.rawValue } case let page as Page: parentPageID = page.parentID?.intValue default: @@ -173,8 +173,8 @@ struct PostSettings: Hashable { // Tag names will be resolved asynchronously in PostSettingsViewModel tags = (post.tags ?? []).map { Term(id: Int($0), name: "") } categoryIDs = Set((post.categories ?? []).map { Int($0) }) - allowComments = post.commentStatus == .open - allowPings = post.pingStatus == .open + allowComments = post.commentStatus.map { $0 == .open } + allowPings = post.pingStatus.map { $0 == .open } parentPageID = post.parent.map { Int($0) } @@ -204,10 +204,8 @@ struct PostSettings: Hashable { if let userID = blog.userID { settings.author = Author(id: userID.intValue, displayName: "–", avatarURL: nil) } - if let blogSettings = blog.settings { - settings.allowComments = blogSettings.commentsAllowed - settings.allowPings = blogSettings.pingbackInboundEnabled - } + settings.allowComments = blog.settings?.commentsAllowed?.boolValue + settings.allowPings = blog.settings?.pingbackInboundEnabled?.boolValue return settings } @@ -294,10 +292,10 @@ struct PostSettings: Hashable { } // Update discussion settings - if post.allowComments != allowComments { + if let allowComments, post.allowComments != allowComments { post.allowComments = allowComments } - if post.allowPings != allowPings { + if let allowPings, post.allowPings != allowPings { post.allowPings = allowPings } @@ -374,14 +372,12 @@ struct PostSettings: Hashable { params.featuredMedia = self.featuredImageID.map { MediaId(Int64($0)) } ?? MediaId(0) } - let postAllowsComments = post.commentStatus == .open - if postAllowsComments != self.allowComments { - params.commentStatus = self.allowComments ? .open : .closed + if let allowComments, (post.commentStatus == .open) != allowComments { + params.commentStatus = allowComments ? .open : .closed } - let postAllowsPings = post.pingStatus == .open - if postAllowsPings != self.allowPings { - params.pingStatus = self.allowPings ? .open : .closed + if let allowPings, (post.pingStatus == .open) != allowPings { + params.pingStatus = allowPings ? .open : .closed } if post.format.map({ $0.id }) != self.postFormat { @@ -491,8 +487,8 @@ struct PostSettings: Hashable { params.author = author.map { UserId(Int64($0.id)) } params.excerpt = excerpt.isEmpty ? nil : excerpt params.featuredMedia = featuredImageID.map { MediaId(Int64($0)) } - params.commentStatus = allowComments ? .open : .closed - params.pingStatus = allowPings ? .open : .closed + params.commentStatus = allowComments.map { $0 ? .open : .closed } + params.pingStatus = allowPings.map { $0 ? .open : .closed } params.format = postFormat.flatMap { PostFormat.from(slug: $0) } params.sticky = isStickyPost ? true : nil params.categories = categoryIds diff --git a/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsView.swift b/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsView.swift index 735dfc2ce07e..8babc5965f23 100644 --- a/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsView.swift +++ b/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsView.swift @@ -474,7 +474,7 @@ struct PostSettingsFormContentView: Vi } label: { SettingsRow( Strings.discussionLabel, - value: viewModel.settings.allowComments ? Strings.discussionOpen : Strings.discussionClosed + value: viewModel.settings.allowComments == true ? Strings.discussionOpen : Strings.discussionClosed ) } } diff --git a/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsViewModelProtocol.swift b/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsViewModelProtocol.swift index 5c936f853311..517a2cf2303b 100644 --- a/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsViewModelProtocol.swift +++ b/WordPress/Classes/ViewRelated/Post/PostSettings/PostSettingsViewModelProtocol.swift @@ -148,7 +148,7 @@ extension PostSettingsViewModelProtocol { if isDraftOrPending { options.append(.pendingReview) } - if capabilities.supportsComments { + if capabilities.supportsComments && (settings.allowComments != nil || settings.allowPings != nil) { options.append(.discussion) } if capabilities.supportsPostFormats { From 8fe666e2cda586249d83066239f4da089d4ce951 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 26 May 2026 20:41:59 +1200 Subject: [PATCH 3/3] Route blog settings sync through a single coordinator Centralize remote settings fetch and persistence in a Swift coordinator that selects its source by precedence (WP.com REST, else Core REST with an XML-RPC complement, else XML-RPC) and writes once over the cache. Both full-metadata and settings-only syncs route through it instead of each duplicating transport-specific fetches. --- .../Tests/Services/BlogServiceTest.m | 30 +++ .../Services/BlogService+Settings.swift | 204 ++++++++++++++++++ WordPress/Classes/Services/BlogService.h | 8 + WordPress/Classes/Services/BlogService.m | 100 ++++----- 4 files changed, 278 insertions(+), 64 deletions(-) create mode 100644 WordPress/Classes/Services/BlogService+Settings.swift diff --git a/Tests/KeystoneTests/Tests/Services/BlogServiceTest.m b/Tests/KeystoneTests/Tests/Services/BlogServiceTest.m index b5d7d3edd05a..93c9c86769bb 100644 --- a/Tests/KeystoneTests/Tests/Services/BlogServiceTest.m +++ b/Tests/KeystoneTests/Tests/Services/BlogServiceTest.m @@ -4,6 +4,7 @@ #import "WordPressTest-Swift.h" @import WordPressData; +@import WordPressKitModels; @import OCMock; @@ -68,4 +69,33 @@ - (void)cleanUpNSUserDefaultValues [UserSettings setDefaultDotComUUID:nil]; } +- (void)testUpdateSettingsAppliesPresentValuesIncludingFalse +{ + self.blog.settings.commentsAllowed = @YES; + self.blog.settings.commentsCloseAutomatically = YES; + self.blog.settings.pingbackOutboundEnabled = YES; + self.blog.settings.relatedPostsEnabled = YES; + self.blog.settings.ampEnabled = YES; + self.blog.settings.sharingDisabledLikes = YES; + + RemoteBlogSettings *remoteSettings = [RemoteBlogSettings new]; + remoteSettings.commentsAllowed = @NO; + remoteSettings.commentsCloseAutomatically = @NO; + remoteSettings.pingbackOutboundEnabled = @NO; + remoteSettings.relatedPostsEnabled = @NO; + remoteSettings.ampEnabled = @NO; + remoteSettings.sharingDisabledLikes = @NO; + remoteSettings.tagline = @"New tagline"; + + [self.blogService updateSettings:self.blog.settings withRemoteSettings:remoteSettings]; + + XCTAssertEqualObjects(self.blog.settings.commentsAllowed, @NO); + XCTAssertFalse(self.blog.settings.commentsCloseAutomatically); + XCTAssertFalse(self.blog.settings.pingbackOutboundEnabled); + XCTAssertFalse(self.blog.settings.relatedPostsEnabled); + XCTAssertFalse(self.blog.settings.ampEnabled); + XCTAssertFalse(self.blog.settings.sharingDisabledLikes); + XCTAssertEqualObjects(self.blog.settings.tagline, @"New tagline"); +} + @end diff --git a/WordPress/Classes/Services/BlogService+Settings.swift b/WordPress/Classes/Services/BlogService+Settings.swift new file mode 100644 index 000000000000..9835fea06666 --- /dev/null +++ b/WordPress/Classes/Services/BlogService+Settings.swift @@ -0,0 +1,204 @@ +import Foundation +import WordPressData +import WordPressKit +import WordPressKitObjC +import WordPressShared +import WordPressCore + +enum BlogSettingsFetchError: Error { + case unknown + case allSourcesFailed + case missingSiteID +} + +extension BlogService { + @objc(fetchAndPersistSettingsForBlog:completion:) + public func fetchAndPersistSettings(for blog: Blog, completion: ((Error?) -> Void)?) { + // Capture the ID synchronously on the caller's context; the async work + // must not capture the `blog` managed object across contexts. + let blogID = TaggedManagedObjectID(blog) + Task { @MainActor in + do { + try await fetchAndPersistSettings(for: blogID) + completion?(nil) + } catch { + completion?(error) + } + } + } + + private func fetchAndPersistSettings(for blogID: TaggedManagedObjectID) async throws { + let source = try await settingsFetchSource(for: blogID) + + switch source { + case .wpcom(let remote): + let settings = try await fetchSettings(remote) + await persistSettings(settings, for: blogID) + case .coreREST(let primaryRemote, let complementRemote): + async let primary = try? fetchSettings(primaryRemote) + async let complement = fetchOptionalSettings(complementRemote) + + let fetched = await (primary: primary, complement: complement) + guard let settings = combinedSettings(primary: fetched.primary, complement: fetched.complement) else { + throw BlogSettingsFetchError.allSourcesFailed + } + await persistSettings(settings, for: blogID) + case .xmlrpc(let remote): + let settings = try await fetchSettings(remote) + await persistSettings(settings, for: blogID) + case .missingSiteID: + throw BlogSettingsFetchError.missingSiteID + case .none: + return + } + } + + /// Combines the two payloads of the only fetch path with more than one source: + /// an application-password site, where the Core REST primary omits `privacy` + /// that the XML-RPC options (complement) still carry. The primary always wins; + /// the complement only fills the handful of fields it can provide (name, + /// tagline, privacy, see `RemoteBlogOptionsHelper`). Returns nil only when both + /// sources are nil. Single-source paths skip this and persist their one payload + /// directly, so the snapshot write in `-[BlogService updateSettings:withRemoteSettings:]` + /// is the only merge other sites see. + private func combinedSettings( + primary: RemoteBlogSettings?, + complement: RemoteBlogSettings? + ) -> RemoteBlogSettings? { + guard let primary else { + return complement + } + guard let complement else { + return primary + } + primary.name = primary.name ?? complement.name + primary.tagline = primary.tagline ?? complement.tagline + primary.privacy = primary.privacy ?? complement.privacy + return primary + } + + @MainActor + private func settingsFetchSource(for blogID: TaggedManagedObjectID) throws -> SettingsFetchSource { + // Resolve the blog on the main context here (on the main actor) so the + // remotes are built from a managed object bound to a known context. The + // remotes capture value-typed credentials, so they're safe to use from the + // async fetches that follow. + let blog = try coreDataStack.mainContext.existingObject(with: blogID) + if blog.supports(.wpComRESTAPI), let api = blog.wordPressComRestApi { + guard let dotComID = blog.dotComID else { + return .missingSiteID + } + return .wpcom(BlogServiceRemoteREST(wordPressComRestApi: api, siteID: dotComID)) + } + + if let coreREST = BlogServiceRemoteCoreREST(blog: blog) { + let complement = xmlrpcRemote(for: blog) + return .coreREST(primary: coreREST, complement: complement) + } + + if let xmlrpcRemote = xmlrpcRemote(for: blog) { + return .xmlrpc(xmlrpcRemote) + } + + return .none + } + + private func xmlrpcRemote(for blog: Blog) -> BlogServiceRemoteXMLRPC? { + // The Objective-C initializer returns nil for missing credentials, which Swift imports as non-optional. + guard let xmlrpcApi = blog.xmlrpcApi, + let username = blog.username, + let password = blog.password + else { + return nil + } + + return BlogServiceRemoteXMLRPC(api: xmlrpcApi, username: username, password: password) + } + + private func fetchSettings(_ remote: BlogServiceRemoteREST) async throws -> RemoteBlogSettings { + try await withCheckedThrowingContinuation { continuation in + remote.syncBlogSettings( + success: { settings in + guard let settings else { + continuation.resume(throwing: BlogSettingsFetchError.unknown) + return + } + continuation.resume(returning: settings) + }, + failure: { error in + continuation.resume(throwing: error ?? BlogSettingsFetchError.unknown) + } + ) + } + } + + private func fetchSettings(_ remote: BlogServiceRemoteCoreREST) async throws -> RemoteBlogSettings { + try await withCheckedThrowingContinuation { continuation in + remote.syncBlogSettings( + success: { settings in + guard let settings else { + continuation.resume(throwing: BlogSettingsFetchError.unknown) + return + } + continuation.resume(returning: settings) + }, + failure: { error in + continuation.resume(throwing: error ?? BlogSettingsFetchError.unknown) + } + ) + } + } + + private func fetchSettings(_ remote: BlogServiceRemoteXMLRPC) async throws -> RemoteBlogSettings { + try await withCheckedThrowingContinuation { continuation in + remote.syncBlogOptions( + success: { options in + guard let options else { + continuation.resume(throwing: BlogSettingsFetchError.unknown) + return + } + let settings = RemoteBlogOptionsHelper.remoteBlogSettings( + fromXMLRPCDictionaryOptions: options as NSDictionary + ) + continuation.resume(returning: settings) + }, + failure: { error in + continuation.resume(throwing: error ?? BlogSettingsFetchError.unknown) + } + ) + } + } + + private func fetchOptionalSettings(_ remote: BlogServiceRemoteXMLRPC?) async -> RemoteBlogSettings? { + guard let remote else { + return nil + } + return try? await fetchSettings(remote) + } + + private func persistSettings( + _ remoteSettings: RemoteBlogSettings, + for blogID: TaggedManagedObjectID + ) async { + // The throwing `performAndSave` lives on `CoreDataStackSwift`, but `coreDataStack` here + // is the base `CoreDataStack`. Rather than downcast to propagate the error (and handle a + // cast failure that can't realistically happen), we ignore the unlikely blog-resolution failure. + await coreDataStack.performAndSave { context in + if let blog = try? context.existingObject(with: blogID), let settings = blog.settings { + self.update(settings, withRemoteSettings: remoteSettings) + } + } + } +} + +private enum SettingsFetchSource { + case wpcom(BlogServiceRemoteREST) + case coreREST(primary: BlogServiceRemoteCoreREST, complement: BlogServiceRemoteXMLRPC?) + case xmlrpc(BlogServiceRemoteXMLRPC) + /// Not reachable in practice: a blog that supports the WP.com REST API always + /// has a `dotComID`, so the site ID is never missing once that branch is taken. + case missingSiteID + /// Not reachable in practice: every blog has at least one usable transport + /// (WP.com REST, Core REST, or XML-RPC), so a source is always found. + case none +} diff --git a/WordPress/Classes/Services/BlogService.h b/WordPress/Classes/Services/BlogService.h index 2055fe5e1a00..3500fd5581c9 100644 --- a/WordPress/Classes/Services/BlogService.h +++ b/WordPress/Classes/Services/BlogService.h @@ -123,4 +123,12 @@ extern NSString *const WPBlogSettingsUpdatedNotification; @end +@class BlogSettings; +@class RemoteBlogSettings; + +/// Internal methods exposed for `BlogService+Settings.swift`. +@interface BlogService (Settings) +- (void)updateSettings:(BlogSettings *)settings withRemoteSettings:(RemoteBlogSettings *)remoteSettings; +@end + NS_ASSUME_NONNULL_END diff --git a/WordPress/Classes/Services/BlogService.m b/WordPress/Classes/Services/BlogService.m index 158d91fba187..9df30485a4e6 100644 --- a/WordPress/Classes/Services/BlogService.m +++ b/WordPress/Classes/Services/BlogService.m @@ -78,6 +78,14 @@ - (void)syncBlogAndAllMetadata:(Blog *)blog completionHandler:(void (^)(void))co NSManagedObjectID *blogObjectID = blog.objectID; id remote = [self remoteForBlog:blog]; + dispatch_group_enter(syncGroup); + [self fetchAndPersistSettingsForBlog:blog completion:^(NSError *error) { + if (error) { + DDLogError(@"Failed syncing settings for blog %@: %@", blog.url, error); + } + dispatch_group_leave(syncGroup); + }]; + if ([remote isKindOfClass:[BlogServiceRemoteREST class]]) { dispatch_group_enter(syncGroup); BlogServiceRemoteREST *restRemote = remote; @@ -89,46 +97,17 @@ - (void)syncBlogAndAllMetadata:(Blog *)blog completionHandler:(void (^)(void))co DDLogError(@"Failed syncing site details for blog %@: %@", blog.url, error); dispatch_group_leave(syncGroup); }]; - - dispatch_group_enter(syncGroup); - [restRemote syncBlogSettingsWithSuccess:^(RemoteBlogSettings *settings) { - [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { - Blog *blogInContext = (Blog *)[context existingObjectWithID:blogObjectID error:nil]; - if (blogInContext) { - [self updateSettings:blogInContext.settings withRemoteSettings:settings]; - } - } completion:^{ - dispatch_group_leave(syncGroup); - } onQueue:dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0)]; - } failure:^(NSError *error) { - DDLogError(@"Failed syncing settings for blog %@: %@", blog.url, error); - dispatch_group_leave(syncGroup); - }]; } else { + // Self-hosted sites still need a `wp.getOptions` call to refresh `blog.options` + // (version, capabilities) and `isXMLRPCDisabled`. Settings are handled above by + // the coordinator, so pass `updateSettings:NO` to avoid a redundant write. dispatch_group_enter(syncGroup); OptionsHandler handler = [self optionsHandlerWithBlogObjectID:blogObjectID + updateSettings:NO completionHandler:^{ dispatch_group_leave(syncGroup); }]; [self syncXMLRPCOptionsIfApplicableFor:blog optionsHandler:handler failure:^{ dispatch_group_leave(syncGroup); }]; - - if ([remote isKindOfClass:[BlogServiceRemoteCoreREST class]]) { - BlogServiceRemoteCoreREST *coreRestRemote = (BlogServiceRemoteCoreREST *)remote; - dispatch_group_enter(syncGroup); - [coreRestRemote syncBlogSettingsWithSuccess:^(RemoteBlogSettings *settings) { - [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { - Blog *blogInContext = (Blog *)[context existingObjectWithID:blogObjectID error:nil]; - if (blogInContext) { - [self updateSettings:blogInContext.settings withRemoteSettings:settings]; - } - } completion:^{ - dispatch_group_leave(syncGroup); - } onQueue:dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0)]; - } failure:^(NSError *error) { - DDLogError(@"Failed syncing settings for blog %@: %@", blog.url, error); - dispatch_group_leave(syncGroup); - }]; - } } dispatch_group_enter(syncGroup); @@ -263,34 +242,17 @@ - (void)syncSettingsForBlog:(Blog *)blog return; } - void(^updateOnSuccess)(RemoteBlogSettings *) = ^(RemoteBlogSettings *remoteSettings) { - [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { - Blog *blogInContext = (Blog *)[context objectWithID:blogID]; - [self updateSettings:blogInContext.settings withRemoteSettings:remoteSettings]; - } completion:success onQueue:dispatch_get_main_queue()]; - }; - id remote = [self remoteForBlog:blogInContext]; - if ([remote isKindOfClass:[BlogServiceRemoteXMLRPC class]]) { - - BlogServiceRemoteXMLRPC *xmlrpcRemote = remote; - [xmlrpcRemote syncBlogOptionsWithSuccess:^(NSDictionary *options) { - RemoteBlogSettings *remoteSettings = [RemoteBlogOptionsHelper remoteBlogSettingsFromXMLRPCDictionaryOptions:options]; - updateOnSuccess(remoteSettings); - } failure:failure]; - - } else if ([remote isKindOfClass:[BlogServiceRemoteREST class]]) { - - BlogServiceRemoteREST *restRemote = remote; - [restRemote syncBlogSettingsWithSuccess:^(RemoteBlogSettings *settings) { - updateOnSuccess(settings); - } failure:failure]; - - } else if ([remote isKindOfClass:[BlogServiceRemoteCoreREST class]]) { - BlogServiceRemoteCoreREST *coreRestRemote = (BlogServiceRemoteCoreREST *)remote; - [coreRestRemote syncBlogSettingsWithSuccess:^(RemoteBlogSettings *settings) { - updateOnSuccess(settings); - } failure:failure]; - } + [self fetchAndPersistSettingsForBlog:blogInContext completion:^(NSError *error) { + if (error) { + if (failure) { + failure(error); + } + } else { + if (success) { + success(); + } + } + }]; }]; } @@ -694,6 +656,15 @@ - (BlogDetailsHandler)blogDetailsHandlerWithBlogObjectID:(NSManagedObjectID *)bl - (OptionsHandler)optionsHandlerWithBlogObjectID:(NSManagedObjectID *)blogObjectID completionHandler:(void (^)(void))completion +{ + return [self optionsHandlerWithBlogObjectID:blogObjectID + updateSettings:YES + completionHandler:completion]; +} + +- (OptionsHandler)optionsHandlerWithBlogObjectID:(NSManagedObjectID *)blogObjectID + updateSettings:(BOOL)updateSettings + completionHandler:(void (^)(void))completion { return ^void(NSDictionary *options) { [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { @@ -705,9 +676,10 @@ - (OptionsHandler)optionsHandlerWithBlogObjectID:(NSManagedObjectID *)blogObject blog.options = [NSDictionary dictionaryWithDictionary:options]; blog.isXMLRPCDisabled = NO; - RemoteBlogSettings *remoteSettings = [RemoteBlogOptionsHelper remoteBlogSettingsFromXMLRPCDictionaryOptions:options]; - [self updateSettings:blog.settings withRemoteSettings:remoteSettings]; - + if (updateSettings) { + RemoteBlogSettings *remoteSettings = [RemoteBlogOptionsHelper remoteBlogSettingsFromXMLRPCDictionaryOptions:options]; + [self updateSettings:blog.settings withRemoteSettings:remoteSettings]; + } // NOTE: `[blog version]` can return nil. If this happens `version` will be `0` CGFloat version = [[blog version] floatValue];