Unregister from application launch notifications #29

Merged
merged 4 commits into from May 7, 2013

Conversation

Projects
None yet
4 participants
Contributor

eriksundin commented Mar 6, 2013

I ran into problems when using the AFOAuth1Client in a process where there is log-out functionality in the application and the authentication process needs to be restarted.
The client never unregisters from the kAFApplicationLaunchedWithURLNotification notification, getting the result of multiple observation blocks getting executed.

Added functionality

  • unregisters from the notification inside the notification block.
  • handle the case where the user cancels an auth flow and returns to the app:
    -- unregister any previous registered notification block before registering a new one
    -- unregister in dealloc to fullfil the contract to addObserverForName:object:queue:usingBlock.

yelled3 commented Mar 18, 2013

looks great - I was just about to write it :-)
merge?

@yelled3 yelled3 referenced this pull request in OldPodsHome/SHAccountManager Mar 18, 2013

Closed

Unregister from application launch notifications #1

Contributor

seivan commented Mar 27, 2013

👍
Nicely done.

The method name should have an indicator that it's about observers. As far as we're concerned, there is no such thing as registerToApplicationLaunchNotifications
The private property is called applicationLaunchNotificationObserver.

Also I think starting bracers should be on a new line :) (Like the rest of the application).
Look at your dealloc.
I personally prefer this style (I even add semicolons at the end of a selector name like the interface).

Contributor

eriksundin commented Mar 27, 2013

Good points @seivan. The naming of the unregister method is not really clear, I used removeApplicationLaunchNotificationObserver instead to clearly reference the observer.

Also fixed the braces :-)

@mattt mattt merged commit aa789ce into AFNetworking:master May 7, 2013

Contributor

mattt commented May 7, 2013

Merged! Thanks, @eriksundin! Tagging a new release now. Really happy to have this in for 0.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment