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

Push Notification - TwoFactor Notification #2525

Merged
merged 9 commits into from
Jul 11, 2023
Merged

Push Notification - TwoFactor Notification #2525

merged 9 commits into from
Jul 11, 2023

Conversation

marinofaggiana
Copy link
Member

@marinofaggiana marinofaggiana commented Jul 10, 2023

Signed-off-by: Marino Faggiana <marino@marinofaggiana.com>
Signed-off-by: Marino Faggiana <marino@marinofaggiana.com>
Signed-off-by: Marino Faggiana <marino@marinofaggiana.com>
Signed-off-by: Marino Faggiana <marino@marinofaggiana.com>
@marinofaggiana marinofaggiana changed the title Push Notification - Push Notification - twofactorNotificatio Jul 10, 2023
@marinofaggiana marinofaggiana changed the title Push Notification - twofactorNotificatio Push Notification - twofactor Notification Jul 10, 2023
@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.05 🎉

Comparison is base (01ee269) 9.27% compared to head (4044ce4) 9.32%.

❗ Current head 4044ce4 differs from pull request most recent head eddc3bf. Consider uploading reports for the commit eddc3bf to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #2525      +/-   ##
==========================================
+ Coverage     9.27%   9.32%   +0.05%     
==========================================
  Files          185     185              
  Lines        25995   26026      +31     
  Branches      9715    9731      +16     
==========================================
+ Hits          2411    2428      +17     
- Misses       23364   23387      +23     
+ Partials       220     211       -9     
Impacted Files Coverage Δ
ExternalResources/NCApplicationHandle.swift 5.55% <0.00%> (-0.33%) ⬇️
iOSClient/AppDelegate.swift 28.59% <0.00%> (-0.79%) ⬇️
iOSClient/NCGlobal.swift 87.09% <ø> (+54.83%) ⬆️
iOSClient/Notification/NCNotification.swift 0.00% <0.00%> (ø)

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Marino Faggiana <marino@marinofaggiana.com>
@marinofaggiana marinofaggiana merged commit d6d2687 into develop Jul 11, 2023
4 checks passed
@@ -280,11 +287,15 @@ class NCNotification: UITableViewController, NCNotificationCellDelegate, NCEmpty
self.notifications.remove(at: index)
}
self.tableView.reloadData()
if self.navigationController?.presentingViewController != nil, notification.app == NCGlobal.shared.twofactorNotificatioName {
Copy link
Collaborator

@mpivchev mpivchev Jul 12, 2023

Choose a reason for hiding this comment

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

twofactorNotificatioName is a typo -> twoFactorNotificationName

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

@@ -32,7 +32,8 @@ class NCApplicationHandle: NSObject {

// class: AppDelegate
// func nextcloudPushNotificationAction(data: [String: AnyObject])
func nextcloudPushNotificationAction(data: [String: AnyObject]) {
func nextcloudPushNotificationAction(data: [String: AnyObject], completion: @escaping (_ detected: Bool) -> Void) {
completion(false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is detected always false?

Copy link
Member Author

Choose a reason for hiding this comment

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

was a old check (removed)

navigationController.modalPresentationStyle = .fullScreen
self.window?.rootViewController?.present(navigationController, animated: true)
}
} else if !findAccount {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need the else if ? We can just do else

@mpivchev mpivchev changed the title Push Notification - twofactor Notification Push Notification - TwoFactor Notification Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants