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

Weekly Roundup Improvements (Core Data + Localization) #17766

Merged
merged 3 commits into from
Jan 18, 2022
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
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
-----
* [*] Signup: Fixed bug where username selection screen could be pushed twice. [#17624]
* [**] Accessibility: VoiceOver improvements on Activity Log and Schedule Post calendars [#17756, #17761]
* [*] Weekly Roundup: Fix a crash which was preventing weekly roundup notifications from appearing [#17765]

19.0
-----
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import CoreData

/// The main data provider for Weekly Roundup information.
///
Expand Down Expand Up @@ -191,14 +192,12 @@ class WeeklyRoundupDataProvider {
NSSortDescriptor(key: "settings.name", ascending: true, selector: #selector(NSString.localizedCaseInsensitiveCompare(_:)))
]

let controller = NSFetchedResultsController<Blog>(fetchRequest: request, managedObjectContext: context, sectionNameKeyPath: nil, cacheName: nil)
do {
try controller.performFetch()
let result = try context.fetch(request)
return .success(result)
} catch {
return .failure(DataRequestError.siteFetchingError(error))
}

return .success(controller.fetchedObjects ?? [])
}
}

Expand Down Expand Up @@ -377,7 +376,8 @@ class WeeklyRoundupBackgroundTask: BackgroundTask {
}
}

let dataProvider = WeeklyRoundupDataProvider(context: ContextManager.shared.newDerivedContext(), onError: onError)
let context = ContextManager.shared.newDerivedContext()
let dataProvider = WeeklyRoundupDataProvider(context: context, onError: onError)
var siteStats: [Blog: StatsSummaryData]? = nil

let requestData = BlockOperation {
Expand Down Expand Up @@ -422,7 +422,8 @@ class WeeklyRoundupBackgroundTask: BackgroundTask {
views: stats.viewsCount,
comments: stats.commentsCount,
likes: stats.likesCount,
periodEndDate: self.currentRunPeriodEndDate()) { result in
periodEndDate: self.currentRunPeriodEndDate(),
context: context) { result in

switch result {
case .success:
Expand Down Expand Up @@ -502,8 +503,8 @@ class WeeklyRoundupNotificationScheduler {
}

func scheduleStaticNotification(completion: @escaping (Result<Void, Error>) -> Void) {
let title = "Weekly Roundup"
let body = "Your weekly roundup is ready, tap here to see the details!"
let title = TextContent.staticNotificationTitle
let body = TextContent.staticNotificationBody

scheduleNotification(
identifier: staticNotificationIdentifier,
Expand All @@ -526,21 +527,31 @@ class WeeklyRoundupNotificationScheduler {
comments: Int,
likes: Int,
periodEndDate: Date,
context: NSManagedObjectContext,
completion: @escaping (Result<Void, Error>) -> Void) {

guard let dotComID = site.dotComID?.intValue else {
// Error
return
}
var siteTitle: String?
var dotComID: Int?

let title: String = {
if let siteTitle = site.title {
return "Weekly Roundup: \(siteTitle)"
} else {
return "Weekly Roundup"
context.performAndWait {
dotComID = site.dotComID?.intValue
siteTitle = site.title
}
}()
let body = "Last week you had \(views) views, \(comments) comments and \(likes) likes."

guard let dotComID = dotComID else {
// Error
return
}

let title: String = {
if let siteTitle = siteTitle {
return String(format: TextContent.dynamicNotificationTitle, siteTitle)
} else {
return TextContent.staticNotificationTitle
}
}()

let body = String(format: TextContent.dynamicNotificationBody, views, comments, likes)

// The dynamic notification date is defined by when the background task is run.
// Since these lines of code execute when the BG Task is run, we can just schedule
Expand All @@ -563,13 +574,13 @@ class WeeklyRoundupNotificationScheduler {
userInfo: userInfo,
dateComponents: dateComponents) { result in

switch result {
case .success:
completion(.success(()))
case .failure(let error):
completion(.failure(NotificationSchedulingError.dynamicNotificationSchedulingError(error: error)))
switch result {
case .success:
completion(.success(()))
case .failure(let error):
completion(.failure(NotificationSchedulingError.dynamicNotificationSchedulingError(error: error)))
}
}
}
}

private func scheduleNotification(
Expand Down Expand Up @@ -636,4 +647,11 @@ class WeeklyRoundupNotificationScheduler {
completion(true)
}
}

enum TextContent {
static let staticNotificationTitle = NSLocalizedString("Weekly Roundup", comment: "Title of Weekly Roundup push notification")
static let dynamicNotificationTitle = NSLocalizedString("Weekly Roundup: %@", comment: "Title of Weekly Roundup push notification. %@ is a placeholder and will be replaced with the title of one of the user's websites.")
static let staticNotificationBody = NSLocalizedString("Your weekly roundup is ready, tap here to see the details!", comment: "Prompt displayed as part of the stats Weekly Roundup push notification.")
static let dynamicNotificationBody = NSLocalizedString("Last week you had %1$d views, %2$d comments and %3$d likes.", comment: "Content of a weekly roundup push notification containing stats about the user's site. The % markers are placeholders and will be replaced by the appropriate number of views, comments, and likes. The numbers indicate the order, so they can be rearranged if necessary – 1 is views, 2 is comments, 3 is likes.")
}
}