Database refactor phase 1

valllllll2000 commented 7 years ago

The SurveyDbAdapter is quite big and is used everywhere in the app. I would like to see if we can separe it in and maybe move tables to another database to simplify the refactor.

Posible scenarios: 1) Move PREFERENCES table to another database 2) Move USER to another database 3) Have the PREFERENCES in SharedPreferences instead of database.

But after talking to @stellanl he explained to me that none of these scenarios are actually possible at the moment because users want to be able to provide all the settings in a single .sql file that would be placed in a folder and then executed in the app.

I would like to discuss what are the actual requirements, who uses the sqlite importing tool? Would it be possible to import a json or a configuration file?

@muloem @janagombitova would love your thoughts on this.

valllllll2000 commented 7 years ago

This issue may benefit from refactoring the big database class: https://github.com/akvo/akvo-flow-mobile/issues/552

stellanl commented 7 years ago

To clarify, I meant that the everything-in-one-db design was intended to make it easier to pre-load settings and data onto phones. I don't think we ever imagined partners would do it by themselves; they'd tell us what they wanted and we'd prepare an SQL file that they would then put on all their phones before handing them out.

valllllll2000 commented 7 years ago

Current situation: SurveyDbAdapter.open() is called in 31 places, inside activities even onCreate() method All these classes depend on this big class. There are these tables: response _id INTEGER PRIMARY KEY AUTOINCREMENT survey_instance_id INTEGER NOT NULL question_id TEXT NOT NULL answer TEXT NOT NULL type TEXT NOT NULL include INTEGER NOT NULL DEFAULT 1 filename TEXT

CREATE INDEX response_idx ON response(survey_instance_id, question_id);

survey_instance
_id INTEGER PRIMARY KEY AUTOINCREMENT uuid TEXT survey_id TEXT NOT NULL user_id INTEGER start_date INTEGER saved_date INTEGER submitted_date INTEGER surveyed_locale_id TEXT status INTEGER exported_date INTEGER sync_date INTEGER duration INTEGER NOT NULL DEFAULT 0 submitter TEXT version REAL

UNIQUE (uuid) ON CONFLICT REPLACE

CREATE INDEX response_status_idx ON survey_instance(status); CREATE INDEX response_modified_idx ON survey_instance(submitted_date);

survey
_id INTEGER PRIMARY KEY AUTOINCREMENT survey_id TEXT NOT NULL survey_group_id INTEGER display_name TEXT NOT NULL version REAL type TEXT location TEXT filename TEXT language TEXT help_downloaded_flag INTEGER NOT NULL DEFAULT 0 deleted INTEGER NOT NULL DEFAULT 0

UNIQUE (survey_id) ON CONFLICT REPLACE

sync_time
id INTEGER PRIMARY KEY AUTOINCREMENT survey_group_id INTEGER time TEXT

UNIQUE (survey_group_id) ON CONFLICT REPLACE

record
_id INTEGER PRIMARY KEY AUTOINCREMENT record_id TEXT survey_group_id INTEGER name TEXT latitude REAL longitude REAL last_modified INTEGER NOT NULL DEFAULT 0

UNIQUE (record_id) ON CONFLICT REPLACE

CREATE INDEX record_name_idx ON record(name)

survey_group
_id INTEGER PRIMARY KEY AUTOINCREMENT survey_group_id INTEGER name TEXT register_survey_id TEXT monitored INTEGER NOT NULL DEFAULT 0

UNIQUE (survey_group_id) ON CONFLICT REPLACE

transmission
_id INTEGER PRIMARY KEY AUTOINCREMENT survey_instance_id INTEGER NOT NULL survey_id TEXT filename TEXT status INTEGER start_date INTEGER end_date INTEGER

UNIQUE (filename) ON CONFLICT REPLACE

user
_id INTEGER PRIMARY KEY AUTOINCREMENT name TEXT NOT NULL email TEXT deleted INTEGER NOT NULL DEFAULT 0

preferences
key TEXT PRIMARY KEY value TEXT

valllllll2000 commented 7 years ago

My idea:

Phase 1:

Preferences should be in SharedPreferences because they have to be accessed faster and multiple times. If we need to import user preferences somehow, lets provide a tool for that which in the end will write the ser preferences to shared preferences. Current user should also be in shared preferences -> some people will never change user and it has to be accessed often. We can keep the user table for all the users if needed (do people use multiuser?) The rest could stay as is for now. This will drastically reduce access to database (reduce the number of classes that access it directly by half).

Phase 2:

Abstract access to database within a data layer, the views should not know the database even exists.

Possibles issues:

We need to make sure the current user preferences are kept so there needs to be a migration mechanism.

muloem commented 7 years ago

