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

Study concluded screen #177

Merged
merged 5 commits into from
Feb 7, 2025
Merged

Study concluded screen #177

merged 5 commits into from
Feb 7, 2025

Conversation

eldcn
Copy link
Contributor

@eldcn eldcn commented Feb 2, 2025

Study concluded screen

♻️ Current situation & Problem

  • Fixes Handle disabling of users #175
  • Added snapshot listener after in main app screen to render three states: 1. Loading (until the first emission) 2. Normal app screen state if user not disabled. 3. Study concluded otherwise
  • Extended and adapted related unit tests
  • Account settings are still accessible, however I noticed that generation of Health summary fails with: com.google.firebase.functions.FirebaseFunctionsException: User is disabled.
  • I used @PSchmiedmayer account in stage environment since it was already marked as disabled

⚙️ 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.
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 and Contributing Guidelines:

Copy link

codecov bot commented Feb 2, 2025

Codecov Report

Attention: Patch coverage is 43.94904% with 88 lines in your changes missing coverage. Please review.

Project coverage is 40.80%. Comparing base (7fac04d) to head (d4a904d).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...nford/bdh/engagehf/navigation/screens/AppScreen.kt 0.00% 74 Missing ⚠️
...spezi/module/account/manager/UserSessionManager.kt 78.13% 2 Missing and 5 partials ⚠️
.../engagehf/navigation/screens/AppScreenViewModel.kt 89.14% 2 Missing and 3 partials ⚠️
...tlin/edu/stanford/spezi/core/design/theme/Sizes.kt 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #177      +/-   ##
============================================
+ Coverage     40.72%   40.80%   +0.08%     
- Complexity      863      866       +3     
============================================
  Files           299      299              
  Lines         11430    11514      +84     
  Branches       1722     1736      +14     
============================================
+ Hits           4654     4697      +43     
- Misses         6316     6352      +36     
- Partials        460      465       +5     
Flag Coverage Δ
uitests 35.99% <0.00%> (-0.02%) ⬇️
unittests 32.25% <43.95%> (+0.15%) ⬆️

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

Files with missing lines Coverage Δ
...stanford/spezi/module/account/manager/UserState.kt 100.00% <100.00%> (ø)
...tlin/edu/stanford/spezi/core/design/theme/Sizes.kt 8.34% <0.00%> (-1.66%) ⬇️
.../engagehf/navigation/screens/AppScreenViewModel.kt 90.45% <89.14%> (+0.90%) ⬆️
...spezi/module/account/manager/UserSessionManager.kt 76.09% <78.13%> (-0.03%) ⬇️
...nford/bdh/engagehf/navigation/screens/AppScreen.kt 0.00% <0.00%> (ø)

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 7fac04d...d4a904d. Read the comment docs.

@eldcn eldcn self-assigned this Feb 2, 2025
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; looks good to me but I might want to have @pauljohanneskraft or @Basler182 take an other look at it.

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.

Happy to see this merged once @pauljohanneskraft synced up with you about the tests 👍

Copy link
Contributor

@Basler182 Basler182 left a comment

Choose a reason for hiding this comment

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

Looks great. Nice work! 🚀

@eldcn
Copy link
Contributor Author

eldcn commented Feb 7, 2025

Happy to see this merged once @pauljohanneskraft synced up with you about the tests 👍

We just tested with @pauljohanneskraft that the screen updates correctly once we do a live toggle of the disabled flag 👍 Merging this PR then

@PSchmiedmayer
Copy link
Member

Amazing; great news! 🚀

@eldcn eldcn merged commit e32d00a into main Feb 7, 2025
11 checks passed
@eldcn eldcn deleted the task/issue-175 branch February 7, 2025 20:29
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.

Handle disabling of users
4 participants