First draft of import support for Clue #140 #254
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
app/src/main/java/com/mensinator/app/settings/ExportImportDialog.kt
Outdated
Show resolved
Hide resolved
| var expanded by remember { mutableStateOf(false) } | ||
|
|
||
| Column(modifier = modifier) { | ||
| OutlinedTextField( |
There was a problem hiding this comment.
Would be nice if the value would be clickable, so that the user doesn't have to specifically tap on the arrow. I couldn't find a way to do it nicely though...
It's possible to set a clickable on the OutlinedTextField, but the ripple animation does not look good. I think a different UI element should be used, not OutlinedTextField.
| val impSuccess = stringResource(id = R.string.import_success_toast) | ||
| val impFailure = stringResource(id = R.string.import_failure_toast) | ||
|
|
||
| var selectedOption by remember { mutableStateOf("Mensinator") } |
There was a problem hiding this comment.
Use an enum for this, not a String.
There was a problem hiding this comment.
Introduced enum but have not gotten it to work with the dialog. When clicked it crashes...
| // Call the import function | ||
| onImportClick(file.absolutePath) | ||
| if (selectedOption == "Clue") { | ||
| ClueImport(context).importFileToDatabase(file.absolutePath) // Pass the Uri |
There was a problem hiding this comment.
Don't handle this on the UI layer :P. The mensinator import is handled from the ViewModel, same thing should be done for the Clue import
There was a problem hiding this comment.
onImportClick should be called with the path and the import option. The VM then decides what to do with it.
There was a problem hiding this comment.
Please rename IExportImport/ExportImport to IMensinatorExportImport/MensinatorExportImport.
|
|
||
| Toast.makeText(context, "Importing file: ${file.name}", Toast.LENGTH_SHORT).show() | ||
|
|
||
| if (file.name != "measurements.json") { |
There was a problem hiding this comment.
This should not be there. The user should be allowed to name their export/backup in the way they want.
There was a problem hiding this comment.
Ah, but here is the problem, when exporting from Clue you get 12 json-files;
Consents.json (we do not need this information)
CycleSettings.json (contains if Clue should track ovulation, we do not have this right now)
Devices.json (we do not need this information)
Integrations.json (weird stuff, not needed we do not integrate to other stuff)
Measurements.json (Contains period dates, what we want to import)
Medical_records.json (we do not need this information)
Profile.json (contains users birthday and cycle length set by user, we dont need this)
Push.json (contains weird stuff, do not need. Does not contain anything about push notifications)
Reminders.json (contains information about reminders, we could use this for notification settings also, but Im not sure this is something the users want. In any case that could be next step when import of period data works)
Subscriptions.json (we do not need this information)
User.json (we do not need this information since we dont store information about email and names)
UserModeEvents.json (information about which 'mode' you are using in Clue, we do not need this).
The export from Clue is in a password protected zipfile and the folder then contains the 12 files mentioned above. I want to make it easy for the user that they should only select the Measurements file. So that's why I have that check, for Clue we are only interested in that specific file.
|
|
||
| private fun cleanUpTables(db: SQLiteDatabase) { | ||
| // Clean up the "periods" table before importing new data | ||
| db.delete("periods", null, null) |
There was a problem hiding this comment.
While importing it should be made clear that existing data gets removed.
There was a problem hiding this comment.
Ah, this is something we do currently for Mensinator import because I did not have a unique constraint on the periods and ovulations table. I have implemented it now so for Clue, we don't need to delete any period data.
There was a problem hiding this comment.
Added new db-functionality for this, so we do not need to delete data in periods/ovulations when importing data (if we use the function addDateToPeriod).
| private val context: Context | ||
| ) : IClueImport { | ||
|
|
||
| private val dbHelper = PeriodDatabaseHelper(context, DefaultDispatcherProvider()) |
There was a problem hiding this comment.
This must not be done! We don't want to create new classes manually. This must be injected via koin, same as at ExportImport.kt.
There was a problem hiding this comment.
Agreed! Not yet familiar with Koin...
|
Also please include an example import json into the project. Else this is difficult to maintain or test. For now just include it in a testing folder, at some point unit tests should be written for the imports. |
|
In the linked issue I have example of exportfile from Clue and some information that is good to know: |
…ded enum. Updated strings
| @@ -136,6 +136,7 @@ | |||
| <string name="import_path_label">Import Path: %1$s</string> | |||
| <string name="import_dialog_message">Select file to import data.</string> | |||
| <string name="export_dialog_message">Select the output path to export calendar entries, symptoms and other data.</string> | |||
| <string name="select_source">Select app source</string> | |||
Check warning
Code scanning / Android Lint
Incomplete translation Warning
@carstenhag Need help with getting the file to new class ClueImport. I think the rest might work?