Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Global Styles endpoint to support non-experimental paths #16823

Merged
merged 7 commits into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def wordpress_ui
end

def wordpress_kit
pod 'WordPressKit', '~> 4.37.0'
pod 'WordPressKit', '~> 4.38.0-beta'
# pod 'WordPressKit', :git => 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', :tag => ''
# pod 'WordPressKit', :git => 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', :branch => ''
# pod 'WordPressKit', :git => 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', :commit => ''
Expand Down
10 changes: 5 additions & 5 deletions Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ PODS:
- WordPressKit (~> 4.18-beta)
- WordPressShared (~> 1.12-beta)
- WordPressUI (~> 1.7-beta)
- WordPressKit (4.37.0):
- WordPressKit (4.38.0-beta.1):
- Alamofire (~> 4.8.0)
- CocoaLumberjack (~> 3.4)
- NSObject-SafeExpectations (= 0.0.4)
Expand Down Expand Up @@ -553,7 +553,7 @@ DEPENDENCIES:
- SVProgressHUD (= 2.2.5)
- WordPress-Editor-iOS (~> 1.19.4)
- WordPressAuthenticator (~> 1.39.0)
- WordPressKit (~> 4.37.0)
- WordPressKit (~> 4.38.0-beta)
- WordPressMocks (~> 0.0.13)
- WordPressShared (~> 1.16.0)
- WordPressUI (~> 1.12.1)
Expand All @@ -565,7 +565,6 @@ DEPENDENCIES:
SPEC REPOS:
https://github.com/wordpress-mobile/cocoapods-specs.git:
- WordPressAuthenticator
- WordPressKit
trunk:
- 1PasswordExtension
- Alamofire
Expand Down Expand Up @@ -605,6 +604,7 @@ SPEC REPOS:
- UIDeviceIdentifier
- WordPress-Aztec-iOS
- WordPress-Editor-iOS
- WordPressKit
- WordPressMocks
- WordPressShared
- WordPressUI
Expand Down Expand Up @@ -810,7 +810,7 @@ SPEC CHECKSUMS:
WordPress-Aztec-iOS: 870c93297849072aadfc2223e284094e73023e82
WordPress-Editor-iOS: 068b32d02870464ff3cb9e3172e74234e13ed88c
WordPressAuthenticator: c50b9737d6f459b1010bd07daa9885d19505c504
WordPressKit: a14d76dc405d87a9598b560a870bf455673cea4f
WordPressKit: ce775ba520533744bd576bd895296e0e13ef0119
WordPressMocks: dfac50a938ac74dddf5f7cce5a9110126408dd19
WordPressShared: 5477f179c7fe03b5d574f91adda66f67d131827e
WordPressUI: 414bf3a7d007618f94a1c7969d6e849779877d5d
Expand All @@ -826,6 +826,6 @@ SPEC CHECKSUMS:
ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba
ZIPFoundation: e27423c004a5a1410c15933407747374e7c6cb6e

PODFILE CHECKSUM: 52c2af0a42adb83ba873ef477a56852a521f74d9
PODFILE CHECKSUM: 5385720b9e21e3e8abe3a3d654d65fc2b18f7c97

COCOAPODS: 1.10.1
96 changes: 63 additions & 33 deletions WordPress/Classes/Services/BlockEditorSettingsService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,16 @@ import Foundation
import WordPressKit

