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.

Sign up for GitHub

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

chore: notifications preferences screen implementation with edge cases #111

Merged
merged 14 commits into from
Jan 14, 2025
Merged

chore: notifications preferences screen implementation with edge cases #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
393607e
chore: notifications preferences screen implementation with edge cases
saeedbashir Jan 1, 2025
8716ec3
refactor: address review feedback
saeedbashir Jan 3, 2025
c60e49c
refactor: address review feedback
saeedbashir Jan 3, 2025
118c68c
chore: implement seen and read APIs
saeedbashir Jan 3, 2025
f18d521
refactor: address review feedback
saeedbashir Jan 3, 2025
97949a1
refactor: address review feedback
saeedbashir Jan 3, 2025
90ef35b
fix: address review feedback
saeedbashir Jan 3, 2025
a310d6b
chore: fix spelling mistakes
mta452 Jan 10, 2025
1b48556
chore: fix indentation
mta452 Jan 10, 2025
74ed02f
chore: present system alert for push notifications
mta452 Jan 10, 2025
1a20310
chore: set optional handler parameter to nil
mta452 Jan 10, 2025
8d1ca84
chore: fix ui issues on ipad
mta452 Jan 13, 2025
7cf97d8
chore: minor refactoring
mta452 Jan 13, 2025
15fe1fc
chore: make ui consistent with other settings screens
mta452 Jan 14, 2025
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
36 changes: 36 additions & 0 deletions Authorization/AuthorizationTests/AuthorizationMock.generated.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,13 @@ open class AuthorizationRouterMock: AuthorizationRouter, Mock {
perform?()
}

@MainActor
open func performNotificationRegistration() {
addInvocation(.m_performNotificationRegistration)
let perform = methodPerformValue(.m_performNotificationRegistration) as? () -> Void
perform?()
}


fileprivate enum MethodType {
case m_showUpdateRequiredView__showAccountLink_showAccountLink(Parameter<Bool>)
Expand All @@ -1025,6 +1032,7 @@ open class AuthorizationRouterMock: AuthorizationRouter, Mock {
case m_hideUpgradeLoaderView__animated_animated(Parameter<Bool>)
case m_showRestoreProgressView
case m_hideRestoreProgressView
case m_performNotificationRegistration

static func compareParameters(lhs: MethodType, rhs: MethodType, matcher: Matcher) -> Matcher.ComparisonResult {
switch (lhs, rhs) {
Expand Down Expand Up @@ -1159,6 +1167,8 @@ open class AuthorizationRouterMock: AuthorizationRouter, Mock {
case (.m_showRestoreProgressView, .m_showRestoreProgressView): return .match

case (.m_hideRestoreProgressView, .m_hideRestoreProgressView): return .match

case (.m_performNotificationRegistration, .m_performNotificationRegistration): return .match
default: return .none
}
}
Expand Down Expand Up @@ -1189,6 +1199,7 @@ open class AuthorizationRouterMock: AuthorizationRouter, Mock {
case let .m_hideUpgradeLoaderView__animated_animated(p0): return p0.intValue
case .m_showRestoreProgressView: return 0
case .m_hideRestoreProgressView: return 0
case .m_performNotificationRegistration: return 0
}
}
func assertionName() -> String {
Expand Down Expand Up @@ -1217,6 +1228,7 @@ open class AuthorizationRouterMock: AuthorizationRouter, Mock {
case .m_hideUpgradeLoaderView__animated_animated: return ".hideUpgradeLoaderView(animated:)"
case .m_showRestoreProgressView: return ".showRestoreProgressView()"
case .m_hideRestoreProgressView: return ".hideRestoreProgressView()"
case .m_performNotificationRegistration: return ".performNotificationRegistration()"
}
}
}
Expand Down Expand Up @@ -1265,6 +1277,8 @@ open class AuthorizationRouterMock: AuthorizationRouter, Mock {
public static func showRestoreProgressView() -> Verify { return Verify(method: .m_showRestoreProgressView)}
@MainActor
public static func hideRestoreProgressView() -> Verify { return Verify(method: .m_hideRestoreProgressView)}
@MainActor
public static func performNotificationRegistration() -> Verify { return Verify(method: .m_performNotificationRegistration)}
}

public struct Perform {
Expand Down Expand Up @@ -1349,6 +1363,10 @@ open class AuthorizationRouterMock: AuthorizationRouter, Mock {
public static func hideRestoreProgressView(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_hideRestoreProgressView, performs: perform)
}
@MainActor
public static func performNotificationRegistration(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_performNotificationRegistration, performs: perform)
}
}

