Feature auth refactor

[DemisChan]

Refactor auth module to use generic Result type

Overview

Refactored authentication domain and data layers to use the shared Result<D, E> type from core:domain:util, replacing feature-specific LoginResult and RegisterResult sealed classes with a type-safe, generic error handling approach.

Key Changes

1. Created AuthError sealed interface

sealed interface AuthError : Error {
    enum class Network : AuthError {
        NO_INTERNET, TIMEOUT, SERVER_ERROR, UNKNOWN
    }
    enum class Auth : AuthError {
        INVALID_CREDENTIALS, USER_ALREADY_EXISTS, VALIDATION_FAILED
    }
}

• Implements core Error marker interface for use with generic Result type
• Network errors: IOException, SocketTimeoutException, HTTP 5xx
• Auth errors: 401 Unauthorized, 409 Conflict, 400 Bad Request

2. Added type aliases for backward compatibility

typealias LoginResult = Result<String, AuthError>      // Returns auth token
typealias RegisterResult = EmptyResult<AuthError>      // Returns Unit

3. Updated repository layer

• DefaultAuthRepository now returns Result.Success(data) / Result.Error(error)
• Proper exception handling with specific error mapping:
  • HttpException → Mapped by status code (401, 409, 400, 5xx)
  • IOException → AuthError.Network.NO_INTERNET
  • SocketTimeoutException → AuthError.Network.TIMEOUT
  • Unknown exceptions → AuthError.Network.UNKNOWN

4. Refactored ViewModels to use functional API

authRepository.register(...)
    .onSuccess { 
        state = state.copy(registrationSuccess = true)
    }
    .onError { error -> 
        state = state.copy(error = error.toUiMessage())
    }

5. Centralized error-to-UI mapping

fun AuthError.toUiMessage(): String {
    return when (this) {
        AuthError.Auth.INVALID_CREDENTIALS -> "Invalid email or password"
        AuthError.Network.NO_INTERNET -> "No internet connection"
        // ... all error cases mapped once
    }
}

6. Updated test fakes

class FakeAuthRepository : AuthRepository {
    var registerResult: RegisterResult = Result.Success(Unit)
}

• Tests updated to use Result.Success(Unit) and Result.Error(AuthError...)
• All tests passing ✅

Benefits

• ✅ Type-safe: Compiler catches unhandled error cases
• ✅ Testable: Each error type can be tested independently
• ✅ Maintainable: Error messages defined once, used everywhere
• ✅ Scalable: Easy to add new error types across features

Files Changed

Added:

• AuthError.kt - Error sealed interface + type aliases + toUiMessage()

Modified:

• DefaultAuthRepository.kt - Updated to return Result types
• LoginViewModel.kt - Refactored to use .onSuccess/.onError
• RegisterViewModel.kt - Refactored to use .onSuccess/.onError
• FakeAuthRepository.kt - Updated for new Result API
• RegisterViewModelTest.kt - All tests updated and passing

Deleted:

• LoginResult.kt - Replaced by type alias
• RegisterResult.kt - Replaced by type alias

Testing

• ✅ Manual testing: Login/Register flows work correctly
• ✅ Error scenarios tested: Network errors, validation errors, user exists

[juandgaines]

You could enable type-safe project accessors to avoid hardcoding module paths like ":core:domain:util" and avoid typing errors while referencing them.

Once enabled, you can reference it like this instead:

implementation(projects.core.domain.util)

You can add this line to your settings.gradle.kts and that would be enough:

enableFeaturePreview("TYPESAFE_PROJECT_ACCESSORS")

[juandgaines]

Should we maybe look for a way to avoid duplicating this try/catch logic in every API call? 😊
We could extract it into a safeCall utility that wraps the network request and handles all the exceptions and response mapping in one place. Something like this:

inline fun <reified T> safeCall(
    execute: () -> Response<T>,
    mapHttpError: (Int) -> AuthError
): Result<T, AuthError> {
    val response = try {
        execute()
    } catch (e: UnknownHostException) {
        return Result.Error(AuthError.Network.NoInternet)
    } catch (e: SocketTimeoutException) {
        return Result.Error(AuthError.Network.Timeout)
    } catch (e: IOException) {
        return Result.Error(AuthError.Network.NoInternet)
    } catch (e: Exception) {
        if (e is CancellationException) throw e
        return Result.Error(AuthError.Network.Unknown)
    }

    return when (response.code()) {
        in 200..299 -> Result.Success(response.body() as T)
        else -> Result.Error(mapHttpError(response.code()))
    }
}

That way, the repository methods become much cleaner - something like:

class DefaultAuthRepository(private val api: AuthApi) : AuthRepository {

    override suspend fun login(email: String, password: String): Result<String, AuthError> {
        return safeCall(
            execute = { api.login(LoginRequest(email, password)) },
            mapHttpError = { code ->
                when (code) {
                    400 -> AuthError.Auth.ValidationFailed
                    401 -> AuthError.Auth.InvalidCredentials
                    409 -> AuthError.Auth.UserAlreadyExists
                    in 500..599 -> AuthError.Network.ServerError
                    else -> AuthError.Network.Unknown
                }
            }
        )
    }
}

In the other hand, I’d stick with the approach Philipp video and you referenced in the slack - it’s much simpler since the backend already assumes a consistent response structure across features. That way, you can handle everything in the presentation layer instead of creating separate classes or enums for each feature.

[DemisChan]

so if I stick to the approach on the video ...this is what i have done right?
if i understood correctly?

[juandgaines]

suggestion: You could consider introducing Philipp’s UiText class - it’s a handy utility that lets you handle both string resources and dynamic text in a consistent way across the app, while also making localization much simpler.

sealed interface UiText {
  data class DynamicString(val value: String): UiText
  class StringResource(
    @StringRes val id: Int,
    val args: Array<Any> = arrayOf()
  ): UiText

  @Composable
  fun asString(): String {
    return when(this) {
      is DynamicString -> value
      is StringResource -> stringResource(id = id, *args)
    }
  }
  
  fun asString(context: Context): String {
    return when(this) {
      is DynamicString -> value
      is StringResource -> context.getString(id, *args)
    }
  }
}

Here’s how you could refactor your AuthError mapping to use it:

fun AuthError.toUiText(): UiText {
    return when (this) {
        AuthError.Auth.INVALID_CREDENTIALS ->
            UiText.StringResource(R.string.error_invalid_credentials)
        AuthError.Auth.USER_ALREADY_EXISTS ->
            UiText.StringResource(R.string.error_user_already_exists)
        AuthError.Auth.VALIDATION_FAILED ->
            UiText.StringResource(R.string.error_validation_failed)

        AuthError.Network.NO_INTERNET ->
            UiText.StringResource(R.string.error_no_internet)
        AuthError.Network.SERVER_ERROR ->
            UiText.StringResource(R.string.error_server)
        AuthError.Network.TIMEOUT ->
            UiText.StringResource(R.string.error_timeout)
        AuthError.Network.UNKNOWN ->
            UiText.StringResource(R.string.error_unknown)
    }
}

A final suggestion - we should make sure this AuthError-> UiText mapper lives in the presentation layer, since it depends on Android string resources (R.string) and UiText.

[juandgaines]

You could move this contentDescription text to strings.xml resources 😊

[juandgaines]

suggestion: So far, the parameter name text makes it seem like it’s the actual input value🤔 - maybe renaming it to hint would make the intent clearer, since it’s really just the placeholder text shown in the field.

[juandgaines]

Right now, this composable feels like handles internally both regular text inputs and password fields 🤔, which makes the logic a bit mixed and doesn't communicate well that handles both. It might be cleaner to split them into two separate composables - one for standard text inputs and another specifically for password fields. That way, each stays focused and easier to read without conditional logic for both scenarios.

[juandgaines]

If the intention is to reuse this composable in the register screen, it might be worth renaming it - the current name LoginInputField makes it sound like it’s only meant for the login screen 🤔. Giving it a more generic name, like TaskyTextField, would better communicate that it’s reusable across different authentication screens.

[juandgaines]

I personally got away from using mocks as much as possible. They're good in two scenarios:

1. You're testing legacy code which isn't written with testability in mind.
2. You're testing code you don't own (e.g. you need to make a library's function return something specific and it's not extendable)

In all other cases, I found fakes/stubs much more reliable and less error prone. I can't count how often my tests broke at some point, not because there was a bug, but because the mock itself broke (for example when the mocked functionality changed/was extended, there were unmocked function calls, etc.).
Having a fake instead will make you implement new functionality before you can even run your tests and also doesn't rely on that much 3rd party library magic as it's just a plain Kotlin class.

[DemisChan]

i have used fakes in my registerViewmodeltest and left this as is

[juandgaines]

I would recommend to look into assertion libraries, since the default assertions aren't so readable for more complex ones. My personal favorite is AssertK. You can add it like this:

1. Version catalog declaration

versions]
...
assertk = "0.28.1"

libraries]
...
assertk = { module = "com.willowtreeapps.assertk:assertk", version.ref = "assertk" }

2. Reference it in your Gradle file:

dependencies {
    ...
    testImplementation(libs.assertk)
    androidTestImplementation(libs.assertk)
}

3. Then your assertions could look like this for example:

assertThat(initials).isEqualTo("PL")

assertThat(myList).hasSize(5)

assertThat(myList).contains(element)

assertThat(myBoolean).isTrue()

Reads like a book :D

[DemisChan]

for my next refactor

[Kai-Cilliers]

Minor: If you have the same padding values for opposite ends, you can use one of the following overload functions:

Modifier.padding(vertical = 16.dp) // top + bottom
Modifier.padding(horizontal = 24.dp) // start + end