v5 repeated values compatibility with brapi field sync

[chaneylc]

fix #548 fix #682 added repeated values to brapi sync feature

Description

Provide a summary of your changes including motivation and context. If these changes fix a bug or resolves a feature request, be sure to link to that issue.

Type of change

What type of changes does your code introduce? Put an x in boxes that apply.

• [ ] CHANGE (fix or feature that would cause existing functionality to not work as expected)
• [ ] FEATURE (non-breaking change which adds functionality)
• [ ] BUGFIX (non-breaking change which fixes an issue)
• [ ] ENHANCEMENT (non-breaking change which improves existing functionality)
• [ ] NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Checklist:

• [ ] I have performed a self-review of my code
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have updated relevant documentation

Changelog entry

Please add a one-line changelog entry below. This will be copied to the changelog file during the release process.

[bellerbrock]

@chaneylc looking at the getExportDbData function the export uses, should be straightforward to calculate rep in the SQL. Just adding RANK() OVER (PARTITION BY obs.observation_unit_id, obs.observation_variable_name ORDER BY obs.observation_time_stamp) AS calculated_rep to the select clause should do the trick.

[chaneylc]

@chaneylc looking at the getExportDbData function the export uses, should be straightforward to calculate rep in the SQL. Just adding RANK() OVER (PARTITION BY obs.observation_unit_id, obs.observation_variable_name ORDER BY obs.observation_time_stamp) AS calculated_rep to the select clause should do the trick.

That is definitely a good use of window functions but we can't do it this way. API 30 is when sqlite 3.28 was introduced which was the first version with window functions, we still have a minimum of 21. I'm guessing even android studio's sqlite version is below this for API 26 compatibility.

[bellerbrock]

@chaneylc Gotcha, that would explain some of the syntax errors I kept getting when trying the window function approach in the observationDao (though for some reason it works fine in the observationUnitPropertyDao).

Its not as concise, but a subquery should also do the trick. But rather than repeat it everywhere the observations table is accessed, what do you think about making an ObservationsDetail View? I see a TODO here suggestiing one here, I could add it and have rep calculated using a subquery.

[chaneylc]

@chaneylc Gotcha, that would explain some of the syntax errors I kept getting when trying the window function approach in the observationDao (though for some reason it works fine in the observationUnitPropertyDao).

Its not as concise, but a subquery should also do the trick. But rather than repeat it everywhere the observations table is accessed, what do you think about making an ObservationsDetail View? I see a TODO here suggestiing one here, I could add it and have rep calculated using a subquery.

Yeah that sounds ok, it would be nice to combine that query and getHostImageObservations too. These were originally meant to return brapi observations, so be careful there with the data source null check to make this robust enough for local repeated measures.

[bellerbrock]

@chaneylc Ok, working on the view approach now. Looks like the existing views are defined in Migrator.kt then its imported into the DataHelper and Daos for view creation and view name retrieval, respectively. But the Migrator seems written to only be used for v8 to v9 db upgrade. Do you think I should refactor it so it can be used for handling additional views, or would you suggest a different approach?

[chaneylc]

@chaneylc Ok, working on the view approach now. Looks like the existing views are defined in Migrator.kt then its imported into the DataHelper and Daos for view creation and view name retrieval, respectively. But the Migrator seems written to only be used for v8 to v9 db upgrade. Do you think I should refactor it so it can be used for handling additional views, or would you suggest a different approach?

Instead of refactoring I think it would be best to encapsulate your view definition, and any functions necessary to maintain it in its own DAO class.

[bellerbrock]

@chaneylc There were a lot of messy downstream effects when using the view, so I've thought better of it, reverted those commits, and have gone with calculating rep for the brapi obs based on the existing obs during the sync save process. Let me know what you think of this solution.