-
Notifications
You must be signed in to change notification settings - Fork 65
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
Validate extracted fhir resources while in debug #2874
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2874 +/- ##
========================================
Coverage 27.0% 27.1%
- Complexity 720 733 +13
========================================
Files 269 275 +6
Lines 13517 13756 +239
Branches 2433 2491 +58
========================================
+ Hits 3662 3733 +71
- Misses 9373 9528 +155
- Partials 482 495 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
|
3d36461
to
f8b6965
Compare
android/engine/src/main/java/org/smartregister/fhircore/engine/di/FhirValidatorModule.kt
Outdated
Show resolved
Hide resolved
android/engine/src/main/java/org/smartregister/fhircore/engine/di/FhirValidatorModule.kt
Outdated
Show resolved
Hide resolved
f8b6965
to
33fd86e
Compare
8a36cfd
to
b74de76
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor code change request. Otherwise looks good 👍
android/engine/src/main/java/org/smartregister/fhircore/engine/di/FhirValidatorModule.kt
Outdated
Show resolved
Hide resolved
b74de76
to
1cf49f6
Compare
Getting this error
|
fb327b9
to
27d7949
Compare
882883f
to
f3fff5e
Compare
hi @LZRS is the only remaining issue that the error dialogs are not great? I think we should get this merged and have that be in a cleanup issue cc @ellykits @ndegwamartin |
Yeah it is the only remaining issue. I can finish up on the tests and have it as ready for review |
f315d06
to
3ca54c0
Compare
3ca54c0
to
e093834
Compare
69aeb05
to
452efa7
Compare
...est/src/main/java/org/smartregister/fhircore/quest/ui/questionnaire/QuestionnaireActivity.kt
Outdated
Show resolved
Hide resolved
android/engine/src/main/java/org/smartregister/fhircore/engine/di/FhirValidatorModule.kt
Outdated
Show resolved
Hide resolved
...ine/src/main/java/org/smartregister/fhircore/engine/util/extension/FhirValidatorExtension.kt
Outdated
Show resolved
Hide resolved
vararg resource: Resource, | ||
isDebug: Boolean = BuildConfig.BUILD_TYPE.contains("debug", ignoreCase = true), | ||
): List<ValidationResult> { | ||
if (!isDebug) return emptyList() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean validation will only be triggered for debug apks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it would only be triggered for debug because the performance implications it might have for non-debug apps
...ine/src/main/java/org/smartregister/fhircore/engine/util/extension/FhirValidatorExtension.kt
Outdated
Show resolved
Hide resolved
...est/src/main/java/org/smartregister/fhircore/quest/ui/questionnaire/QuestionnaireActivity.kt
Outdated
Show resolved
Hide resolved
Recommending we strip out the UI stuff in this version, see my comment here. |
dc4c215
to
5e59ef5
Compare
@LZRS Create a follow-up GitHub issue based on @ndegwamartin's comment below.
|
* Validate extracted fhir resources while in debug * Format validation error messages * Show validation result in dialog after submission * Use macos-13 for ci job runner * Fix failing QuestionnaireViewModelTest tests * add missing paren * Refactor to just send FhirValidator error messages to logcat --------- Co-authored-by: Peter Lubell-Doughtie <[email protected]> Co-authored-by: Sebastian <[email protected]> Co-authored-by: Benjamin Mwalimu <[email protected]> Co-authored-by: Elly Kitoto <[email protected]>
IMPORTANT: Where possible all PRs must be linked to a Github issue
Depends on #2825
Fixes #2853
Engineer Checklist
strings.xml
file./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the project's style guideCode Reviewer Checklist
strings.xml
file