feat: Add error in uploadfile#2013
feat: Add error in uploadfile#2013benjaminVadon wants to merge 8 commits intosimplify-update-UploadFilefrom
Conversation
|
This PR/issue depends on:
|
|
Failed to generate code suggestions for PR |
There was a problem hiding this comment.
Pull request overview
Adds a persisted upload error identifier on Android (mirroring iOS error keys) and a resolver (DriveError) to turn those keys into localized user-facing messages.
Changes:
- Add new error string resources (
errorGeneric,errorPhotoLibraryAccessLimited,uploadOverDataRestrictedError) across supported locales. - Introduce
DriveErrorsealed model to map error keys to messages/plurals. - Persist an upload error key in
UploadFile(Realm schema bump + migration) and set it on upload session start failure.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Adds new base (EN) strings for generic/upload-related errors. |
| app/src/main/res/values-sv/strings.xml | Adds Swedish translations for new error strings. |
| app/src/main/res/values-pt/strings.xml | Adds Portuguese translations for new error strings. |
| app/src/main/res/values-pl/strings.xml | Adds Polish translations for new error strings. |
| app/src/main/res/values-nl/strings.xml | Adds Dutch translations for new error strings. |
| app/src/main/res/values-nb/strings.xml | Adds Norwegian Bokmål translations for new error strings. |
| app/src/main/res/values-it/strings.xml | Adds Italian translations for new error strings. |
| app/src/main/res/values-fr/strings.xml | Adds French translations for new error strings. |
| app/src/main/res/values-fi/strings.xml | Adds Finnish translations for new error strings. |
| app/src/main/res/values-es/strings.xml | Adds Spanish translations for new error strings. |
| app/src/main/res/values-el/strings.xml | Adds Greek translations for new error strings. |
| app/src/main/res/values-de/strings.xml | Adds German translations for new error strings. |
| app/src/main/res/values-da/strings.xml | Adds Danish translations for new error strings. |
| app/src/main/java/com/infomaniak/drive/data/sync/UploadMigration.kt | Bumps Realm DB version and migrates UploadFile to include _driveErrorKey. |
| app/src/main/java/com/infomaniak/drive/data/models/up/DriveError.kt | New error-key-to-message model (local/network/server). |
| app/src/main/java/com/infomaniak/drive/data/models/UploadFile.kt | Stores _driveErrorKey and exposes resolved error: DriveError? plus update helper. |
| app/src/main/java/com/infomaniak/drive/data/api/UploadTask.kt | Stores error key when upload session start fails. |
| app/build.gradle.kts | Adds kotlin("reflect") dependency to support reflection-based key lookup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| androidTestRuntimeOnly(libs.android.test.runner) | ||
|
|
||
| implementation(core.coil.gif) | ||
| implementation(kotlin("reflect")) |
There was a problem hiding this comment.
Adding kotlin-reflect to the app increases APK size and can add startup/runtime overhead. Since the current usage is to enumerate sealed subclasses for DriveError, consider replacing the reflection-based registry with an explicit listOf(...)/mapOf(key -> DriveError) so this dependency can be avoided.
| implementation(kotlin("reflect")) |
| companion object { | ||
| fun find(key: String?): DriveError? = key?.let { allValues.find { it.key == key } } | ||
|
|
||
| private val allValues: List<DriveError> by lazy { buildAllValuesList() } | ||
|
|
||
| private fun buildAllValuesList(): List<DriveError> { | ||
| return Local::class.values() + Network::class.values() + ServerError::class.values() + ServerPlurals::class.values() | ||
| } | ||
|
|
||
| private inline fun <reified T : DriveError> KClass<T>.values(): List<DriveError> = | ||
| sealedSubclasses.map { it.objectInstance as DriveError } | ||
|
|
There was a problem hiding this comment.
DriveError builds its registry via Kotlin reflection (sealedSubclasses/objectInstance) and then does a linear search in find(). This is brittle (will crash if a non-object subclass is added) and requires kotlin-reflect. Prefer an explicit Map<String, DriveError> registry (can be generated manually) to remove reflection and guarantee O(1) lookups. Also note that many server keys here duplicate data/api/ErrorCode.kt; reusing that source of truth would reduce drift.
benjaminVadon
force-pushed
the
add-error-in-uploadfile
branch
from
124d66a to
018fbfb
Compare
|
Failed to generate code suggestions for PR |
… manageError call
|
Failed to generate code suggestions for PR |
|
Failed to generate code suggestions for PR |
|
Failed to generate code suggestions for PR |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Failed to generate code suggestions for PR |
|
Add errors from iOS code to the UploadFile type. For sake of simplicity, I choose to only store the key and retrieve the error when needed (thus don't store resIs in DB, don't manage to store an enum, etc)
depends on #2012
Part of feature on updating upload error management