First steps into starting onesignal on DidFinishLaunching event #38

williamrijksen · 2017-07-05T19:21:05Z

Context

#28.

What has been done

[iOS] Trust on the UIApplicationDidFinishLaunchingNotification, because it looks like it has missing context.
[iOS] Use the promptForPushNotificationsWithUserResponse to skip the didRegisterForRemoteNotificationsWithDeviceToken method
[Android] Start one signal onAppCreate

How to test

build module and verify the module is working again.

Todo

[iOS] Fix the notificationOpened / notificationReceived event. @hansemannn do you have any suggestions to fire the event from a static context?
[iOS] Remove the launch function
[iOS] Update the readme by introducing onesignal.promptForPushNotificationsWithUserResponse

Notes

~~[iOS] The notificationopened / notificationreceived is broken since it's removed (see todo)~~

chmiiller · 2017-07-06T16:05:30Z

Hi @williamrijksen
Unfortunately it does not solve the issue #7 but seems promising 😬 Let me try to explain...

We have the method + (void)load that assigns onAppCreate as a selector to the NSNotificationCenter.
Inside onAppCreate we have the OneSignal library initialization and everything seems to work well. Then on the OneSignal initWithLaunchOptions we have:

handleNotificationAction:^(OSNotificationOpenedResult *result) {
    NSLog(@"User opened notification");
}

And if I send and click to open a notification it works and I can see the User opened notification message on the Console! Awesome!
Then I was trying to call the method actionHandler using [self actionHandler:result]; but this doesn't work. I think something goes wrong trying to find self in this scope, but I'm not sure why.
I've tried OneSignal's implementation of handleNotificationAction like here: https://documentation.onesignal.com/docs/ios-native-sdk#section--initwithlaunchoptions- with

id notificationReceiverBlock = ^(OSNotification *notification) {
    NSLog(@"Received Notification - %@", notification.payload.notificationID);
  };
  
  id notificationOpenedBlock = ^(OSNotificationOpenedResult *result) {
        // This block gets called when the user reacts to a notification received
        OSNotificationPayload* payload = result.notification.payload;
        
        NSString* messageTitle = @"OneSignal Example";
        NSString* fullMessage = [payload.body copy];
        
        if (payload.additionalData) {
            
            if(payload.title)
                messageTitle = payload.title;
            
            NSDictionary* additionalData = payload.additionalData;
            
            if (additionalData[@"actionSelected"])
                fullMessage = [fullMessage stringByAppendingString:[NSString stringWithFormat:@"\nPressed ButtonId:%@", additionalData[@"actionSelected"]]];
        }
        
        UIAlertView* alertView = [[UIAlertView alloc] initWithTitle:messageTitle
                                                            message:fullMessage
                                                           delegate:self
                                                  cancelButtonTitle:@"Close"
                                                  otherButtonTitles:nil, nil];
        [alertView show];

   };
  
   id onesignalInitSettings = @{kOSSettingsKeyAutoPrompt : @YES};
  
   [OneSignal initWithLaunchOptions:launchOptions
                              appId:@"YOUR_ONESIGNAL_APP_ID"
         handleNotificationReceived:notificationReceiverBlock
           handleNotificationAction:notificationOpenedBlock
                           settings:onesignalInitSettings];

but inside onAppCreate instead and it displays the AlertView successfully even if the app was closed, which means it works. But because I can't call self, I'm not able to use our

[self _hasListeners:@"notificationOpened"]

or

[self fireEvent:@"notificationOpened" withObject:notificationData];

for example. Can you understand why we can't use "self" here? I really want to have this feature but I'm running out of ideas now.
Thanks for looking at this

hansemannn · 2017-07-06T16:40:37Z

@chmiiller It's a class method, so there is no self scope. Try posting another (inner)-notification to handle inside your module. UrbanAirship does something similar I think.

williamrijksen · 2017-07-06T18:02:21Z

@hansemannn @chmiiller I created a way to get the module instance, but it looks like the opened/received events are not always triggered. I'm looking forward for a better solution, do you have some suggestions?

hansemannn · 2017-07-06T18:21:14Z

ios/Classes/ComWilliamrijksenOnesignalModule.m