public func given(_ method: Given) {
Expand Down Expand Up @@ -1612,6 +1630,13 @@ open class BaseRouterMock: BaseRouter, Mock {
perform?()
}

@MainActor
open func performNotificationRegistration() {
addInvocation(.m_performNotificationRegistration)
let perform = methodPerformValue(.m_performNotificationRegistration) as? () -> Void
perform?()
}


fileprivate enum MethodType {
case m_backToRoot__animated_animated(Parameter<Bool>)
Expand All @@ -1637,6 +1662,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case m_hideUpgradeLoaderView__animated_animated(Parameter<Bool>)
case m_showRestoreProgressView
case m_hideRestoreProgressView
case m_performNotificationRegistration

static func compareParameters(lhs: MethodType, rhs: MethodType, matcher: Matcher) -> Matcher.ComparisonResult {
switch (lhs, rhs) {
Expand Down Expand Up @@ -1766,6 +1792,8 @@ open class BaseRouterMock: BaseRouter, Mock {
case (.m_showRestoreProgressView, .m_showRestoreProgressView): return .match

case (.m_hideRestoreProgressView, .m_hideRestoreProgressView): return .match

case (.m_performNotificationRegistration, .m_performNotificationRegistration): return .match
default: return .none
}
}
Expand Down Expand Up @@ -1795,6 +1823,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case let .m_hideUpgradeLoaderView__animated_animated(p0): return p0.intValue
case .m_showRestoreProgressView: return 0
case .m_hideRestoreProgressView: return 0
case .m_performNotificationRegistration: return 0
}
}
func assertionName() -> String {
Expand Down Expand Up @@ -1822,6 +1851,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case .m_hideUpgradeLoaderView__animated_animated: return ".hideUpgradeLoaderView(animated:)"
case .m_showRestoreProgressView: return ".showRestoreProgressView()"
case .m_hideRestoreProgressView: return ".hideRestoreProgressView()"
case .m_performNotificationRegistration: return ".performNotificationRegistration()"
}
}
}
Expand Down Expand Up @@ -1869,6 +1899,8 @@ open class BaseRouterMock: BaseRouter, Mock {
public static func showRestoreProgressView() -> Verify { return Verify(method: .m_showRestoreProgressView)}
@MainActor
public static func hideRestoreProgressView() -> Verify { return Verify(method: .m_hideRestoreProgressView)}
@MainActor
public static func performNotificationRegistration() -> Verify { return Verify(method: .m_performNotificationRegistration)}
}

public struct Perform {
Expand Down Expand Up @@ -1950,6 +1982,10 @@ open class BaseRouterMock: BaseRouter, Mock {
public static func hideRestoreProgressView(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_hideRestoreProgressView, performs: perform)
}
@MainActor
public static func performNotificationRegistration(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_performNotificationRegistration, performs: perform)
}
}