Some notes from our discussion around this...

Current database structure stores user preferences together with survey definitions and response data. Since the user preferences are accessed quite often, and yet are mostly static, its better we store them separately from the rest of the database that will be changing frequently e.g survey updates, survey responses etc
The current setup, i.e., having the preferences in the database was motived by having the possibility for pre loading a large number of devices with preferences and data as mentioned by @stellanl. After discussing this we think If a large scale installation of the application is needed, we can create another method of handling such imports
We reviewed the current preferences and will migrate them to a more static shared preferences location.
We need to take care when migrating to make sure that we provide some conservative defaults for certain settings like the Upload via cellular data setting.
Finally, we should cross check the DROP TABLE logic to be clear we are not dropping survey responses when upgrading the database

@stellanl @valllllll2000 did I miss something?

valllllll2000 commented 7 years ago

Part1 - Preferences table

Preferences that will be migrated to shared preferences (using onUpgrade method)

CELL_UPLOAD_SETTING_KEY = "data.cellular.upload" default value -> false

SERVER_SETTING_KEY = "backend.server" default value -> use property file "serverBase" value on migration if database value is empty use property file "serverBase" value

SCREEN_ON_KEY = "screen.keepon" default value -> true

DEVICE_IDENT_KEY = "device.identifier" default value -> ??? not sure, use android_id???

MAX_IMG_SIZE = "media.img.maxsize"

displayed to the user values in strings

    <string-array name="max_image_size_pref">
        <item>320x240px</item>
        <item>640x480px</item>
        <item>1280x960px</item>
    </string-array>

positions in the array:

int IMAGE_SIZE_320_240 = 0;
int IMAGE_SIZE_640_480 = 1;
int IMAGE_SIZE_1280_960 = 2;

to the database it is saved the position in the array converted to String!!! FormActivity expects a string and the converts to int we should save it as int default value -> 0

Settings inside the database which are no longer used (to be deleted)

'user.storelast','false' 'user.lastuser.id','' 'survey.textsize','LARGE'

App Languages

PREF_LOCALE = "pref.locale" -> default English (android sets it already to English by default) Used in FlowApp when initializing the app and also in PreferencesActivity -> App Language These are the available languages:

  <!-- App languages -->
    <string-array name="app_languages">
        <item>Bahasa Indonesia</item>
        <item>English</item>
        <item>Español</item>
        <item>Français</item>
        <item>Hindi</item>
        <item>Khmer</item>
        <item>Portuguese</item>
        <item>Vietnamese</item>
        <item>Nepali</item>
    </string-array>

<string-array name="app_language_codes">
        <item>in</item>
        <item>en</item>
        <item>es</item>
        <item>fr</item>
        <item>hi</item>
        <item>km</item>
        <item>pt</item>
        <item>vi</item>
        <item>ne</item>
    </string-array>

Survey languages

Survey languages will be left as is for now and they will be handled in the issues: #576 and #575

Still not sure what these represent, exactly. For English I get: survey.languagespresent -> "0" survey.language -> "0"

SURVEY_LANG_PRESENT_KEY = "survey.languagespresent" available languages for survey

SURVEY_LANG_SETTING_KEY = "survey.language"; selected languages for a survey. It is possible to display the survey in 2 languages at the same time.

default values for those seem to be empty string

It seems to be possible to select multiple languages for one survey.

valllllll2000 commented 7 years ago

Test plan:

For Preference migration: the following preferences will be migrated:

keep screen on during survey input
sync data over mobile network
device identifier
resize large images

Steps: 1) Install app previous version, for example 2.2.9 2) Login a user so you can access preferences 3) Go to settings -> preferences, leave everything as is and remember the default settings 4) Install the new version without uninstalling the old one, try manually updating 5) Reopen the app, go to settings and verify everything is the same 6) Uninstall and repeat steps 1 and 2 7) No modify all the settings: Screen on -> disable Sync data -> disable change the device identifier to something else for example test_indentifier change default image size to 640x480 8) update app again 9) Make sure settings are preserved

janagombitova commented 7 years ago

👍 works as a charm, all preferences are kept the same

janagombitova commented 7 years ago

@valllllll2000 just to confirm: after uninstalling the app and installing a new version, the preferences I saved previously are not saved and the default preferences are in place. Is this expected? For me as a user this makes sense.

When updating the app version, the preferences are saved as in the same way as I have modified them

valllllll2000 commented 7 years ago

@janagombitova if you uninstall the app it is as if you never had it installed in the first place. We do not send anything to the backend to save user preferences so it gets lost.

janagombitova commented 7 years ago

@valllllll2000 ok. then all works as expected 👍

akvo / akvo-flow-mobile