Add configuration to generate MeasureReports in the background (#2515) #2520

ndegwamartin · 2023-07-04T10:08:40Z

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

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 configuration to generate MeasureReports in the background * Set default report config startPeriod, remove subject from existing measureReport retrieval * Exclude Subjects from existing measure report retrieval search --------- Co-authored-by: Martin Ndegwa <ndegwamartin@users.noreply.github.com>

codecov · 2023-07-04T10:53:54Z

Codecov Report

Merging #2520 (b723c7e) into main (d84c2c8) will decrease coverage by 0.3%.
The diff coverage is 60.5%.

@@            Coverage Diff            @@
##              main   #2520     +/-   ##
=========================================
- Coverage     65.4%   65.1%   -0.3%     
- Complexity    1101    1110      +9     
=========================================
  Files          213     215      +2     
  Lines         9385    9544    +159     
  Branches      1866    1899     +33     
=========================================
+ Hits          6140    6217     +77     
- Misses        2047    2120     +73     
- Partials      1198    1207      +9

Flag	Coverage Δ
engine	`72.9% <66.8%> (-0.3%)`	⬇️
geowidget	`62.9% <37.5%> (-0.3%)`	⬇️
quest	`60.1% <57.2%> (-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%> (ø)`
...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%> (ø)`
...data/remote/fhir/resource/FhirResourceConverter.kt	`100.0% <ø> (ø)`
...hircore/engine/domain/model/ActionParameterType.kt	`100.0% <ø> (ø)`
...hircore/engine/domain/model/FhirResourceConfigs.kt	`44.4% <0.0%> (ø)`
... and 157 more

... and 10 files with indirect coverage changes

pld · 2023-07-05T16:41:17Z

@ndegwamartin this doesn't seem like the correct fixes ref

...org/smartregister/fhircore/engine/configuration/report/measure/MeasureReportConfiguration.kt

pld · 2023-07-10T16:34:33Z

.../main/java/org/smartregister/fhircore/quest/data/report/measure/MeasureReportPagingSource.kt

+  override suspend fun load(params: LoadParams<Int>): LoadResult<Int, MeasureReportConfig> {
+    return try {
+      LoadResult.Page(data = measureReportConfiguration.reports, prevKey = null, nextKey = null)
+    } catch (exception: Exception) {


can we catch something more specific?

...rc/main/java/org/smartregister/fhircore/quest/data/report/measure/MeasureReportRepository.kt

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

.../java/org/smartregister/fhircore/quest/ui/report/measure/worker/MeasureReportConfigWorker.kt

pld · 2023-07-10T17:12:26Z

.../java/org/smartregister/fhircore/quest/ui/report/measure/worker/MeasureReportConfigWorker.kt

+          }
+        }
+      Timber.i("Result.success  / . . . MeasureReportWorker . . ./")
+    } catch (e: Exception) {


is there a more specific exception?

.../java/org/smartregister/fhircore/quest/ui/report/measure/worker/MeasureReportConfigWorker.kt

pld · 2023-07-11T16:01:36Z

.../org/smartregister/fhircore/quest/ui/report/measure/worker/MeasureReportMonthPeriodWorker.kt

+   * This method always sets the start and end date of the month to be the first and last day of the
+   * current month. If you are using this worker to generate monthly reports it enqueue a new worker
+   * at least every 24 hours.


I'm still not sure this is correct, need to think about this a bit more to see if there's an edge case that misses data from the previous month when the date changes.

I think there is actually still a problem, since it’s using “today” to figure out the current month instead of the date it was enqueued. This is what I’m thinking, if it’s the last day of the month, you run the worker, and enqueue another worker for tomorrow. Then you collect more data. That new data you collected will never appear in the previous months measure report, because by the time the enqueued measure report runs it will be tomorrow, in the next month, and that run will be for the next month.

One (simple?) solution is to have today be set when the worker is enqueued, instead of when the worker is run. then when you enqueue a worker on the last day of the month, and proceed to collect some data, when that worker runs it will cover all the data for the previous month, during which it was enqueued.

This also seems to match expected behavior... better?

created this issue to track this, #2567

pld

follow-up tasks in #2567

#2520) * add configuration to generate MeasureReports in the background (#2515) * Add configuration to generate MeasureReports in the background * Set default report config startPeriod, remove subject from existing measureReport retrieval * Exclude Subjects from existing measure report retrieval search --------- Co-authored-by: Martin Ndegwa <ndegwamartin@users.noreply.github.com> * cleanup and notes * fix typo * remove comment * add a failing test to run eval pop measure * catch exception with bad measure url * change MeasureReportConfig to ReportConfiguration, add tests * upgrade spotless and lint, use trailing commas per new lint recs --------- Co-authored-by: Roy <84201465+roywanyaga@users.noreply.github.com> Co-authored-by: Benjamin Mwalimu <dubdabasoduba@gmail.com> Co-authored-by: Peter Lubell-Doughtie <peter@ona.io>

Merge branch 'main' into scheduled-report-config-main

366377e

roywanyaga added the DNM DO NOT MERGE label Jul 10, 2023

Merge branch 'main' into scheduled-report-config-main

547526d

pld assigned roywanyaga Jul 10, 2023

pld requested changes Jul 10, 2023

View reviewed changes

Merge branch 'main' into scheduled-report-config-main

cba976d

pld assigned pld and unassigned roywanyaga Jul 11, 2023

Merge branch 'main' into scheduled-report-config-main

5923172

roywanyaga assigned ellykits Jul 11, 2023

pld added 3 commits July 11, 2023 11:58

cleanup and notes

1205db1

fix typo

55d50a5

remove comment

9610a6e

pld reviewed Jul 11, 2023

View reviewed changes

pld added 2 commits July 11, 2023 13:00

add a failing test to run eval pop measure

2f1c9f2

catch exception with bad measure url

21cfb9c

pld force-pushed the scheduled-report-config-main branch from d90a84e to 21cfb9c Compare July 11, 2023 18:23

change MeasureReportConfig to ReportConfiguration, add tests

543158b

pld force-pushed the scheduled-report-config-main branch from 532b917 to 543158b Compare July 12, 2023 01:01

pld mentioned this pull request Jul 12, 2023

Resolve background workers missing data since run month based on today not when enqueed #2567

Closed

6 tasks

pld force-pushed the scheduled-report-config-main branch 2 times, most recently from 14bf673 to 4c6ae86 Compare July 12, 2023 02:11

upgrade spotless and lint, use trailing commas per new lint recs

b723c7e

pld force-pushed the scheduled-report-config-main branch from 4c6ae86 to b723c7e Compare July 12, 2023 02:19

pld approved these changes Jul 12, 2023

View reviewed changes

pld merged commit 3bbbb7a into main Jul 12, 2023

pld deleted the scheduled-report-config-main branch July 12, 2023 03:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add configuration to generate MeasureReports in the background (#2515) #2520

Add configuration to generate MeasureReports in the background (#2515) #2520

ndegwamartin commented Jul 4, 2023 •

edited by pld

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading

pld commented Jul 5, 2023

pld Jul 10, 2023

pld Jul 10, 2023

pld Jul 11, 2023

pld Jul 11, 2023 •

edited

Loading

pld Jul 11, 2023

pld Jul 12, 2023

pld left a comment

Add configuration to generate MeasureReports in the background (#2515) #2520

Add configuration to generate MeasureReports in the background (#2515) #2520

Conversation

ndegwamartin commented Jul 4, 2023 • edited by pld Loading

codecov bot commented Jul 4, 2023 • edited Loading

Codecov Report

pld commented Jul 5, 2023

pld Jul 10, 2023

Choose a reason for hiding this comment

pld Jul 10, 2023

Choose a reason for hiding this comment

pld Jul 11, 2023

Choose a reason for hiding this comment

pld Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

pld Jul 11, 2023

Choose a reason for hiding this comment

pld Jul 12, 2023

Choose a reason for hiding this comment

pld left a comment

Choose a reason for hiding this comment

ndegwamartin commented Jul 4, 2023 •

edited by pld

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading

pld Jul 11, 2023 •

edited

Loading