Fix refreshing profile data #2523

ellykits · 2023-07-04T12:37:47Z

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

Fixes https://github.com/opensrp/fhir-resources/issues/1940
Fixes https://github.com/opensrp/fhir-resources/issues/1695

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

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

codecov · 2023-07-04T13:03:35Z

Codecov Report

Merging #2523 (001628c) into main (2ea8fa1) will increase coverage by 0.5%.
The diff coverage is 65.8%.

@@            Coverage Diff            @@
##              main   #2523     +/-   ##
=========================================
+ Coverage     64.8%   65.4%   +0.5%     
- Complexity    1050    1095     +45     
=========================================
  Files          209     213      +4     
  Lines         9156    9344    +188     
  Branches      1805    1860     +55     
=========================================
+ Hits          5942    6111    +169     
+ Misses        2059    2038     -21     
- Partials      1155    1195     +40

Flag	Coverage Δ
engine	`73.1% <65.5%> (+<0.1%)`	⬆️
geowidget	`63.2% <ø> (ø)`
quest	`60.3% <66.1%> (+0.8%)`	⬆️

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

Impacted Files	Coverage Δ
...core/engine/configuration/view/ColumnProperties.kt	`100.0% <ø> (ø)`
...ore/engine/configuration/view/DividerProperties.kt	`0.0% <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%> (-2.6%)`	⬇️
...e/engine/ui/bottomsheet/RegisterBottomSheetView.kt	`51.2% <0.0%> (-2.5%)`	⬇️
...g/smartregister/fhircore/quest/QuestApplication.kt	`44.2% <ø> (+0.8%)`	⬆️
...org/smartregister/fhircore/quest/event/AppEvent.kt	`100.0% <ø> (ø)`
...g/smartregister/fhircore/quest/event/EventQueue.kt	`100.0% <ø> (ø)`
...r/fhircore/quest/ui/shared/QuestionnaireHandler.kt	`12.5% <ø> (+12.5%)`	⬆️
... and 52 more

... and 5 files with indirect coverage changes

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

ellykits · 2023-07-05T15:34:54Z

Breaking config changes

Removed

property: "refreshContent"
Affected config: QuestionnaireConfig

Added

Affected config: QuestionnaireConfig
New property: onSubmitActions
New property type: List<ActionConfig>

Sample

             "onSubmitActions": [
                  {
                    "trigger": "ON_QUESTIONNAIRE_SUBMISSION",
                    "workflow": "LAUNCH_REGISTER",
                    "id": "householdRegister",
                    "display": "All Households",
                    "popNavigationBackStack": true
                  }
            ]

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

pld

lgtm! test failure looks related to missing fun call mock on eventBus

android/engine/src/main/java/org/smartregister/fhircore/engine/task/FhirTaskUtil.kt

pld · 2023-07-05T19:50:18Z

android/engine/src/main/java/org/smartregister/fhircore/engine/task/FhirTaskUtil.kt

+   * Check in the task is part of another task and if so check if the parent task is
+   * completed,cancelled,failed or entered in error.
+   */
+  private suspend fun Task.preReqConditionSatisfied() =


I think we can rename this fun and consolidate the precondition

Suggested change

private suspend fun Task.preReqConditionSatisfied() =

private suspend fun Task.preconditionSatisfied() =

this.executionStartIsBeforeOrToday() &&

this.status == Task.TaskStatus.REQUESTED &&

pld · 2023-07-05T19:53:48Z

android/engine/src/main/java/org/smartregister/fhircore/engine/task/FhirTaskUtil.kt

