-
Notifications
You must be signed in to change notification settings - Fork 22
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
[#228] Add TokenAuthenticator to template-xml #382
Conversation
Kover report for template-xml:🧛 Template - XML Unit Tests Code Coverage:
|
File | Coverage |
---|---|
ApiServiceProvider.kt |
0.00% |
ApplicationRequestAuthenticator.kt |
0.00% |
AuthStatus.kt |
0.00% |
AuthenticateResponse.kt |
0.00% |
Error.kt |
100.00% |
ErrorResponse.kt |
100.00% |
ResponseMapping.kt |
71.70% |
SessionManagerImpl.kt |
0.00% |
TokenRefresherImpl.kt |
0.00% |
Modified Files Not Found In Coverage Report:
AuthModule.kt
AuthService.kt
Authenticate.kt
MockUtil.kt
OkHttpClientModule.kt
RetrofitModule.kt
SessionManager.kt
TokenRefresher.kt
Codebase cunningly covered by count Shroud 🧛
Kover report for template-compose:
🧛 Template - Compose Unit Tests Code Coverage: 14.62%
Coverage of Modified Files:
File | Coverage |
---|---|
ApiServiceProvider.kt |
0.00% |
Error.kt |
100.00% |
ErrorResponse.kt |
100.00% |
ResponseMapping.kt |
87.69% |
Modified Files Not Found In Coverage Report:
ApplicationRequestAuthenticator.kt
AuthModule.kt
AuthService.kt
AuthStatus.kt
Authenticate.kt
AuthenticateResponse.kt
MockUtil.kt
OkHttpClientModule.kt
RetrofitModule.kt
SessionManager.kt
SessionManagerImpl.kt
TokenRefresher.kt
TokenRefresherImpl.kt
Codebase cunningly covered by count Shroud 🧛
Generated by 🚫 Danger
275d6f3
to
a0a9a43
Compare
a0a9a43
to
c79ef76
Compare
@luongvo @kaungkhantsoe I think let's postpone this for now, this issue is a lot bigger than I originally estimated. It's best to create another issue first, to integrate a new API that works with a (refresh) token. Because our current API doesn't have that. What do you think? 🙏 |
@Tuubz Do you mean to integrate an API into the template? It might be a better idea to add that to the sample projects instead (perhaps the survey API?) and keep only the authenticator stuff in the template, what do you think? |
@ryan-conway Sorry for the late reply, but yes, you're correct. 👍 Please have a look at this thread: https://nimble-co.slack.com/archives/G41UCTGHL/p1614156128022800 IMO, I think we should go with GitHub API (convenient) or create a Mock API with Postman (flexible). I'll create an RFC for this, what do you think? 🙏 |
@Tuubz An RFC sounds like a good idea 😄 |
@ryan-conway @kaungkhantsoe RFC created: #434 🚀 |
What happened 👀
Provide a description of the changes this pull request brings to the codebase. Additionally, when the pull request is still being worked on, a checklist of the planned changes is welcome to track progress.
Insight 📝
Describe in detail why this solution is the most appropriate, which solution you tried but did not go with, and how to test the changes. References to relevant documentation are welcome as well.
Proof Of Work 📹
Show us the implementation: screenshots, GIFs, etc.