diff --git a/Tests/KeystoneTests/Tests/Services/BloggingPromptsServiceTests.swift b/Tests/KeystoneTests/Tests/Services/BloggingPromptsServiceTests.swift index 85a00717394a..23ed9ea1ae97 100644 --- a/Tests/KeystoneTests/Tests/Services/BloggingPromptsServiceTests.swift +++ b/Tests/KeystoneTests/Tests/Services/BloggingPromptsServiceTests.swift @@ -153,8 +153,9 @@ final class BloggingPromptsServiceTests: CoreDataTestCase { // call the fetch just to trigger default parameter assignment. the completion blocks can be ignored. service.fetchPrompts(from: expectedDate, number: 10, success: { _ in }, failure: { _ in }) - let date = try passedDate() - XCTAssertEqual("2022-01-02", try passedDate()) + // `force_year` is now always the current year, so the reconstructed date carries it. + let currentYear = Calendar(identifier: .gregorian).component(.year, from: Date()) + XCTAssertEqual("\(currentYear)-01-02", try passedDate()) } // MARK: - Upsert Tests @@ -341,7 +342,7 @@ private extension BloggingPromptsServiceTests { } func makeBlog() -> Blog { - return BlogBuilder(mainContext).isHostedAtWPcom().with(blogID: siteID).build() + BlogBuilder(mainContext).isHostedAtWPcom().with(blogID: siteID).build() } func stubFetchPromptsResponse(with fileName: String? = nil) { @@ -369,7 +370,7 @@ private extension BloggingPromptsServiceTests { } func passedNumber() throws -> Int { - return try XCTUnwrap(passedParameter("per_page") as? Int) + try XCTUnwrap(passedParameter("per_page") as? Int) } func passedDate() throws -> String { @@ -387,8 +388,9 @@ private extension BloggingPromptsServiceTests { return [ String(forcedYear), String(format: "%02d", month), - String(format: "%02d", day), - ].joined(separator: "-") + String(format: "%02d", day) + ] + .joined(separator: "-") } // MARK: Test Prompts @@ -396,8 +398,9 @@ private extension BloggingPromptsServiceTests { private func loadTestPrompts(from fileName: String) -> [BloggingPromptRemoteObject] { let bundle = Bundle(for: BloggingPromptsServiceTests.self) guard let url = bundle.url(forResource: fileName, withExtension: "json"), - let data = try? Data(contentsOf: url), - let prompts = try? Self.jsonDecoder.decode([BloggingPromptRemoteObject].self, from: data) else { + let data = try? Data(contentsOf: url), + let prompts = try? Self.jsonDecoder.decode([BloggingPromptRemoteObject].self, from: data) + else { return [] } return prompts diff --git a/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift b/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift index fea6df46655e..c8bfac522640 100644 --- a/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift +++ b/WordPress/Classes/Services/BloggingPrompts/BloggingPromptsService.swift @@ -54,11 +54,13 @@ class BloggingPromptsService { /// - number: The amount of prompts to return. Defaults to 25 when unspecified (10 days back, today, 14 days ahead). /// - success: Closure to be called when the fetch process succeeded. /// - failure: Closure to be called when the fetch process failed. - func fetchPrompts(from startDate: Date? = nil, - to endDate: Date? = nil, - number: Int = defaultNumberOfResults, - success: (([BloggingPrompt]) -> Void)? = nil, - failure: ((Error?) -> Void)? = nil) { + func fetchPrompts( + from startDate: Date? = nil, + to endDate: Date? = nil, + number: Int = defaultNumberOfResults, + success: (([BloggingPrompt]) -> Void)? = nil, + failure: ((Error?) -> Void)? = nil + ) { let fromDate = startDate ?? defaultStartDate fetchRemotePrompts(number: number, fromDate: fromDate, ignoresYear: true) { result in @@ -83,11 +85,18 @@ class BloggingPromptsService { /// - Parameters: /// - success: Closure to be called when the fetch process succeeded. /// - failure: Closure to be called when the fetch process failed. - func fetchTodaysPrompt(success: ((BloggingPrompt?) -> Void)? = nil, - failure: ((Error?) -> Void)? = nil) { - fetchPrompts(from: Date(), number: 1, success: { prompts in - success?(prompts.first) - }, failure: failure) + func fetchTodaysPrompt( + success: ((BloggingPrompt?) -> Void)? = nil, + failure: ((Error?) -> Void)? = nil + ) { + fetchPrompts( + from: Date(), + number: 1, + success: { prompts in + success?(prompts.first) + }, + failure: failure + ) } /// Convenience method to obtain the blogging prompt for the current day, @@ -96,8 +105,10 @@ class BloggingPromptsService { /// - Parameters: /// - success: Closure to be called when the fetch process succeeded. /// - failure: Closure to be called when the fetch process failed. - func todaysPrompt(success: @escaping (BloggingPrompt?) -> Void, - failure: @escaping (Error?) -> Void) { + func todaysPrompt( + success: @escaping (BloggingPrompt?) -> Void, + failure: @escaping (Error?) -> Void + ) { guard localTodaysPrompt == nil else { success(localTodaysPrompt) return @@ -112,8 +123,10 @@ class BloggingPromptsService { /// - Parameters: /// - success: Closure to be called when the fetch process succeeded. /// - failure: Closure to be called when the fetch process failed. - func fetchListPrompts(success: @escaping ([BloggingPrompt]) -> Void, - failure: @escaping (Error?) -> Void) { + func fetchListPrompts( + success: @escaping ([BloggingPrompt]) -> Void, + failure: @escaping (Error?) -> Void + ) { fetchPrompts(from: listStartDate, to: Date(), number: maxListPrompts, success: success, failure: failure) } @@ -129,7 +142,11 @@ class BloggingPromptsService { } let fetchRequest = BloggingPrompt.fetchRequest() - fetchRequest.predicate = NSPredicate(format: "\(#keyPath(BloggingPrompt.siteID)) = %@ AND \(#keyPath(BloggingPrompt.promptID)) = %@", siteID, NSNumber(value: promptID)) + fetchRequest.predicate = NSPredicate( + format: "\(#keyPath(BloggingPrompt.siteID)) = %@ AND \(#keyPath(BloggingPrompt.promptID)) = %@", + siteID, + NSNumber(value: promptID) + ) fetchRequest.fetchLimit = 1 return (try? self.contextManager.mainContext.fetch(fetchRequest))?.first @@ -142,8 +159,10 @@ class BloggingPromptsService { /// - Parameters: /// - success: Closure to be called on success with an optional `BloggingPromptSettings` object. /// - failure: Closure to be called on failure with an optional `Error` object. - func fetchSettings(success: @escaping (BloggingPromptSettings?) -> Void, - failure: @escaping (Error?) -> Void) { + func fetchSettings( + success: @escaping (BloggingPromptSettings?) -> Void, + failure: @escaping (Error?) -> Void + ) { remote.fetchSettings(for: siteID) { result in switch result { case .success(let remoteSettings): @@ -164,9 +183,11 @@ class BloggingPromptsService { /// - success: Closure to be called on success with an optional `BloggingPromptSettings` object. `nil` is passed /// when the call is successful but there were no updated settings on the remote. /// - failure: Closure to be called on failure with an optional `Error` object. - func updateSettings(settings: RemoteBloggingPromptsSettings, - success: @escaping (BloggingPromptSettings?) -> Void, - failure: @escaping (Error?) -> Void) { + func updateSettings( + settings: RemoteBloggingPromptsSettings, + success: @escaping (BloggingPromptSettings?) -> Void, + failure: @escaping (Error?) -> Void + ) { remote.updateSettings(for: siteID, with: settings) { result in switch result { case .success(let remoteSettings): @@ -196,10 +217,12 @@ class BloggingPromptsService { /// Otherwise, this falls back to the default account's primary blog. /// - api: When supplied, the WordPressComRestApi instance to use to fetch the prompts. /// Otherwise, an default or anonymous instance will be computed based on whether there is an account available. - required init?(contextManager: CoreDataStackSwift = ContextManager.shared, - api: WordPressComRestApi? = nil, - remote: BloggingPromptsServiceRemote? = nil, - blog: Blog? = nil) { + required init?( + contextManager: CoreDataStackSwift = ContextManager.shared, + api: WordPressComRestApi? = nil, + remote: BloggingPromptsServiceRemote? = nil, + blog: Blog? = nil + ) { let blogObjectID = blog?.objectID let (siteID, remoteInstance, api) = contextManager.performQuery { mainContext in // if a blog exists, then try to use the blog's ID. @@ -213,8 +236,11 @@ class BloggingPromptsService { return ( blogInContext?.dotComID, remote, - api ?? WordPressComRestApi.anonymousApi(userAgent: WPUserAgent.wordPress(), - localeKey: WordPressComRestApi.LocaleKeyV2) + api + ?? WordPressComRestApi.anonymousApi( + userAgent: WPUserAgent.wordPress(), + localeKey: WordPressComRestApi.LocaleKeyV2 + ) ) } @@ -226,7 +252,8 @@ class BloggingPromptsService { } guard let siteID, - let remoteInstance else { + let remoteInstance + else { return nil } @@ -251,7 +278,7 @@ class BloggingPromptsServiceFactory { } func makeService(for blog: Blog) -> BloggingPromptsService? { - return .init(contextManager: contextManager, remote: remote, blog: blog) + .init(contextManager: contextManager, remote: remote, blog: blog) } } @@ -297,14 +324,17 @@ private extension BloggingPromptsService { /// - ignoresYear: When set to true, this will convert the date to a custom format that ignores the year part. Defaults to true. /// - forceYear: Forces the year value on the prompt's date to the specified value. Defaults to the current year. /// - completion: A closure that will be called when the fetch request completes. - func fetchRemotePrompts(number: Int? = nil, - fromDate: Date? = nil, - ignoresYear: Bool = true, - forceYear: Int? = nil, - completion: @escaping (Result<[BloggingPromptRemoteObject], Error>) -> Void) { + func fetchRemotePrompts( + number: Int? = nil, + fromDate: Date? = nil, + ignoresYear: Bool = true, + forceYear: Int? = nil, + completion: @escaping (Result<[BloggingPromptRemoteObject], Error>) -> Void + ) { let path = "wpcom/v3/sites/\(siteID)/blogging-prompts" let requestParameter: [String: AnyHashable] = { var params = [String: AnyHashable]() + params["order"] = "desc" if let number, number > 0 { params["per_page"] = number @@ -322,24 +352,27 @@ private extension BloggingPromptsService { params["after"] = dateString } - if let forceYear = forceYear ?? fromDate?.dateAndTimeComponents().year { - params["force_year"] = forceYear - } + params["force_year"] = forceYear ?? Calendar(identifier: .gregorian).component(.year, from: Date()) return params }() - api.GET(path, parameters: requestParameter as [String: AnyObject], success: { responseObject, _ in - do { - let data = try JSONSerialization.data(withJSONObject: responseObject, options: []) - let remotePrompts = try Self.jsonDecoder.decode([BloggingPromptRemoteObject].self, from: data) - completion(.success(remotePrompts)) - } catch { + api.GET( + path, + parameters: requestParameter as [String: AnyObject], + success: { responseObject, _ in + do { + let data = try JSONSerialization.data(withJSONObject: responseObject, options: []) + let remotePrompts = try Self.jsonDecoder.decode([BloggingPromptRemoteObject].self, from: data) + completion(.success(remotePrompts)) + } catch { + completion(.failure(error)) + } + }, + failure: { error, _ in completion(.failure(error)) } - }, failure: { error, _ in - completion(.failure(error)) - }) + ) } /// Loads local prompts based on the given parameters. @@ -357,7 +390,8 @@ private extension BloggingPromptsService { let fetchRequest = BloggingPrompt.fetchRequest() if let utcEndDate = utcDateIgnoringTime(from: endDate) { - let format = "\(#keyPath(BloggingPrompt.siteID)) = %@ AND \(#keyPath(BloggingPrompt.date)) >= %@ AND \(#keyPath(BloggingPrompt.date)) <= %@" + let format = + "\(#keyPath(BloggingPrompt.siteID)) = %@ AND \(#keyPath(BloggingPrompt.date)) >= %@ AND \(#keyPath(BloggingPrompt.date)) <= %@" fetchRequest.predicate = NSPredicate(format: format, siteID, utcStartDate as NSDate, utcEndDate as NSDate) } else { let format = "\(#keyPath(BloggingPrompt.siteID)) = %@ AND \(#keyPath(BloggingPrompt.date)) >= %@" @@ -383,48 +417,56 @@ private extension BloggingPromptsService { // incoming remote prompts should have unique dates. // fetch requests require the date to be `NSDate` specifically, hence the cast. let incomingDates = Set(remotePrompts.map(\.dateString)) - let promptsByDate = remotePrompts.reduce(into: [String: BloggingPromptRemoteObject]()) { partialResult, remotePrompt in + let promptsByDate = remotePrompts.reduce(into: [String: BloggingPromptRemoteObject]()) { + partialResult, + remotePrompt in partialResult[remotePrompt.dateString] = remotePrompt } let fetchRequest = BloggingPrompt.fetchRequest() fetchRequest.predicate = NSPredicate(format: "\(#keyPath(BloggingPrompt.siteID)) = %@", siteID) - contextManager.performAndSave({ derivedContext in - /// Try to overwrite prompts that have the same dates. - /// - /// Perf. notes: since we're at most updating 25 entries, it should be acceptable to update them one by one. - /// However, if requirements change and we need to work through a larger data set, consider switching to - /// a drop-and-replace strategy with `NSBatchDeleteRequest` as it's more performant. - var updatedExistingDates = Set() - - try derivedContext.fetch(fetchRequest).forEach { prompt in - guard let incoming = promptsByDate[prompt.dateString] else { - return - } - - // ensure that there's only one prompt for each date. - // if the prompt with this date has been updated before, then it's a duplicate. Let's delete it. - if updatedExistingDates.contains(prompt.dateString) { - derivedContext.deleteObject(prompt) - return - } - - // otherwise, we can update the prompt matching the date with the incoming prompt. - prompt.configure(with: incoming, for: self.siteID.int32Value) - updatedExistingDates.insert(incoming.dateString) - } + contextManager.performAndSave( + { derivedContext in + /// Try to overwrite prompts that have the same dates. + /// + /// Perf. notes: since we're at most updating 25 entries, it should be acceptable to update them one by one. + /// However, if requirements change and we need to work through a larger data set, consider switching to + /// a drop-and-replace strategy with `NSBatchDeleteRequest` as it's more performant. + var updatedExistingDates = Set() + + try derivedContext.fetch(fetchRequest) + .forEach { prompt in + guard let incoming = promptsByDate[prompt.dateString] else { + return + } + + // ensure that there's only one prompt for each date. + // if the prompt with this date has been updated before, then it's a duplicate. Let's delete it. + if updatedExistingDates.contains(prompt.dateString) { + derivedContext.deleteObject(prompt) + return + } + + // otherwise, we can update the prompt matching the date with the incoming prompt. + prompt.configure(with: incoming, for: self.siteID.int32Value) + updatedExistingDates.insert(incoming.dateString) + } - // process the remaining new prompts. - let datesToInsert = incomingDates.subtracting(updatedExistingDates) - datesToInsert.forEach { date in - guard let incoming = promptsByDate[date], - let newPrompt = BloggingPrompt.newObject(in: derivedContext) else { - return + // process the remaining new prompts. + let datesToInsert = incomingDates.subtracting(updatedExistingDates) + datesToInsert.forEach { date in + guard let incoming = promptsByDate[date], + let newPrompt = BloggingPrompt.newObject(in: derivedContext) + else { + return + } + newPrompt.configure(with: incoming, for: self.siteID.int32Value) } - newPrompt.configure(with: incoming, for: self.siteID.int32Value) - } - }, completion: completion, on: .main) + }, + completion: completion, + on: .main + ) } // MARK: Prompt Settings @@ -435,10 +477,15 @@ private extension BloggingPromptsService { /// - remoteSettings: The blogging prompt settings from the remote. /// - completion: Closure to be called on completion. func saveSettings(_ remoteSettings: RemoteBloggingPromptsSettings, completion: @escaping () -> Void) { - contextManager.performAndSave({ derivedContext in - let settings = self.loadSettings(context: derivedContext) ?? BloggingPromptSettings(context: derivedContext) - settings.configure(with: remoteSettings, siteID: self.siteID.int32Value, context: derivedContext) - }, completion: completion, on: .main) + contextManager.performAndSave( + { derivedContext in + let settings = + self.loadSettings(context: derivedContext) ?? BloggingPromptSettings(context: derivedContext) + settings.configure(with: remoteSettings, siteID: self.siteID.int32Value, context: derivedContext) + }, + completion: completion, + on: .main + ) } private func loadSettings(context: NSManagedObjectContext) -> BloggingPromptSettings? {