+    instance = nil;
+    // release any resources that have been retained by the module
+    [super dealloc];
+}


Make it ARC ;-)

chmiiller · 2017-07-07T08:54:22Z

Thanks guys! It makes sense. Now the actionHandler gets called but

[self _hasListeners:@"notificationOpened"]

is false. Is it because we shouldn't use self in this scope or because it has no listeners at all?
Gonna double check Urban Airship module @hansemannn , thanks!

Update:
Made a instance method and tried to call it from actionHandler with self and it worked, so self exists, but don't know why has no listeners =/

Update 2
Made some progress. I've tried to add a timeout or "performSelector afterDelay" as iOS names it and it works!

- (void) actionHandler:(OSNotificationOpenedResult *)result {
    [self performSelector:@selector(delayedActionHandler:) withObject:nil afterDelay:1.0];
}

And then, delayedActionHandler does what actionHandler was doing with firing the event and stuff. What will be the best way to make sure actionHandler gets called after listeners were added?
And btw, as @williamrijksen mentioned, it seems very unstable. Do we need to dealloc something else from memory?

williamrijksen · 2017-07-10T09:36:12Z

@chmiiller Cool! Would we introduce performSelector afterDelay in this case? Or should we put it towards 5sec or so?

hansemannn · 2017-07-10T15:26:19Z

Guys, please check this PR for a solution that does not require delays.

chmiiller · 2017-07-11T13:31:00Z

Thanks @hansemannn but I can't understand what we're missing. As far as I see, One Signal SDK has implemented it already here: https://github.com/OneSignal/OneSignal-iOS-SDK/blob/01a340b9a86fb2add5a7407da763bd615265d1b6/iOS_SDK/OneSignalSDK/Source/OneSignal.m
Maybe @jkasten2 could guide us on how to implement it

williamrijksen · 2017-07-11T21:20:36Z

@hansemannn @chmiiller This evening I worked on the NSNotificationCenter within this PR. I introduced a launch event to be able to set the event listeners first.


    onesignal = require('com.williamrijksen.onesignal');
    onesignal.addEventListener('notificationOpened', function (evt) {
        alert('notificationOpened');
        Ti.API.info(JSON.stringify(evt));
    });
    onesignal.addEventListener('notificationReceived', function (evt) {
        alert('notificationReceived');
        Ti.API.info(JSON.stringify(evt));
    });
    if (OS_IOS) {
        onesignal.promptForPushNotificationsWithUserResponse();
    }
   _.delay(function() {
       onesignal.launch();
   }, 2000);

The delay is necessary because otherwise there's something wrong with the event listeners. @hansemannn Do you know why that's delay is necessary?

Thanks in advance!

williamrijksen · 2017-07-11T21:21:31Z

ios/Classes/ComWilliamrijksenOnesignalModule.m

+    [super startup];
+    NSLog(@"[INFO] started %@", self);
+    _instance = self;
+//    [self performSelector:@selector(handleQueuedBootEvents) withObject:self afterDelay:10.0];


@hansemannn Do you know why this is not working?

I do not see any selector for that message. But as you moved to queuing now, it should be fine.

Oh yes, my bad. It should be launch. When it's launch, I see launch is being called, but the fireEvent is not working. When I call launch` like this (#38 (comment)) it's working properly. Do you know why that's failing?

chmiiller · 2017-07-12T12:55:10Z

Sounds great! You have implemented what @hansemannn suggested, hope we manage to make it work somehow.
I've added some logs on tryToPostNotification so it looks like this:

+ (void)tryToPostNotification:(NSDictionary *)_notification withNotificationName:(NSString *)_notificationName
{
	NSLog(@"------------------------->>>>>>>>>>>>>>>>> on tryToPostNotification");
    if ([self instance] != nil) {
		NSLog(@"------------------------->>>>>>>>>>>>>>>>> self instance is NOT NULL");
        [[NSNotificationCenter defaultCenter] postNotificationName:_notificationName object:self userInfo:_notification];
        return;
    }else{
		NSLog(@"------------------------->>>>>>>>>>>>>>>>> self instance DOESNT EXIST");
	}

    if (_queuedBootEvents == nil) {
		NSLog(@"------------------------->>>>>>>>>>>>>>>>> no _queuedBootEvents found");
        _queuedBootEvents = [[NSMutableDictionary alloc] init];
    }else{
		NSLog(@"------------------------->>>>>>>>>>>>>>>>> _queuedBootEvents not null");
	}
    [_queuedBootEvents setObject:_notification forKey:_notificationName];
}

If we click the notification to start the app it prints self instance is NOT NULL, which is good, but doesn't go further because of this return. Is there a specific reason for this?
Commenting it out just works! 🎉

hansemannn · 2017-07-12T17:06:00Z

@chmiiller For the null-checks, you could use the NULL_IF_NIL() macro. In general, validate all return values to be mappable values, e.g. NSNumber, NSString, NSDate, etc. For dictionaries / arrays, loop through the collection and ensure the key-value types there.

hansemannn · 2017-07-12T17:11:38Z

ios/Classes/ComWilliamrijksenOnesignalModule.m

+    NSDictionary* notificationData = @{
+                                    @"title": title,
+                                    @"body": body,
+                                    @"additionalData": additionalData};


@chmiiller This for example could be a dictionaty holding non-JS types like NSData, so ensure to map those (in this example, an NSData instance would be mapped with [[TiBlob alloc] initWithData:data mimetype:@"text/plain"]).

What do you want to change? The additionalData or the notificationData?

williamrijksen · 2017-07-12T17:12:11Z

@chmiiller If there's an [self instance], the boot events should not being queued. That's the reason of the early return in tryToPostNotification.

williamrijksen · 2017-07-14T18:54:00Z

Blocked by experiencing this issue: https://jira.appcelerator.org/browse/MOD-2249. @hansemannn @chmiiller do we have a workaround for this?

hansemannn · 2017-07-28T19:28:42Z

Commented on the ticket why it is invalid and proposed a fix (use the delegate-pattern) for it. Same would apply for this one, with the exact same reasons to avoid the race-condition.

hansemannn · 2017-07-28T19:29:45Z

ios/Classes/ComWilliamrijksenOnesignalModule.m

+-(void)notificationOpened:(NSNotification*)info
+{
+    if (_instance != nil) {
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{


Use TiDispatchOnMainThread instead.

@hansemannn is this a method or something else? I can't find anything in the titanium_mobile repository with the name TiDispatchOnMainThread.

Found it.... I think you meant TiThreadPerformOnMainThread

hansemannn · 2017-07-28T19:30:26Z

ios/Classes/ComWilliamrijksenOnesignalModule.m

+                                                                                        @"body" : [[info userInfo] valueForKey:@"body"],
+                                                                                        }];
+            NSLog(@"[DEBUG] com.williamrijksen.onesignal FIRE notificationOpened: %@" , event);
+            [self fireEvent:TiNotificationOpened withObject:event];


Check if the proxy has listeners first, e.g if ([self _hasListener:fireEvent:TiNotificationOpened])

jvandijk · 2017-08-02T09:51:12Z

@hansemannn we've updated the PR by removing the NSNotification approach and replace it with a delegate pattern solution. The code is now way cleaner, but in the end we face the same problem. The event is not fired. Once the fireevent is executed, we end up here:
TiBindingEvent.m#L199
The runloopcount is '0', and the newTarget has no listeners.

Do you have any tips why this could happen? It has do something with context.... but what?

Should we create separate TiProxy based class with its own context to be able to overcome this issue?

jvandijk · 2017-08-02T21:04:49Z

ios/Classes/OneSignalManager.m

+                                       @"title": title,
+                                       @"body": body,
+                                       @"additionalData": additionalData};
+    [self.delegate notificationReceived:notificationData];


@williamrijksen a copy and paste issue here... this should be notificationOpened

jvandijk · 2017-08-02T21:06:47Z

OK the non firing events only seem to be an issue on 5.5.1, not 6.1.1. @chmiiller could you test this PR as well? I have successful results now on 6.1.1.

hansemannn · 2017-08-06T06:23:27Z

@williamrijksen @jvandijk You got me between two holidays.

The reason that the event was not firing (the loopCount was 0, there were no event listeners) is because the proxy did not load up at the state that you trier to communicate with it. That's also the reason we handle background-notifications and background services in the ‘booted‘ delegate and moved from timeouts to that solution recently.

Not 100 % sute if that was in 6.1.x, but the change may also affect / fix other state-related issues like the one you had. Let me know if 6.1.1/6.1.2 works for everyone now, I'll return tomorrow 🏄.

gimdongwoo · 2017-08-07T13:19:15Z

I also tried to build it because travis-ci got an error.

I can not build last commit with error.

[TRACE] [TRACE] ld: warning: directory not found for option '-F/Users/ingrowth/Library/Application Support/Titanium/modules/iphone/com.williamrijksen.onesignal/1.6.1/platform'
[TRACE] [TRACE] Undefined symbols for architecture x86_64:
[TRACE] [TRACE]   "_OBJC_CLASS_$_OneSignalManager", referenced from:
[TRACE] [TRACE]       objc-class-ref in libcom.williamrijksen.onesignal.a(ComWilliamrijksenOnesignalModule.o)
[TRACE] [TRACE] ld: symbol(s) not found for architecture x86_64
[TRACE] [TRACE] clang: error: linker command failed with exit code 1 (use -v to see invocation)
[TRACE] [ERROR] ** BUILD FAILED **
[TRACE] [ERROR] The following build commands failed:
[TRACE] [ERROR]         Ld build/Products/Debug-iphonesimulator/com.williamrijksen.onesignal.app/com.williamrijksen.onesignal normal x86_64
[TRACE] [ERROR] (1 failure)
[ERROR] Failed to run ti build

But, Its one pre-commit is no problem, build is successful.

jvandijk · 2017-08-08T08:38:06Z

@hansemannn thanks for replying / @gimdongwoo thanks for testing. I think I've found a way combining OneSignal and the structure of Titanium that fits. When all listeners are setup, the notification is handled via the OneSignal library.
If this is not the case yet (in cold boot) I'm retrieving the data from the TiApp context.

Unfortunately OneSignal doesn't expose the way they create their notification data structure, so I had to replicate that.

Due to taking this approach, the code became way simpler again. Hopefully this works out for everyone.

chmiiller · 2017-08-08T09:11:02Z

awesome! Looking forward to see it. @gimdongwoo maybe you're not able to build it because of the /platform/OneSignal.framework missing in your "xcodeproj" file. You can open the Xcode Project and import the OneSignal.framework again. This should fix your issue.

First steps into starting onesignal on DidFinishLaunching event

a78fd98

williamrijksen added the Work in progress label Jul 5, 2017

williamrijksen mentioned this pull request Jul 6, 2017

[iOS] handleNotificationReceived when app is closed #7

Closed

Start android onesignal onAppCreate to align with the iOS behaviour

38c4160

Set module instance to use it on notificationreceived/notificationopened

5b4103d

hansemannn reviewed Jul 6, 2017

View reviewed changes

chmiiller mentioned this pull request Jul 11, 2017

iOS 10 Rich Push notifications #13

Open

Use NotificationCenter for notification-callbacks

5dcfa81

williamrijksen commented Jul 11, 2017

View reviewed changes

hansemannn reviewed Jul 12, 2017

View reviewed changes

Added releaseQueuedBootEvents after loading, and remove launch event

31f69b4

hansemannn reviewed Jul 28, 2017

View reviewed changes

Creates a onesignal delegate to fire events

743864c

jvandijk reviewed Aug 2, 2017

View reviewed changes

williamrijksen mentioned this pull request Aug 3, 2017

The application has crashed with an uncaught exception 'NSInvalidArgumentException'. #39

Closed

Also notify on cold boot, by using TiApp and clone of OS payload

5653cb1

jvandijk changed the base branch from master to develop August 8, 2017 08:25

Update version to 1.7.0

6f8b0f4

Jeroen van Dijk added 6 commits August 8, 2017 11:44

Update to OneSignal SDK version 2.5.4

a36169a

Update OneSignal Android version to 3.5.8

44be056

Implement opened event for parity with iOS

4a9a26c

Update version number

1b54905

Add dist for build.properties

07b7ed5

Update travis environment settings

