diff --git a/Modules/Sources/WordPressCore/WordPressClient.swift b/Modules/Sources/WordPressCore/WordPressClient.swift index f3c27ded44dd..225c649fe15f 100644 --- a/Modules/Sources/WordPressCore/WordPressClient.swift +++ b/Modules/Sources/WordPressCore/WordPressClient.swift @@ -300,6 +300,15 @@ public actor WordPressClient { } } + /// Updates site settings and replaces the cached settings with the + /// server's response, so `fetchSiteSettings()` stays coherent after a + /// save without a refetch. + public func updateSiteSettings(params: SiteSettingsUpdateParams) async throws -> SiteSettingsWithEditContext { + let updated = try await api.siteSettings.update(params: params).data + self.loadSiteSettingsTask = Task { updated } + return updated + } + /// Creates a new task to fetch the site settings from the server. private func newSiteSettingsTask() -> Task { Task { diff --git a/Tests/KeystoneTests/Tests/Services/BlogServiceRemoteCoreRESTSettingsTests.swift b/Tests/KeystoneTests/Tests/Services/BlogServiceRemoteCoreRESTSettingsTests.swift index 674120c83870..f7b6bcf44f50 100644 --- a/Tests/KeystoneTests/Tests/Services/BlogServiceRemoteCoreRESTSettingsTests.swift +++ b/Tests/KeystoneTests/Tests/Services/BlogServiceRemoteCoreRESTSettingsTests.swift @@ -17,7 +17,8 @@ struct BlogServiceRemoteCoreRESTSettingsTests { startOfWeek: UInt64 = 1, defaultCategory: UInt64 = 1, defaultPostFormat: String = "standard", - postsPerPage: UInt64 = 10 + postsPerPage: UInt64 = 10, + siteIcon: UInt64 = 0 ) -> SiteSettingsWithEditContext { SiteSettingsWithEditContext( title: title, @@ -39,7 +40,7 @@ struct BlogServiceRemoteCoreRESTSettingsTests { defaultPingStatus: .closed, defaultCommentStatus: .closed, siteLogo: nil, - siteIcon: 0, + siteIcon: siteIcon, additionalFields: WpAdditionalFields() ) } @@ -124,4 +125,81 @@ struct BlogServiceRemoteCoreRESTSettingsTests { ) #expect(result.postsPerPage == NSNumber(value: 25)) } + + // MARK: - Write mapping + + @Test func writeMapsTitleOnly() { + let sparse = RemoteBlogSettings() + sparse.name = "New Title" + let params = BlogServiceRemoteCoreREST.makeUpdateParams(from: sparse) + #expect(params.title == "New Title") + #expect(params.description == nil) + #expect(params.timezone == nil) + #expect(params.defaultCommentStatus == nil) + #expect(params.defaultPingStatus == nil) + #expect(params.siteIcon == nil) + } + + @Test func writeMapsWritingFields() { + let sparse = RemoteBlogSettings() + sparse.tagline = "tag" + sparse.timezoneString = "Europe/Vienna" + sparse.dateFormat = "F j, Y" + sparse.timeFormat = "g:i a" + sparse.startOfWeek = "1" + sparse.defaultCategoryID = 7 + sparse.postsPerPage = 12 + let params = BlogServiceRemoteCoreREST.makeUpdateParams(from: sparse) + #expect(params.description == "tag") + #expect(params.timezone == "Europe/Vienna") + #expect(params.dateFormat == "F j, Y") + #expect(params.timeFormat == "g:i a") + #expect(params.startOfWeek == 1) + #expect(params.defaultCategory == 7) + #expect(params.postsPerPage == 12) + } + + @Test func writeMapsStandardPostFormatToZero() { + let sparse = RemoteBlogSettings() + sparse.defaultPostFormat = "standard" + #expect(BlogServiceRemoteCoreREST.makeUpdateParams(from: sparse).defaultPostFormat == "0") + } + + @Test func writeMapsNonStandardPostFormatVerbatim() { + let sparse = RemoteBlogSettings() + sparse.defaultPostFormat = "aside" + #expect(BlogServiceRemoteCoreREST.makeUpdateParams(from: sparse).defaultPostFormat == "aside") + } + + @Test func writeOmitsNonNumericStartOfWeek() { + let sparse = RemoteBlogSettings() + sparse.startOfWeek = "monday" + #expect(BlogServiceRemoteCoreREST.makeUpdateParams(from: sparse).startOfWeek == nil) + } + + @Test func writeMapsDiscussionBooleans() { + let sparse = RemoteBlogSettings() + sparse.commentsAllowed = true + sparse.pingbackInboundEnabled = false + let params = BlogServiceRemoteCoreREST.makeUpdateParams(from: sparse) + #expect(params.defaultCommentStatus == .open) + #expect(params.defaultPingStatus == .closed) + } + + @Test func writeMapsIconAndRemoval() { + let set = RemoteBlogSettings() + set.iconMediaID = 42 + #expect(BlogServiceRemoteCoreREST.makeUpdateParams(from: set).siteIcon == 42) + + let removal = RemoteBlogSettings() + removal.iconMediaID = 0 + #expect(BlogServiceRemoteCoreREST.makeUpdateParams(from: removal).siteIcon == 0) + } + + // MARK: - Icon read mapping + + @Test func readMapsSiteIcon() { + let result = BlogServiceRemoteCoreREST.mapSiteSettings(makeSiteSettings(siteIcon: 42)) + #expect(result.iconMediaID == 42) + } } diff --git a/Tests/KeystoneTests/Tests/Services/BlogServiceRemoteCoreRESTUpdateTests.swift b/Tests/KeystoneTests/Tests/Services/BlogServiceRemoteCoreRESTUpdateTests.swift new file mode 100644 index 000000000000..70ffe88a4cbf --- /dev/null +++ b/Tests/KeystoneTests/Tests/Services/BlogServiceRemoteCoreRESTUpdateTests.swift @@ -0,0 +1,57 @@ +import XCTest +import OHHTTPStubs +import OHHTTPStubsSwift +import WordPressAPI +@testable import WordPress +@testable import WordPressCore + +final class BlogServiceRemoteCoreRESTUpdateTests: XCTestCase { + + override func tearDown() { + HTTPStubs.removeAllStubs() + super.tearDown() + } + + func testTitleOnlyUpdateSendsOnlyTitle() async throws { + let api = try WordPressAPI( + urlSession: URLSession(configuration: .ephemeral), + siteInfo: .selfHosted( + siteUrl: .parse(input: "https://example.com"), + apiRoot: .parse(input: "https://example.com/wp-json") + ), + authentication: .none + ) + // WordPressClient's init eagerly starts api-root/user/theme/settings + // tasks. Install a host-wide stub before constructing it so those hit + // the stub instead of the network; the more specific settings stub + // registered below takes precedence for the request under test. + stub(condition: isHost("example.com")) { _ in + HTTPStubsResponse(data: Data(), statusCode: 200, headers: nil) + } + let client = WordPressClient(api: api, siteURL: URL(string: "https://example.com")!) + let remote = BlogServiceRemoteCoreREST(client: client) + + var capturedBody: [String: Any] = [:] + stub(condition: { $0.url?.absoluteString.contains("/wp/v2/settings") == true || $0.url?.query?.contains("rest_route") == true }) { request in + if let body = request.ohhttpStubs_httpBody, + let json = try? JSONSerialization.jsonObject(with: body) as? [String: Any] + { + capturedBody = json + } + return HTTPStubsResponse( + data: WordPressClientSiteSettingsTests.settingsJSON.data(using: .utf8)!, + statusCode: 200, + headers: ["Content-Type": "application/json"] + ) + } + + let sparse = RemoteBlogSettings() + sparse.name = "Updated Title" + try await remote.updateBlogSettings(sparse) + + XCTAssertEqual(capturedBody["title"] as? String, "Updated Title") + XCTAssertNil(capturedBody["description"]) + XCTAssertNil(capturedBody["default_comment_status"]) + XCTAssertNil(capturedBody["site_icon"]) + } +} diff --git a/Tests/KeystoneTests/Tests/Services/BlogServiceUpdateSettingsTests.swift b/Tests/KeystoneTests/Tests/Services/BlogServiceUpdateSettingsTests.swift new file mode 100644 index 000000000000..e58b96a786e2 --- /dev/null +++ b/Tests/KeystoneTests/Tests/Services/BlogServiceUpdateSettingsTests.swift @@ -0,0 +1,342 @@ +import XCTest +import OHHTTPStubs +import OHHTTPStubsSwift +import WordPressKitModels +@testable import WordPress +@testable import WordPressData + +final class BlogServiceUpdateSettingsTests: CoreDataTestCase { + + private var service: BlogService! + + override func setUp() { + super.setUp() + service = BlogService(coreDataStack: contextManager) + } + + override func tearDown() { + HTTPStubs.removeAllStubs() + super.tearDown() + } + + private func makeChanges(name: String) -> BlogSettingsChanges { + let changes = BlogSettingsChanges() + changes.name = name + return changes + } + + private func attachSettings(to blog: Blog) { + blog.settings = NSEntityDescription.insertNewObject( + forEntityName: "BlogSettings", into: mainContext) as? BlogSettings + } + + // MARK: WP.com + + private func makeDotComBlog(dotComID: Int = 12345) -> Blog { + let blog = BlogBuilder(mainContext) + .withAnAccount() + .with(dotComID: dotComID) + .build() + attachSettings(to: blog) + blog.settings?.name = "Old Title" + try! mainContext.save() + return blog + } + + func testDotComTitleOnlySaveSendsOnlyTitleAndSucceedsOnce() { + let blog = makeDotComBlog() + var capturedParams: [String: Any] = [:] + stub(condition: pathEndsWith("/sites/12345/settings")) { request in + if let body = request.ohhttpStubs_httpBody, + let json = try? JSONSerialization.jsonObject(with: body) as? [String: Any] + { + capturedParams = json + } + return HTTPStubsResponse( + jsonObject: ["updated": ["blogname": "New Title"]], statusCode: 200, headers: nil) + } + + let success = expectation(description: "success called exactly once") + service.updateSettings(for: blog, changes: makeChanges(name: "New Title"), success: { + success.fulfill() + }, failure: { _ in + XCTFail("failure must not be called") + }) + wait(for: [success], timeout: 5) + + XCTAssertEqual(capturedParams["blogname"] as? String, "New Title") + XCTAssertNil(capturedParams["blogdescription"]) + XCTAssertNil(capturedParams["amp_is_enabled"]) + } + + func testDotComServerErrorFailsOnceAndPersistsNothing() { + let blog = makeDotComBlog() + stub(condition: pathEndsWith("/sites/12345/settings")) { _ in + HTTPStubsResponse(jsonObject: ["error": "unauthorized"], statusCode: 403, headers: nil) + } + + let failure = expectation(description: "failure called exactly once") + service.updateSettings(for: blog, changes: makeChanges(name: "New Title"), success: { + XCTFail("success must not be called") + }, failure: { _ in + failure.fulfill() + }) + wait(for: [failure], timeout: 5) + + // Nothing was persisted by the write path: the committed value is unchanged. + mainContext.refreshAllObjects() + XCTAssertEqual(blog.settings?.name, "Old Title") + } + + func testFailedSaveDoesNotBlockNextSave() { + let blog = makeDotComBlog() + var requestCount = 0 + stub(condition: pathEndsWith("/sites/12345/settings")) { _ in + requestCount += 1 + if requestCount == 1 { + return HTTPStubsResponse( + jsonObject: ["error": "unauthorized"], + statusCode: 403, + headers: nil + ) + } + return HTTPStubsResponse( + jsonObject: ["updated": ["blogname": "Second Title"]], + statusCode: 200, + headers: nil + ) + } + + let firstFailure = expectation(description: "first save fails") + let secondSuccess = expectation(description: "second save succeeds") + service.updateSettings( + for: blog, + changes: makeChanges(name: "First Title"), + success: { + XCTFail("first save must not succeed") + }, + failure: { _ in + firstFailure.fulfill() + } + ) + service.updateSettings( + for: blog, + changes: makeChanges(name: "Second Title"), + success: { + secondSuccess.fulfill() + }, + failure: { _ in + XCTFail("second save must not fail") + } + ) + wait(for: [firstFailure, secondSuccess], timeout: 10) + + XCTAssertEqual(requestCount, 2) + } + + // MARK: XML-RPC + + private func makeSelfHostedBlog() -> Blog { + let blog = BlogBuilder(mainContext) + .with(username: "admin") + .build() + blog.xmlrpc = "https://selfhosted.example/xmlrpc.php" + blog.password = "secret" + attachSettings(to: blog) + try! mainContext.save() + return blog + } + + // The XML-RPC transport can only write the title/tagline subset. Its wire + // payload is `RemoteBlogOptionsHelper.remoteOptionsForUpdatingBlogTitleAndTagline` + // applied to the sparse settings the write path maps from `changes` (see + // `performSettingsUpdate`'s `.xmlrpc` branch). This asserts that subset + // directly: OHHTTPStubs does not intercept `WordPressOrgXMLRPCApi`'s + // URLSession in this test target, so an end-to-end XML-RPC round trip is + // verified in `WordPressKitTests` instead. The one-callback contract for + // the XML-RPC branch is covered by `testXMLRPCUnsupportedFieldsSkipRequestAndSucceed`. + func testXMLRPCTaglineOnlyMapsToTaglineOptionOnly() { + let changes = BlogSettingsChanges() + changes.tagline = "New tagline" + let options = RemoteBlogOptionsHelper.remoteOptionsForUpdatingBlogTitleAndTagline( + changes.toRemoteBlogSettings()) + XCTAssertEqual(options["blog_tagline"] as? String, "New tagline") + XCTAssertNil(options["blog_title"]) + } + + func testXMLRPCUnsupportedFieldsSkipRequestAndSucceed() { + let blog = makeSelfHostedBlog() + var requestCount = 0 + stub(condition: isHost("selfhosted.example")) { _ in + requestCount += 1 + return HTTPStubsResponse(data: Data(), statusCode: 200, headers: nil) + } + + let changes = BlogSettingsChanges() + changes.dateFormat = "F j, Y" // XML-RPC cannot write this + let success = expectation(description: "success without request") + service.updateSettings(for: blog, changes: changes, success: { success.fulfill() }, + failure: { _ in XCTFail("unexpected failure") }) + wait(for: [success], timeout: 5) + XCTAssertEqual(requestCount, 0) + } + + // MARK: Empty changes / no transport + + func testEmptyChangesSucceedWithoutRequest() { + let blog = makeSelfHostedBlog() + var requestCount = 0 + stub(condition: { _ in true }) { _ in + requestCount += 1 + return HTTPStubsResponse(data: Data(), statusCode: 200, headers: nil) + } + let success = expectation(description: "success") + service.updateSettings(for: blog, changes: BlogSettingsChanges(), success: { success.fulfill() }, + failure: { _ in XCTFail("unexpected failure") }) + wait(for: [success], timeout: 5) + XCTAssertEqual(requestCount, 0) + } + + func testNoTransportFailsWithNoAvailableTransport() { + // No account (so not WP.com), no application password (so not Core + // REST), and no XML-RPC endpoint: every transport is unavailable. + let blog = BlogBuilder(mainContext, dotComID: nil).with(username: "admin").build() + try! mainContext.save() + + let failure = expectation(description: "failure") + service.updateSettings(for: blog, changes: makeChanges(name: "T"), success: { + XCTFail("success must not be called") + }, failure: { error in + XCTAssertTrue(error is BlogSettingsServiceError) + failure.fulfill() + }) + wait(for: [failure], timeout: 5) + } + + // MARK: Persistence + + // On success, exactly the acknowledged changes are persisted; undeclared + // fields are untouched. Driven over WP.com because its transport is the + // one OHHTTPStubs reliably intercepts in this target; persistence itself + // is transport-agnostic (`persist(_:for:)` runs after any successful save). + func testSuccessPersistsOnlyDeclaredFields() { + let blog = makeDotComBlog() + blog.settings?.name = "Old Title" + blog.settings?.tagline = "Old tagline" + try! mainContext.save() + stub(condition: pathEndsWith("/sites/12345/settings")) { _ in + HTTPStubsResponse(jsonObject: ["updated": ["blogname": "New Title"]], statusCode: 200, headers: nil) + } + + let success = expectation(description: "success") + service.updateSettings(for: blog, changes: makeChanges(name: "New Title"), + success: { success.fulfill() }, + failure: { _ in XCTFail("unexpected failure") }) + wait(for: [success], timeout: 5) + + mainContext.refreshAllObjects() + XCTAssertEqual(blog.settings?.name, "New Title") + XCTAssertEqual(blog.settings?.tagline, "Old tagline") + } + + // MARK: Serialization + + func testOverlappingSavesFromTwoServiceInstancesAreSequential() { + let blog = makeDotComBlog() + var responseTimes: [Date] = [] + var requestTimes: [Date] = [] + stub(condition: pathEndsWith("/sites/12345/settings")) { _ in + requestTimes.append(Date()) + let response = HTTPStubsResponse( + jsonObject: ["updated": [:]], statusCode: 200, headers: nil) + response.responseTime = 0.5 + responseTimes.append(Date().addingTimeInterval(0.5)) + return response + } + + let first = expectation(description: "first completes") + let second = expectation(description: "second completes") + let serviceA = BlogService(coreDataStack: contextManager) + let serviceB = BlogService(coreDataStack: contextManager) + serviceA.updateSettings(for: blog, changes: makeChanges(name: "A"), + success: { first.fulfill() }, failure: { _ in XCTFail() }) + serviceB.updateSettings(for: blog, changes: makeChanges(name: "B"), + success: { second.fulfill() }, failure: { _ in XCTFail() }) + wait(for: [first, second], timeout: 10) + + XCTAssertEqual(requestTimes.count, 2) + // The second request must not start before the first response finished. + XCTAssertGreaterThanOrEqual(requestTimes[1], responseTimes[0]) + } + + func testOverlappingSavesForDifferentBlogsAreSequential() { + let firstBlog = makeDotComBlog(dotComID: 12345) + let secondBlog = makeDotComBlog(dotComID: 67890) + var responseTimes: [Date] = [] + var requestTimes: [Date] = [] + stub(condition: pathEndsWith("/settings")) { _ in + requestTimes.append(Date()) + let response = HTTPStubsResponse( + jsonObject: ["updated": [:]], + statusCode: 200, + headers: nil + ) + response.responseTime = 0.5 + responseTimes.append(Date().addingTimeInterval(0.5)) + return response + } + + let first = expectation(description: "first completes") + let second = expectation(description: "second completes") + let serviceA = BlogService(coreDataStack: contextManager) + let serviceB = BlogService(coreDataStack: contextManager) + serviceA.updateSettings( + for: firstBlog, + changes: makeChanges(name: "A"), + success: { first.fulfill() }, + failure: { _ in XCTFail() } + ) + serviceB.updateSettings( + for: secondBlog, + changes: makeChanges(name: "B"), + success: { second.fulfill() }, + failure: { _ in XCTFail() } + ) + wait(for: [first, second], timeout: 10) + + XCTAssertEqual(requestTimes.count, 2) + XCTAssertGreaterThanOrEqual(requestTimes[1], responseTimes[0]) + } + + // An application-password blog must resolve to the Core REST transport. + // Asserted at the source-resolution seam (the plan's documented fallback): + // the full-path Core REST wire behavior is covered by + // `BlogServiceRemoteCoreRESTUpdateTests`, and driving the request here + // would depend on the `WordPressClientFactory` session being stub-backed. + @MainActor + func testApplicationPasswordBlogUsesCoreRESTSource() throws { + let keychain = TestKeychain() + // Set the URL before storing the app password: the token is keyed by + // the blog's URL string, so it must match what resolution reads back. + let blog = BlogBuilder(mainContext, dotComID: nil) + .with(username: "admin") + .with(url: "https://selfhosted.example") + .with(restApiRootURL: "https://selfhosted.example/wp-json/") + .withApplicationPassword("app-password", using: keychain) + .build() + try mainContext.save() + + // Building the Core REST remote constructs a factory-backed + // WordPressClient whose eager caches fire background requests. Stub the + // host so this unit test stays offline and quiet; the assertion below + // only cares about which transport resolution selects. + stub(condition: isHost("selfhosted.example")) { _ in + HTTPStubsResponse(data: Data(), statusCode: 200, headers: nil) + } + + let source = try service.settingsSource(for: TaggedManagedObjectID(blog), keychain: keychain) + guard case .coreREST = source else { + return XCTFail("expected Core REST source, got \(source)") + } + } +} diff --git a/Tests/KeystoneTests/Tests/Services/BlogSettingsChangesTests.swift b/Tests/KeystoneTests/Tests/Services/BlogSettingsChangesTests.swift new file mode 100644 index 000000000000..f17a8324900a --- /dev/null +++ b/Tests/KeystoneTests/Tests/Services/BlogSettingsChangesTests.swift @@ -0,0 +1,63 @@ +import Testing +import WordPressKitModels +@testable import WordPress + +struct BlogSettingsChangesTests { + + @Test func emptyByDefault() { + #expect(BlogSettingsChanges().isEmpty) + } + + @Test func notEmptyWithOneField() { + let changes = BlogSettingsChanges() + changes.name = "Title" + #expect(!changes.isEmpty) + } + + @Test func sparseMappingCopiesOnlyDeclaredFields() { + let changes = BlogSettingsChanges() + changes.name = "My Blog" + let remote = changes.toRemoteBlogSettings() + #expect(remote.name == "My Blog") + #expect(remote.tagline == nil) + #expect(remote.postsPerPage == nil) + #expect(remote.commentsAllowed == nil) + #expect(remote.iconMediaID == nil) + } + + @Test func mapsAllScalarFields() { + let changes = BlogSettingsChanges() + changes.tagline = "tag" + changes.privacy = 1 + changes.languageID = 2 + changes.gmtOffset = NSNumber(value: -5.0) + changes.timezoneString = "America/New_York" + changes.defaultCategoryID = 3 + changes.defaultPostFormat = "aside" + changes.dateFormat = "F j, Y" + changes.timeFormat = "g:i a" + changes.startOfWeek = "1" + changes.postsPerPage = 10 + changes.iconMediaID = 42 + let remote = changes.toRemoteBlogSettings() + #expect(remote.tagline == "tag") + #expect(remote.privacy == 1) + #expect(remote.languageID == 2) + #expect(remote.gmtOffset == NSNumber(value: -5.0)) + #expect(remote.timezoneString == "America/New_York") + #expect(remote.defaultCategoryID == 3) + #expect(remote.defaultPostFormat == "aside") + #expect(remote.dateFormat == "F j, Y") + #expect(remote.timeFormat == "g:i a") + #expect(remote.startOfWeek == "1") + #expect(remote.postsPerPage == 10) + #expect(remote.iconMediaID == 42) + } + + @Test func mapsCommentSortOrderThroughAscendingBridge() { + let changes = BlogSettingsChanges() + changes.commentsSortOrderAscending = NSNumber(value: false) + let remote = changes.toRemoteBlogSettings() + #expect(!remote.commentsSortOrderAscending) + } +} diff --git a/Tests/KeystoneTests/Tests/Services/WordPressClientSiteSettingsTests.swift b/Tests/KeystoneTests/Tests/Services/WordPressClientSiteSettingsTests.swift new file mode 100644 index 000000000000..9fbdd59793f5 --- /dev/null +++ b/Tests/KeystoneTests/Tests/Services/WordPressClientSiteSettingsTests.swift @@ -0,0 +1,65 @@ +import XCTest +import OHHTTPStubs +import OHHTTPStubsSwift +import WordPressAPI +import WordPressAPIInternal +@testable import WordPress +@testable import WordPressCore + +final class WordPressClientSiteSettingsTests: XCTestCase { + + override func tearDown() { + HTTPStubs.removeAllStubs() + super.tearDown() + } + + private func makeClient() throws -> WordPressClient { + let api = try WordPressAPI( + urlSession: URLSession(configuration: .ephemeral), + siteInfo: .selfHosted( + siteUrl: .parse(input: "https://example.com"), + apiRoot: .parse(input: "https://example.com/wp-json") + ), + authentication: .none + ) + // WordPressClient's init eagerly starts api-root/user/theme/settings + // tasks. Install a host-wide stub before constructing it so those hit + // the stub instead of the network; each test's more specific stub, + // registered later, takes precedence for the request under test. + stub(condition: isHost("example.com")) { _ in + HTTPStubsResponse(data: Data(), statusCode: 200, headers: nil) + } + return WordPressClient(api: api, siteURL: URL(string: "https://example.com")!) + } + + static let settingsJSON = """ + {"title":"Updated Title","description":"tag","url":"https://example.com", + "email":"a@example.com","timezone":"","date_format":"F j, Y","time_format":"g:i a", + "start_of_week":1,"language":"en_US","use_smilies":false,"default_category":1, + "default_post_format":"0","posts_per_page":10,"show_on_front":"posts", + "page_on_front":0,"page_for_posts":0,"default_ping_status":"open", + "default_comment_status":"open","site_logo":null,"site_icon":0} + """ + + func testUpdateReplacesCachedSettings() async throws { + let client = try makeClient() + var requestCount = 0 + stub(condition: { $0.url?.absoluteString.contains("/wp/v2/settings") == true || $0.url?.query?.contains("rest_route") == true }) { _ in + requestCount += 1 + return HTTPStubsResponse( + data: Self.settingsJSON.data(using: .utf8)!, + statusCode: 200, + headers: ["Content-Type": "application/json"] + ) + } + + let updated = try await client.updateSiteSettings(params: SiteSettingsUpdateParams(title: "Updated Title")) + XCTAssertEqual(updated.title, "Updated Title") + let updateRequests = requestCount + + // The cache must serve the update response without another request. + let cached = try await client.fetchSiteSettings() + XCTAssertEqual(cached.title, "Updated Title") + XCTAssertEqual(requestCount, updateRequests) + } +} diff --git a/WordPress/Classes/Services/BlogService+Settings.swift b/WordPress/Classes/Services/BlogService+Settings.swift index 9835fea06666..cb40cf5d9fd7 100644 --- a/WordPress/Classes/Services/BlogService+Settings.swift +++ b/WordPress/Classes/Services/BlogService+Settings.swift @@ -5,10 +5,11 @@ import WordPressKitObjC import WordPressShared import WordPressCore -enum BlogSettingsFetchError: Error { +enum BlogSettingsServiceError: Error { case unknown case allSourcesFailed case missingSiteID + case noAvailableTransport } extension BlogService { @@ -28,7 +29,7 @@ extension BlogService { } private func fetchAndPersistSettings(for blogID: TaggedManagedObjectID) async throws { - let source = try await settingsFetchSource(for: blogID) + let source = try await settingsSource(for: blogID) switch source { case .wpcom(let remote): @@ -40,14 +41,14 @@ extension BlogService { let fetched = await (primary: primary, complement: complement) guard let settings = combinedSettings(primary: fetched.primary, complement: fetched.complement) else { - throw BlogSettingsFetchError.allSourcesFailed + throw BlogSettingsServiceError.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 + throw BlogSettingsServiceError.missingSiteID case .none: return } @@ -78,7 +79,10 @@ extension BlogService { } @MainActor - private func settingsFetchSource(for blogID: TaggedManagedObjectID) throws -> SettingsFetchSource { + func settingsSource( + for blogID: TaggedManagedObjectID, + keychain: KeychainAccessible = AppKeychain() + ) throws -> SettingsSource { // 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 @@ -91,7 +95,7 @@ extension BlogService { return .wpcom(BlogServiceRemoteREST(wordPressComRestApi: api, siteID: dotComID)) } - if let coreREST = BlogServiceRemoteCoreREST(blog: blog) { + if let coreREST = BlogServiceRemoteCoreREST(blog: blog, keychain: keychain) { let complement = xmlrpcRemote(for: blog) return .coreREST(primary: coreREST, complement: complement) } @@ -120,13 +124,13 @@ extension BlogService { remote.syncBlogSettings( success: { settings in guard let settings else { - continuation.resume(throwing: BlogSettingsFetchError.unknown) + continuation.resume(throwing: BlogSettingsServiceError.unknown) return } continuation.resume(returning: settings) }, failure: { error in - continuation.resume(throwing: error ?? BlogSettingsFetchError.unknown) + continuation.resume(throwing: error ?? BlogSettingsServiceError.unknown) } ) } @@ -137,13 +141,13 @@ extension BlogService { remote.syncBlogSettings( success: { settings in guard let settings else { - continuation.resume(throwing: BlogSettingsFetchError.unknown) + continuation.resume(throwing: BlogSettingsServiceError.unknown) return } continuation.resume(returning: settings) }, failure: { error in - continuation.resume(throwing: error ?? BlogSettingsFetchError.unknown) + continuation.resume(throwing: error ?? BlogSettingsServiceError.unknown) } ) } @@ -154,7 +158,7 @@ extension BlogService { remote.syncBlogOptions( success: { options in guard let options else { - continuation.resume(throwing: BlogSettingsFetchError.unknown) + continuation.resume(throwing: BlogSettingsServiceError.unknown) return } let settings = RemoteBlogOptionsHelper.remoteBlogSettings( @@ -163,7 +167,7 @@ extension BlogService { continuation.resume(returning: settings) }, failure: { error in - continuation.resume(throwing: error ?? BlogSettingsFetchError.unknown) + continuation.resume(throwing: error ?? BlogSettingsServiceError.unknown) } ) } @@ -191,7 +195,7 @@ extension BlogService { } } -private enum SettingsFetchSource { +enum SettingsSource { case wpcom(BlogServiceRemoteREST) case coreREST(primary: BlogServiceRemoteCoreREST, complement: BlogServiceRemoteXMLRPC?) case xmlrpc(BlogServiceRemoteXMLRPC) @@ -202,3 +206,130 @@ private enum SettingsFetchSource { /// (WP.com REST, Core REST, or XML-RPC), so a source is always found. case none } + +extension BlogService { + /// Serializes settings writes that can be triggered rapidly from the UI. + /// + /// For example, toggling settings in `SharingButtonsViewController` can + /// call `updateSettings` again before the previous request finishes. A + /// shared queue preserves the user's change order even when each call uses + /// a new `BlogService` instance. + private static let settingsWriteOperationQueue: OperationQueue = { + let queue = OperationQueue() + queue.name = "org.wordpress.BlogService.settingsWrites" + queue.maxConcurrentOperationCount = 1 + return queue + }() + + /// Saves the declared settings changes to the blog's transport. + /// + /// Callers keep mutating `blog.settings` in memory for immediate UI + /// feedback and pass the same values here. Only acknowledged changes + /// are persisted by this path; on failure it persists nothing (pending + /// in-memory edits share the main context and can still be persisted + /// by unrelated saves, exactly as before this API existed). + @objc(updateSettingsForBlog:changes:success:failure:) + public func updateSettings( + for blog: Blog, + changes: BlogSettingsChanges, + success: (() -> Void)?, + failure: ((Error) -> Void)? + ) { + updateSettings(for: blog, changes: changes, keychain: AppKeychain(), success: success, failure: failure) + } + + func updateSettings( + for blog: Blog, + changes: BlogSettingsChanges, + keychain: KeychainAccessible, + success: (() -> Void)?, + failure: ((Error) -> Void)? + ) { + let blogID = TaggedManagedObjectID(blog) + Self.settingsWriteOperationQueue.addOperation( + AsyncBlockOperation { operationCompletion in + Task { @MainActor in + defer { operationCompletion() } + + do { + try await self.performSettingsUpdate(for: blogID, changes: changes, keychain: keychain) + success?() + } catch { + failure?(error) + } + } + } + ) + } + + @MainActor + private func performSettingsUpdate( + for blogID: TaggedManagedObjectID, + changes: BlogSettingsChanges, + keychain: KeychainAccessible + ) async throws { + guard !changes.isEmpty else { + return + } + + let sparse = changes.toRemoteBlogSettings() + let source = try settingsSource(for: blogID, keychain: keychain) + + switch source { + case .wpcom(let remote): + try await updateSettings(sparse, via: remote) + case .coreREST(let primary, _): + // The XML-RPC complement is read-path-only: the only fields the + // XML-RPC writer ever handled (title, tagline) are written by + // Core REST itself. + try await primary.updateBlogSettings(sparse) + case .xmlrpc(let remote): + let options = RemoteBlogOptionsHelper.remoteOptionsForUpdatingBlogTitleAndTagline(sparse) as? [AnyHashable: Any] ?? [:] + guard !options.isEmpty else { + // This transport can only write title and tagline; other + // declared fields are silently unsupported, as they always + // were. Nothing to send is a success, not an error. + return + } + try await updateOptions(options, via: remote) + case .missingSiteID, .none: + throw BlogSettingsServiceError.noAvailableTransport + } + + // Persist the acknowledged changes on a background context. The cast is + // safe: ContextManager is the app's only CoreDataStack and conforms to + // CoreDataStackSwift (see EditorSettingsService for the same pattern). + // Failure is best-effort: the server write already succeeded, so a local + // save failure must not turn this save into a failure callback. + try? await (coreDataStack as! CoreDataStackSwift) + .performAndSave { context in + if let blog = try? context.existingObject(with: blogID), let settings = blog.settings { + changes.apply(to: settings) + } + } + } + + private func updateSettings(_ settings: RemoteBlogSettings, via remote: BlogServiceRemoteREST) async throws { + try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in + remote.update( + settings, + success: { continuation.resume() }, + failure: { error in + continuation.resume(throwing: error ?? BlogSettingsServiceError.unknown) + } + ) + } + } + + private func updateOptions(_ options: [AnyHashable: Any], via remote: BlogServiceRemoteXMLRPC) async throws { + try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in + remote.updateBlogOptions( + with: options, + success: { continuation.resume() }, + failure: { error in + continuation.resume(throwing: error ?? BlogSettingsServiceError.unknown) + } + ) + } + } +} diff --git a/WordPress/Classes/Services/BlogService.h b/WordPress/Classes/Services/BlogService.h index 3500fd5581c9..c47f8ab1f802 100644 --- a/WordPress/Classes/Services/BlogService.h +++ b/WordPress/Classes/Services/BlogService.h @@ -97,17 +97,6 @@ extern NSString *const WPBlogSettingsUpdatedNotification; success:(void (^)(void))success failure:(void (^)(NSError *error))failure; -/** - * Update blog settings to the server - * - * @param blog the blog to update - * @param success a block that is invoked when the update is successful - * @param failure a block that in invoked when the update fails. - */ -- (void)updateSettingsForBlog:(Blog *)blog - success:(nullable void (^)(void))success - failure:(nullable void (^)(NSError *error))failure; - /** * Associate synced blogs to the specified Jetpack account. * diff --git a/WordPress/Classes/Services/BlogService.m b/WordPress/Classes/Services/BlogService.m index 323d96c64d27..40b95c9b4a72 100644 --- a/WordPress/Classes/Services/BlogService.m +++ b/WordPress/Classes/Services/BlogService.m @@ -241,47 +241,6 @@ - (void)syncAuthorsForBlog:(Blog *)blog }]; } -- (void)updateSettingsForBlog:(Blog *)blog - success:(void (^)(void))success - failure:(void (^)(NSError *error))failure -{ - NSManagedObjectID *blogID = [blog objectID]; - NSManagedObjectContext *context = self.coreDataStack.mainContext; - [context performBlock:^{ - Blog *blogInContext = (Blog *)[context objectWithID:blogID]; - id remote = [self remoteForBlog:blogInContext]; - RemoteBlogSettings *remoteSettings = [self remoteSettingFromSettings:blogInContext.settings]; - - void(^onSuccess)(void) = ^() { - [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) { - Blog *blogInContext = (Blog *)[context existingObjectWithID:blogID error:nil]; - if (blogInContext) { - [self updateSettings:blogInContext.settings withRemoteSettings:remoteSettings]; - } - } completion:^{ - if (success) { - success(); - } - } onQueue:dispatch_get_main_queue()]; - }; - - if ([remote isKindOfClass:[BlogServiceRemoteXMLRPC class]]) { - - BlogServiceRemoteXMLRPC *xmlrpcRemote = remote; - [xmlrpcRemote updateBlogOptionsWith:[RemoteBlogOptionsHelper remoteOptionsForUpdatingBlogTitleAndTagline:remoteSettings] - success:onSuccess - failure:failure]; - - } else if([remote isKindOfClass:[BlogServiceRemoteREST class]]) { - - BlogServiceRemoteREST *restRemote = remote; - [restRemote updateBlogSettings:remoteSettings - success:onSuccess - failure:failure]; - } - }]; -} - - (void)syncPostTypesForBlog:(Blog *)blog success:(void (^)(void))success failure:(void (^)(NSError *error))failure @@ -758,75 +717,4 @@ - (void)updateSettings:(BlogSettings *)settings withRemoteSettings:(RemoteBlogSe settings.sharingDisabledReblogs = [remoteSettings.sharingDisabledReblogs boolValue]; } -- (RemoteBlogSettings *)remoteSettingFromSettings:(BlogSettings *)settings -{ - NSParameterAssert(settings); - RemoteBlogSettings *remoteSettings = [RemoteBlogSettings new]; - - // Transformables - NSString *joinedBlocklistKeys = [[settings.commentsBlocklistKeys allObjects] componentsJoinedByString:@"\n"]; - NSString *joinedModerationKeys = [[settings.commentsModerationKeys allObjects] componentsJoinedByString:@"\n"]; - - // General - remoteSettings.name = settings.name; - remoteSettings.tagline = settings.tagline; - remoteSettings.privacy = settings.privacy; - remoteSettings.languageID = settings.languageID; - remoteSettings.iconMediaID = settings.iconMediaID; - remoteSettings.gmtOffset = settings.gmtOffset; - remoteSettings.timezoneString = settings.timezoneString; - - // Writing - remoteSettings.defaultCategoryID = settings.defaultCategoryID; - remoteSettings.defaultPostFormat = settings.defaultPostFormat; - remoteSettings.dateFormat = settings.dateFormat; - remoteSettings.timeFormat = settings.timeFormat; - remoteSettings.startOfWeek = settings.startOfWeek; - remoteSettings.postsPerPage = settings.postsPerPage; - - // Discussion - remoteSettings.commentsAllowed = settings.commentsAllowed; - remoteSettings.commentsBlocklistKeys = joinedBlocklistKeys; - remoteSettings.commentsCloseAutomatically = @(settings.commentsCloseAutomatically); - remoteSettings.commentsCloseAutomaticallyAfterDays = settings.commentsCloseAutomaticallyAfterDays; - remoteSettings.commentsFromKnownUsersAllowlisted = @(settings.commentsFromKnownUsersAllowlisted); - - remoteSettings.commentsMaximumLinks = settings.commentsMaximumLinks; - remoteSettings.commentsModerationKeys = joinedModerationKeys; - - remoteSettings.commentsPagingEnabled = @(settings.commentsPagingEnabled); - remoteSettings.commentsPageSize = settings.commentsPageSize; - - remoteSettings.commentsRequireManualModeration = @(settings.commentsRequireManualModeration); - remoteSettings.commentsRequireNameAndEmail = @(settings.commentsRequireNameAndEmail); - remoteSettings.commentsRequireRegistration = @(settings.commentsRequireRegistration); - - remoteSettings.commentsSortOrderAscending = settings.commentsSortOrderAscending; - - remoteSettings.commentsThreadingDepth = settings.commentsThreadingDepth; - remoteSettings.commentsThreadingEnabled = @(settings.commentsThreadingEnabled); - - remoteSettings.pingbackInboundEnabled = settings.pingbackInboundEnabled; - remoteSettings.pingbackOutboundEnabled = @(settings.pingbackOutboundEnabled); - - // AMP - remoteSettings.ampEnabled = @(settings.ampEnabled); - - // Related Posts - remoteSettings.relatedPostsAllowed = @(settings.relatedPostsAllowed); - remoteSettings.relatedPostsEnabled = @(settings.relatedPostsEnabled); - remoteSettings.relatedPostsShowHeadline = @(settings.relatedPostsShowHeadline); - remoteSettings.relatedPostsShowThumbnails = @(settings.relatedPostsShowThumbnails); - - // Sharing - remoteSettings.sharingButtonStyle = settings.sharingButtonStyle; - remoteSettings.sharingLabel = settings.sharingLabel; - remoteSettings.sharingTwitterName = settings.sharingTwitterName; - remoteSettings.sharingCommentLikesEnabled = @(settings.sharingCommentLikesEnabled); - remoteSettings.sharingDisabledLikes = @(settings.sharingDisabledLikes); - remoteSettings.sharingDisabledReblogs = @(settings.sharingDisabledReblogs); - - return remoteSettings; -} - @end diff --git a/WordPress/Classes/Services/BlogServiceRemoteCoreREST.swift b/WordPress/Classes/Services/BlogServiceRemoteCoreREST.swift index 0ac01a954fe3..cca494db8980 100644 --- a/WordPress/Classes/Services/BlogServiceRemoteCoreREST.swift +++ b/WordPress/Classes/Services/BlogServiceRemoteCoreREST.swift @@ -4,6 +4,7 @@ import WordPressCore import WordPressData import WordPressAPI import WordPressAPIInternal +import WordPressShared @objc public class BlogServiceRemoteCoreREST: NSObject, BlogServiceRemote { let client: WordPressClient @@ -16,6 +17,14 @@ import WordPressAPIInternal ) } + public convenience init?(blog: Blog, keychain: KeychainAccessible) { + guard let site = try? WordPressSite(blog: blog, keychain: keychain) else { return nil } + + self.init( + client: WordPressClientFactory.shared.instance(for: site) + ) + } + init(client: WordPressClient) { self.client = client } @@ -128,11 +137,13 @@ import WordPressAPIInternal settings.pingbackInboundEnabled = siteSettings.defaultPingStatus .map { NSNumber(value: $0.allowsDiscussion) } + // Site icon + settings.iconMediaID = NSNumber(value: siteSettings.siteIcon) + // The following properties are not available from the Core REST API // site settings endpoint. settings.privacy = nil settings.languageID = nil - settings.iconMediaID = nil settings.gmtOffset = nil settings.commentsBlocklistKeys = nil settings.commentsCloseAutomatically = nil @@ -165,6 +176,39 @@ import WordPressAPIInternal return settings } + /// Maps a sparse `RemoteBlogSettings` (only the caller's declared fields + /// are non-nil) to the Core REST update params. A nil field is left out + /// of the params so it is not written. + static func makeUpdateParams(from settings: RemoteBlogSettings) -> SiteSettingsUpdateParams { + // "standard" is stored as "0" server-side; mirror of the read mapping above. + let postFormat: String? = settings.defaultPostFormat.map { $0 == "standard" ? "0" : $0 } + + // A plugin-defined custom status cannot be expressed by the app's + // Bool model. The field is only ever non-nil when the user toggled + // it, so writing open/closed is their explicit choice. + return SiteSettingsUpdateParams( + title: settings.name, + description: settings.tagline, + timezone: settings.timezoneString, + dateFormat: settings.dateFormat, + timeFormat: settings.timeFormat, + startOfWeek: settings.startOfWeek.flatMap { UInt64($0) }, + defaultCategory: settings.defaultCategoryID.map { UInt64(truncating: $0) }, + defaultPostFormat: postFormat, + postsPerPage: settings.postsPerPage.map { UInt64(truncating: $0) }, + defaultPingStatus: settings.pingbackInboundEnabled.map { $0.boolValue ? .open : .closed }, + defaultCommentStatus: settings.commentsAllowed.map { $0.boolValue ? .open : .closed }, + siteIcon: settings.iconMediaID.map { UInt64(truncating: $0) } + ) + } + + /// Writes the declared settings (a sparse `RemoteBlogSettings`) through + /// the Core REST `/wp/v2/settings` endpoint. + func updateBlogSettings(_ settings: RemoteBlogSettings) async throws { + let params = Self.makeUpdateParams(from: settings) + _ = try await client.updateSiteSettings(params: params) + } + @objc public func syncBlogSettings( success: SettingsHandler?, failure: (((any Error)?) -> Void)? diff --git a/WordPress/Classes/Services/BlogSettingsChanges.swift b/WordPress/Classes/Services/BlogSettingsChanges.swift new file mode 100644 index 000000000000..8f8356b32d32 --- /dev/null +++ b/WordPress/Classes/Services/BlogSettingsChanges.swift @@ -0,0 +1,217 @@ +import Foundation +import WordPressData +import WordPressKitModels + +/// The fields of one settings save. Callers declare exactly what they +/// changed; nil fields are not sent and not persisted. This exists because +/// deriving the delta from Core Data dirty state is unreliable across +/// in-flight saves (see the design doc's "Why not infer the delta" section). +@objc public final class BlogSettingsChanges: NSObject { + // General + @objc public var name: String? + @objc public var tagline: String? + @objc public var privacy: NSNumber? + @objc public var languageID: NSNumber? + @objc public var gmtOffset: NSNumber? + @objc public var timezoneString: String? + // Writing + @objc public var defaultCategoryID: NSNumber? + @objc public var defaultPostFormat: String? + @objc public var dateFormat: String? + @objc public var timeFormat: String? + @objc public var startOfWeek: String? + @objc public var postsPerPage: NSNumber? + // Discussion (Bool-valued entries are NSNumber-wrapped Bools) + @objc public var commentsAllowed: NSNumber? + @objc public var commentsBlocklistKeys: String? + @objc public var commentsCloseAutomatically: NSNumber? + @objc public var commentsCloseAutomaticallyAfterDays: NSNumber? + @objc public var commentsFromKnownUsersAllowlisted: NSNumber? + @objc public var commentsMaximumLinks: NSNumber? + @objc public var commentsModerationKeys: String? + @objc public var commentsPagingEnabled: NSNumber? + @objc public var commentsPageSize: NSNumber? + @objc public var commentsRequireManualModeration: NSNumber? + @objc public var commentsRequireNameAndEmail: NSNumber? + @objc public var commentsRequireRegistration: NSNumber? + @objc public var commentsSortOrderAscending: NSNumber? + @objc public var commentsThreadingDepth: NSNumber? + @objc public var commentsThreadingEnabled: NSNumber? + @objc public var pingbackInboundEnabled: NSNumber? + @objc public var pingbackOutboundEnabled: NSNumber? + // Related posts + @objc public var relatedPostsEnabled: NSNumber? + @objc public var relatedPostsShowHeadline: NSNumber? + @objc public var relatedPostsShowThumbnails: NSNumber? + // Traffic + @objc public var ampEnabled: NSNumber? + // Sharing + @objc public var sharingButtonStyle: String? + @objc public var sharingLabel: String? + @objc public var sharingTwitterName: String? + @objc public var sharingCommentLikesEnabled: NSNumber? + @objc public var sharingDisabledLikes: NSNumber? + @objc public var sharingDisabledReblogs: NSNumber? + // Site icon (0 = remove, mirroring the icon UI's convention) + @objc public var iconMediaID: NSNumber? + + private var allValues: [Any?] { + [ + name, tagline, privacy, languageID, gmtOffset, timezoneString, + defaultCategoryID, defaultPostFormat, dateFormat, timeFormat, + startOfWeek, postsPerPage, + commentsAllowed, commentsBlocklistKeys, commentsCloseAutomatically, + commentsCloseAutomaticallyAfterDays, commentsFromKnownUsersAllowlisted, + commentsMaximumLinks, commentsModerationKeys, commentsPagingEnabled, + commentsPageSize, commentsRequireManualModeration, + commentsRequireNameAndEmail, commentsRequireRegistration, + commentsSortOrderAscending, commentsThreadingDepth, + commentsThreadingEnabled, pingbackInboundEnabled, pingbackOutboundEnabled, + relatedPostsEnabled, relatedPostsShowHeadline, relatedPostsShowThumbnails, + ampEnabled, + sharingButtonStyle, sharingLabel, sharingTwitterName, + sharingCommentLikesEnabled, sharingDisabledLikes, sharingDisabledReblogs, + iconMediaID + ] + } + + @objc public var isEmpty: Bool { + allValues.allSatisfy { $0 == nil } + } + + func toRemoteBlogSettings() -> RemoteBlogSettings { + let remote = RemoteBlogSettings() + remote.name = name + remote.tagline = tagline + remote.privacy = privacy + remote.languageID = languageID + remote.gmtOffset = gmtOffset + remote.timezoneString = timezoneString + remote.defaultCategoryID = defaultCategoryID + remote.defaultPostFormat = defaultPostFormat + remote.dateFormat = dateFormat + remote.timeFormat = timeFormat + remote.startOfWeek = startOfWeek + remote.postsPerPage = postsPerPage + remote.commentsAllowed = commentsAllowed + remote.commentsBlocklistKeys = commentsBlocklistKeys + remote.commentsCloseAutomatically = commentsCloseAutomatically + remote.commentsCloseAutomaticallyAfterDays = commentsCloseAutomaticallyAfterDays + remote.commentsFromKnownUsersAllowlisted = commentsFromKnownUsersAllowlisted + remote.commentsMaximumLinks = commentsMaximumLinks + remote.commentsModerationKeys = commentsModerationKeys + remote.commentsPagingEnabled = commentsPagingEnabled + remote.commentsPageSize = commentsPageSize + remote.commentsRequireManualModeration = commentsRequireManualModeration + remote.commentsRequireNameAndEmail = commentsRequireNameAndEmail + remote.commentsRequireRegistration = commentsRequireRegistration + if let commentsSortOrderAscending { + remote.commentsSortOrderAscending = commentsSortOrderAscending.boolValue + } + remote.commentsThreadingDepth = commentsThreadingDepth + remote.commentsThreadingEnabled = commentsThreadingEnabled + remote.pingbackInboundEnabled = pingbackInboundEnabled + remote.pingbackOutboundEnabled = pingbackOutboundEnabled + remote.relatedPostsEnabled = relatedPostsEnabled + remote.relatedPostsShowHeadline = relatedPostsShowHeadline + remote.relatedPostsShowThumbnails = relatedPostsShowThumbnails + remote.ampEnabled = ampEnabled + remote.sharingButtonStyle = sharingButtonStyle + remote.sharingLabel = sharingLabel + remote.sharingTwitterName = sharingTwitterName + remote.sharingCommentLikesEnabled = sharingCommentLikesEnabled + remote.sharingDisabledLikes = sharingDisabledLikes + remote.sharingDisabledReblogs = sharingDisabledReblogs + remote.iconMediaID = iconMediaID + return remote + } + + /// Applies only the declared fields, mirroring the transforms in + /// `-[BlogService updateSettings:withRemoteSettings:]` (the read-path + /// merge, which must never be used for sparse writes because it + /// assigns every field unconditionally). + func apply(to settings: BlogSettings) { + if let name { settings.name = name } + if let tagline { settings.tagline = tagline } + if let privacy { settings.privacy = privacy } + if let languageID { settings.languageID = languageID } + if let gmtOffset { settings.gmtOffset = gmtOffset } + if let timezoneString { settings.timezoneString = timezoneString } + if let defaultCategoryID { settings.defaultCategoryID = defaultCategoryID } + if let defaultPostFormat { settings.defaultPostFormat = defaultPostFormat } + if let dateFormat { settings.dateFormat = dateFormat } + if let timeFormat { settings.timeFormat = timeFormat } + if let startOfWeek { settings.startOfWeek = startOfWeek } + if let postsPerPage { settings.postsPerPage = postsPerPage } + if let commentsAllowed { settings.commentsAllowed = commentsAllowed } + if let commentsBlocklistKeys { + settings.commentsBlocklistKeys = Set( + commentsBlocklistKeys + .components(separatedBy: .newlines) + .filter { !$0.isEmpty } + ) + } + if let commentsCloseAutomatically { + settings.commentsCloseAutomatically = commentsCloseAutomatically.boolValue + } + if let commentsCloseAutomaticallyAfterDays { + settings.commentsCloseAutomaticallyAfterDays = commentsCloseAutomaticallyAfterDays + } + if let commentsFromKnownUsersAllowlisted { + settings.commentsFromKnownUsersAllowlisted = commentsFromKnownUsersAllowlisted.boolValue + } + if let commentsMaximumLinks { settings.commentsMaximumLinks = commentsMaximumLinks } + if let commentsModerationKeys { + settings.commentsModerationKeys = Set( + commentsModerationKeys + .components(separatedBy: .newlines) + .filter { !$0.isEmpty } + ) + } + if let commentsPagingEnabled { + settings.commentsPagingEnabled = commentsPagingEnabled.boolValue + } + if let commentsPageSize { settings.commentsPageSize = commentsPageSize } + if let commentsRequireManualModeration { + settings.commentsRequireManualModeration = commentsRequireManualModeration.boolValue + } + if let commentsRequireNameAndEmail { + settings.commentsRequireNameAndEmail = commentsRequireNameAndEmail.boolValue + } + if let commentsRequireRegistration { + settings.commentsRequireRegistration = commentsRequireRegistration.boolValue + } + if let commentsSortOrderAscending { + settings.commentsSortOrderAscending = commentsSortOrderAscending.boolValue + } + if let commentsThreadingDepth { settings.commentsThreadingDepth = commentsThreadingDepth } + if let commentsThreadingEnabled { + settings.commentsThreadingEnabled = commentsThreadingEnabled.boolValue + } + if let pingbackInboundEnabled { settings.pingbackInboundEnabled = pingbackInboundEnabled } + if let pingbackOutboundEnabled { + settings.pingbackOutboundEnabled = pingbackOutboundEnabled.boolValue + } + if let relatedPostsEnabled { settings.relatedPostsEnabled = relatedPostsEnabled.boolValue } + if let relatedPostsShowHeadline { + settings.relatedPostsShowHeadline = relatedPostsShowHeadline.boolValue + } + if let relatedPostsShowThumbnails { + settings.relatedPostsShowThumbnails = relatedPostsShowThumbnails.boolValue + } + if let ampEnabled { settings.ampEnabled = ampEnabled.boolValue } + if let sharingButtonStyle { settings.sharingButtonStyle = sharingButtonStyle } + if let sharingLabel { settings.sharingLabel = sharingLabel } + if let sharingTwitterName { settings.sharingTwitterName = sharingTwitterName } + if let sharingCommentLikesEnabled { + settings.sharingCommentLikesEnabled = sharingCommentLikesEnabled.boolValue + } + if let sharingDisabledLikes { + settings.sharingDisabledLikes = sharingDisabledLikes.boolValue + } + if let sharingDisabledReblogs { + settings.sharingDisabledReblogs = sharingDisabledReblogs.boolValue + } + if let iconMediaID { settings.iconMediaID = iconMediaID } + } +} diff --git a/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController+SiteIcon.swift b/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController+SiteIcon.swift index 67ee5c233ed8..7702ec0e1e99 100644 --- a/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController+SiteIcon.swift +++ b/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController+SiteIcon.swift @@ -90,7 +90,7 @@ extension HomeSiteHeaderViewController { func removeSiteIcon() { blogDetailHeaderView.updatingIcon = true blog.settings?.iconMediaID = NSNumber(value: 0) - updateBlogSettingsAndRefreshIcon() + updateBlogSettingsAndRefreshIcon(iconURL: nil) WPAnalytics.track(.siteSettingsSiteIconRemoved) } @@ -101,17 +101,48 @@ extension HomeSiteHeaderViewController { func updateBlogIconWithMedia(_ media: Media) { blog.settings?.iconMediaID = media.mediaID - updateBlogSettingsAndRefreshIcon() + updateBlogSettingsAndRefreshIcon(iconURL: media.remoteURL) } - func updateBlogSettingsAndRefreshIcon() { - blogService.updateSettings(for: blog, success: { [weak self] in + /// - Parameter iconURL: the icon's image URL to reflect in the header once + /// the write is acknowledged, or `nil` when the icon is removed. The + /// header renders from `blog.icon`, which is otherwise only refreshed by + /// `syncBlog`; that has no Core REST path, so we set it from this + /// authoritative value to keep the header correct on every transport. + func updateBlogSettingsAndRefreshIcon(iconURL: String?) { + let changes = BlogSettingsChanges() + changes.iconMediaID = blog.settings?.iconMediaID + let blogID = blog.objectID + blogService.updateSettings(for: blog, changes: changes, success: { [weak self] in guard let self else { return } - self.blogService.syncBlog(self.blog, success: { - self.blogDetailHeaderView.updatingIcon = false - self.blogDetailHeaderView.refreshIconImage() + self.blog.icon = iconURL + // Persist just this blog's icon URL so it survives a context reset + // or relaunch. Core REST has no `syncBlog` to repair `blog.icon` + // from the stored `iconMediaID`, and the settings write only saves + // the setting itself. Scoped to the icon field on a background + // context so no unrelated pending main-context edits are saved. + ContextManager.shared.performAndSave( + { context in + guard let blog = (try? context.existingObject(with: blogID)) as? Blog else { + return + } + blog.icon = iconURL + }, + completion: nil, + on: .main + ) + // Clear the loading state as soon as the write is acknowledged. + // `syncBlog` cannot gate this: it has no Core REST implementation + // and never invokes its callbacks for application-password sites, + // which would otherwise leave the icon spinner stuck forever. + self.blogDetailHeaderView.updatingIcon = false + self.blogDetailHeaderView.refreshIconImage() + // Best-effort metadata refresh so transports that support blog sync + // (WP.com REST, XML-RPC) reconcile `blog.icon` with the server. + self.blogService.syncBlog(self.blog, success: { [weak self] in + self?.blogDetailHeaderView.refreshIconImage() }, failure: { _ in }) }, failure: { [weak self] _ in self?.showErrorForSiteIconUpdate() diff --git a/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController.swift b/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController.swift index 7198fa3c1874..0fa4b25d7738 100644 --- a/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController.swift +++ b/WordPress/Classes/ViewRelated/Blog/My Site/Header/HomeSiteHeaderViewController.swift @@ -210,7 +210,9 @@ extension HomeSiteHeaderViewController { blog.settings?.name = title blogDetailHeaderView.setTitleLoading(true) - blogService.updateSettings(for: blog, success: { [weak self] in + let changes = BlogSettingsChanges() + changes.name = title + blogService.updateSettings(for: blog, changes: changes, success: { [weak self] in let notice = Notice(title: title, message: SiteTitleStrings.titleChangeSuccessfulMessage, diff --git a/WordPress/Classes/ViewRelated/Blog/Sharing/SharingButtonsViewController.swift b/WordPress/Classes/ViewRelated/Blog/Sharing/SharingButtonsViewController.swift index bf85f7c25f0a..41cfce214b7b 100644 --- a/WordPress/Classes/ViewRelated/Blog/Sharing/SharingButtonsViewController.swift +++ b/WordPress/Classes/ViewRelated/Blog/Sharing/SharingButtonsViewController.swift @@ -208,7 +208,9 @@ class SharingButtonsViewController: UITableViewController { switchCell.onChange = { newValue in self.blog.settings!.sharingDisabledReblogs = !newValue self.didMakeChanges = true - self.saveBlogSettingsChanges(false) + let changes = BlogSettingsChanges() + changes.sharingDisabledReblogs = NSNumber(value: !newValue) + self.saveBlogSettingsChanges(false, changes: changes) let properties = [ "checked": NSNumber(value: newValue) @@ -231,7 +233,9 @@ class SharingButtonsViewController: UITableViewController { switchCell.onChange = { newValue in self.blog.settings!.sharingDisabledLikes = !newValue self.didMakeChanges = true - self.saveBlogSettingsChanges(false) + let changes = BlogSettingsChanges() + changes.sharingDisabledLikes = NSNumber(value: !newValue) + self.saveBlogSettingsChanges(false, changes: changes) } } } @@ -258,7 +262,9 @@ class SharingButtonsViewController: UITableViewController { switchCell.onChange = { newValue in self.blog.settings!.sharingCommentLikesEnabled = newValue self.didMakeChanges = true - self.saveBlogSettingsChanges(false) + let changes = BlogSettingsChanges() + changes.sharingCommentLikesEnabled = NSNumber(value: newValue) + self.saveBlogSettingsChanges(false, changes: changes) } } } @@ -528,7 +534,7 @@ class SharingButtonsViewController: UITableViewController { /// /// - Parameter refresh: True if the tableview should be reloaded. /// - private func saveBlogSettingsChanges(_ refresh: Bool) { + private func saveBlogSettingsChanges(_ refresh: Bool, changes: BlogSettingsChanges) { if refresh { tableView.reloadData() } @@ -537,6 +543,7 @@ class SharingButtonsViewController: UITableViewController { let dotComID = blog.dotComID service.updateSettings( for: self.blog, + changes: changes, success: { WPAppAnalytics.track(.sharingButtonSettingsChanged, blogID: dotComID) }, @@ -701,7 +708,9 @@ class SharingButtonsViewController: UITableViewController { } WPAnalytics.track(.sharingButtonsLabelChanged, properties: [:], blog: blog) self.blog.settings!.sharingLabel = value - self.saveBlogSettingsChanges(true) + let changes = BlogSettingsChanges() + changes.sharingLabel = value + self.saveBlogSettingsChanges(true, changes: changes) } navigationController?.pushViewController(controller, animated: true) @@ -736,7 +745,9 @@ class SharingButtonsViewController: UITableViewController { } self.blog.settings!.sharingButtonStyle = str - self.saveBlogSettingsChanges(true) + let changes = BlogSettingsChanges() + changes.sharingButtonStyle = str + self.saveBlogSettingsChanges(true, changes: changes) } } navigationController?.pushViewController(controller!, animated: true) @@ -761,7 +772,9 @@ class SharingButtonsViewController: UITableViewController { var str = NSString(string: value) str = str.replacingOccurrences(of: "@", with: "") as NSString self.blog.settings!.sharingTwitterName = str as String - self.saveBlogSettingsChanges(true) + let changes = BlogSettingsChanges() + changes.sharingTwitterName = str as String + self.saveBlogSettingsChanges(true, changes: changes) } navigationController?.pushViewController(controller, animated: true) diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/DateAndTimeFormatSettingsViewController.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/DateAndTimeFormatSettingsViewController.swift index 4e22633100dd..eaa164aae388 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/DateAndTimeFormatSettingsViewController.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/DateAndTimeFormatSettingsViewController.swift @@ -135,7 +135,9 @@ open class DateAndTimeFormatSettingsViewController: UITableViewController { settingsViewController.onItemSelected = { [weak self] (selected: Any?) in if let newDateFormat = selected as? String { self?.settings.dateFormat = newDateFormat - self?.saveSettings() + let changes = BlogSettingsChanges() + changes.dateFormat = newDateFormat + self?.saveSettings(changes) WPAnalytics.trackSettingsChange("date_format", fieldName: "date_format") } } @@ -169,7 +171,9 @@ open class DateAndTimeFormatSettingsViewController: UITableViewController { settingsViewController.onItemSelected = { [weak self] (selected: Any?) in if let newTimeFormat = selected as? String { self?.settings.timeFormat = newTimeFormat - self?.saveSettings() + let changes = BlogSettingsChanges() + changes.timeFormat = newTimeFormat + self?.saveSettings(changes) WPAnalytics.trackSettingsChange("date_format", fieldName: "time_format") } } @@ -189,7 +193,9 @@ open class DateAndTimeFormatSettingsViewController: UITableViewController { settingsViewController.onItemSelected = { [weak self] (selected: Any?) in if let newStartOfWeek = selected as? String { self?.settings.startOfWeek = newStartOfWeek - self?.saveSettings() + let changes = BlogSettingsChanges() + changes.startOfWeek = newStartOfWeek + self?.saveSettings(changes) WPAnalytics.trackSettingsChange("date_format", fieldName: "start_of_week", value: newStartOfWeek as Any) @@ -218,8 +224,8 @@ open class DateAndTimeFormatSettingsViewController: UITableViewController { // MARK: - Persistance - fileprivate func saveSettings() { - service.updateSettings(for: blog, + fileprivate func saveSettings(_ changes: BlogSettingsChanges) { + service.updateSettings(for: blog, changes: changes, success: { SiteStatsInformation.sharedInstance.updateTimeZone() }, failure: { [weak self] (error: Error) -> Void in self?.refreshSettings() diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift index 9ae3783d7e79..a8cdad6ef7d2 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift @@ -11,6 +11,7 @@ open class DiscussionSettingsViewController: UITableViewController { private let tracksDiscussionSettingsKey = "site_settings_discussion" private var isChangingSettings = false private var isSettingsChangeNeeded = false + private var pendingChanges = BlogSettingsChanges() // MARK: - Initializers / Deinitializers @objc public convenience init(blog: Blog) { @@ -77,9 +78,12 @@ open class DiscussionSettingsViewController: UITableViewController { isSettingsChangeNeeded = false navigationItem.rightBarButtonItem = .activityIndicator + let changes = pendingChanges + pendingChanges = BlogSettingsChanges() let service = BlogService(coreDataStack: ContextManager.shared) service.updateSettings( for: blog, + changes: changes, success: { [weak self] in self?.didFinishChangingSettings(nil) }, @@ -190,40 +194,45 @@ open class DiscussionSettingsViewController: UITableViewController { guard let enabled = payload as? Bool else { return } - didChangeSetting("allow_comments", value: enabled as Any) settings.commentsAllowed = NSNumber(value: enabled) + pendingChanges.commentsAllowed = NSNumber(value: enabled) + didChangeSetting("allow_comments", value: enabled as Any) } private func pressedPingbacksInbound(_ payload: AnyObject?) { guard let enabled = payload as? Bool else { return } - didChangeSetting("receive_pingbacks", value: enabled as Any) settings.pingbackInboundEnabled = NSNumber(value: enabled) + pendingChanges.pingbackInboundEnabled = NSNumber(value: enabled) + didChangeSetting("receive_pingbacks", value: enabled as Any) } private func pressedPingbacksOutbound(_ payload: AnyObject?) { guard let enabled = payload as? Bool else { return } - didChangeSetting("send_pingbacks", value: enabled as Any) settings.pingbackOutboundEnabled = enabled + pendingChanges.pingbackOutboundEnabled = NSNumber(value: enabled) + didChangeSetting("send_pingbacks", value: enabled as Any) } private func pressedRequireNameAndEmail(_ payload: AnyObject?) { guard let enabled = payload as? Bool else { return } - didChangeSetting("require_name_and_email", value: enabled as Any) settings.commentsRequireNameAndEmail = enabled + pendingChanges.commentsRequireNameAndEmail = NSNumber(value: enabled) + didChangeSetting("require_name_and_email", value: enabled as Any) } private func pressedRequireRegistration(_ payload: AnyObject?) { guard let enabled = payload as? Bool else { return } - didChangeSetting("require_registration", value: enabled as Any) settings.commentsRequireRegistration = enabled + pendingChanges.commentsRequireRegistration = NSNumber(value: enabled) + didChangeSetting("require_registration", value: enabled as Any) } private func pressedCloseCommenting(_ payload: AnyObject?) { @@ -248,6 +257,8 @@ open class DiscussionSettingsViewController: UITableViewController { pickerViewController.onChange = { [weak self] (enabled: Bool, newValue: Int) in self?.settings.commentsCloseAutomatically = enabled self?.settings.commentsCloseAutomaticallyAfterDays = newValue as NSNumber + self?.pendingChanges.commentsCloseAutomatically = NSNumber(value: enabled) + self?.pendingChanges.commentsCloseAutomaticallyAfterDays = newValue as NSNumber let value: Any = enabled ? newValue : "disabled" self?.didChangeSetting("close_commenting", value: value) @@ -265,8 +276,11 @@ open class DiscussionSettingsViewController: UITableViewController { guard let newSortOrder = CommentsSorting(rawValue: selected as! Int) else { return } - self?.didChangeSetting("comments_sort_by", value: selected as Any) self?.settings.commentsSorting = newSortOrder + if let self { + self.pendingChanges.commentsSortOrderAscending = NSNumber(value: self.settings.commentsSortOrderAscending) + } + self?.didChangeSetting("comments_sort_by", value: selected as Any) } navigationController?.pushViewController(settingsViewController, animated: true) } @@ -282,6 +296,10 @@ open class DiscussionSettingsViewController: UITableViewController { return } self?.settings.commentsThreading = newThreadingDepth + if let self { + self.pendingChanges.commentsThreadingEnabled = NSNumber(value: self.settings.commentsThreadingEnabled) + self.pendingChanges.commentsThreadingDepth = self.settings.commentsThreadingDepth + } self?.didChangeSetting("comments_threading", value: selected as Any) } navigationController?.pushViewController(settingsViewController, animated: true) @@ -304,6 +322,8 @@ open class DiscussionSettingsViewController: UITableViewController { pickerViewController.onChange = { [weak self] (enabled: Bool, newValue: Int) in self?.settings.commentsPagingEnabled = enabled self?.settings.commentsPageSize = newValue as NSNumber + self?.pendingChanges.commentsPagingEnabled = NSNumber(value: enabled) + self?.pendingChanges.commentsPageSize = newValue as NSNumber let value: Any = enabled ? newValue : "disabled" self?.didChangeSetting("comments_paging", value: value) @@ -323,6 +343,10 @@ open class DiscussionSettingsViewController: UITableViewController { return } self?.settings.commentsAutoapproval = newApprovalStatus + if let self { + self.pendingChanges.commentsRequireManualModeration = NSNumber(value: self.settings.commentsRequireManualModeration) + self.pendingChanges.commentsFromKnownUsersAllowlisted = NSNumber(value: self.settings.commentsFromKnownUsersAllowlisted) + } self?.didChangeSetting("comments_automatically_approve", value: selected as Any) } navigationController?.pushViewController(settingsViewController, animated: true) @@ -342,6 +366,7 @@ open class DiscussionSettingsViewController: UITableViewController { pickerViewController.pickerSelectedValue = settings.commentsMaximumLinks as? Int pickerViewController.onChange = { [weak self] (_: Bool, newValue: Int) in self?.settings.commentsMaximumLinks = newValue as NSNumber + self?.pendingChanges.commentsMaximumLinks = newValue as NSNumber self?.didChangeSetting("comments_links", value: newValue as Any) } navigationController?.pushViewController(pickerViewController, animated: true) @@ -365,6 +390,7 @@ open class DiscussionSettingsViewController: UITableViewController { ) settingsViewController.onChange = { [weak self] (updated: Set) in self?.settings.commentsModerationKeys = updated + self?.pendingChanges.commentsModerationKeys = updated.joined(separator: "\n") self?.didChangeSetting("comments_hold_for_moderation", value: updated.count as Any) } navigationController?.pushViewController(settingsViewController, animated: true) @@ -388,6 +414,7 @@ open class DiscussionSettingsViewController: UITableViewController { ) settingsViewController.onChange = { [weak self] (updated: Set) in self?.settings.commentsBlocklistKeys = updated + self?.pendingChanges.commentsBlocklistKeys = updated.joined(separator: "\n") self?.didChangeSetting("comments_block_list", value: updated.count as Any) } navigationController?.pushViewController(settingsViewController, animated: true) diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsRelatedPostsView.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsRelatedPostsView.swift index 7621b0fe8ae6..4443c4c2e5e8 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsRelatedPostsView.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsRelatedPostsView.swift @@ -27,13 +27,19 @@ struct RelatedPostsSettingsView: View { } .toggleStyle(SwitchToggleStyle(tint: Color(UIAppColor.jetpackGreen))) .onChange(of: settings.relatedPostsEnabled) { - save(field: "show_related_posts", value: $1) + let changes = BlogSettingsChanges() + changes.relatedPostsEnabled = NSNumber(value: $1) + save(field: "show_related_posts", value: $1, changes: changes) } .onChange(of: settings.relatedPostsShowHeadline) { - save(field: "show_related_posts_header", value: $1) + let changes = BlogSettingsChanges() + changes.relatedPostsShowHeadline = NSNumber(value: $1) + save(field: "show_related_posts_header", value: $1, changes: changes) } .onChange(of: settings.relatedPostsShowThumbnails) { - save(field: "show_related_posts_thumbnail", value: $1) + let changes = BlogSettingsChanges() + changes.relatedPostsShowThumbnails = NSNumber(value: $1) + save(field: "show_related_posts_thumbnail", value: $1, changes: changes) } .navigationTitle(Strings.title) .navigationBarTitleDisplayMode(.inline) @@ -98,10 +104,10 @@ struct RelatedPostsSettingsView: View { } } - private func save(field: String, value: Any) { + private func save(field: String, value: Any, changes: BlogSettingsChanges) { WPAnalytics.trackSettingsChange("related_posts", fieldName: field, value: value) isSaving = true - BlogService(coreDataStack: ContextManager.shared).updateSettings(for: blog, success: { + BlogService(coreDataStack: ContextManager.shared).updateSettings(for: blog, changes: changes, success: { isSaving = false }, failure: { _ in isSaving = false diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController+Swift.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController+Swift.swift index 906d650d6265..01b0d7537906 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController+Swift.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController+Swift.swift @@ -28,7 +28,9 @@ extension SiteSettingsViewController { let view = SiteSettingsPrivacyPicker(blog: blog, selection: blog.siteVisibility) { [weak self] in guard let self, self.blog.siteVisibility != $0 else { return } self.blog.siteVisibility = $0 - self.saveSettings() + let changes = BlogSettingsChanges() + changes.privacy = NSNumber(value: $0.rawValue) + self.saveSettings(with: changes) self.trackSettingsChange(fieldName: "site_settings", value: $0.rawValue) } let viewController = UIHostingController(rootView: view) @@ -151,7 +153,10 @@ extension SiteSettingsViewController { let view = TimeZoneSelectorView(selectedValue: timezoneValue) { [weak self] newValue in self?.blog.settings?.gmtOffset = newValue.gmtOffset as NSNumber? self?.blog.settings?.timezoneString = newValue.timezoneString - self?.saveSettings() + let changes = BlogSettingsChanges() + changes.gmtOffset = newValue.gmtOffset as NSNumber? + changes.timezoneString = newValue.timezoneString + self?.saveSettings(with: changes) self? .trackSettingsChange( fieldName: "timezone", @@ -185,7 +190,9 @@ extension SiteSettingsViewController { } pickerViewController.onChange = { [weak self] (_: Bool, newValue: Int) in self?.blog.settings?.postsPerPage = newValue as NSNumber? - self?.saveSettings() + let changes = BlogSettingsChanges() + changes.postsPerPage = newValue as NSNumber + self?.saveSettings(with: changes) self?.trackSettingsChange(fieldName: "posts_per_page", value: newValue as Any) } @@ -456,7 +463,9 @@ extension SiteSettingsViewController { if value != self.blog.settings?.name { self.blog.settings?.name = value - self.saveSettings() + let changes = BlogSettingsChanges() + changes.name = value + self.saveSettings(with: changes) self.trackSettingsChange(fieldName: "site_title") } @@ -499,7 +508,9 @@ extension SiteSettingsViewController { if normalizedTagline != self.blog.settings?.tagline { self.blog.settings?.tagline = normalizedTagline - self.saveSettings() + let changes = BlogSettingsChanges() + changes.tagline = normalizedTagline + self.saveSettings(with: changes) self.trackSettingsChange(fieldName: "tagline") } diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.h b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.h index f4208145cf7e..55c54be2ad42 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.h +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.h @@ -2,6 +2,7 @@ @class Blog; @class SettingTableViewCell; +@class BlogSettingsChanges; typedef NS_ENUM(NSInteger, SiteSettingsSection) { SiteSettingsSectionGeneral = 0, @@ -24,7 +25,7 @@ typedef NS_ENUM(NSInteger, SiteSettingsSection) { - (instancetype)initWithBlog:(Blog *)blog; -- (void)saveSettings; +- (void)saveSettingsWithChanges:(BlogSettingsChanges *)changes; // General Settings: These were made available here to help with the transition to Swift. diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.m b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.m index 6ab90dce3dfb..1d7856aab4de 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.m +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteSettingsViewController.m @@ -500,7 +500,9 @@ - (SwitchTableViewCell *)ampSettingCell __weak __typeof__(self) weakSelf = self; _ampSettingCell.onChange = ^(BOOL value){ weakSelf.blog.settings.ampEnabled = value; - [weakSelf saveSettings]; + BlogSettingsChanges *changes = [BlogSettingsChanges new]; + changes.ampEnabled = @(value); + [weakSelf saveSettingsWithChanges:changes]; [WPAnalytics trackSettingsChange:@"site_settings" fieldName:@"amp_enabled" value:@(value)]; }; @@ -823,7 +825,9 @@ - (void)showLanguageSelectorForBlog:(Blog *)blog LanguageViewController *languageViewController = [[LanguageViewController alloc] initWithBlog:blog]; languageViewController.onChange = ^(NSNumber *newLanguageID){ weakSelf.blog.settings.languageID = newLanguageID; - [weakSelf saveSettings]; + BlogSettingsChanges *changes = [BlogSettingsChanges new]; + changes.languageID = newLanguageID; + [weakSelf saveSettingsWithChanges:changes]; [WPAnalytics trackSettingsChange:@"site_settings" fieldName:@"language" value:newLanguageID]; }; @@ -896,7 +900,9 @@ - (void)showPostFormatSelector if ([weakSelf savingWritingDefaultsIsAvailable]) { [WPAnalytics trackSettingsChange:@"site_settings" fieldName:@"default_post_format"]; - [weakSelf saveSettings]; + BlogSettingsChanges *changes = [BlogSettingsChanges new]; + changes.defaultPostFormat = status; + [weakSelf saveSettingsWithChanges:changes]; } } } @@ -1119,15 +1125,15 @@ - (NSString *)getTagsCountPresentableString:(NSInteger)tagCount #pragma mark - Saving methods -- (void)saveSettings +- (void)saveSettingsWithChanges:(BlogSettingsChanges *)changes { - if (!self.blog.settings.hasChanges) { + if (changes.isEmpty) { return; } [self showActivityIndicator]; BlogService *blogService = [[BlogService alloc] initWithCoreDataStack:[ContextManager sharedInstance]]; - [blogService updateSettingsForBlog:self.blog success:^{ + [blogService updateSettingsForBlog:self.blog changes:changes success:^{ [self hideActivityIndicator]; [NSNotificationCenter.defaultCenter postNotificationName:WPBlogSettingsUpdatedNotification object:nil]; } failure:^(NSError *error) { @@ -1216,7 +1222,9 @@ - (void)postCategoriesViewController:(PostCategoriesViewController *)controller [WPAnalytics trackSettingsChange:@"site_settings" fieldName:@"default_category"]; - [self saveSettings]; + BlogSettingsChanges *changes = [BlogSettingsChanges new]; + changes.defaultCategoryID = category.categoryID; + [self saveSettingsWithChanges:changes]; } }