RAMPcapture V1 adjustments#29
Conversation
…n history charts & tables
…otes tab instead of following it in the tab row
…w on TEI enrolment screen when History tab is selected and viewed full-width - for easier navigation
There was a problem hiding this comment.
Pull request overview
This PR applies RAMPcapture V1 adjustments across the Android app to improve RAMP history chart/table correctness, update TEI/event navigation to replace the upstream Notes tab with History, and surface fork-specific version info on the About screen.
Changes:
- Fix history charts/tables to (a) keep current in-progress values plotted and (b) exclude locally deleted follow-up events from history.
- Update TEI dashboard and event capture navigation so Notes is not shown and History takes its place, with improved landscape navigation.
- Add RAMPcapture version information to the About screen, wired through CI and BuildConfig.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates the fork README index to include the new RAMPcapture version info entries. |
| form/src/test/java/org/dhis2/form/simprints/ramp/data/FormHistoryChartRepositoryTest.kt | Adds coverage for unsaved current values and deleted follow-up filtering; updates stubbing and expectations. |
| form/src/main/java/org/dhis2/form/simprints/ramp/ui/FormHistoryChartView.kt | Increases chart text size factor for improved readability. |
| form/src/main/java/org/dhis2/form/simprints/ramp/data/FormHistoryChartRepository.kt | Loads tracked-entity data values, plots current in-progress values, and filters out deleted follow-ups. |
| app/src/test/java/org/dhis2/usescases/teiDashboard/TeiDashboardPageConfiguratorTest.kt | Updates expectation that Notes is not displayed. |
| app/src/test/java/org/dhis2/usescases/teiDashboard/DashboardViewModelTest.kt | Adds tests around showing Details when History is selected in landscape; adjusts resource stubbing. |
| app/src/test/java/org/dhis2/usescases/eventsWithoutRegistration/eventCapture/EventPageConfiguratorTest.kt | Adds coverage that Notes is not displayed. |
| app/src/test/java/org/dhis2/simprints/ramp/data/EventHistoryTableRepositoryTest.kt | Adds coverage that deleted follow-up events are excluded from history tables. |
| app/src/main/res/values/strings.xml | Adds “RAMPcapture Version” label and renames upstream app label to “DHIS2 App Version”. |
| app/src/main/res/layout/fragment_about.xml | Adds a new About row for RAMPcapture version. |
| app/src/main/java/org/dhis2/usescases/teiDashboard/TeiDashboardPageConfigurator.kt | Disables Notes for TEI dashboard; keeps other page visibility logic intact. |
| app/src/main/java/org/dhis2/usescases/teiDashboard/TeiDashboardMobileActivity.kt | Adds landscape navigation handling for restoring History fullscreen state; updates selection wiring. |
| app/src/main/java/org/dhis2/usescases/teiDashboard/DashboardViewModel.kt | Makes Notes conditional, adds landscape “Details” visibility behavior tied to History selection. |
| app/src/main/java/org/dhis2/usescases/eventsWithoutRegistration/eventCapture/EventPageConfigurator.kt | Disables Notes for event capture screens. |
| app/src/main/java/org/dhis2/usescases/eventsWithoutRegistration/eventCapture/EventCapturePagerAdapter.kt | Makes Notes page inclusion conditional via a new constructor flag. |
| app/src/main/java/org/dhis2/usescases/eventsWithoutRegistration/eventCapture/EventCaptureActivity.kt | Passes displayNotes() into the pager adapter. |
| app/src/main/java/org/dhis2/usescases/about/AboutFragment.kt | Binds the new RAMPcapture version text on the About screen. |
| app/src/main/java/org/dhis2/simprints/ramp/data/EventHistoryTableRepository.kt | Filters out locally deleted events at the repository query level. |
| app/src/main/java/org/dhis2/bindings/ContextExtensions.kt | Adds formatting helper for RAMPcapture build/version info. |
| app/src/androidTest/java/org/dhis2/usescases/teidashboard/TeiDashboardTest.kt | Replaces Notes behavior tests with an assertion that Notes tab is absent. |
| app/src/androidTest/java/org/dhis2/usescases/teidashboard/robot/TeiDashboardRobot.kt | Updates robot helper to assert Notes tab does not exist. |
| app/src/androidTest/java/org/dhis2/usescases/about/AboutTest.kt | Updates About UI test to include the new RAMPcapture version row. |
| app/src/androidTest/java/org/dhis2/usescases/about/AboutRobot.kt | Updates robot assertions to check the new RAMPcapture version view. |
| app/build.gradle.kts | Exposes RAMP_CAPTURE_VERSION via BuildConfig (env-driven with a local fallback). |
| .github/workflows/rampcapture-github-release-signed-apk.yml | Exports computed fork version number to Gradle build via RAMP_CAPTURE_VERSION. |
Comments suppressed due to low confidence (1)
form/src/main/java/org/dhis2/form/simprints/ramp/data/FormHistoryChartRepository.kt:24
getChartloads the current event from the DB before checking whether the requested field is even configured for a history chart. SincewithSimprintsRampHistoryChart()calls this for every field, this can introduce an O(#fields)blockingGet()cost per form render even when most fields are not chart-configured.
val currentEvent = loadCurrentEvent() ?: return null
val chartConfig =
configs.firstOrNull { config ->
config.programId?.trim() == currentEvent.program() &&
config.followUpVisitProgramStageId?.trim() == currentEvent.programStage() &&
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tory if no chart configured for the field
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
app/src/main/java/org/dhis2/usescases/searchTrackEntity/SearchTEActivity.kt:199
- In landscape, the clipping logic reads
viewModel.isSearchEnabled.valueinside thefiltersOpenedobserver, butisSearchEnabledis updated asynchronously and isn’t observed here. IfisSearchEnabledchanges whilefiltersOpenedstays the same, the clipping won’t be refreshed.
if (isLandscape()) {
viewModel.filtersOpened.observe(this) { isOpened: Boolean ->
if (java.lang.Boolean.TRUE == isOpened || viewModel.isSearchEnabled.value == false) {
binding.mainComponent.clipWithRoundedCorners(16.dp)
} else {
binding.mainComponent.clipWithTopRightRoundedCorner(16.dp)
}
}
cd7087a to
62b6392
Compare
… besides Form/Details. Forced full-width view on Chart tab item press on landscape
…ogram stage via config
| return try { | ||
| getConfig() | ||
| .programSpecificSettings | ||
| .firstOrNull { it.programId == programId } |
There was a problem hiding this comment.
Elseware we seem to trim the whitespace (.trimToValue()) for the program ID. I have no idea if this is needed or not here but I'd check because if DHIS2 is doing it in other places this could cause different behaviour for these searches.
| this.metadataIconProvider = metadataIconProvider; | ||
| this.profilePictureProvider = profilePictureProvider; | ||
| this.customIntentRepository = customIntentRepository; | ||
| this.rampDatastoreRepository = new RampDatastoreRepository(d2, new Gson()); |
There was a problem hiding this comment.
We don't want to inject this?
| .firstOrNull { config -> | ||
| config.programId.trimToValue() == programId && | ||
| ( | ||
| programStageId == null || |
There was a problem hiding this comment.
Hmmm maybe I am reading thins wrong but If a program ever has more than one programStageHistoryTables entry, the selection is arbitrary, and since the chosen config's followUpVisitProgramStageId drives the event query in getTable(), the table can silently show the wrong data.
It's fine if there is only ever one config per program, but there are no checks for this anywhere. IDK if it's scope for this PR.
There was a problem hiding this comment.
A program indeed is intended to have 1 config at most. While we do have some degree of defensive parsing where IDs can have accidentally copied leading/trailing whitespaces (can happen while copied from the Maintenance web UI), here I think it would be excessive. "Only ever one config per program" is true.
Changes: