From 0f479b24311006a9f53da3d0a0453879805a5b86 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 15:01:11 -0500 Subject: [PATCH 1/2] fix(mailbox): de-duplicate folder names in the drawer The drawer rendered every standard-role folder with a generic friendly name (e.g. "Drafts") and discarded the server name, so a Gmail account with both a provider built-in folder and a same-named user folder showed two identical entries (Drafts, Archive, Spam). De-duplicate labels provider-agnostically: when 2+ folders would render the same name, the provider's built-in special folder (identified by RFC 6154 SPECIAL-USE flags, now persisted on the folder cache) gets the provider name appended ("Archive - Gmail"), a nested user folder gets its parent location ("Reports (Work)"), and a top-level user folder keeps its plain name. Only triggers on a real collision, so stock accounts are unchanged. Adds a `specialUse` column to the folders table (Room v11 -> v12). Co-Authored-By: Claude Opus 4.8 --- .../12.json | 606 ++++++++++++++++++ .../data/local/LibreMailDatabaseTest.kt | 14 +- .../libremail/ui/mailbox/FolderDrawerTest.kt | 30 +- .../libremail/data/local/LibreMailDatabase.kt | 2 +- .../org/libremail/data/local/Mappers.kt | 2 + .../org/libremail/data/local/Migrations.kt | 13 + .../data/local/entity/FolderEntity.kt | 7 + .../kotlin/org/libremail/di/DatabaseModule.kt | 2 + .../org/libremail/domain/model/Folder.kt | 16 + .../libremail/domain/model/MailProvider.kt | 4 + .../org/libremail/ui/mailbox/FolderDrawer.kt | 6 +- .../org/libremail/ui/mailbox/FolderLabels.kt | 67 ++ .../libremail/domain/model/FolderRoleTest.kt | 12 + .../libremail/ui/mailbox/FolderLabelsTest.kt | 131 ++++ 14 files changed, 902 insertions(+), 10 deletions(-) create mode 100644 app/schemas/org.libremail.data.local.LibreMailDatabase/12.json create mode 100644 app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt create mode 100644 app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt diff --git a/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json b/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json new file mode 100644 index 0000000..f27dd67 --- /dev/null +++ b/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json @@ -0,0 +1,606 @@ +{ + "formatVersion": 1, + "database": { + "version": 12, + "identityHash": "45329c820e9325adbfd6b7de89a9996a", + "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, `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": "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": "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, 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 + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + }, + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "messages", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `sender` TEXT NOT NULL, `senderEmail` TEXT NOT NULL, `subject` TEXT NOT NULL, `snippet` TEXT NOT NULL, `body` TEXT NOT NULL, `isHtml` INTEGER NOT NULL, `timestampMillis` INTEGER NOT NULL, `isRead` INTEGER NOT NULL, `isStarred` INTEGER NOT NULL, `folder` TEXT NOT NULL DEFAULT 'INBOX', `inInbox` INTEGER NOT NULL, `bodyFetched` INTEGER NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sender", + "columnName": "sender", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "senderEmail", + "columnName": "senderEmail", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "snippet", + "columnName": "snippet", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isHtml", + "columnName": "isHtml", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "timestampMillis", + "columnName": "timestampMillis", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isRead", + "columnName": "isRead", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isStarred", + "columnName": "isStarred", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'INBOX'" + }, + { + "fieldPath": "inInbox", + "columnName": "inInbox", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "bodyFetched", + "columnName": "bodyFetched", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_messages_accountId", + "unique": false, + "columnNames": [ + "accountId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId` ON `${TABLE_NAME}` (`accountId`)" + }, + { + "name": "index_messages_timestampMillis", + "unique": false, + "columnNames": [ + "timestampMillis" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_timestampMillis` ON `${TABLE_NAME}` (`timestampMillis`)" + } + ] + }, + { + "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": "attachments", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`messageId` TEXT NOT NULL, `partIndex` INTEGER NOT NULL, `filename` TEXT NOT NULL, `mimeType` TEXT NOT NULL, `sizeBytes` INTEGER NOT NULL, PRIMARY KEY(`messageId`, `partIndex`), FOREIGN KEY(`messageId`) REFERENCES `messages`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "messageId", + "columnName": "messageId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "partIndex", + "columnName": "partIndex", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "filename", + "columnName": "filename", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "mimeType", + "columnName": "mimeType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sizeBytes", + "columnName": "sizeBytes", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "messageId", + "partIndex" + ] + }, + "indices": [ + { + "name": "index_attachments_messageId", + "unique": false, + "columnNames": [ + "messageId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_attachments_messageId` ON `${TABLE_NAME}` (`messageId`)" + } + ], + "foreignKeys": [ + { + "table": "messages", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "messageId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "outbox", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `createdAt` INTEGER NOT NULL, `lastError` TEXT, `bodyHtml` TEXT, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "createdAt", + "columnName": "createdAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastError", + "columnName": "lastError", + "affinity": "TEXT" + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "drafts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `updatedAt` INTEGER NOT NULL, `attachments` TEXT NOT NULL, `bodyHtml` TEXT, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT" + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "updatedAt", + "columnName": "updatedAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "attachments", + "columnName": "attachments", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "folders", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `fullName` TEXT NOT NULL, `displayName` TEXT NOT NULL, `role` TEXT NOT NULL, `selectable` INTEGER NOT NULL, `sortOrder` INTEGER NOT NULL, `specialUse` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`accountId`, `fullName`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "fullName", + "columnName": "fullName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "role", + "columnName": "role", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "selectable", + "columnName": "selectable", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "sortOrder", + "columnName": "sortOrder", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "specialUse", + "columnName": "specialUse", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "fullName" + ] + } + }, + { + "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, '45329c820e9325adbfd6b7de89a9996a')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt index 59c8b69..3019b28 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -132,15 +132,17 @@ class LibreMailDatabaseTest { folderDao.replaceForAccount( "acct", listOf( - FolderEntity("acct", "[Gmail]/Sent Mail", "Sent Mail", "SENT", selectable = true, sortOrder = 1), + FolderEntity( + "acct", "[Gmail]/Sent Mail", "Sent Mail", "SENT", + selectable = true, sortOrder = 1, specialUse = true, + ), FolderEntity("acct", "INBOX", "INBOX", "INBOX", selectable = true, sortOrder = 0), ), ) - // observeForAccount returns folders ordered by sortOrder. - assertEquals( - listOf("INBOX", "[Gmail]/Sent Mail"), - folderDao.observeForAccount("acct").first().map { it.fullName }, - ) + // observeForAccount returns folders ordered by sortOrder, with specialUse round-tripped. + val stored = folderDao.observeForAccount("acct").first() + assertEquals(listOf("INBOX", "[Gmail]/Sent Mail"), stored.map { it.fullName }) + assertEquals(listOf(false, true), stored.map { it.specialUse }) // replaceForAccount swaps the whole set (delete + insert). folderDao.replaceForAccount("acct", listOf(FolderEntity("acct", "Archive", "Archive", "ARCHIVE", true, 0))) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt index 87516f6..fbe7abb 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt @@ -58,6 +58,27 @@ class FolderDrawerTest { composeTestRule.onNodeWithText(string(R.string.folder_all_inboxes)).assertDoesNotExist() } + @Test + fun duplicateFolderNames_areDisambiguatedWithProviderSuffix() { + val gmail = account("imap:g", "user@gmail.com").copy( + imap = ServerConfig("imap.gmail.com", 993, MailSecurity.SSL_TLS), + ) + setContent( + accounts = listOf(gmail), + drawerAccount = gmail, + folders = listOf( + folder("imap:g", "INBOX", "INBOX", FolderRole.INBOX), + // Gmail's built-in Drafts (server special-use) alongside a same-named user folder. + folder("imap:g", "[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("imap:g", "Drafts", "Drafts", FolderRole.DRAFTS), + ), + ) + + // The provider's built-in folder is suffixed; the user folder keeps the plain name. + composeTestRule.onNodeWithText("Drafts - Gmail").assertIsDisplayed() + composeTestRule.onNodeWithText("Drafts").assertIsDisplayed() + } + @Test fun tappingAFolder_reportsItsAccountAndFullName() { var picked: Pair? = null @@ -131,6 +152,11 @@ class FolderDrawerTest { smtp = ServerConfig("smtp.example.org", 465, MailSecurity.SSL_TLS), ) - private fun folder(accountId: String, fullName: String, displayName: String, role: FolderRole) = - Folder(accountId, fullName, displayName, role, selectable = true) + private fun folder( + accountId: String, + fullName: String, + displayName: String, + role: FolderRole, + specialUse: Boolean = false, + ) = Folder(accountId, fullName, displayName, role, selectable = true, specialUse = specialUse) } diff --git a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt index d65ee85..30e87ab 100644 --- a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt +++ b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt @@ -34,7 +34,7 @@ import org.libremail.data.local.entity.SignatureEntity FolderEntity::class, SignatureEntity::class, ], - version = 11, + version = 12, exportSchema = true, ) abstract class LibreMailDatabase : RoomDatabase() { diff --git a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt index 0b970b9..831bc2f 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt @@ -150,6 +150,7 @@ internal fun FolderEntity.toDomain(): Folder = Folder( displayName = displayName, role = runCatching { FolderRole.valueOf(role) }.getOrDefault(FolderRole.NORMAL), selectable = selectable, + specialUse = specialUse, ) internal fun FetchedFolder.toEntity(accountId: String, sortOrder: Int): FolderEntity = FolderEntity( @@ -159,6 +160,7 @@ internal fun FetchedFolder.toEntity(accountId: String, sortOrder: Int): FolderEn role = FolderRole.roleOf(fullName, displayName, attributes).name, selectable = selectable, sortOrder = sortOrder, + specialUse = FolderRole.isServerSpecial(attributes), ) internal fun AttachmentEntity.toDomain(): Attachment = Attachment( diff --git a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt index dcece0e..03dd030 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -216,3 +216,16 @@ val MIGRATION_10_11 = object : Migration(10, 11) { ) } } + +/** + * v11 -> v12: drawer folder de-duplication (preserves existing data). Adds a `specialUse` column to + * `folders` recording whether the server advertises the folder as special-use (RFC 6154), so the + * drawer can tell a provider's built-in folder from a same-named user folder. `DEFAULT 0` matches + * the entity's `@ColumnInfo(defaultValue = "0")` so fresh-install and migrated schemas validate + * identically (the MIGRATION_9_10 pattern); the next folder refresh backfills the real value. + */ +val MIGRATION_11_12 = object : Migration(11, 12) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE `folders` ADD COLUMN `specialUse` INTEGER NOT NULL DEFAULT 0") + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt index d0523cb..10ee226 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt @@ -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.Entity /** A cached IMAP folder for an account. [sortOrder] preserves the server's listing order. */ @@ -13,4 +14,10 @@ data class FolderEntity( val role: String, val selectable: Boolean, val sortOrder: Int, + /** + * True when the server advertises this as a special-use folder (RFC 6154, e.g. `\Drafts`, + * `\Junk`, `\All`). Distinguishes a provider's built-in folder from a same-named user folder + * when the drawer de-duplicates display labels. + */ + @ColumnInfo(defaultValue = "0") val specialUse: Boolean = false, ) diff --git a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt index 1920ea3..3bbfe19 100644 --- a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt +++ b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt @@ -14,6 +14,7 @@ import net.zetetic.database.sqlcipher.SupportOpenHelperFactory import org.libremail.data.local.DatabaseEncryption import org.libremail.data.local.LibreMailDatabase import org.libremail.data.local.MIGRATION_10_11 +import org.libremail.data.local.MIGRATION_11_12 import org.libremail.data.local.MIGRATION_1_2 import org.libremail.data.local.MIGRATION_2_3 import org.libremail.data.local.MIGRATION_3_4 @@ -59,6 +60,7 @@ object DatabaseModule { MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, + MIGRATION_11_12, ) // No destructive fallback: the migration chain is complete, and silently dropping the // accounts/credentials/mail tables would lose stored secrets. A missing migration should diff --git a/app/src/main/kotlin/org/libremail/domain/model/Folder.kt b/app/src/main/kotlin/org/libremail/domain/model/Folder.kt index f3f2ba2..0b8de14 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/Folder.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/Folder.kt @@ -11,6 +11,11 @@ data class Folder( val role: FolderRole, /** False for \Noselect containers (e.g. Gmail's "[Gmail]" parent) that hold no messages. */ val selectable: Boolean, + /** + * True when the server advertises this as a special-use folder (RFC 6154). Lets the drawer tell + * a provider's built-in folder from a same-named user folder when de-duplicating labels. + */ + val specialUse: Boolean = false, ) /** @@ -47,6 +52,17 @@ enum class FolderRole { return byAttribute ?: roleFromDisplayName(displayName) } + /** + * The RFC 6154 SPECIAL-USE attributes (plus Gmail's `\All`) that mark a folder as one the + * server provisions itself, as opposed to a user-created folder. + */ + private val SPECIAL_USE_ATTRIBUTES = + setOf("\\all", "\\archive", "\\drafts", "\\flagged", "\\junk", "\\sent", "\\trash") + + /** True when the server advertises any SPECIAL-USE attribute for the folder (RFC 6154). */ + fun isServerSpecial(attributes: List): Boolean = + attributes.any { it.lowercase() in SPECIAL_USE_ATTRIBUTES } + /** Best-effort role from a folder's display name, for servers without SPECIAL-USE flags. */ private fun roleFromDisplayName(displayName: String): FolderRole = when (displayName.lowercase().trim()) { "sent", "sent mail", "sent items", "sent messages" -> SENT diff --git a/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt b/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt index c5a7188..927b32e 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt @@ -86,5 +86,9 @@ enum class MailProvider( companion object { /** Resolves a provider by its [key], or null if none matches (case-insensitive). */ fun fromKey(key: String): MailProvider? = entries.firstOrNull { it.key.equals(key, ignoreCase = true) } + + /** Resolves a provider by its IMAP host, or null if none matches (case-insensitive). */ + fun forImapHost(host: String): MailProvider? = + entries.firstOrNull { it.imapHost.equals(host, ignoreCase = true) } } } diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt index a6b0a55..edef7eb 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt @@ -77,6 +77,10 @@ fun FolderDrawer( val sorted = remember(folders) { folders.sortedWith(compareBy({ it.role.ordinal }, { it.displayName.lowercase() })) } + // De-duplicate labels: two distinct folders that would render the same name (e.g. a + // provider's built-in Drafts and a same-named user folder) get disambiguated. + val baseLabels = sorted.associate { it.fullName to folderDisplayLabel(it) } + val resolvedLabels = resolveDrawerLabels(sorted, baseLabels, drawerAccount?.let(::providerLabel).orEmpty()) sorted.forEach { folder -> val isSelected = selectedAccountId != null && selectedAccountId == drawerAccount?.id && @@ -85,7 +89,7 @@ fun FolderDrawer( { Icon(vector, contentDescription = null) } } NavigationDrawerItem( - label = { Text(folderDisplayLabel(folder)) }, + label = { Text(resolvedLabels[folder.fullName] ?: folderDisplayLabel(folder)) }, icon = iconContent, selected = isSelected, onClick = { diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt new file mode 100644 index 0000000..c65b85b --- /dev/null +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.mailbox + +import org.libremail.domain.model.Account +import org.libremail.domain.model.AuthType +import org.libremail.domain.model.Folder +import org.libremail.domain.model.MailProvider + +/** + * The provider name appended when de-duplicating a folder label (the "Gmail" in "Drafts - Gmail"). + * A recognized brand where possible, else the account's email domain so any account has a suffix. + */ +fun providerLabel(account: Account): String = when { + account.authType == AuthType.OAUTH_OUTLOOK || + account.imap.host.contains("outlook", ignoreCase = true) || + account.imap.host.contains("office365", ignoreCase = true) -> "Outlook" + + else -> MailProvider.forImapHost(account.imap.host)?.displayName + ?: account.email.substringAfterLast('@', account.imap.host) +} + +/** + * Resolves each folder's drawer label so no two entries collide. [baseLabels] maps a folder's + * [Folder.fullName] to its localized base label (a friendly role name, or the raw display name). + * + * A base label shared by 2+ folders is disambiguated: the provider's built-in special folder gets + * " - [providerLabel]" (e.g. "Archive - Gmail"); a nested user folder gets its parent location in + * parentheses (e.g. "Reports (Work)"); a top-level user folder keeps its base label. Among colliding + * user folders at most one is top-level (paths are unique), so the result is distinct. A final pass + * appends the full path to any labels that still tie, guaranteeing uniqueness. + */ +fun resolveDrawerLabels( + folders: List, + baseLabels: Map, + providerLabel: String, +): Map { + fun base(folder: Folder) = baseLabels[folder.fullName] ?: folder.displayName + val duplicated = folders.groupingBy(::base).eachCount().filterValues { it > 1 }.keys + + val resolved = folders.associate { folder -> + val base = base(folder) + val label = when { + base !in duplicated -> base + folder.specialUse -> "$base - $providerLabel" + else -> parentOf(folder.fullName, folder.displayName)?.let { "$base ($it)" } ?: base + } + folder.fullName to label + } + + // Safety net for a residual tie (e.g. two special folders mapping to one role): fall back to the + // unambiguous full path so every drawer entry stays distinct. + val stillTied = resolved.values.groupingBy { it }.eachCount().filterValues { it > 1 }.keys + if (stillTied.isEmpty()) return resolved + return resolved.mapValues { (fullName, label) -> if (label in stillTied) "$label [$fullName]" else label } +} + +/** + * The immediate parent segment of [fullName] (its location), or null when the folder is top-level. + * [displayName] is the leaf, so the character just before it in [fullName] is the server's hierarchy + * separator and the segment before that is the parent. + */ +private fun parentOf(fullName: String, displayName: String): String? { + if (fullName.length <= displayName.length || !fullName.endsWith(displayName)) return null + val separator = fullName[fullName.length - displayName.length - 1] + val parentPath = fullName.substring(0, fullName.length - displayName.length - 1) + return parentPath.substringAfterLast(separator).ifEmpty { null } +} diff --git a/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt b/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt index aa623dc..d426cfd 100644 --- a/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt +++ b/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt @@ -3,6 +3,8 @@ package org.libremail.domain.model import org.junit.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue class FolderRoleTest { @@ -30,4 +32,14 @@ class FolderRoleTest { assertEquals(FolderRole.ARCHIVE, FolderRole.roleOf("Archive", "Archive", emptyList())) assertEquals(FolderRole.NORMAL, FolderRole.roleOf("Receipts", "Receipts", emptyList())) } + + @Test + fun `isServerSpecial is true only for special-use attributes`() { + assertTrue(FolderRole.isServerSpecial(listOf("\\Junk"))) + assertTrue(FolderRole.isServerSpecial(listOf("\\All"))) + // Case-insensitive, and ignores non-special-use flags mixed in. + assertTrue(FolderRole.isServerSpecial(listOf("\\HasNoChildren", "\\drafts"))) + assertFalse(FolderRole.isServerSpecial(emptyList())) + assertFalse(FolderRole.isServerSpecial(listOf("\\HasNoChildren"))) + } } diff --git a/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt new file mode 100644 index 0000000..ef4c4da --- /dev/null +++ b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.mailbox + +import org.junit.Test +import org.libremail.domain.model.Account +import org.libremail.domain.model.AuthType +import org.libremail.domain.model.Folder +import org.libremail.domain.model.FolderRole +import org.libremail.domain.model.MailSecurity +import org.libremail.domain.model.ServerConfig +import kotlin.test.assertEquals + +class FolderLabelsTest { + + private fun folder( + fullName: String, + displayName: String = fullName.substringAfterLast('/'), + role: FolderRole = FolderRole.NORMAL, + specialUse: Boolean = false, + ) = Folder("acct", fullName, displayName, role, selectable = true, specialUse = specialUse) + + private fun account(email: String, imapHost: String, authType: AuthType = AuthType.PASSWORD_IMAP) = + Account( + id = "acct", + email = email, + displayName = email, + authType = authType, + imap = ServerConfig(imapHost, 993, MailSecurity.SSL_TLS), + smtp = ServerConfig("smtp", 587, MailSecurity.STARTTLS), + ) + + /** Builds base labels the way the drawer does: the friendly role name, else the raw display name. */ + private fun baseLabelsOf(folders: List, friendly: Map) = + folders.associate { it.fullName to (friendly[it.role] ?: it.displayName) } + + private val friendlyNames = mapOf( + FolderRole.INBOX to "Inbox", + FolderRole.SENT to "Sent", + FolderRole.DRAFTS to "Drafts", + FolderRole.ARCHIVE to "Archive", + FolderRole.SPAM to "Spam", + FolderRole.TRASH to "Trash", + ) + + private fun resolve(folders: List, provider: String) = + resolveDrawerLabels(folders, baseLabelsOf(folders, friendlyNames), provider) + + @Test + fun `provider special folder gets the suffix and the user folder keeps its name`() { + val folders = listOf( + folder("[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("Drafts", "Drafts", FolderRole.DRAFTS), + ) + val labels = resolve(folders, "Gmail") + assertEquals("Drafts - Gmail", labels["[Gmail]/Drafts"]) + assertEquals("Drafts", labels["Drafts"]) + } + + @Test + fun `all three reported Gmail duplicates are disambiguated`() { + val folders = listOf( + folder("[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("Drafts", "Drafts", FolderRole.DRAFTS), + folder("[Gmail]/All Mail", "All Mail", FolderRole.ARCHIVE, specialUse = true), + folder("Archive", "Archive", FolderRole.ARCHIVE), + folder("[Gmail]/Spam", "Spam", FolderRole.SPAM, specialUse = true), + folder("Spam", "Spam", FolderRole.SPAM), + ) + val labels = resolve(folders, "Gmail") + assertEquals("Drafts - Gmail", labels["[Gmail]/Drafts"]) + assertEquals("Drafts", labels["Drafts"]) + assertEquals("Archive - Gmail", labels["[Gmail]/All Mail"]) + assertEquals("Archive", labels["Archive"]) + assertEquals("Spam - Gmail", labels["[Gmail]/Spam"]) + assertEquals("Spam", labels["Spam"]) + } + + @Test + fun `two nested user folders with the same leaf get their parent location`() { + val folders = listOf( + folder("Work/Reports"), + folder("Personal/Reports"), + ) + val labels = resolve(folders, "example.org") + assertEquals("Reports (Work)", labels["Work/Reports"]) + assertEquals("Reports (Personal)", labels["Personal/Reports"]) + } + + @Test + fun `a top-level user folder keeps its name against a nested sibling`() { + val folders = listOf( + folder("Reports"), + folder("Work/Reports"), + ) + val labels = resolve(folders, "example.org") + assertEquals("Reports", labels["Reports"]) + assertEquals("Reports (Work)", labels["Work/Reports"]) + } + + @Test + fun `unique labels are left untouched`() { + val folders = listOf( + folder("[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("Receipts"), + ) + val labels = resolve(folders, "Gmail") + assertEquals("Drafts", labels["[Gmail]/Drafts"]) + assertEquals("Receipts", labels["Receipts"]) + } + + @Test + fun `two special folders for one role stay distinct via the full-path safety net`() { + val folders = listOf( + folder("[Gmail]/All Mail", "All Mail", FolderRole.ARCHIVE, specialUse = true), + folder("Archives", "Archives", FolderRole.ARCHIVE, specialUse = true), + ) + val labels = resolve(folders, "Gmail") + assertEquals(2, labels.values.toSet().size, "every drawer entry must be unique") + } + + @Test + fun `providerLabel resolves brands, Outlook, and falls back to the email domain`() { + assertEquals("Gmail", providerLabel(account("a@gmail.com", "imap.gmail.com"))) + assertEquals("iCloud Mail", providerLabel(account("a@icloud.com", "imap.mail.me.com"))) + assertEquals( + "Outlook", + providerLabel(account("a@outlook.com", "outlook.office365.com", AuthType.OAUTH_OUTLOOK)), + ) + assertEquals("example.org", providerLabel(account("alice@example.org", "imap.example.org"))) + } +} From e20981d083387ea4cf183325a7bfc7bbcfbd592e Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 15:15:24 -0500 Subject: [PATCH 2/2] style(test): satisfy ktlint in new folder-label tests Body expression on the signature line (function-signature) and one argument per wrapped line (argument-list-wrapping) in the tests added for drawer folder de-duplication. Co-Authored-By: Claude Opus 4.8 --- .../data/local/LibreMailDatabaseTest.kt | 9 +++++++-- .../libremail/ui/mailbox/FolderLabelsTest.kt | 17 ++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt index 3019b28..61ce90d 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -133,8 +133,13 @@ class LibreMailDatabaseTest { "acct", listOf( FolderEntity( - "acct", "[Gmail]/Sent Mail", "Sent Mail", "SENT", - selectable = true, sortOrder = 1, specialUse = true, + accountId = "acct", + fullName = "[Gmail]/Sent Mail", + displayName = "Sent Mail", + role = "SENT", + selectable = true, + sortOrder = 1, + specialUse = true, ), FolderEntity("acct", "INBOX", "INBOX", "INBOX", selectable = true, sortOrder = 0), ), diff --git a/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt index ef4c4da..65644ed 100644 --- a/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt @@ -19,15 +19,14 @@ class FolderLabelsTest { specialUse: Boolean = false, ) = Folder("acct", fullName, displayName, role, selectable = true, specialUse = specialUse) - private fun account(email: String, imapHost: String, authType: AuthType = AuthType.PASSWORD_IMAP) = - Account( - id = "acct", - email = email, - displayName = email, - authType = authType, - imap = ServerConfig(imapHost, 993, MailSecurity.SSL_TLS), - smtp = ServerConfig("smtp", 587, MailSecurity.STARTTLS), - ) + private fun account(email: String, imapHost: String, authType: AuthType = AuthType.PASSWORD_IMAP) = Account( + id = "acct", + email = email, + displayName = email, + authType = authType, + imap = ServerConfig(imapHost, 993, MailSecurity.SSL_TLS), + smtp = ServerConfig("smtp", 587, MailSecurity.STARTTLS), + ) /** Builds base labels the way the drawer does: the friendly role name, else the raw display name. */ private fun baseLabelsOf(folders: List, friendly: Map) =