Remove PushKit #3268

ismailgulek · 2020-05-22T11:29:11Z

Fix #2714

Signed-off-by: ismailgulek <[email protected]>

manuroe

I have not finished the review yet. Will come back to it in 2h.

manuroe · 2020-05-25T09:26:14Z

Riot/Managers/PushNotification/PushNotificationService.m

+- (void)didReceiveRemoteNotification:(NSDictionary *)userInfo
+              fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler
+{
+//    NSLog(@"[PushNotificationService][Push] didReceiveRemoteNotification: applicationState: %tu - payload: %@", [UIApplication sharedApplication].applicationState, userInfo);


Can we still log the call of this function?
It could be useful for sanity check.

manuroe · 2020-05-25T09:43:34Z

RiotNSE/NotificationService.swift

+        Bundle.mxk_customizeLocalizedStringTableName("Vector")
+
+        if isatty(STDERR_FILENO) == 0 {
+//            MXLogger.setSubLogName("nse")


To fix. Logs are useful

manuroe · 2020-05-25T09:50:25Z

RiotNSE/NotificationService.swift

+            }
+
+            theUserAccount.mxSession.event(withEventId: theEventId, inRoom: theRoomId, success: { [weak self] (event) in
+                guard let strongSelf = self else {


You can use the following swift 4.x:

Suggested change

guard let strongSelf = self else {

guard let self = self else {

manuroe · 2020-05-25T10:00:28Z

RiotNSE/NotificationService.swift

+                    return
+                }
+
+                guard let theEvent = event else {


Suggested change

guard let theEvent = event else {

guard let event = event else {

manuroe · 2020-05-25T10:02:08Z

RiotNSE/NotificationService.swift

+    }
+
+    func processEvent(_ event: MXEvent) {
+        if let content = originalContent, let theUserAccount = userAccount {


You could use userAcccount = self.userAccount to unwrap the value.

manuroe · 2020-05-25T10:14:07Z

RiotNSE/NotificationService.swift

+                // Modify the notification content here...
+                guard let newContent = notificationContent else {
+                    //  remove
+                    UNUserNotificationCenter.current().removePendingNotificationRequests(withIdentifiers: [self.requestIdentifier!])


We should always unwrap optionals for long term sanity.

Signed-off-by: ismailgulek <[email protected]>

manuroe · 2020-05-26T13:29:54Z

Riot/Managers/PushNotification/PushNotificationService.m

+{
+    NSLog(@"[PushNotificationService][Push] didReceiveRemoteNotification: applicationState: %tu - payload: %@", [UIApplication sharedApplication].applicationState, userInfo);
+//
+//    // Display local notifications only when the app is running in background.


Could we remove commented code?
This is to avoid to keep dead comments for years. Specially when there is no "comment" for the commented section.
We will be able to retrieve this piece of code in git if we need it one day.

manuroe · 2020-05-26T13:33:54Z

Riot/Modules/Settings/SettingsViewController.m

-    CALLS_ENABLE_CALLKIT_INDEX = 0,
-    CALLS_CALLKIT_DESCRIPTION_INDEX,
-    CALLS_ENABLE_STUN_SERVER_FALLBACK_INDEX,
+    //CALLS_ENABLE_CALLKIT_INDEX = 0,


I think we can clear this commented piece of code as well.

Riot/Utils/Constants.swift

manuroe · 2020-05-26T14:08:43Z

RiotNSE/NotificationService.swift

+    func fetchEvent() {
+        guard let content = originalContent, let userAccount = self.userAccount else {
+            //  there is something wrong, do not change the content
+            NSLog("[NotificationService] Fallback case 4")


Could we have more detail in the log for future debugging?
Like:

Suggested change

NSLog("[NotificationService] Fallback case 4")

NSLog("[NotificationService] fetchEvent: Fallback case 4. There is something wrong, do not change the content")

This worths for all Fallback case X :)

I do not understand the order for X :/

You're totally right. It was easy for debugging on development. Changing them all.

manuroe · 2020-05-26T14:17:59Z

RiotNSE/NotificationService.swift

+            }
+
+            guard let event = event else {
+                return


Should we call self.fallbackToOriginalContent() here?

Yes, it will be more clear.

manuroe · 2020-05-26T14:21:00Z

RiotNSE/NotificationService.swift

+            }
+
+            //  should show decrypted content in notification
+            guard event.clear == nil else {


Suggested change

guard event.clear == nil else {

if event.clear {

RiotNSE/NotificationService.swift

manuroe · 2020-05-26T14:22:23Z

RiotNSE/NotificationService.swift

+    }
+
+    func launchBackgroundSync() {
+        guard let userAccount = userAccount else { return }


Should we call self.fallbackToOriginalContent() here?

Also adding this

RiotNSE/NotificationService.swift

Co-authored-by: manuroe <[email protected]>

Signed-off-by: ismailgulek <[email protected]>

Co-authored-by: manuroe <[email protected]>

Signed-off-by: ismailgulek <[email protected]>

manuroe

Using a new implementation NSEMemoryStore is really smart 👍

manuroe · 2020-05-28T16:14:46Z

RiotNSE/NotificationService.swift

-            store = MXFileStore(credentials: userAccount.mxCredentials)
+        if let userAccount = MXKAccountManager.shared()?.activeAccounts.first {
+            store = NSEMemoryStore(withCredentials: userAccount.mxCredentials)
+            store.open(with: userAccount.mxCredentials, onComplete: nil, failure: nil)


It can be removed. It is called by MXSession.setStore()

That's right. Removing.

RiotNSE/NotificationService.swift

manuroe · 2020-05-28T16:19:43Z

RiotNSE/NotificationService.swift

-        room.state({ (roomState: MXRoomState!) in
-
+        //  initialize a temporary file store, just to load the room state
+        let fileStore = MXFileStore(credentials: session.credentials)


Why not using the existing NSEMemoryStore and implement stateOfRoom there?

Yes, totally makes sense. Updating.

Signed-off-by: ismailgulek <[email protected]>

manuroe

All good for me!

Signed-off-by: ismailgulek <[email protected]>

Create notification service extension

53bb87d

Signed-off-by: ismailgulek <[email protected]>

ismailgulek marked this pull request as ready for review May 25, 2020 10:11

ismailgulek requested review from manuroe and SBiOSoftWhare May 25, 2020 10:11

ismailgulek added 2 commits May 25, 2020 13:27

update fastlane commands for new target

a3890c6

Signed-off-by: ismailgulek <[email protected]>

Configure log files correctly

7335682

Signed-off-by: ismailgulek <[email protected]>

manuroe reviewed May 25, 2020

View reviewed changes

ismailgulek added 9 commits May 25, 2020 15:50

Fix Manu's comments

9475cd7

Signed-off-by: ismailgulek <[email protected]>

Implement a workaround to delete some notifications afterwards

f85aee0

Signed-off-by: ismailgulek <[email protected]>

Swiftify some ifs

87e702a

Signed-off-by: ismailgulek <[email protected]>

Swiftify some ifs

23203af

Signed-off-by: ismailgulek <[email protected]>

Swiftify some ifs

db01577

Signed-off-by: ismailgulek <[email protected]>

Swiftify code, ignore session state

4e25323

Signed-off-by: ismailgulek <[email protected]>

Swiftify content function, use switch for ifs when possible

09353f4

Signed-off-by: ismailgulek <[email protected]>

Swiftify code, style changes

f25e3b6

Signed-off-by: ismailgulek <[email protected]>

Swiftify extension methods

7f39581

Signed-off-by: ismailgulek <[email protected]>

manuroe reviewed May 26, 2020

View reviewed changes

ismailgulek and others added 10 commits May 26, 2020 17:51

Update Riot/Utils/Constants.swift

63d4a16

Co-authored-by: manuroe <[email protected]>

Update RiotNSE/NotificationService.swift

db8b5a4

Co-authored-by: manuroe <[email protected]>

Remove some commented code

ba50f47

Signed-off-by: ismailgulek <[email protected]>

Change logs for fallback cases

b1a654d

Signed-off-by: ismailgulek <[email protected]>

Add fallback method calls on some points

69d9b5b

Signed-off-by: ismailgulek <[email protected]>

Remove some redundant semicolons

c4bdf33

Signed-off-by: ismailgulek <[email protected]>

Update RiotNSE/NotificationService.swift

fa44dae

Co-authored-by: manuroe <[email protected]>

Update RiotNSE/NotificationService.swift

a0e9540

Co-authored-by: manuroe <[email protected]>

Update RiotNSE/NotificationService.swift

f5324ae

Co-authored-by: manuroe <[email protected]>

Change some guarding to if check

3bd9e6b

Signed-off-by: ismailgulek <[email protected]>

manuroe approved these changes May 26, 2020

View reviewed changes

ismailgulek added this to the Sprint 27: iOS13 Maintenance milestone May 27, 2020

Implement a memory store specific to NSE

bcd9683

Signed-off-by: ismailgulek <[email protected]>

Drop MXKAccount and MXFileStore usages

d859ce8

Signed-off-by: ismailgulek <[email protected]>

manuroe reviewed May 28, 2020

View reviewed changes

ismailgulek added 2 commits May 29, 2020 09:55

Fix Manu's comments

845363b

Signed-off-by: ismailgulek <[email protected]>

Add some comments

4ef5e17

Signed-off-by: ismailgulek <[email protected]>

manuroe approved these changes May 29, 2020

View reviewed changes

Adapt to new method, change timeout to seconds

0b639e8

Signed-off-by: ismailgulek <[email protected]>

manuroe approved these changes May 29, 2020

View reviewed changes

ismailgulek merged commit 0d73c8d into xcode11 May 29, 2020

ismailgulek deleted the remove_pushkit branch May 29, 2020 09:34

ismailgulek mentioned this pull request May 29, 2020

Find an alternative to VoIP push when building the app for iOS 13 #2714

Closed

ismailgulek mentioned this pull request Oct 1, 2020

Fix Message Not Shown After Push Error #3680

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove PushKit #3268

Remove PushKit #3268

ismailgulek commented May 22, 2020 •

edited

Loading

manuroe left a comment

manuroe May 25, 2020

ismailgulek May 25, 2020

manuroe May 25, 2020

ismailgulek May 25, 2020

manuroe May 25, 2020

ismailgulek May 25, 2020

manuroe May 25, 2020

ismailgulek May 25, 2020

manuroe May 25, 2020

ismailgulek May 25, 2020

manuroe May 25, 2020

ismailgulek May 25, 2020

manuroe May 26, 2020

ismailgulek May 26, 2020

manuroe May 26, 2020

ismailgulek May 26, 2020

manuroe May 26, 2020

ismailgulek May 26, 2020

manuroe May 26, 2020

ismailgulek May 26, 2020

manuroe May 26, 2020

ismailgulek May 26, 2020

manuroe May 26, 2020

ismailgulek May 26, 2020

manuroe left a comment

manuroe May 28, 2020

ismailgulek May 29, 2020

manuroe May 28, 2020

ismailgulek May 29, 2020 •

edited

Loading

manuroe left a comment

	guard let strongSelf = self else {
	guard let self = self else {

	guard let theEvent = event else {
	guard let event = event else {

	NSLog("[NotificationService] Fallback case 4")
	NSLog("[NotificationService] fetchEvent: Fallback case 4. There is something wrong, do not change the content")

Remove PushKit #3268

Remove PushKit #3268

Conversation

ismailgulek commented May 22, 2020 • edited Loading

manuroe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuroe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ismailgulek May 29, 2020 • edited Loading

Choose a reason for hiding this comment

manuroe left a comment

Choose a reason for hiding this comment

ismailgulek commented May 22, 2020 •

edited

Loading

ismailgulek May 29, 2020 •

edited

Loading