public func given(_ method: Given) {
Expand Down
6 changes: 6 additions & 0 deletions Core/Core/Configuration/BaseRouter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ public protocol BaseRouter {

@MainActor
func hideRestoreProgressView()

@MainActor
func performNotificationRegistration()
Copy link

Choose a reason for hiding this comment

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

Isn't router responsible for navigation purposes only?

Copy link
Author

Choose a reason for hiding this comment

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

PushNotificationsManager belongs to OpenedX target and Notifications is added as a framework to it. And accessing the parent from the child isn't a good idea. That's why it's here.

Copy link

Choose a reason for hiding this comment

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

Maybe we should create a protocol in Core, implement it in PushNotificationsManager and access it from there?

Copy link
Author

Choose a reason for hiding this comment

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

It already move to the OEXFoundation a plugin based architecture, moving this there will make it harder to move to the plugin architecture, thats why I keep it simple.

}

extension BaseRouter {
Expand Down Expand Up @@ -179,5 +182,8 @@ open class BaseRouterMock: BaseRouter {

@MainActor
public func hideRestoreProgressView() {}

@MainActor
public func performNotificationRegistration() {}
}
#endif
2 changes: 2 additions & 0 deletions Core/Core/Data/CoreStorage.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public protocol CoreStorage {
var userSettings: UserSettings? {get set}
var resetAppSupportDirectoryUserData: Bool? {get set}
var lastUsedSocialAuth: String? {get set}
var discussionNotificationsSettingStatus: Bool? {get set}
func clear()
}

Expand All @@ -37,6 +38,7 @@ public class CoreStorageMock: CoreStorage {
public var userSettings: UserSettings?
public var resetAppSupportDirectoryUserData: Bool?
public var lastUsedSocialAuth: String?
public var discussionNotificationsSettingStatus: Bool?
public func clear() {}

public init() {}
Expand Down
2 changes: 2 additions & 0 deletions Core/Core/Extensions/Notification.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ public extension Notification.Name {
static let courseUpgradeCompletionNotification = Notification.Name("CourseUpgradeCompletionNotification")
static let getCourseDates = Notification.Name("getCourseDates")
static let refreshEnrollments = Notification.Name("refreshEnrollments")
static let notificationRegistration = Notification.Name("notificationRegistration")
}

public extension Notification {
enum UserInfoKey: String {
case isForced
case status
}
}
18 changes: 18 additions & 0 deletions Course/CourseTests/CourseMock.generated.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,13 @@ open class BaseRouterMock: BaseRouter, Mock {
perform?()
}

@MainActor
open func performNotificationRegistration() {
addInvocation(.m_performNotificationRegistration)
let perform = methodPerformValue(.m_performNotificationRegistration) as? () -> Void
perform?()
}


fileprivate enum MethodType {
case m_backToRoot__animated_animated(Parameter<Bool>)
Expand All @@ -678,6 +685,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case m_hideUpgradeLoaderView__animated_animated(Parameter<Bool>)
case m_showRestoreProgressView
case m_hideRestoreProgressView
case m_performNotificationRegistration

static func compareParameters(lhs: MethodType, rhs: MethodType, matcher: Matcher) -> Matcher.ComparisonResult {
switch (lhs, rhs) {
Expand Down Expand Up @@ -807,6 +815,8 @@ open class BaseRouterMock: BaseRouter, Mock {
case (.m_showRestoreProgressView, .m_showRestoreProgressView): return .match

case (.m_hideRestoreProgressView, .m_hideRestoreProgressView): return .match

case (.m_performNotificationRegistration, .m_performNotificationRegistration): return .match
default: return .none
}
}
Expand Down Expand Up @@ -836,6 +846,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case let .m_hideUpgradeLoaderView__animated_animated(p0): return p0.intValue
case .m_showRestoreProgressView: return 0
case .m_hideRestoreProgressView: return 0
case .m_performNotificationRegistration: return 0
}
}
func assertionName() -> String {
Expand Down Expand Up @@ -863,6 +874,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case .m_hideUpgradeLoaderView__animated_animated: return ".hideUpgradeLoaderView(animated:)"
case .m_showRestoreProgressView: return ".showRestoreProgressView()"
case .m_hideRestoreProgressView: return ".hideRestoreProgressView()"
case .m_performNotificationRegistration: return ".performNotificationRegistration()"
}
}
}
Expand Down Expand Up @@ -910,6 +922,8 @@ open class BaseRouterMock: BaseRouter, Mock {
public static func showRestoreProgressView() -> Verify { return Verify(method: .m_showRestoreProgressView)}
@MainActor
public static func hideRestoreProgressView() -> Verify { return Verify(method: .m_hideRestoreProgressView)}
@MainActor
public static func performNotificationRegistration() -> Verify { return Verify(method: .m_performNotificationRegistration)}
}

public struct Perform {
Expand Down Expand Up @@ -991,6 +1005,10 @@ open class BaseRouterMock: BaseRouter, Mock {
public static func hideRestoreProgressView(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_hideRestoreProgressView, performs: perform)
}
@MainActor
public static func performNotificationRegistration(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_performNotificationRegistration, performs: perform)
}
}

public func given(_ method: Given) {
Expand Down
50 changes: 26 additions & 24 deletions Dashboard/Dashboard/Presentation/PrimaryCourseDashboardView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,31 +331,33 @@ public struct PrimaryCourseDashboardView<ProgramView: View>: View {
.foregroundColor(Theme.Colors.textPrimary)
.accessibilityIdentifier("courses_header_text")
Spacer()
Button(action: {
router.showNotificationsScreen()
}, label: {
CoreAssets.notificationsIcon.swiftUIImage
.renderingMode(.template)
.foregroundColor(Theme.Colors.accentColor)
})
.frame(width: 24, height: 24)
.overlay {
if viewModel.hasUnreadNotifications {
if #available(iOS 17.0, *) {
Circle()
.stroke(Theme.Colors.background, lineWidth: 5)
.fill(Theme.Colors.accentButtonColor)
.frame(width: 8, height: 8)
.offset(x: 7, y: -6)
} else {
Circle()
.strokeBorder(Theme.Colors.background, lineWidth: 2)
.frame(width: 10, height: 10)
.background(
if viewModel.config.pushNotificationsEnabled {
mta452 marked this conversation as resolved.
Show resolved Hide resolved
Button(action: {
router.showNotificationsScreen()
}, label: {
CoreAssets.notificationsIcon.swiftUIImage
.renderingMode(.template)
.foregroundColor(Theme.Colors.accentColor)
})
.frame(width: 24, height: 24)
.overlay {
if viewModel.hasUnreadNotifications {
if #available(iOS 17.0, *) {
Circle()
.foregroundColor(Theme.Colors.accentButtonColor)
)
.offset(x: 5, y: -6)
.stroke(Theme.Colors.background, lineWidth: 5)
.fill(Theme.Colors.accentButtonColor)
.frame(width: 8, height: 8)
.offset(x: 7, y: -6)
} else {
Circle()
.strokeBorder(Theme.Colors.background, lineWidth: 2)
.frame(width: 10, height: 10)
.background(
Circle()
.foregroundColor(Theme.Colors.accentButtonColor)
)
.offset(x: 5, y: -6)
}
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions Dashboard/DashboardTests/DashboardMock.generated.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,13 @@ open class BaseRouterMock: BaseRouter, Mock {
perform?()
}

@MainActor
open func performNotificationRegistration() {
addInvocation(.m_performNotificationRegistration)
let perform = methodPerformValue(.m_performNotificationRegistration) as? () -> Void
perform?()
}


fileprivate enum MethodType {
case m_backToRoot__animated_animated(Parameter<Bool>)
Expand All @@ -678,6 +685,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case m_hideUpgradeLoaderView__animated_animated(Parameter<Bool>)
case m_showRestoreProgressView
case m_hideRestoreProgressView
case m_performNotificationRegistration

static func compareParameters(lhs: MethodType, rhs: MethodType, matcher: Matcher) -> Matcher.ComparisonResult {
switch (lhs, rhs) {
Expand Down Expand Up @@ -807,6 +815,8 @@ open class BaseRouterMock: BaseRouter, Mock {
case (.m_showRestoreProgressView, .m_showRestoreProgressView): return .match

case (.m_hideRestoreProgressView, .m_hideRestoreProgressView): return .match

case (.m_performNotificationRegistration, .m_performNotificationRegistration): return .match
default: return .none
}
}
Expand Down Expand Up @@ -836,6 +846,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case let .m_hideUpgradeLoaderView__animated_animated(p0): return p0.intValue
case .m_showRestoreProgressView: return 0
case .m_hideRestoreProgressView: return 0
case .m_performNotificationRegistration: return 0
}
}
func assertionName() -> String {
Expand Down Expand Up @@ -863,6 +874,7 @@ open class BaseRouterMock: BaseRouter, Mock {
case .m_hideUpgradeLoaderView__animated_animated: return ".hideUpgradeLoaderView(animated:)"
case .m_showRestoreProgressView: return ".showRestoreProgressView()"
case .m_hideRestoreProgressView: return ".hideRestoreProgressView()"
case .m_performNotificationRegistration: return ".performNotificationRegistration()"
}
}
}
Expand Down Expand Up @@ -910,6 +922,8 @@ open class BaseRouterMock: BaseRouter, Mock {
public static func showRestoreProgressView() -> Verify { return Verify(method: .m_showRestoreProgressView)}
@MainActor
public static func hideRestoreProgressView() -> Verify { return Verify(method: .m_hideRestoreProgressView)}
@MainActor
public static func performNotificationRegistration() -> Verify { return Verify(method: .m_performNotificationRegistration)}
}

public struct Perform {
Expand Down Expand Up @@ -991,6 +1005,10 @@ open class BaseRouterMock: BaseRouter, Mock {
public static func hideRestoreProgressView(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_hideRestoreProgressView, performs: perform)
}
@MainActor
public static func performNotificationRegistration(perform: @escaping () -> Void) -> Perform {
return Perform(method: .m_performNotificationRegistration, performs: perform)
}
}

public func given(_ method: Given) {
Expand Down
Loading
Loading