bf28a71

jvandijk mentioned this pull request Aug 9, 2017

The behavior of notificationReceived on Android is the same as iOS. #37

Closed

Jeroen van Dijk added 2 commits August 9, 2017 13:54

Add callback to notification usage prompt

99e4808

Update README

b6cdf87

jvandijk removed the Work in progress label Aug 9, 2017

jvandijk merged commit 6772a0b into develop Aug 9, 2017

This was referenced Aug 9, 2017

NSInvalidArgumentException Crash #28

Closed

App crashes on notification click #34

Closed

jvandijk deleted the feature/fix-invalidargument-exception branch August 10, 2017 09:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First steps into starting onesignal on DidFinishLaunching event #38

First steps into starting onesignal on DidFinishLaunching event #38

williamrijksen commented Jul 5, 2017 •

edited by jvandijk

Loading

chmiiller commented Jul 6, 2017

hansemannn commented Jul 6, 2017 •

edited

Loading

williamrijksen commented Jul 6, 2017

hansemannn Jul 6, 2017

chmiiller commented Jul 7, 2017 •

edited

Loading

williamrijksen commented Jul 10, 2017

hansemannn commented Jul 10, 2017

chmiiller commented Jul 11, 2017 •

edited

Loading

williamrijksen commented Jul 11, 2017

williamrijksen Jul 11, 2017

hansemannn Jul 12, 2017

williamrijksen Jul 12, 2017 •

edited

Loading

chmiiller commented Jul 12, 2017 •

edited

Loading

hansemannn commented Jul 12, 2017

hansemannn Jul 12, 2017

williamrijksen Jul 12, 2017

williamrijksen commented Jul 12, 2017

williamrijksen commented Jul 14, 2017

hansemannn commented Jul 28, 2017

hansemannn Jul 28, 2017

jvandijk Jul 31, 2017

jvandijk Jul 31, 2017

hansemannn Jul 28, 2017

jvandijk commented Aug 2, 2017 •

edited

Loading

jvandijk Aug 2, 2017

jvandijk commented Aug 2, 2017

hansemannn commented Aug 6, 2017 •

edited

Loading

gimdongwoo commented Aug 7, 2017 •

edited

Loading

jvandijk commented Aug 8, 2017

chmiiller commented Aug 8, 2017

First steps into starting onesignal on DidFinishLaunching event #38

First steps into starting onesignal on DidFinishLaunching event #38

Conversation

williamrijksen commented Jul 5, 2017 • edited by jvandijk Loading

Context

What has been done

How to test

Todo

Notes

chmiiller commented Jul 6, 2017

hansemannn commented Jul 6, 2017 • edited Loading

williamrijksen commented Jul 6, 2017

Choose a reason for hiding this comment

chmiiller commented Jul 7, 2017 • edited Loading

williamrijksen commented Jul 10, 2017

hansemannn commented Jul 10, 2017

chmiiller commented Jul 11, 2017 • edited Loading

williamrijksen commented Jul 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamrijksen Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

chmiiller commented Jul 12, 2017 • edited Loading

hansemannn commented Jul 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamrijksen commented Jul 12, 2017

williamrijksen commented Jul 14, 2017

hansemannn commented Jul 28, 2017

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

jvandijk commented Aug 2, 2017 • edited Loading

Choose a reason for hiding this comment

jvandijk commented Aug 2, 2017

hansemannn commented Aug 6, 2017 • edited Loading

gimdongwoo commented Aug 7, 2017 • edited Loading

jvandijk commented Aug 8, 2017

chmiiller commented Aug 8, 2017

williamrijksen commented Jul 5, 2017 •

edited by jvandijk

Loading

hansemannn commented Jul 6, 2017 •

edited

Loading

chmiiller commented Jul 7, 2017 •

edited

Loading

chmiiller commented Jul 11, 2017 •

edited

Loading

williamrijksen Jul 12, 2017 •

edited

Loading

chmiiller commented Jul 12, 2017 •

edited

Loading

jvandijk commented Aug 2, 2017 •

edited

Loading

hansemannn commented Aug 6, 2017 •

edited

Loading

gimdongwoo commented Aug 7, 2017 •

edited

Loading