Fixes the app showing a "Sync Complete" message even on sync failure #2599

ndegwamartin · 2023-07-18T06:34:51Z

IMPORTANT: Where possible all PRs must be linked to a Github issue

Closes #2575

Additionally Includes:

Fix Missing Manifest Resources on Setup
Enhance Sync Progress View logic
Fix NON PROXY flag incorrect state in dev mode
Improve UX when launching Insights

Engineer Checklist

I have written Unit tests for any new feature(s) and edge cases for bug fixes
I have added any strings visible on UI components to the strings.xml file
I have updated the CHANGELOG.md file for any notable changes to the codebase
I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the project's style guide
I have built and run the FHIRCore app to verify my change fixes the issue and/or does not break the app
I have checked that this PR does NOT introduce breaking changes that require an update to Content and/or Configs? If it does add a sample here or a link to exactly what changes need to be made to the content.

Code Reviewer Checklist

I have verified Unit tests have been written for any new feature(s) and edge cases
I have verified any strings visible on UI components are in the strings.xml file
I have verifed the CHANGELOG.md file has any notable changes to the codebase
I have verified the solution has been implemented in a configurable and generic way for reuseable components
I have built and run the FHIRCore app to verify the change fixes the issue and/or does not break the app

- Add progress indicator

codecov · 2023-07-18T07:00:09Z

Codecov Report

Merging #2599 (efd8464) into main (d84c2c8) will increase coverage by 0.2%.
The diff coverage is 67.8%.

@@            Coverage Diff            @@
##              main   #2599     +/-   ##
=========================================
+ Coverage     65.4%   65.7%   +0.2%     
- Complexity    1101    1150     +49     
=========================================
  Files          213     216      +3     
  Lines         9385    9693    +308     
  Branches      1866    1927     +61     
=========================================
+ Hits          6140    6370    +230     
- Misses        2047    2103     +56     
- Partials      1198    1220     +22

Flag	Coverage Δ
engine	`73.1% <70.4%> (+<0.1%)`	⬆️
geowidget	`62.9% <37.5%> (-0.3%)`	⬇️

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

Impacted Files	Coverage Δ
...ter/fhircore/engine/configuration/Configuration.kt	`25.0% <ø> (ø)`
...ngine/configuration/event/EventTriggerCondition.kt	`50.0% <0.0%> (ø)`
...ircore/engine/configuration/event/EventWorkflow.kt	`33.3% <0.0%> (ø)`
...ngine/configuration/view/PersonalDataProperties.kt	`0.0% <0.0%> (ø)`
...hircore/engine/configuration/view/RowProperties.kt	`0.0% <ø> (ø)`
...ine/configuration/view/ViewPropertiesSerializer.kt	`23.8% <0.0%> (ø)`
...data/remote/fhir/resource/FhirResourceConverter.kt	`100.0% <ø> (ø)`
...e/data/remote/fhir/resource/FhirResourceService.kt	`0.0% <0.0%> (ø)`
...hircore/engine/domain/model/FhirResourceConfigs.kt	`44.4% <0.0%> (ø)`
...rtregister/fhircore/engine/sync/SyncBroadcaster.kt	`33.3% <0.0%> (ø)`
... and 149 more

... and 17 files with indirect coverage changes

Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com>

- Add IS NON PROXY flag documentation

pld · 2023-07-18T13:08:41Z

...ngine/src/main/java/org/smartregister/fhircore/engine/configuration/ConfigurationRegistry.kt