+      if (task.executionStartIsBeforeOrToday() &&
+          task.status == Task.TaskStatus.REQUESTED &&
+          task.preReqConditionSatisfied()
+      ) {


why not also consider the above 2 conditions part of the precondition, and add them to that function?

Suggested change

if (task.executionStartIsBeforeOrToday() &&

task.status == Task.TaskStatus.REQUESTED &&

task.preReqConditionSatisfied()

) {

if task.preReqConditionSatisfied() {

I see. I think the conditions here are breaking the functionality for marking Tasks as ready. The conditions for updating the dependent Tasks should be separate from the parent Task.

...d/engine/src/main/java/org/smartregister/fhircore/engine/util/extension/ResourceExtension.kt

android/quest/src/main/java/org/smartregister/fhircore/quest/ui/profile/ProfileFragment.kt

…i/profile/ProfileFragment.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

…/util/extension/ResourceExtension.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

…/task/FhirTaskUtil.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

- Fix Failing tests - Add unit tests ✅

ndegwamartin · 2023-07-06T17:14:12Z

@dubdabasoduba before we split the PR for the 0.2.4 branch could you please confirm that the logic for updating task status is correct?

From the logic seems like we:

Look for any tasks that are due with a task status of REQUESTED
Look for the parent task they are a partOf
If the parent task is found and it as the status CANCELLED, COMPLETE, FAILED or ENTEREDINERROR then we mark the (child)task status as READY

Note: The above logic was not added by this PR just refactored into it.

Also please review the code to confirm my assertions above.

dubdabasoduba · 2023-07-06T18:28:51Z

@dubdabasoduba before we split the PR for the 0.2.4 branch could you please confirm that the logic for updating task status is correct?

From the logic seems like we:

Look for any tasks that are due with a task status of REQUESTED

Look for the parent task they are a partOf

If the parent task is found and it as the status CANCELLED, COMPLETE, FAILED or ENTEREDINERROR then we mark the (child)task status as READY

Note: The above logic was not added by this PR just refactored into it.

Also please review the code to confirm my assertions above.

@ndegwamartin this is correct. We might want to check in with the PMs on enabling dependent tasks when the pre-requisite was CANCELLED, FAILED or ENTEREDINERROR

dubdabasoduba · 2023-07-06T21:39:08Z

@ndegwamartin this works well.

The above config allows redirecting to a register.
- It however breaks with redirecting to a profile.
The Profile refreshes correctly.

This PR is also needed to avoid a crash on remove family functionality

ellykits · 2023-07-07T07:46:29Z

@ndegwamartin this works well.

The above config allows redirecting to a register.

It however breaks with redirecting to a profile.

The Profile refreshes correctly.

This PR is also needed to avoid a crash on remove family functionality

Could you let me know which profile you tried opening here? if the profile is related to a household it would work with the Group resource being the base. Otherwise, the ResourceData would be null (ResourceData is required to open the profile).

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

ellykits · 2023-07-07T13:33:41Z

@dubdabasoduba I deleted the conditional checks that relied on the dependent Tasks, for which I needed confirmation from you.

* Fix refreshing profile data Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> * Resolve list view not refreshing data Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> * Handle questionnaire on submission actions Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> * Fix failing tests Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> * Update android/quest/src/main/java/org/smartregister/fhircore/quest/ui/profile/ProfileFragment.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io> * Update android/engine/src/main/java/org/smartregister/fhircore/engine/util/extension/ResourceExtension.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io> * Update android/engine/src/main/java/org/smartregister/fhircore/engine/util/extension/ResourceExtension.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io> * Update android/engine/src/main/java/org/smartregister/fhircore/engine/task/FhirTaskUtil.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io> * Fix Build 💚 - Fix Failing tests - Add unit tests ✅ * Fix Build 💚 * Fix updating Task status to READY Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> --------- Signed-off-by: Elly Kitoto <junkmailstoelly@gmail.com> Co-authored-by: Peter Lubell-Doughtie <peter@ona.io> Co-authored-by: Martin Ndegwa <ndegwamartin@users.noreply.github.com> Co-authored-by: Martin Ndegwa <mndegwa@ona.io>

Fix refreshing profile data

85b74c2

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

ellykits added Work In Progress Describes an items that is not complete. Mostly used for scoping issues of discussions DNM DO NOT MERGE 0.2.4 labels Jul 4, 2023

ellykits added 3 commits July 4, 2023 16:09

Resolve list view not refreshing data

f2db662

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

Merge branch 'main' into fix-profile-data-refresh

4b6a099

Handle questionnaire on submission actions

8178273

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

ellykits added 2 commits July 5, 2023 18:35

Merge branch 'main' into fix-profile-data-refresh

31dd23b

Fix failing tests

ff097b1

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

ellykits marked this pull request as ready for review July 5, 2023 16:10

ellykits removed the DNM DO NOT MERGE label Jul 5, 2023

pld added 2 commits July 5, 2023 13:10

Merge branch 'main' into fix-profile-data-refresh

7f92a82

Merge branch 'main' into fix-profile-data-refresh

358922a

pld reviewed Jul 5, 2023

View reviewed changes

ndegwamartin and others added 5 commits July 6, 2023 14:19

Update android/quest/src/main/java/org/smartregister/fhircore/quest/u…

6a43063

…i/profile/ProfileFragment.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

Update android/engine/src/main/java/org/smartregister/fhircore/engine…

dbef611

…/util/extension/ResourceExtension.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

Update android/engine/src/main/java/org/smartregister/fhircore/engine…

0e38ec0

…/util/extension/ResourceExtension.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

Update android/engine/src/main/java/org/smartregister/fhircore/engine…

9b2e65d

…/task/FhirTaskUtil.kt Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

Fix Build 💚

55c7e1b

- Fix Failing tests - Add unit tests ✅

Fix Build 💚

ab0c116

ellykits added 2 commits July 7, 2023 10:55

Merge branch 'main' into fix-profile-data-refresh

bfca407

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

Fix updating Task status to READY

001628c

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

ndegwamartin approved these changes Jul 7, 2023

View reviewed changes

pld approved these changes Jul 7, 2023

View reviewed changes

pld merged commit d32c982 into main Jul 7, 2023

pld deleted the fix-profile-data-refresh branch July 7, 2023 13:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix refreshing profile data #2523

Fix refreshing profile data #2523

ellykits commented Jul 4, 2023 •

edited by pld

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading

ellykits commented Jul 5, 2023

pld left a comment

pld Jul 5, 2023

pld Jul 5, 2023

ellykits Jul 7, 2023

ellykits Jul 7, 2023

ndegwamartin commented Jul 6, 2023

dubdabasoduba commented Jul 6, 2023

dubdabasoduba commented Jul 6, 2023

ellykits commented Jul 7, 2023

ellykits commented Jul 7, 2023

-  private suspend fun Task.preReqConditionSatisfied() =
+  private suspend fun Task.preconditionSatisfied() =
+      this.executionStartIsBeforeOrToday() &&
+      this.status == Task.TaskStatus.REQUESTED &&

Fix refreshing profile data #2523

Fix refreshing profile data #2523

Conversation

ellykits commented Jul 4, 2023 • edited by pld Loading

codecov bot commented Jul 4, 2023 • edited Loading

Codecov Report

ellykits commented Jul 5, 2023

Breaking config changes

Removed

Added

pld left a comment

Choose a reason for hiding this comment

pld Jul 5, 2023

Choose a reason for hiding this comment

pld Jul 5, 2023

Choose a reason for hiding this comment

ellykits Jul 7, 2023

Choose a reason for hiding this comment

ellykits Jul 7, 2023

Choose a reason for hiding this comment

ndegwamartin commented Jul 6, 2023

dubdabasoduba commented Jul 6, 2023

dubdabasoduba commented Jul 6, 2023

ellykits commented Jul 7, 2023

ellykits commented Jul 7, 2023

ellykits commented Jul 4, 2023 •

edited by pld

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading