Refactor backup and import processes to use profile for CSV#367
Refactor backup and import processes to use profile for CSV#367yogeshpaliyal merged 2 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors bookmark backup/export and import flows so CSV backups include profile information and CSV imports can map entries to profiles.
Changes:
- Add a
profileNamecolumn to CSV exports and include it in the CSV writer output. - Update CSV import to resolve/create profiles based on the CSV’s profile column, with a fallback to the currently selected profile.
- Replace
Flow.singleOrNull()usage withfirst()/firstOrNull()when reading the selected profile id from DataStore.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/java/com/yogeshpaliyal/deepr/util/Constants.kt | Adds PROFILE_NAME CSV header constant. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/importer/MozillaBookmarkImporter.kt | Uses DataStore first() for selected profile id during import. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/importer/HtmlBookmarkImporter.kt | Uses DataStore firstOrNull() for selected profile id during import. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/importer/CsvBookmarkImporter.kt | Injects AppPreferenceDataStore and maps CSV rows to profiles. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/importer/ChromeBookmarkImporter.kt | Uses DataStore first() for selected profile id during import. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/ImportRepositoryImpl.kt | Wires AppPreferenceDataStore into CsvBookmarkImporter. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/ExportRepositoryImpl.kt | Switches export query to getLinksForBackup() to include profile name. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/CsvWriter.kt | Writes profileName column and uses GetLinksForBackup query model. |
| app/src/main/java/com/yogeshpaliyal/deepr/backup/AutoBackupWorker.kt | Uses getLinksForBackup() for auto-backup CSV generation. |
app/src/main/java/com/yogeshpaliyal/deepr/backup/importer/CsvBookmarkImporter.kt
Show resolved
Hide resolved
| createdAt = createdAt, | ||
| profileId = profileID ?: 1L, | ||
| profileId = profileID, | ||
| ) |
There was a problem hiding this comment.
Variable profileID (used to populate profileId) is inconsistently named compared to Kotlin style and the surrounding code. Renaming it to profileId would improve readability and avoid confusion.
| val profileID = | ||
| profileName?.let { | ||
| val profile = deeprQueries.getProfileByName(profileName).executeAsOneOrNull() | ||
| if (profile == null) { | ||
| deeprQueries.insertProfile(profileName) | ||
| deeprQueries.lastInsertRowId().executeAsOneOrNull() | ||
| } else { |
There was a problem hiding this comment.
When the profile doesn’t exist, the code inserts via insertProfile (which is INSERT OR IGNORE) and then uses lastInsertRowId() as the new profile id. If the insert is ignored (e.g., race with another import/profile creation), lastInsertRowId() can be unrelated, leading to an incorrect profileId on imported links. Safer approach: after the insert, re-query getProfileByName(...) and use its id (or change the insert/query to return the id deterministically).
| bookmarks.forEach { bookmark -> | ||
| val existing = deeprQueries.getDeeprByLink(bookmark.url).executeAsOneOrNull() | ||
| if (bookmark.url.isNotBlank() && existing == null) { | ||
| try { | ||
| val profileId = appPreferenceDataStore.getSelectedProfileId.singleOrNull() ?: 1L | ||
| val profileId = appPreferenceDataStore.getSelectedProfileId.firstOrNull() ?: 1L | ||
| deeprQueries.transaction { |
There was a problem hiding this comment.
getSelectedProfileId.firstOrNull() is called for every bookmark inside the loop. Since the selected profile id is the same for the whole import, read it once before iterating to avoid repeated DataStore reads (can be noticeable for large bookmark files).
| links.forEach { link -> | ||
| val url = link.attr("href") | ||
| val title = link.text() | ||
| val folder = extractMozillaFolder(link) | ||
|
|
||
| // Firefox-specific attributes | ||
| val addDate = link.attr("add_date") | ||
| val lastModified = link.attr("last_modified") | ||
| val shortcutUrl = link.attr("shortcuturl") | ||
| val tags = link.attr("tags") | ||
| val profileId = appPreferenceDataStore.getSelectedProfileId.singleOrNull() ?: 1L | ||
| val profileId = appPreferenceDataStore.getSelectedProfileId.first() | ||
| if (url.isNotBlank() && !url.startsWith("place:")) { |
There was a problem hiding this comment.
getSelectedProfileId.first() is executed inside the links.forEach loop. Fetch the selected profile id once before iterating so importing large Firefox exports doesn’t repeatedly hit DataStore.
| return RequestResult.Error(context.getString(R.string.no_data_to_export)) | ||
| } | ||
| val dataToExportInCsvFormat = deeprQueries.listDeeprWithTagsAsc().executeAsList() | ||
| val dataToExportInCsvFormat = deeprQueries.getLinksForBackup().executeAsList() |
There was a problem hiding this comment.
Switching to getLinksForBackup() changes the export ordering: the SQL query has no ORDER BY, whereas the previous listDeeprWithTagsAsc() was ordered by createdAt. If deterministic CSV output matters (diffing backups, stable user experience), add ordering (in SQL or sort the list before writing).
| val dataToExportInCsvFormat = deeprQueries.getLinksForBackup().executeAsList() | |
| val dataToExportInCsvFormat = | |
| deeprQueries.getLinksForBackup().executeAsList().sortedBy { it.createdAt } |
|
@yogeshpaliyal I've opened a new pull request, #368, to work on those changes. Once the pull request is ready, I'll request review from you. |
📱 APK Build Complete!Your debug APK has been built successfully and is ready for testing. 📥 Download APKNote: Click the link above, scroll down to the "Artifacts" section, and download the Retention: This artifact will be available for 3 days. |
…eation (#368) * Initial plan * Handle blank/whitespace profile names in CSV import Co-authored-by: yogeshpaliyal <9381846+yogeshpaliyal@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: yogeshpaliyal <9381846+yogeshpaliyal@users.noreply.github.com>
🧪 Integrated Test ResultsThe integrated UI tests have completed. View full test results: Test Run #21785475248 Test reports are available in the artifacts section of the workflow run. |
📱 APK Build Complete!Your debug APK has been built successfully and is ready for testing. 📥 Download APKNote: Click the link above, scroll down to the "Artifacts" section, and download the Retention: This artifact will be available for 3 days. |
🧪 Integrated Test ResultsThe integrated UI tests have completed. View full test results: Test Run #21785625646 Test reports are available in the artifacts section of the workflow run. |
This pull request updates the backup and import functionality to support exporting and importing bookmarks with associated profile information. The main change is switching from the
ListDeeprWithTagsAscdata structure to the newGetLinksForBackupstructure, which includes the profile name. Several importers now use the correct coroutine method for retrieving the selected profile ID, improving reliability.Backup and Export Enhancements:
GetLinksForBackupinstead ofListDeeprWithTagsAsc, allowing profile information to be included in exported data. (AutoBackupWorker.kt,ExportRepositoryImpl.kt,CsvWriter.kt) [1] [2] [3] [4] [5]PROFILE_NAMEto CSV export header and data rows, so exported bookmarks now include the profile name. (CsvWriter.kt,Constants.kt) [1] [2] [3]Import Functionality Improvements:
CsvBookmarkImporterto acceptAppPreferenceDataStoreand use the selected profile ID from preferences, defaulting correctly when not found. (ImportRepositoryImpl.kt,CsvBookmarkImporter.kt) [1] [2] [3]CsvBookmarkImporter.kt) [1] [2]Consistency in Profile ID Retrieval:
singleOrNull()tofirst()orfirstOrNull()for retrieving the selected profile ID in Chrome, Mozilla, and HTML importers, improving coroutine usage and reliability. (ChromeBookmarkImporter.kt,MozillaBookmarkImporter.kt,HtmlBookmarkImporter.kt) [1] [2] [3] [4] [5] [6]