-        create(resource)
+        try {
+          create(resource)
+        } catch (e: Exception) {


what type of exceptions are getting thrown here, can we make this more specific? let's add a test for this try/catch too

...ngine/src/main/java/org/smartregister/fhircore/engine/configuration/ConfigurationRegistry.kt

Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com>

pld

I'm going to try and add some more tests for this, I will get it merged before my EOD

pld · 2023-07-18T16:40:33Z

...id/quest/src/main/java/org/smartregister/fhircore/quest/ui/appsetting/AppSettingViewModel.kt

-                    val config =
-                      it.tryDecodeJson<RegisterConfiguration>()
-                        ?: it.tryDecodeJson<ProfileConfiguration>()
-
-                    when (config) {
-                      is RegisterConfiguration ->
-                        config.fhirResource.dependentResourceTypes(patientRelatedResourceTypes)
-                      is ProfileConfiguration ->
-                        config.fhirResource.dependentResourceTypes(patientRelatedResourceTypes)
+                    val registerConfig = it.tryDecodeJson<RegisterConfiguration>()
+                    if (registerConfig != null) {
+                      if (registerConfig.configType == ConfigType.Profile.name) {
+                        val profileConfig = it.tryDecodeJson<ProfileConfiguration>()
+                        profileConfig
+                          ?.fhirResource
+                          ?.dependentResourceTypes(
+                            patientRelatedResourceTypes,
+                          )
+                      } else {
+                        registerConfig.fhirResource.dependentResourceTypes(
+                          patientRelatedResourceTypes,
+                        )
+                      }


because we are ignoring unknown keys, as it was written it always decodes as RegisterConfiguration

with the new implementation looks like we might experience a crash if registerConfig was actually ever null

actually scratch that, the if-else closes correctly

maybe, it would be a pre-existing issue since that's the same as config getting set to null in the previous version, I think we have a test for that, but it might be passing b/c of mocks

* main: Add View and Row widget documentation (#2614) Document spacer and divider widgets. (#2612) Add the Practitioner selection screen on the In-app reporting workflow (#2605) Add enableFamilyRegistration test for GeoWidgetfragment (#2607) Improved UX for non proxy mode development 🔨 Update docs - added sync insights (#2600) Fixes the app showing a "Sync Complete" message even on sync failure (#2599) Retrieve related resources without necessarily having to pass a reference. (#2595) Add tests to verify LoginScreen error states (#2589) Rw docs update (#2588) Manifest Binaries request batching (#2583) remove about section (#2587) Pld doc updates (#2584) fix cname (#2586) Add Encounter_Location type (#2585) passing empty bundle instead of null to trigger the careplan when we don't have any bundle resource (#2432) Configure resource id for profile launch (#2581)

…2599) * Fix Missing Manifest Resources on Setup 🐛 * Enhance Sync Progress View logic * Fix NON PROXY flag incorrect state in dev mode * Improve UX when launching Insights - Add progress indicator * Fix bug: Sync Complete shown even after sync failed * Bump up Engine Artifact Version * Fix failing engine module tests Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> * Fix Unit Tests 💚 - Add IS NON PROXY flag documentation * Catch SQLException also rename String constant Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> * test passing params * fix deserialization bug and add test * spotless * add button test * test high error status * properties tests --------- Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> Co-authored-by: Elly Kitoto <junkmailstoelly@gmail.com> Co-authored-by: pld <peter@ona.io>

ndegwamartin added 5 commits July 18, 2023 08:40

Fix Missing Manifest Resources on Setup 🐛

a908cf4

Enhance Sync Progress View logic

4cccce1

Fix NON PROXY flag incorrect state in dev mode

59f871d

Improve UX when launching Insights

e9e4d35

- Add progress indicator

Fix bug: Sync Complete shown even after sync failed

561a398

ndegwamartin and others added 2 commits July 18, 2023 11:34

Bump up Engine Artifact Version

6d1f020

Fix failing engine module tests

f346e3c

Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com>

ellykits changed the title ~~Issue 2575~~ Fixes the app showing a "Sync Complete" message even on sync failure Jul 18, 2023

Fix Unit Tests 💚

ca1fb91

- Add IS NON PROXY flag documentation

ndegwamartin force-pushed the issue-2575-main branch from 07fc29a to ca1fb91 Compare July 18, 2023 12:24

pld reviewed Jul 18, 2023

View reviewed changes

Catch SQLException also rename String constant

c706c9f

Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com>

ellykits previously approved these changes Jul 18, 2023

View reviewed changes

pld requested changes Jul 18, 2023

View reviewed changes

pld self-assigned this Jul 18, 2023

pld added 2 commits July 18, 2023 11:40

test passing params

899d2db

fix deserialization bug and add test

fa9b6e6

pld dismissed ellykits’s stale review via fa9b6e6 July 18, 2023 16:39

pld reviewed Jul 18, 2023

View reviewed changes

pld added 4 commits July 18, 2023 12:58

spotless

0968769

add button test

03aaf6c

test high error status

40d1f17

properties tests

efd8464

pld approved these changes Jul 18, 2023

View reviewed changes

pld merged commit a496ce9 into main Jul 18, 2023

pld deleted the issue-2575-main branch July 18, 2023 19:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes the app showing a "Sync Complete" message even on sync failure #2599

Fixes the app showing a "Sync Complete" message even on sync failure #2599

ndegwamartin commented Jul 18, 2023

codecov bot commented Jul 18, 2023 •

edited

Loading

pld Jul 18, 2023

pld left a comment

pld Jul 18, 2023

ndegwamartin Jul 19, 2023

ndegwamartin Jul 19, 2023

pld Jul 19, 2023

Fixes the app showing a "Sync Complete" message even on sync failure #2599

Fixes the app showing a "Sync Complete" message even on sync failure #2599

Conversation

ndegwamartin commented Jul 18, 2023

codecov bot commented Jul 18, 2023 • edited Loading

Codecov Report

pld Jul 18, 2023

Choose a reason for hiding this comment

pld left a comment

Choose a reason for hiding this comment

pld Jul 18, 2023

Choose a reason for hiding this comment

ndegwamartin Jul 19, 2023

Choose a reason for hiding this comment

ndegwamartin Jul 19, 2023

Choose a reason for hiding this comment

pld Jul 19, 2023

Choose a reason for hiding this comment

codecov bot commented Jul 18, 2023 •

edited

Loading