feat(settings): reorder accounts by drag in settings
Give accounts a user-controlled order (issue #164). Every surface that lists accounts -- the Settings list, the drawer account switcher, and the compose account picker -- reads the same `ORDER BY sortOrder` query, so a reorder in Settings is honored app-wide. In Settings a row can be long-pressed and dragged to a new position; the order persists and survives restart. Data layer: - AccountEntity gains `sortOrder` (@ColumnInfo defaultValue "0"); AccountDao orders by it and adds insertAtEnd / reorder / nextSortOrder / setSortOrder, the mutations wrapped in transactions. - New accounts are appended (current max + 1) via insertAtEnd. Migration (AccountDatabase v1 -> v2): - ACCOUNT_MIGRATION_1_2 adds the column and backfills existing accounts by their previous alphabetical (email) rank, so the already-shown order does not shuffle on upgrade. Registered in AccountDatabaseModule; 2.json is exported and AccountMigrationTest replays and validates it. - AccountDataMigrator (the pre-#111 cache->account-db move) creates the v2-shaped table and applies the same email-rank backfill, since ACCOUNT_MIGRATION_1_2 does not run for that path. UI: - AccountReorderList drives long-press drag over a plain Column (no nested lazy list inside the scrolling settings column, no extra dependency); the pure reorder-index maths (commitDrag) is unit-tested. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,241 @@
|
||||
{
|
||||
"formatVersion": 1,
|
||||
"database": {
|
||||
"version": 2,
|
||||
"identityHash": "6705bfa56d02b55f47c5c66ba10143b0",
|
||||
"entities": [
|
||||
{
|
||||
"tableName": "accounts",
|
||||
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, `displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `sortOrder` INTEGER NOT NULL DEFAULT 0, `imap_host` TEXT NOT NULL, `imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, `smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))",
|
||||
"fields": [
|
||||
{
|
||||
"fieldPath": "id",
|
||||
"columnName": "id",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "email",
|
||||
"columnName": "email",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "displayName",
|
||||
"columnName": "displayName",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "authType",
|
||||
"columnName": "authType",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "sortOrder",
|
||||
"columnName": "sortOrder",
|
||||
"affinity": "INTEGER",
|
||||
"notNull": true,
|
||||
"defaultValue": "0"
|
||||
},
|
||||
{
|
||||
"fieldPath": "imap.host",
|
||||
"columnName": "imap_host",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "imap.port",
|
||||
"columnName": "imap_port",
|
||||
"affinity": "INTEGER",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "imap.security",
|
||||
"columnName": "imap_security",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "smtp.host",
|
||||
"columnName": "smtp_host",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "smtp.port",
|
||||
"columnName": "smtp_port",
|
||||
"affinity": "INTEGER",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "smtp.security",
|
||||
"columnName": "smtp_security",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
}
|
||||
],
|
||||
"primaryKey": {
|
||||
"autoGenerate": false,
|
||||
"columnNames": [
|
||||
"id"
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"tableName": "credentials",
|
||||
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `encryptedSecret` TEXT NOT NULL, PRIMARY KEY(`accountId`))",
|
||||
"fields": [
|
||||
{
|
||||
"fieldPath": "accountId",
|
||||
"columnName": "accountId",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "encryptedSecret",
|
||||
"columnName": "encryptedSecret",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
}
|
||||
],
|
||||
"primaryKey": {
|
||||
"autoGenerate": false,
|
||||
"columnNames": [
|
||||
"accountId"
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"tableName": "account_settings",
|
||||
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `signature` TEXT NOT NULL, `signatureEnabled` INTEGER NOT NULL, `notificationsEnabled` INTEGER NOT NULL, `retentionCount` INTEGER, `retentionMonths` INTEGER, PRIMARY KEY(`accountId`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
|
||||
"fields": [
|
||||
{
|
||||
"fieldPath": "accountId",
|
||||
"columnName": "accountId",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "signature",
|
||||
"columnName": "signature",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "signatureEnabled",
|
||||
"columnName": "signatureEnabled",
|
||||
"affinity": "INTEGER",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "notificationsEnabled",
|
||||
"columnName": "notificationsEnabled",
|
||||
"affinity": "INTEGER",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "retentionCount",
|
||||
"columnName": "retentionCount",
|
||||
"affinity": "INTEGER"
|
||||
},
|
||||
{
|
||||
"fieldPath": "retentionMonths",
|
||||
"columnName": "retentionMonths",
|
||||
"affinity": "INTEGER"
|
||||
}
|
||||
],
|
||||
"primaryKey": {
|
||||
"autoGenerate": false,
|
||||
"columnNames": [
|
||||
"accountId"
|
||||
]
|
||||
},
|
||||
"foreignKeys": [
|
||||
{
|
||||
"table": "accounts",
|
||||
"onDelete": "CASCADE",
|
||||
"onUpdate": "NO ACTION",
|
||||
"columns": [
|
||||
"accountId"
|
||||
],
|
||||
"referencedColumns": [
|
||||
"id"
|
||||
]
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"tableName": "signatures",
|
||||
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `name` TEXT NOT NULL, `contentHtml` TEXT NOT NULL, `isDefault` INTEGER NOT NULL, PRIMARY KEY(`id`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
|
||||
"fields": [
|
||||
{
|
||||
"fieldPath": "id",
|
||||
"columnName": "id",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "accountId",
|
||||
"columnName": "accountId",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "name",
|
||||
"columnName": "name",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "contentHtml",
|
||||
"columnName": "contentHtml",
|
||||
"affinity": "TEXT",
|
||||
"notNull": true
|
||||
},
|
||||
{
|
||||
"fieldPath": "isDefault",
|
||||
"columnName": "isDefault",
|
||||
"affinity": "INTEGER",
|
||||
"notNull": true
|
||||
}
|
||||
],
|
||||
"primaryKey": {
|
||||
"autoGenerate": false,
|
||||
"columnNames": [
|
||||
"id"
|
||||
]
|
||||
},
|
||||
"indices": [
|
||||
{
|
||||
"name": "index_signatures_accountId",
|
||||
"unique": false,
|
||||
"columnNames": [
|
||||
"accountId"
|
||||
],
|
||||
"orders": [],
|
||||
"createSql": "CREATE INDEX IF NOT EXISTS `index_signatures_accountId` ON `${TABLE_NAME}` (`accountId`)"
|
||||
}
|
||||
],
|
||||
"foreignKeys": [
|
||||
{
|
||||
"table": "accounts",
|
||||
"onDelete": "CASCADE",
|
||||
"onUpdate": "NO ACTION",
|
||||
"columns": [
|
||||
"accountId"
|
||||
],
|
||||
"referencedColumns": [
|
||||
"id"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"setupQueries": [
|
||||
"CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
|
||||
"INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '6705bfa56d02b55f47c5c66ba10143b0')"
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -221,11 +221,38 @@ class AccountDataMigratorTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun copiedAccountsGetAStableAlphabeticalInitialSortOrder() = runBlocking<Unit> {
|
||||
// Insert three accounts in non-alphabetical order so the assertion proves the initial order is
|
||||
// by email (the pre-#111 listing) rather than the copy's insertion order (issue #164). sortOrder
|
||||
// is a destination-only column the v14 cache never had, so it is assigned entirely by the copy.
|
||||
helper.createDatabase(cacheName, 14).apply {
|
||||
listOf("c" to "carol@example.org", "a" to "ada@example.org", "b" to "bob@example.org")
|
||||
.forEach { (id, email) ->
|
||||
execSQL(
|
||||
"INSERT INTO accounts (id, email, displayName, authType, imap_host, imap_port, " +
|
||||
"imap_security, smtp_host, smtp_port, smtp_security) VALUES " +
|
||||
"('$id', '$email', '$id', 'PASSWORD_IMAP', 'imap.example.org', 993, 'SSL_TLS', " +
|
||||
"'smtp.example.org', 465, 'SSL_TLS')",
|
||||
)
|
||||
}
|
||||
close()
|
||||
}
|
||||
|
||||
AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile)
|
||||
|
||||
openAccountsDb().apply {
|
||||
// getAll orders by sortOrder; the copy ranked by email, so ada(0) < bob(1) < carol(2).
|
||||
assertEquals(listOf("a", "b", "c"), accountDao().getAll().map { it.id })
|
||||
close()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun migratorDdlMatchesExportedAccountDatabaseSchema() {
|
||||
val schema = JSONObject(
|
||||
InstrumentationRegistry.getInstrumentation().context.assets
|
||||
.open("org.libremail.data.local.AccountDatabase/1.json")
|
||||
.open("org.libremail.data.local.AccountDatabase/2.json")
|
||||
.bufferedReader().use { it.readText() },
|
||||
).getJSONObject("database")
|
||||
val entities = schema.getJSONArray("entities")
|
||||
|
||||
@@ -39,9 +39,9 @@ class AccountDatabaseTest {
|
||||
@After
|
||||
fun tearDown() = db.close()
|
||||
|
||||
private fun account(id: String = "acct") = AccountEntity(
|
||||
private fun account(id: String = "acct", email: String = "$id@example.org") = AccountEntity(
|
||||
id = id,
|
||||
email = "a@example.org",
|
||||
email = email,
|
||||
displayName = "A",
|
||||
authType = "PASSWORD_IMAP",
|
||||
imap = ServerConfigEmbedded("imap.example.org", 993, "SSL_TLS"),
|
||||
@@ -69,6 +69,34 @@ class AccountDatabaseTest {
|
||||
assertNull("account_settings must cascade-delete with its account", db.accountSettingsDao().get("acct"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun insertAtEndAppendsAccountsInIncreasingSortOrder() = runBlocking<Unit> {
|
||||
db.accountDao().insertAtEnd(account("a"))
|
||||
db.accountDao().insertAtEnd(account("b"))
|
||||
db.accountDao().insertAtEnd(account("c"))
|
||||
|
||||
// Each new account lands at the end: current max + 1, starting from 0 (issue #164).
|
||||
assertEquals(0, db.accountDao().getById("a")?.sortOrder)
|
||||
assertEquals(1, db.accountDao().getById("b")?.sortOrder)
|
||||
assertEquals(2, db.accountDao().getById("c")?.sortOrder)
|
||||
// observeAll now orders by sortOrder, i.e. the append (insertion) order.
|
||||
assertEquals(listOf("a", "b", "c"), db.accountDao().observeAll().first().map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun reorderPersistsAndBothQueriesReflectTheNewOrder() = runBlocking<Unit> {
|
||||
listOf("a", "b", "c").forEach { db.accountDao().insertAtEnd(account(it)) }
|
||||
|
||||
db.accountDao().reorder(listOf("c", "a", "b"))
|
||||
|
||||
assertEquals(listOf("c", "a", "b"), db.accountDao().observeAll().first().map { it.id })
|
||||
assertEquals(listOf("c", "a", "b"), db.accountDao().getAll().map { it.id })
|
||||
// sortOrder is renumbered to the new positions, so the order survives an app restart.
|
||||
assertEquals(0, db.accountDao().getById("c")?.sortOrder)
|
||||
assertEquals(1, db.accountDao().getById("a")?.sortOrder)
|
||||
assertEquals(2, db.accountDao().getById("b")?.sortOrder)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun signaturesCascadeWithTheirAccount() = runBlocking<Unit> {
|
||||
db.accountDao().upsert(account())
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.local
|
||||
|
||||
import androidx.room.testing.MigrationTestHelper
|
||||
import androidx.sqlite.db.SupportSQLiteDatabase
|
||||
import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
|
||||
/**
|
||||
* Replays [AccountDatabase]'s migrations against the schema JSONs exported to `app/schemas` (shipped
|
||||
* to the test APK as assets). AccountDatabase is the non-auth account store split out of the cache in
|
||||
* issue #111; like the cache database it registers no destructive fallback, so a drifted migration
|
||||
* would crash upgrading users at first open — this test makes such drift fail in CI instead. Kept
|
||||
* separate from [MigrationTest] because each database has its own version line and MigrationTestHelper
|
||||
* binds to a single [androidx.room.RoomDatabase] class.
|
||||
*/
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class AccountMigrationTest {
|
||||
|
||||
@get:Rule
|
||||
val helper = MigrationTestHelper(
|
||||
InstrumentationRegistry.getInstrumentation(),
|
||||
AccountDatabase::class.java,
|
||||
emptyList(),
|
||||
FrameworkSQLiteOpenHelperFactory(),
|
||||
)
|
||||
|
||||
/**
|
||||
* v1 -> v2 (issue #164): `accounts.sortOrder` appears, and existing accounts are backfilled with a
|
||||
* stable initial order matching the previous alphabetical (`ORDER BY email`) listing. The rows are
|
||||
* inserted out of alphabetical order to prove the rank is by email, not insertion order.
|
||||
*/
|
||||
@Test
|
||||
fun migrate1To2_addsSortOrderBackfilledByEmailRank() {
|
||||
helper.createDatabase(TEST_DB, 1).apply {
|
||||
insertAccount("c", "carol@example.org")
|
||||
insertAccount("a", "ada@example.org")
|
||||
insertAccount("b", "bob@example.org")
|
||||
close()
|
||||
}
|
||||
|
||||
val db = helper.runMigrationsAndValidate(TEST_DB, 2, true, ACCOUNT_MIGRATION_1_2)
|
||||
|
||||
db.query("SELECT id, sortOrder FROM accounts ORDER BY sortOrder").use { c ->
|
||||
assertTrue(c.moveToFirst())
|
||||
assertEquals("ada ranks first alphabetically, so sortOrder 0", "a", c.getString(0))
|
||||
assertEquals(0, c.getInt(1))
|
||||
assertTrue(c.moveToNext())
|
||||
assertEquals("b", c.getString(0))
|
||||
assertEquals(1, c.getInt(1))
|
||||
assertTrue(c.moveToNext())
|
||||
assertEquals("c", c.getString(0))
|
||||
assertEquals(2, c.getInt(1))
|
||||
assertFalse("all three pre-upgrade accounts must survive, and nothing else", c.moveToNext())
|
||||
}
|
||||
db.close()
|
||||
}
|
||||
|
||||
private fun SupportSQLiteDatabase.insertAccount(id: String, email: String) {
|
||||
execSQL(
|
||||
"INSERT INTO accounts (id, email, displayName, authType, imap_host, imap_port, imap_security, " +
|
||||
"smtp_host, smtp_port, smtp_security) VALUES ('$id', '$email', '$id', 'PASSWORD_IMAP', " +
|
||||
"'imap.example.org', 993, 'SSL_TLS', 'smtp.example.org', 465, 'SSL_TLS')",
|
||||
)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val TEST_DB = "account-migration-test.db"
|
||||
}
|
||||
}
|
||||
@@ -62,6 +62,11 @@ class FakeAccountRepository(
|
||||
accountsFlow.value = accountsFlow.value.filterNot { it.id == id }
|
||||
}
|
||||
|
||||
override suspend fun reorderAccounts(orderedIds: List<String>) {
|
||||
val byId = accountsFlow.value.associateBy { it.id }
|
||||
accountsFlow.value = orderedIds.mapNotNull { byId[it] }
|
||||
}
|
||||
|
||||
override suspend fun resetBackfillProgress(accountId: String?) = Unit
|
||||
}
|
||||
|
||||
|
||||
@@ -108,7 +108,8 @@ class AccountDataMigrator @Inject constructor(
|
||||
internal val CREATE_TABLE_SQL = mapOf(
|
||||
"accounts" to
|
||||
"CREATE TABLE IF NOT EXISTS `accounts` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, " +
|
||||
"`displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `imap_host` TEXT NOT NULL, " +
|
||||
"`displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, " +
|
||||
"`sortOrder` INTEGER NOT NULL DEFAULT 0, `imap_host` TEXT NOT NULL, " +
|
||||
"`imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, " +
|
||||
"`smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))",
|
||||
"credentials" to
|
||||
@@ -168,6 +169,18 @@ class AccountDataMigrator @Inject constructor(
|
||||
val cols = sharedColumns(db, table)
|
||||
db.rawExecSQL("INSERT OR IGNORE INTO `$table` ($cols) SELECT $cols FROM cache.`$table`")
|
||||
}
|
||||
// sortOrder (issue #164) is a destination-only column the pre-#111 cache never had, so
|
||||
// the copy above leaves every account at its DEFAULT 0. Give them the same stable
|
||||
// alphabetical initial order ACCOUNT_MIGRATION_1_2 assigns (rank by email), so a
|
||||
// pre-#111 upgrade lands in the order it already showed rather than an undefined
|
||||
// tie-break. Post-#111 installs migrate via ACCOUNT_MIGRATION_1_2 instead and never
|
||||
// reach this copy; either way the user can then drag to reorder.
|
||||
if ("accounts" in present) {
|
||||
db.rawExecSQL(
|
||||
"UPDATE `accounts` SET `sortOrder` = " +
|
||||
"(SELECT COUNT(*) FROM `accounts` AS ranked WHERE ranked.`email` < `accounts`.`email`)",
|
||||
)
|
||||
}
|
||||
Log.d(TAG, "moved account tables into the account database: $present")
|
||||
} finally {
|
||||
db.rawExecSQL("DETACH DATABASE cache;")
|
||||
|
||||
@@ -40,7 +40,7 @@ import org.libremail.data.local.entity.SignatureEntity
|
||||
AccountSettingsEntity::class,
|
||||
SignatureEntity::class,
|
||||
],
|
||||
version = 1,
|
||||
version = 2,
|
||||
exportSchema = true,
|
||||
)
|
||||
abstract class AccountDatabase : RoomDatabase() {
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.local
|
||||
|
||||
import androidx.room.migration.Migration
|
||||
import androidx.sqlite.db.SupportSQLiteDatabase
|
||||
|
||||
/**
|
||||
* Migrations for [AccountDatabase] — the non-auth account store split out of the cache database in
|
||||
* issue #111. Kept in a SEPARATE file from the cache database's `MIGRATION_x_y` chain in
|
||||
* `Migrations.kt` on purpose: both databases number their own schema versions from 1, and
|
||||
* `MigrationTest` reflectively pulls every `Migration` val out of the `Migrations.kt` file facade to
|
||||
* assert the cache chain is unbroken — an AccountDatabase migration declared alongside them would
|
||||
* pollute that assertion with a second `1 -> 2`. A distinct file (and the `ACCOUNT_` prefix) keeps
|
||||
* the two chains from colliding at the package level.
|
||||
*/
|
||||
|
||||
/**
|
||||
* AccountDatabase v1 -> v2 (issue #164): add the user-controlled [AccountEntity.sortOrder]. Existing
|
||||
* accounts are given a stable initial order matching the previous alphabetical (`ORDER BY email`)
|
||||
* listing — each account's rank among the others by email — so the order they were already shown in
|
||||
* doesn't shuffle on upgrade. `DEFAULT 0` matches the entity's `@ColumnInfo(defaultValue = "0")` so a
|
||||
* fresh install validates identically to a migrated one (the folders `specialUse` MIGRATION_11_12
|
||||
* pattern); the backfill then overwrites that default with the real rank.
|
||||
*/
|
||||
val ACCOUNT_MIGRATION_1_2 = object : Migration(1, 2) {
|
||||
override fun migrate(db: SupportSQLiteDatabase) {
|
||||
db.execSQL("ALTER TABLE `accounts` ADD COLUMN `sortOrder` INTEGER NOT NULL DEFAULT 0")
|
||||
db.execSQL(
|
||||
"UPDATE `accounts` SET `sortOrder` = " +
|
||||
"(SELECT COUNT(*) FROM `accounts` AS ranked WHERE ranked.`email` < `accounts`.`email`)",
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -5,15 +5,16 @@ import androidx.room.Dao
|
||||
import androidx.room.Insert
|
||||
import androidx.room.OnConflictStrategy
|
||||
import androidx.room.Query
|
||||
import androidx.room.Transaction
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import org.libremail.data.local.entity.AccountEntity
|
||||
|
||||
@Dao
|
||||
interface AccountDao {
|
||||
@Query("SELECT * FROM accounts ORDER BY email")
|
||||
@Query("SELECT * FROM accounts ORDER BY sortOrder")
|
||||
fun observeAll(): Flow<List<AccountEntity>>
|
||||
|
||||
@Query("SELECT * FROM accounts ORDER BY email")
|
||||
@Query("SELECT * FROM accounts ORDER BY sortOrder")
|
||||
suspend fun getAll(): List<AccountEntity>
|
||||
|
||||
@Query("SELECT * FROM accounts WHERE id = :id LIMIT 1")
|
||||
@@ -22,6 +23,32 @@ interface AccountDao {
|
||||
@Insert(onConflict = OnConflictStrategy.REPLACE)
|
||||
suspend fun upsert(account: AccountEntity)
|
||||
|
||||
/** The sortOrder that appends a new account to the end of the list (0 when there are none yet). */
|
||||
@Query("SELECT COALESCE(MAX(sortOrder), -1) + 1 FROM accounts")
|
||||
suspend fun nextSortOrder(): Int
|
||||
|
||||
/**
|
||||
* Insert [account] at the end of the user-defined order (issue #164), stamping it with the current
|
||||
* max + 1. Both steps run in one transaction so two near-simultaneous adds can't collide on a
|
||||
* sortOrder.
|
||||
*/
|
||||
@Transaction
|
||||
suspend fun insertAtEnd(account: AccountEntity) {
|
||||
upsert(account.copy(sortOrder = nextSortOrder()))
|
||||
}
|
||||
|
||||
@Query("UPDATE accounts SET sortOrder = :sortOrder WHERE id = :id")
|
||||
suspend fun setSortOrder(id: String, sortOrder: Int)
|
||||
|
||||
/**
|
||||
* Persist a user-chosen ordering (issue #164): each account is stamped with its index in
|
||||
* [orderedIds]. Wrapped in a transaction so the list is never observed half-renumbered.
|
||||
*/
|
||||
@Transaction
|
||||
suspend fun reorder(orderedIds: List<String>) {
|
||||
orderedIds.forEachIndexed { index, id -> setSortOrder(id, index) }
|
||||
}
|
||||
|
||||
@Query("DELETE FROM accounts WHERE id = :id")
|
||||
suspend fun deleteById(id: String)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.local.entity
|
||||
|
||||
import androidx.room.ColumnInfo
|
||||
import androidx.room.Embedded
|
||||
import androidx.room.Entity
|
||||
import androidx.room.PrimaryKey
|
||||
@@ -14,6 +15,14 @@ data class AccountEntity(
|
||||
val authType: String,
|
||||
@Embedded(prefix = "imap_") val imap: ServerConfigEmbedded,
|
||||
@Embedded(prefix = "smtp_") val smtp: ServerConfigEmbedded,
|
||||
/**
|
||||
* The account's position in the user-defined ordering shown across the app (issue #164): the
|
||||
* Settings list, the drawer's account switcher, and the unified-inbox filter chips all list
|
||||
* accounts `ORDER BY sortOrder`. New accounts are appended (current max + 1); the user can drag to
|
||||
* reorder. `DEFAULT 0` matches [ACCOUNT_MIGRATION_1_2] so a fresh install validates identically to
|
||||
* a migrated one (the folders `specialUse` pattern).
|
||||
*/
|
||||
@ColumnInfo(defaultValue = "0") val sortOrder: Int = 0,
|
||||
)
|
||||
|
||||
/** Embedded host/port/security columns (prefixed per server in [AccountEntity]). */
|
||||
|
||||
@@ -44,7 +44,7 @@ class AccountRepositoryImpl @Inject constructor(
|
||||
|
||||
override suspend fun addImapAccount(account: Account, password: String): Result<List<String>> = runCatching {
|
||||
val folders = imapClient.listFolders(account.toImapParams(secret = password, useXoauth2 = false))
|
||||
accountDao.upsert(account.toEntity())
|
||||
accountDao.insertAtEnd(account.toEntity())
|
||||
accountSettingsRepository.ensureDefaults(account.id)
|
||||
credentialStore.saveSecret(account.id, password)
|
||||
mailNotifier.ensureAccountChannel(account)
|
||||
@@ -60,7 +60,7 @@ class AccountRepositoryImpl @Inject constructor(
|
||||
): Result<List<String>> = runCatching {
|
||||
val account = Account.outlook(email)
|
||||
val folders = imapClient.listFolders(account.toImapParams(secret = accessToken, useXoauth2 = true))
|
||||
accountDao.upsert(account.toEntity())
|
||||
accountDao.insertAtEnd(account.toEntity())
|
||||
accountSettingsRepository.ensureDefaults(account.id)
|
||||
credentialStore.saveSecret(account.id, authStateJson)
|
||||
mailNotifier.ensureAccountChannel(account)
|
||||
@@ -69,6 +69,8 @@ class AccountRepositoryImpl @Inject constructor(
|
||||
folders.map { it.fullName }
|
||||
}
|
||||
|
||||
override suspend fun reorderAccounts(orderedIds: List<String>) = accountDao.reorder(orderedIds)
|
||||
|
||||
override suspend fun deleteAccount(id: String) {
|
||||
accountDao.deleteById(id)
|
||||
credentialStore.delete(id)
|
||||
|
||||
@@ -10,6 +10,7 @@ import dagger.hilt.InstallIn
|
||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||
import dagger.hilt.components.SingletonComponent
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.libremail.data.local.ACCOUNT_MIGRATION_1_2
|
||||
import org.libremail.data.local.AccountDatabase
|
||||
import org.libremail.data.local.DatabaseFiles.ACCOUNTS_NAME
|
||||
import org.libremail.data.local.DatabaseProvisioner
|
||||
@@ -43,6 +44,7 @@ object AccountDatabaseModule {
|
||||
@ApplicationContext context: Context,
|
||||
provisioner: DatabaseProvisioner,
|
||||
): AccountDatabase = Room.databaseBuilder(context, AccountDatabase::class.java, ACCOUNTS_NAME)
|
||||
.addMigrations(ACCOUNT_MIGRATION_1_2)
|
||||
.openHelperFactory(
|
||||
DeferredOpenHelperFactory { configuration ->
|
||||
runBlocking { provisioner.prepareCache() }
|
||||
|
||||
@@ -20,6 +20,13 @@ interface AccountRepository {
|
||||
|
||||
suspend fun deleteAccount(id: String)
|
||||
|
||||
/**
|
||||
* Persist a user-chosen account order (issue #164). [orderedIds] is the new top-to-bottom order;
|
||||
* every account's stored position is updated to its index in the list. All surfaces that list
|
||||
* accounts (Settings, the drawer switcher, the unified-inbox filter chips) then follow it.
|
||||
*/
|
||||
suspend fun reorderAccounts(orderedIds: List<String>)
|
||||
|
||||
/**
|
||||
* Discards full-history backfill progress so it re-evaluates against the current retention floor
|
||||
* after a retention change: tightening re-hits the (tighter) floor cheaply, loosening resumes
|
||||
|
||||
@@ -0,0 +1,157 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.settings
|
||||
|
||||
import androidx.compose.animation.core.animateDpAsState
|
||||
import androidx.compose.foundation.clickable
|
||||
import androidx.compose.foundation.gestures.detectDragGesturesAfterLongPress
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.Row
|
||||
import androidx.compose.foundation.layout.fillMaxWidth
|
||||
import androidx.compose.foundation.layout.offset
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.material.icons.Icons
|
||||
import androidx.compose.material.icons.filled.Menu
|
||||
import androidx.compose.material3.Icon
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.Surface
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableFloatStateOf
|
||||
import androidx.compose.runtime.mutableIntStateOf
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.input.pointer.pointerInput
|
||||
import androidx.compose.ui.layout.onSizeChanged
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import androidx.compose.ui.unit.IntOffset
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.compose.ui.zIndex
|
||||
import org.libremail.R
|
||||
import org.libremail.domain.model.Account
|
||||
import kotlin.math.roundToInt
|
||||
|
||||
/**
|
||||
* The Settings accounts list with long-press drag-to-reorder (issue #164). Tapping a row still opens
|
||||
* that account; a long-press lifts it and the drag follows the finger, and on release the new
|
||||
* top-to-bottom order is persisted via [onReorder] (the account ids, in order). Every surface that
|
||||
* lists accounts — this list, the drawer switcher, the unified-inbox filter chips — reads the same
|
||||
* `ORDER BY sortOrder` query, so they all follow the committed order automatically.
|
||||
*
|
||||
* Built with `pointerInput` + `Modifier.offset` over a plain [Column] rather than a `LazyColumn` or a
|
||||
* reorderable-list dependency: the account list is tiny (a handful of rows), it lives inside an
|
||||
* already-scrolling settings column (a nested lazy list can't share that scroll), and the repo
|
||||
* prefers no unnecessary dependencies. Only the dragged row moves during the gesture; the list
|
||||
* commits to the dropped position on release, so no mid-drag recomposition can cancel the gesture.
|
||||
*/
|
||||
@Composable
|
||||
internal fun AccountReorderList(
|
||||
accounts: List<Account>,
|
||||
onOpenAccount: (String) -> Unit,
|
||||
onReorder: (List<String>) -> Unit,
|
||||
) {
|
||||
// Local working order, re-seeded from upstream whenever the account set changes (add / remove / an
|
||||
// external reorder) via the remember key. A drag never changes [accounts], so it stays put until
|
||||
// the drop optimistically sets the new order — which upstream then echoes back identically.
|
||||
var order by remember(accounts) { mutableStateOf(accounts) }
|
||||
var draggingId by remember { mutableStateOf<String?>(null) }
|
||||
var dragOffsetY by remember { mutableFloatStateOf(0f) }
|
||||
var rowHeightPx by remember { mutableIntStateOf(0) }
|
||||
|
||||
Column(Modifier.fillMaxWidth()) {
|
||||
order.forEach { account ->
|
||||
val dragging = account.id == draggingId
|
||||
AccountReorderRow(
|
||||
id = account.id,
|
||||
email = account.email,
|
||||
dragging = dragging,
|
||||
offsetY = if (dragging) dragOffsetY.roundToInt() else 0,
|
||||
onMeasured = { rowHeightPx = it },
|
||||
onOpen = { onOpenAccount(account.id) },
|
||||
onDragStart = {
|
||||
draggingId = account.id
|
||||
dragOffsetY = 0f
|
||||
},
|
||||
onDrag = { delta -> dragOffsetY += delta },
|
||||
onDragEnd = {
|
||||
commitDrag(order, account.id, dragOffsetY, rowHeightPx)?.let { moved ->
|
||||
order = moved
|
||||
onReorder(moved.map { it.id })
|
||||
}
|
||||
draggingId = null
|
||||
dragOffsetY = 0f
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The list [current] reordered by moving the account [id] the number of row-slots its accumulated
|
||||
* [dragOffsetY] spans (at [rowHeightPx] each), or null when the drop leaves it in place. Pure so the
|
||||
* index maths is unit-testable without a gesture.
|
||||
*/
|
||||
internal fun commitDrag(current: List<Account>, id: String, dragOffsetY: Float, rowHeightPx: Int): List<Account>? {
|
||||
if (rowHeightPx <= 0) return null
|
||||
val from = current.indexOfFirst { it.id == id }
|
||||
if (from == -1) return null
|
||||
val to = (from + (dragOffsetY / rowHeightPx).roundToInt()).coerceIn(0, current.lastIndex)
|
||||
if (to == from) return null
|
||||
return current.toMutableList().apply { add(to, removeAt(from)) }
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun AccountReorderRow(
|
||||
id: String,
|
||||
email: String,
|
||||
dragging: Boolean,
|
||||
offsetY: Int,
|
||||
onMeasured: (Int) -> Unit,
|
||||
onOpen: () -> Unit,
|
||||
onDragStart: () -> Unit,
|
||||
onDrag: (Float) -> Unit,
|
||||
onDragEnd: () -> Unit,
|
||||
) {
|
||||
val elevation by animateDpAsState(if (dragging) 6.dp else 0.dp, label = "accountDragElevation")
|
||||
Surface(
|
||||
color = if (dragging) MaterialTheme.colorScheme.surfaceVariant else MaterialTheme.colorScheme.surface,
|
||||
tonalElevation = elevation,
|
||||
shadowElevation = elevation,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.zIndex(if (dragging) 1f else 0f)
|
||||
.offset { IntOffset(0, offsetY) }
|
||||
.onSizeChanged { onMeasured(it.height) },
|
||||
) {
|
||||
Row(
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.clickable(onClick = onOpen)
|
||||
// Keyed on the account id so the gesture restarts only when this slot's account
|
||||
// changes (an add/remove/re-seed) — never mid-drag, when the order is held stable.
|
||||
.pointerInput(id) {
|
||||
detectDragGesturesAfterLongPress(
|
||||
onDragStart = { onDragStart() },
|
||||
onDrag = { change, dragAmount ->
|
||||
change.consume()
|
||||
onDrag(dragAmount.y)
|
||||
},
|
||||
onDragEnd = onDragEnd,
|
||||
onDragCancel = onDragEnd,
|
||||
)
|
||||
}
|
||||
.padding(horizontal = 16.dp, vertical = 16.dp),
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
) {
|
||||
Text(text = email, style = MaterialTheme.typography.bodyLarge, modifier = Modifier.weight(1f))
|
||||
Icon(
|
||||
imageVector = Icons.Filled.Menu,
|
||||
contentDescription = stringResource(R.string.account_reorder_handle),
|
||||
tint = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -125,9 +125,11 @@ fun SettingsScreen(
|
||||
modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp),
|
||||
)
|
||||
} else {
|
||||
accounts.forEach { account ->
|
||||
ClickRow(title = account.email, onClick = { onOpenAccount(account.id) })
|
||||
}
|
||||
AccountReorderList(
|
||||
accounts = accounts,
|
||||
onOpenAccount = onOpenAccount,
|
||||
onReorder = viewModel::reorderAccounts,
|
||||
)
|
||||
}
|
||||
ClickRow(title = stringResource(R.string.settings_add_account), onClick = onAddAccount)
|
||||
HorizontalDivider()
|
||||
|
||||
@@ -64,6 +64,9 @@ class SettingsViewModel @Inject constructor(
|
||||
|
||||
fun toggleAdvanced() = _advancedExpanded.update { !it }
|
||||
|
||||
/** Persist the account order the user produced by dragging in Settings (issue #164). */
|
||||
fun reorderAccounts(orderedIds: List<String>) = update { accountRepository.reorderAccounts(orderedIds) }
|
||||
|
||||
/** Re-read the battery-optimization status; call when the screen resumes (e.g. back from Settings). */
|
||||
fun refreshBatteryStatus() {
|
||||
_batteryUnrestricted.value = batteryOptimizationManager.isIgnoringBatteryOptimizations()
|
||||
|
||||
@@ -260,6 +260,7 @@
|
||||
<string name="settings_new_mail">New mail notifications</string>
|
||||
<string name="settings_new_mail_summary">Show a notification when new mail arrives</string>
|
||||
<string name="settings_no_accounts">No accounts yet</string>
|
||||
<string name="account_reorder_handle">Long-press and drag to reorder account</string>
|
||||
<string name="account_remove">Remove account</string>
|
||||
|
||||
<!-- Backup -->
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.settings
|
||||
|
||||
import org.junit.Test
|
||||
import org.libremail.domain.model.Account
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
|
||||
/** The pure reorder-index maths behind the Settings drag-to-reorder list (issue #164). */
|
||||
class AccountReorderListTest {
|
||||
|
||||
private val accounts = listOf("a", "b", "c").map { Account.outlook("$it@example.org") }
|
||||
private val ids get() = accounts.map { it.id }
|
||||
|
||||
@Test
|
||||
fun `dragging the first row two slots down moves it to the end`() {
|
||||
val moved = commitDrag(accounts, id = ids[0], dragOffsetY = 200f, rowHeightPx = 100)
|
||||
assertEquals(listOf(ids[1], ids[2], ids[0]), moved?.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `dragging the last row two slots up moves it to the front`() {
|
||||
val moved = commitDrag(accounts, id = ids[2], dragOffsetY = -200f, rowHeightPx = 100)
|
||||
assertEquals(listOf(ids[2], ids[0], ids[1]), moved?.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an offset under half a row height keeps the position`() {
|
||||
assertNull(commitDrag(accounts, id = ids[0], dragOffsetY = 40f, rowHeightPx = 100))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a drop dragged past the ends is clamped to a valid index`() {
|
||||
val moved = commitDrag(accounts, id = ids[0], dragOffsetY = 900f, rowHeightPx = 100)
|
||||
assertEquals(listOf(ids[1], ids[2], ids[0]), moved?.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unmeasured row height cannot reorder`() {
|
||||
assertNull(commitDrag(accounts, id = ids[0], dragOffsetY = 200f, rowHeightPx = 0))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unknown id cannot reorder`() {
|
||||
assertNull(commitDrag(accounts, id = "missing", dragOffsetY = 200f, rowHeightPx = 100))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user