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 auth state listener to directly navigate to home when logged in #50

Merged
merged 8 commits into from
Jul 8, 2024

Conversation

Basler182
Copy link
Contributor

auth state listener

♻️ Current situation & Problem

#37

⚙️ Release Notes

  • added auth state listener to directly navigate to home when logged in

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@Basler182 Basler182 self-assigned this Jun 23, 2024
@Basler182 Basler182 added the enhancement New feature or request label Jun 23, 2024
Copy link

codecov bot commented Jun 23, 2024

Codecov Report

Attention: Patch coverage is 73.91304% with 24 lines in your changes missing coverage. Please review.

Project coverage is 44.61%. Comparing base (1c2559c) to head (9fff5d9).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #50      +/-   ##
============================================
+ Coverage     43.67%   44.61%   +0.95%     
- Complexity      310      318       +8     
============================================
  Files           125      125              
  Lines          3813     3829      +16     
  Branches        552      558       +6     
============================================
+ Hits           1665     1708      +43     
+ Misses         1999     1970      -29     
- Partials        149      151       +2     
Flag Coverage Δ
uitests 35.90% <26.32%> (+0.43%) ⬆️
unittests 28.25% <70.66%> (+1.53%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...edu/stanford/bdh/engagehf/MainActivityViewModel.kt 100.00% <100.00%> (+9.31%) ⬆️
...dh/engagehf/education/EngageEducationRepository.kt 0.00% <ø> (ø)
...cation/VideoSectionDocumentToVideoSectionMapper.kt 0.00% <ø> (ø)
...nford/bdh/engagehf/navigation/screens/AppScreen.kt 72.23% <100.00%> (+0.53%) ⬆️
...rd/bdh/engagehf/onboarding/EngageConsentManager.kt 100.00% <100.00%> (+45.46%) ⬆️
.../spezi/core/bluetooth/domain/BLEDeviceConnector.kt 88.24% <ø> (ø)
...nford/spezi/module/account/login/LoginViewModel.kt 40.00% <ø> (ø)
...e/account/manager/FirebaseInvitationAuthManager.kt 0.00% <ø> (ø)
...spezi/module/account/manager/UserSessionManager.kt 100.00% <100.00%> (ø)
...stanford/spezi/module/account/manager/UserState.kt 100.00% <100.00%> (ø)
... and 9 more

Continue to review full report in Codecov by Sentry.

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

@Basler182 Basler182 marked this pull request as draft June 25, 2024 21:22
@PSchmiedmayer PSchmiedmayer removed their request for review June 28, 2024 17:45
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this @Basler182; I would defer to @eldcn and @pauljohanneskraft for a detailed review 👍

@Basler182
Copy link
Contributor Author

Thank you for working on this @Basler182; I would defer to @eldcn and @pauljohanneskraft for a detailed review 👍

i spoke to @eldcn earlier. he is working on the changes we discussed on tuesday and is removing the firebase auth module from the app. he will then merge the changes into this branch and then we can merge everything together into the master 🚀

@PSchmiedmayer
Copy link
Member

Sounds great; thank you @Basler182 and @eldcn!

@eldcn
Copy link
Contributor

eldcn commented Jul 1, 2024

@Basler182, @PSchmiedmayer , @pauljohanneskraft I just merged #55 pointing to this branch 🙌

# Firebase dependencies in Account module

## ♻️ Current situation & Problem
Moves all firebase dependencies in account module, and exposes needed
information and APIs from other modules via custom components. This way,
consumers of `account` module, do not need to include firebase
dependencies additionally in case they use `account`. Firebase remains
this way as an internal implementation detail, that can be easily
replaced without affecting other modules and minimizing breaking changes
in regard to backwards compatibility


## ⚙️ Release Notes 
*Add a bullet point list summary of the feature and possible migration
guides if this is a breaking change so this section can be added to the
release notes.*
*Include code snippets that provide examples of the feature implemented
or links to the documentation if it appends or changes the public
interface.*


## 📚 Documentation
*Please ensure that you properly document any additions in conformance
to [Spezi Documentation
Guide](https://github.com/StanfordSpezi/.github/blob/main/DOCUMENTATIONGUIDE.md).*
*You can use this section to describe your solution, but we encourage
contributors to document your reasoning and changes using in-line
documentation.*


## ✅ Testing
*Please ensure that the PR meets the testing requirements set by CodeCov
and that new functionality is appropriately tested.*
*This section describes important information about the tests and why
some elements might not be testable.*


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [ ] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
@eldcn eldcn marked this pull request as ready for review July 1, 2024 19:51
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @eldcn for the improvements and working on this feature!

Feel free to sync merging this PR with @Basler182 to get this PR merged; thanks to the two of you to get this in place 🚀

@eldcn eldcn mentioned this pull request Jul 6, 2024
1 task
eldcn and others added 4 commits July 8, 2024 19:51
# Conflicts:
#	app/src/main/kotlin/edu/stanford/bdh/engagehf/MainActivityViewModel.kt
#	app/src/main/kotlin/edu/stanford/bdh/engagehf/onboarding/EngageConsentManager.kt
#	app/src/test/kotlin/edu/stanford/bdh/engagehf/MainActivityViewModelTest.kt
Signed-off-by: Basler182 <[email protected]>
@eldcn eldcn merged commit d2701ef into main Jul 8, 2024
11 checks passed
@eldcn eldcn deleted the feature/issue-37 branch July 8, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants