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

MLIBZ-1502: logout async #237

Merged
merged 3 commits into from Sep 8, 2017

Conversation

Projects
None yet
3 participants
@heyzooi
Contributor

heyzooi commented Aug 31, 2017

Description

Make the logout() async which will end up invalidating the current auth token

Changes

  • Adding optional parameters to the logout() method

Tests

  • Unit testing mocking the response from the server

@heyzooi heyzooi self-assigned this Aug 31, 2017

@heyzooi heyzooi requested a review from tejasranade Aug 31, 2017

{
fulfill()
} else {
reject(error ?? buildError(data, response, error, self.client))

This comment has been minimized.

@tejasranade

tejasranade Sep 5, 2017

Member

@heyzooi two suggestions -
1 - let's bring this up in standup and check what we're doing on the other platforms for logout failures... does an error in the network operation get reported as an error, or is it still a success (since we logged the user out locally)?
2 - please make sure the docs describe the failure case properly.

@tejasranade

tejasranade Sep 5, 2017

Member

@heyzooi two suggestions -
1 - let's bring this up in standup and check what we're doing on the other platforms for logout failures... does an error in the network operation get reported as an error, or is it still a success (since we logged the user out locally)?
2 - please make sure the docs describe the failure case properly.

This comment has been minimized.

@heyzooi

heyzooi Sep 7, 2017

Contributor

@tejasranade as discussed in our standup, the current implementation is the expected behaviour

@heyzooi

heyzooi Sep 7, 2017

Contributor

@tejasranade as discussed in our standup, the current implementation is the expected behaviour

@@ -3865,6 +3865,40 @@ extension UserTests {
}
}
func testLogout() {

This comment has been minimized.

@tejasranade

tejasranade Sep 5, 2017

Member

It'll be useful to test the case where the logout network call fails.

@tejasranade

tejasranade Sep 5, 2017

Member

It'll be useful to test the case where the logout network call fails.

This comment has been minimized.

@heyzooi

heyzooi Sep 7, 2017

Contributor

@tejasranade additional unit tests were added

@heyzooi

heyzooi Sep 7, 2017

Contributor

@tejasranade additional unit tests were added

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Sep 6, 2017

Codecov Report

Merging #237 into develop will increase coverage by 0.26%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #237      +/-   ##
===========================================
+ Coverage    90.78%   91.04%   +0.26%     
===========================================
  Files           65       65              
  Lines         7692     7860     +168     
===========================================
+ Hits          6983     7156     +173     
+ Misses         709      704       -5
Flag Coverage Δ
#Mac 88.39% <76.66%> (+0.41%) ⬆️
#iOS 89.91% <100%> (+0.28%) ⬆️
Impacted Files Coverage Δ
Kinvey/Kinvey/RequestFactory.swift 100% <ø> (ø) ⬆️
Kinvey/Kinvey/Endpoint.swift 96.96% <100%> (+0.04%) ⬆️
Kinvey/Kinvey/User.swift 97.03% <100%> (+0.06%) ⬆️
Kinvey/Kinvey/HttpRequestFactory.swift 99.57% <100%> (ø) ⬆️
Kinvey/Kinvey/Error.swift 91.66% <0%> (-1.34%) ⬇️
Kinvey/Kinvey/Kinvey.swift 100% <0%> (ø) ⬆️
Kinvey/Kinvey/PushOperation.swift 89.71% <0%> (+0.94%) ⬆️
Kinvey/Kinvey/Persistable.swift 83.27% <0%> (+1.31%) ⬆️
Kinvey/Kinvey/Realtime.swift 74.62% <0%> (+1.49%) ⬆️
Kinvey/Kinvey/RealmCache.swift 89.58% <0%> (+2.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b11f3f9...dd27be3. Read the comment docs.

codecov-io commented Sep 6, 2017

Codecov Report

Merging #237 into develop will increase coverage by 0.26%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #237      +/-   ##
===========================================
+ Coverage    90.78%   91.04%   +0.26%     
===========================================
  Files           65       65              
  Lines         7692     7860     +168     
===========================================
+ Hits          6983     7156     +173     
+ Misses         709      704       -5
Flag Coverage Δ
#Mac 88.39% <76.66%> (+0.41%) ⬆️
#iOS 89.91% <100%> (+0.28%) ⬆️
Impacted Files Coverage Δ
Kinvey/Kinvey/RequestFactory.swift 100% <ø> (ø) ⬆️
Kinvey/Kinvey/Endpoint.swift 96.96% <100%> (+0.04%) ⬆️
Kinvey/Kinvey/User.swift 97.03% <100%> (+0.06%) ⬆️
Kinvey/Kinvey/HttpRequestFactory.swift 99.57% <100%> (ø) ⬆️
Kinvey/Kinvey/Error.swift 91.66% <0%> (-1.34%) ⬇️
Kinvey/Kinvey/Kinvey.swift 100% <0%> (ø) ⬆️
Kinvey/Kinvey/PushOperation.swift 89.71% <0%> (+0.94%) ⬆️
Kinvey/Kinvey/Persistable.swift 83.27% <0%> (+1.31%) ⬆️
Kinvey/Kinvey/Realtime.swift 74.62% <0%> (+1.49%) ⬆️
Kinvey/Kinvey/RealmCache.swift 89.58% <0%> (+2.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b11f3f9...dd27be3. Read the comment docs.

@tejasranade

LGTM

@tejasranade tejasranade merged commit 15a36af into develop Sep 8, 2017

2 of 3 checks passed

codebeat 7 issues resolved and 8 introduced
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@tejasranade tejasranade deleted the feature/MLIBZ-1502-logout_async branch Sep 8, 2017

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