thoughtbot / ios-on-rails

A guide to building a Rails API and iOS app
Other
77 stars 6 forks source link

Fix +(NSNumber *)userID in HUMUserSession #80

Closed sirdharma closed 10 years ago

sirdharma commented 10 years ago

The fix is to be applied in the book (from which I am writing the iOS code without looking at your codebase).

I believe that the implementation for + (NSNumber *)userID (in HUMUserSession) may have been changed at some point and the code in the book is inconsistent (and actually does not work if followed blindly).

The method returns an (NSNumber *), which triggers a warning in HumRailsAFNClient.m "Incompatible pointer types sending 'NSNumber *' to parameter of type 'NSString *'" at the following line (from chapter "A Rails API Client with AFNetworking"):

[_sharedClient.requestSerializer setValue:[HUMUserSession userID]
                       forHTTPHeaderField:@"X-DEVICE-TOKEN"];

Also, I am quite sure that the following statement in HUMMapViewController.m does not evaluates to the expected behaviour (from chapter "Posting a User with AFNetworking"):

if (![HUMUserSession userID]) {…}

The book's implementation for [HUMUserSession userID] never returns nil.

The same problem appears in + (instancetype)sharedClient in HUMRailsAFNClient.m and in - (instancetype)init in HUMRailsClient.m.

sirdharma commented 10 years ago

I could look at the code in the repository but I would rather not until I am not done with reading the book and implementing the code by myself.

The fix I chose to solve the issue and for minimal changes in the code is the following:

// HUMUserSession.m

+ (NSNumber *)userID
{
    NSString *userIDString = [SSKeychain passwordForService:@"Humon"
                                                    account:@"currentUserID"];

    return userIDString ? [NSNumber numberWithInteger:[userIDString integerValue]] : nil;
}
jessieay commented 10 years ago

Hi @sirdharma - thank you for submitting these issues for iOS on Rails! We really appreciate the input.

We will do our best to incorporate your feedback into the book in order to help future readers. In the meantime, I just wanted to make sure you saw this PR, which was submitted about a month ago: https://github.com/thoughtbot/ios-on-rails/pull/61

It seems like you are using this book as a recipe rather than a guide. It's really cool that you've made a ton of progress without looking at the included code, but I just wanted to set expectations regarding the amount of detail by linking you to that PR. :)

Have a great weekend! :rabbit: :rainbow: :racehorse:

sirdharma commented 10 years ago

Hi @jessieay,

Thank you for your message and please excuse my very late answer. I did read that PR and while I agree with you that the book shall be used as a guide, that is no excuse in my opinion for leaving mistakes in the code (while you may certainly want to omit some details).

Cheers, François

dazmuda commented 10 years ago

@sirdharma Thanks for bringing this up! The discrepancy between types boils down to exactly what you guessed:

1) The rails app now returns a number for the ID and a string for the token. The rails section is all up-to-date. 2) I changed the iOS sample app to save and store these both as strings. The iOS section text was not yet updated for those changes.

This is all fixed in https://github.com/thoughtbot/ios-on-rails/pull/91. I'm going to close this, but if you have more questions please ask!

P.S. That update also includes a change to UserSessions! [HUMUserSession userID] can return nil, but we use the convenience method userIsLoggedIn to judge whether or not we need to make a POST to users now.