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

Added withFreshTokensPerformActionWithAdditionalParameters to OIDAuthState #49

Closed
wants to merge 2 commits into from

Conversation

CraigLn
Copy link
Contributor

@CraigLn CraigLn commented Oct 3, 2016

This PR adds a way to include additional parameters when a token needs to be refreshed. This way previously only achievable by constructing the request, performing the refresh, and updating the auth state manually.


This change is Reviewable

@codecov-io
Copy link

codecov-io commented Oct 3, 2016

Current coverage is 75.27% (diff: 0.00%)

Merging #49 into master will decrease coverage by 0.07%

@@             master        #49   diff @@
==========================================
  Files            33         33          
  Lines          1899       1901     +2   
  Methods         377        378     +1   
  Messages          0          0          
  Branches        101        101          
==========================================
  Hits           1431       1431          
- Misses          423        425     +2   
  Partials         45         45          

Powered by Codecov. Last update ddb7861...cbb7657

@WilliamDenniss
Copy link
Member

Thanks for the pull request @ProjectLane.

Can you eSign the CLA & contributor agreement so we can merge it in?

@CraigLn
Copy link
Contributor Author

CraigLn commented Nov 5, 2016

@WilliamDenniss Sorry for the delay. I just signed the documents, but let me know if anything was missed. Thank you!

@WilliamDenniss
Copy link
Member

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


Source/OIDAuthState.h, line 216 at r1 (raw file):

        thread.
 */
- (void)withFreshTokensPerformActionWithAdditionalParameters:

Can you name this:

performActionWithFreshTokens:(OIDAuthStateAction)action additionalRefreshParameters:(nullable NSDictionary<NSString *, NSString *> *)additionalParameters

Two reasons:

  1. I think it's best to say additional_Refresh_Param to be clear the params relate to the refresh, and
  2. I plan to rename withFreshTokensPerformAction to performActionWithFreshTokens based on some other feedback – may as well not add a new method with the old naming style.

Source/OIDAuthState.m, line 444 at r1 (raw file):

    // refresh the tokens
    OIDTokenRequest *tokenRefreshRequest =
      [self tokenRefreshRequestWithAdditionalParameters:additionalParameters];

This should be indented by 4 space characters not 2, since it's a line wrap.


Comments from Reviewable

@WilliamDenniss
Copy link
Member

Thanks for signing the CLA! I've reviewed your PR, see comments.

@WilliamDenniss
Copy link
Member

Merged as part of #55.

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.

3 participants