Client Refactoring

Summary

The Buendia client is stable but leaves many areas for potential improvement. This document describes known issues and TODO's within the client codebase.

Less Reliance on Sync

Currently, virtually all data retrieved from the server is retrieved via the Buendia sync account. This was an architectural decision made early on, but it comes with some unwanted side effects:

• Even when a sync is forced, Android may delay the beginning of sync indefinitely (though in practice, this is usually seconds, at most)
• The sync process runs outside of the application itself, forcing us to use inter-process communication to control sync, which is slow and can be unreliable
• Currently, all server data is downloaded on initial sync, which can be a slow process when there are lot of patients or a lot of observations

The sync account is still useful for keeping client data up-to-date, but ideally, data would be requested by the app and saved independently of sync for any blocking operation, with only the necessary data retrieved. This would be faster and more reliable and has the side benefit of simplifying some logic, particularly in TentSelectionController.

Improved User Management

User management allows clinicians to identify themselves so that there is an audit trail for data entered via the app; however, there are a number of aspects in which the current implementation should be improved:

• The ActiveUserSetEvent and ActiveUserUnsetEvent events are already implemented, which allow the app to handle the case in which the logged-in user is found to no longer exist on the server. No part of the app actually responds to these events, however, which means that users deleted on the server will continue to be able to use the app.
• Though the app stores a user id internally, only user full names and initials are displayed throughout the app; for two users with the same full name, it is very difficult to determine which is which.
• Encounter forms include a hidden 'clinician' field that is automatically populated with the logged-in user when the form is loaded. There are multiple issues with this process, however:
• Clinician is selected based on the full name of the logged-in user, which may not be unique.
• The application currently has a rare unicode encoding bug that may cause the clinician name and username not to match.
• There is no authentication for users and no concept of authorization. For now, this is working as intended, but at some later point, there may be a need to add auth/auth.

Internationalization

For the most part, the application was designed with internationalization (i18n) in mind. Most strings are built from resource strings, most UI elements use start and end rather than left and right, etc. There are still a handful of instances in which hardcoded strings exist, however, which will need to be resolved before the app can be fully localizable. Most, if not all, of these cases are marked with TODO's in the code to make them easier to find.

Once these cases are fixed, locale selection also needs to be revisited. Currently, English is hard-coded in LocaleSelector and PatientSearchActivity. It may be that using Locale.getDefault() will be sufficient.

Modularization

Currently, there are several parts of the UI that are hardcoded to assume that a particular set of concepts and forms will be downloaded from the server. These assumptions should eventually be generalized so that the app can easily adapt to new/different concepts and forms; for example:

• The vitals shown at the top of the patient chart (condition, consciousness, etc.) are hardcoded concepts. Ideally, the set of vitals to show and their labels should be defined by a form on the server.
• The "Ebola test" section of the patient chart and the UUID for the PCR test form are hardcoded.
• Temperature is hardcoded to display as red or green based on a certain threshold. This may be applicable to other concepts.
• Condition is hardcoded with a set of allowed values and colors, which are all hardcoded as well.
• Allowed values, display colors, etc. should ideally be defined by the server.
• Responsiveness, mobility, pain, and weakness are all hardcoded to display as initials if their localized value starts with 1-2 characters followed by a period. This is on the right track, but this should be applied more widely so that the UUID's for responsiveness, mobility, etc. don't need to be hardcoded in the app at all.
• Several fields are hardcoded to show a numeric value that changes color at a particular threshold, similar to temperature.
• 'Pregnant' and 'IV fitted' are hardcoded to appear under gender and age if present. Ideally the server could define new concepts that would have the same behavior (which also removes the need for pregnancy or IV-fitted to be hardcoded in the client).

For a complete list of hardcoded concepts from the server, see model/Concepts.java. Form ids (encounter form, PCR form, and the patient chart form) can be found in LocalizedChartHelper, SyncAdapter, and PatientChartActivity.

Test Refactoring / TODO's

The client tests have a number of TODO's for improving test reliability, coverage, etc. Test improvements are covered in more detail here.

Miscellaneous

There are a number of smaller TODO's/tasks throughout the codebase, including but not necessarily limited to:

• Reconcile TypedCursorAdapter and PatientTypedCursorAdapter, which serve the same purpose (or remove TypedCursorAdapter, which is unused)
• Remove dependency on PatientChartModel (need to migrate PatientModel#getOdkPatient elsewhere) -- these form the basis of an old data model that has been mostly replaced with AppModel
• Optimize filtering in patient list to use in-memory patients rather than making new database calls (though this may not be worth the added complexity)
• Remove (or make use of) model/LocalizedString, an unused class for i18n
• Remove (or implement and make use of) some unimplemented/unused interfaces in net/Server and net/OpenMrsServer
• Break reliance on singletons (several classes still request singletons from App.java, these should all be migrated to use dependency injection)
• Cleanup SyncAdapter to consolidate and/or extract large sections of code
• Consolidate and generalize the similar AppAddPatientAsyncTask and AppAddEncounterAsyncTask classes
• Improve error handling in AssignLocationDialog by parsing Volley errors in AppUpdatePatientAsyncTask
• Cleanup GsonRequest by parsing charset rather than just assuming UTF-8
• Improve authentication in OpenMrsJsonRequest
• Move content authority definition out of Contracts, as Contracts defines content providers, and content authorities encompass more than just content providers
• Properly update the syncResult counts in the RpcToDb class for user deletion operations, which are currently unchanged
• Remove references to/dependency on the now-unused ViewPagerIndicator
• Remove PatientProjection, moving projection definition to Contracts, to centralize definition of patient fields (otherwise, changes to fields in Contracts will break PatientProjection and vice-versa)
• Optimize insertions in UsersDelegate, which currently performs bulk insertion by performing several individual insertions
• Remove references to Contracts from PatientDatabase--currently, for most tables, there is a one-to-one mapping between the database and the content provider, but this is not guaranteed