class BlockEditorSettingsService {
typealias BlockEditorSettingsServiceCompletion = (_ hasChanges: Bool, _ blockEditorSettings: BlockEditorSettings?) -> Void
struct SettingsServiceResult {
let hasChanges: Bool
let blockEditorSettings: BlockEditorSettings?
}

enum BlockEditorSettingsServiceError: Int, Error {
case blogNotFound
}

typealias BlockEditorSettingsServiceCompletion = (Swift.Result<SettingsServiceResult, Error>) -> Void

let blog: Blog
let remote: BlockEditorSettingsServiceRemote
Expand Down Expand Up @@ -54,8 +63,9 @@ private extension BlockEditorSettingsService {
let originalChecksum = self.blog.blockEditorSettings?.checksum ?? ""
self.updateEditorThemeCache(originalChecksum: originalChecksum, editorTheme: editorTheme, completion: completion)
}
case .failure(let error):
DDLogError("Error loading active theme: \(error)")
case .failure(let err):
DDLogError("Error loading active theme: \(err)")
completion(.failure(err))
}
}
}
Expand All @@ -64,7 +74,8 @@ private extension BlockEditorSettingsService {
let newChecksum = editorTheme?.checksum ?? ""
guard originalChecksum != newChecksum else {
/// The fetched Editor Theme is the same as the cached one so respond with no new changes.
completion(false, self.blog.blockEditorSettings)
let result = SettingsServiceResult(hasChanges: false, blockEditorSettings: self.blog.blockEditorSettings)
completion(.success(result))
return
}

Expand All @@ -76,27 +87,29 @@ private extension BlockEditorSettingsService {

/// The fetched Editor Theme is different than the cached one so persist the new one and delete the old one.
context.perform {
self.persistEditorThemeToCoreData(blogID: self.blog.objectID, editorTheme: editorTheme) { success in
guard success else {
completion(false, nil)
return
}

self.context.perform {
completion(true, self.blog.blockEditorSettings)
self.persistEditorThemeToCoreData(blogID: self.blog.objectID, editorTheme: editorTheme) { callback in
switch callback {
case .success:
self.context.perform {
let result = SettingsServiceResult(hasChanges: true, blockEditorSettings: self.blog.blockEditorSettings)
completion(.success(result))
}
case .failure(let err):
completion(.failure(err))
}
}
}
}

func persistEditorThemeToCoreData(blogID: NSManagedObjectID, editorTheme: RemoteEditorTheme, completion: @escaping (_ success: Bool) -> Void) {
func persistEditorThemeToCoreData(blogID: NSManagedObjectID, editorTheme: RemoteEditorTheme, completion: @escaping (Swift.Result<Void, Error>) -> Void) {
let parsingContext = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType)
parsingContext.parent = context
parsingContext.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump

parsingContext.perform {
guard let blog = parsingContext.object(with: blogID) as? Blog else {
completion(false)
let err = BlockEditorSettingsServiceError.blogNotFound
completion(.failure(err))
return
}

Expand All @@ -106,25 +119,33 @@ private extension BlockEditorSettingsService {
}

blog.blockEditorSettings = BlockEditorSettings(editorTheme: editorTheme, context: parsingContext)
try? parsingContext.save()
completion(true)
do {
try parsingContext.save()
} catch let err {
completion(.failure(err))
}

completion(.success(()))
}
}
}

// MARK: Editor Global Styles support
private extension BlockEditorSettingsService {
func fetchBlockEditorSettings(_ completion: @escaping BlockEditorSettingsServiceCompletion) {
remote.fetchBlockEditorSettings { [weak self] (response) in
remote.fetchBlockEditorSettings(forSiteID: blog.dotComID?.intValue) { [weak self] (response) in
guard let `self` = self else { return }
switch response {
case .success(let remoteSettings):
self.context.perform {
let originalChecksum = self.blog.blockEditorSettings?.checksum ?? ""
self.updateBlockEditorSettingsCache(originalChecksum: originalChecksum, remoteSettings: remoteSettings, completion: completion)
}
case .failure(let error):
DDLogError("Error loading Block Editor Settings: \(error)")
case .failure(let err):
DDLogError("Error fetching editor settings: \(err)")
// The user may not have the gutenberg plugin installed so try /wp/v2/themes to maintain feature support.
// In WP 5.9 we may be able to skip this attempt.
self.fetchTheme(completion)
Copy link

Choose a reason for hiding this comment

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

Retrying on error should be fine. I wonder if we can add an extra check for the specific error type (404).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing specific here but I wasn't sure of about all of the ways this could fail so I decided to play it safe.

}
}
}
Expand All @@ -133,7 +154,8 @@ private extension BlockEditorSettingsService {
let newChecksum = remoteSettings?.checksum ?? ""
guard originalChecksum != newChecksum else {
/// The fetched Block Editor Settings is the same as the cached one so respond with no new changes.
completion(false, self.blog.blockEditorSettings)
let result = SettingsServiceResult(hasChanges: false, blockEditorSettings: self.blog.blockEditorSettings)
completion(.success(result))
return
}

Expand All @@ -145,27 +167,29 @@ private extension BlockEditorSettingsService {

/// The fetched Block Editor Settings is different than the cached one so persist the new one and delete the old one.
context.perform {
self.persistBlockEditorSettingsToCoreData(blogID: self.blog.objectID, remoteSettings: remoteSettings) { success in
guard success else {
completion(false, nil)
return
}

self.context.perform {
completion(true, self.blog.blockEditorSettings)
self.persistBlockEditorSettingsToCoreData(blogID: self.blog.objectID, remoteSettings: remoteSettings) { callback in
switch callback {
case .success:
self.context.perform {
let result = SettingsServiceResult(hasChanges: true, blockEditorSettings: self.blog.blockEditorSettings)
completion(.success(result))
}
case .failure(let err):
completion(.failure(err))
}
}
}
}

func persistBlockEditorSettingsToCoreData(blogID: NSManagedObjectID, remoteSettings: RemoteBlockEditorSettings, completion: @escaping (_ success: Bool) -> Void) {
func persistBlockEditorSettingsToCoreData(blogID: NSManagedObjectID, remoteSettings: RemoteBlockEditorSettings, completion: @escaping (Swift.Result<Void, Error>) -> Void) {
let parsingContext = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType)
parsingContext.parent = context
parsingContext.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump

parsingContext.perform {
guard let blog = parsingContext.object(with: blogID) as? Blog else {
completion(false)
let err = BlockEditorSettingsServiceError.blogNotFound
completion(.failure(err))
return
}

Expand All @@ -175,8 +199,13 @@ private extension BlockEditorSettingsService {
}

blog.blockEditorSettings = BlockEditorSettings(remoteSettings: remoteSettings, context: parsingContext)
try? parsingContext.save()
completion(true)
do {
try parsingContext.save()
} catch let err {
completion(.failure(err))
}

completion(.success(()))
}
}
}
Expand All @@ -189,7 +218,8 @@ private extension BlockEditorSettingsService {
// Block Editor Settings nullify on delete
self.context.delete(blockEditorSettings)
}
completion(true, nil)
let result = SettingsServiceResult(hasChanges: true, blockEditorSettings: nil)
completion(.success(result))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1214,9 +1214,16 @@ extension GutenbergViewController {
}

private func fetchBlockSettings() {
editorSettingsService?.fetchSettings({ [weak self] (hasChanges, settings) in
guard hasChanges, let `self` = self else { return }
self.gutenberg.updateEditorSettings(settings)
editorSettingsService?.fetchSettings({ [weak self] result in
guard let `self` = self else { return }
switch result {
case .success(let response):
if response.hasChanges {
self.gutenberg.updateEditorSettings(response.blockEditorSettings)
}
case .failure(let err):
DDLogError("Error fetching settings: \(err)")
}
})
}
}
Loading