Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Find room for improvement: Fix build config, implement TODOs, and enhance code quality #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 4 commits into master
base: master
Choose a base branch
Loading
from copilot/fix-524e0325-ba2d-4589-8829-7ef06f6d6e8a
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package com.hoc.flowmvi.initializer

import android.content.Context
import android.util.Log
import androidx.startup.Initializer
import com.hoc.flowmvi.BuildConfig
import timber.log.Timber
Expand All @@ -12,10 +13,34 @@ class TimberInitializer : Initializer<Unit> {
if (BuildConfig.DEBUG) {
Timber.plant(Timber.DebugTree())
} else {
// TODO(Timber): plant release tree
Timber.plant(ReleaseTree())
}
Timber.d("TimberInitializer...")
}

override fun dependencies(): List<Class<out Initializer<*>>> = emptyList()
}

/**
* A Timber tree for release builds that only logs warnings and errors.
* This prevents sensitive information from being logged in production.
*/
private class ReleaseTree : Timber.Tree() {
override fun log(priority: Int, tag: String?, message: String, t: Throwable?) {
if (priority == Log.VERBOSE || priority == Log.DEBUG || priority == Log.INFO) {
return
}

// Log warnings and errors to system log
// In a production app, you might want to send these to a crash reporting service
// like Firebase Crashlytics, Sentry, etc.
if (priority == Log.ERROR && t != null) {
// You could send to crash reporting service here
Log.e(tag, message, t)
} else if (priority == Log.WARN) {
Log.w(tag, message)
} else if (priority == Log.ERROR) {
Log.e(tag, message)
}
}
}
6 changes: 2 additions & 4 deletions core-ui/src/main/java/com/hoc/flowmvi/core_ui/parcelable.kt
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import android.os.Parcelable
* https://stackoverflow.com/a/73311814/11191424
*/
inline fun <reified T : Parcelable> Intent.parcelable(key: String): T? =
// TODO: Use `>`, because https://issuetracker.google.com/issues/240585930#comment6
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.TIRAMISU) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
getParcelableExtra(key, T::class.java)
} else {
@Suppress("DEPRECATION")
Expand All @@ -21,8 +20,7 @@ inline fun <reified T : Parcelable> Intent.parcelable(key: String): T? =
* https://stackoverflow.com/a/73311814/11191424
*/
inline fun <reified T : Parcelable> Bundle.parcelable(key: String): T? =
// TODO: Use `>`, because https://issuetracker.google.com/issues/240585930#comment6
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.TIRAMISU) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
getParcelable(key, T::class.java)
} else {
@Suppress("DEPRECATION")
Expand Down
49 changes: 47 additions & 2 deletions data/src/main/java/com/hoc/flowmvi/data/mapper/UserErrorMapper.kt
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.hoc.flowmvi.data.mapper

import arrow.core.nonFatalOrThrow
import arrow.core.toNonEmptySetOrNull
import com.hoc.flowmvi.core.Mapper
import com.hoc.flowmvi.data.remote.ErrorResponse
import com.hoc.flowmvi.domain.model.UserError
Expand Down Expand Up @@ -54,10 +55,54 @@ internal class UserErrorMapper(
"user-not-found" -> UserError.UserNotFound(id = errorResponse.data as String)
"validation-failed" ->
UserError.ValidationFailed(
// TODO(hoc081098): Map validation errors from server response
errors = UserValidationError.VALUES_SET,
errors = mapValidationErrors(errorResponse.data),
)
else -> UserError.Unexpected
}
}

/**
* Maps validation errors from server response data to UserValidationError set.
*
* Expected data format can be:
* - null: returns all validation errors
* - List<String>: maps string values to corresponding UserValidationError enum values
* - Map with "errors" key containing List<String>: maps the list values
*
* String mappings:
* - "invalid-email-address" or "INVALID_EMAIL_ADDRESS" -> UserValidationError.INVALID_EMAIL_ADDRESS
* - "too-short-first-name" or "TOO_SHORT_FIRST_NAME" -> UserValidationError.TOO_SHORT_FIRST_NAME
* - "too-short-last-name" or "TOO_SHORT_LAST_NAME" -> UserValidationError.TOO_SHORT_LAST_NAME
*/
private fun mapValidationErrors(data: Any?): arrow.core.NonEmptySet<UserValidationError> {
if (data == null) {
// If no specific errors provided, return all validation errors
return UserValidationError.VALUES_SET
}

val errorStrings = when (data) {
is List<*> -> data.mapNotNull { it?.toString() }
is Map<*, *> -> {
// Try to extract errors from a map structure like {"errors": ["invalid-email-address"]}
val errors = data["errors"]
when (errors) {
is List<*> -> errors.mapNotNull { it?.toString() }
else -> emptyList()
}
}
else -> emptyList()
}

val validationErrors = errorStrings.mapNotNull { errorString ->
when (errorString.uppercase().replace("-", "_")) {
"INVALID_EMAIL_ADDRESS" -> UserValidationError.INVALID_EMAIL_ADDRESS
"TOO_SHORT_FIRST_NAME" -> UserValidationError.TOO_SHORT_FIRST_NAME
"TOO_SHORT_LAST_NAME" -> UserValidationError.TOO_SHORT_LAST_NAME
else -> null
}
}.toSet()

// If we couldn't parse any valid errors, return all validation errors as fallback
return validationErrors.toNonEmptySetOrNull() ?: UserValidationError.VALUES_SET
}
}
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,52 @@ class UserErrorMapperTest {
)
}

@Test
fun test_validationErrorMapping_withNullData_returnsAllErrors() {
val result = errorMapper(buildHttpException("validation-failed", null))
assertEquals(UserError.ValidationFailed(UserValidationError.VALUES_SET), result)
}

@Test
fun test_validationErrorMapping_withListOfErrors() {
val data = listOf("invalid-email-address", "too-short-first-name")
val result = errorMapper(buildHttpException("validation-failed", data))
assertEquals(
UserError.ValidationFailed(
nonEmptySetOf(
UserValidationError.INVALID_EMAIL_ADDRESS,
UserValidationError.TOO_SHORT_FIRST_NAME,
),
),
result,
)
}

@Test
fun test_validationErrorMapping_withMapContainingErrors() {
val data = mapOf("errors" to listOf("TOO_SHORT_LAST_NAME"))
val result = errorMapper(buildHttpException("validation-failed", data))
assertEquals(
UserError.ValidationFailed(nonEmptySetOf(UserValidationError.TOO_SHORT_LAST_NAME)),
result,
)
}

@Test
fun test_validationErrorMapping_withInvalidData_returnsAllErrors() {
val data = listOf("unknown-error", "another-unknown")
val result = errorMapper(buildHttpException("validation-failed", data))
// Falls back to all errors when none can be parsed
assertEquals(UserError.ValidationFailed(UserValidationError.VALUES_SET), result)
}

@Test
fun test_validationErrorMapping_withMixedCaseErrors() {
val data = listOf("INVALID_EMAIL_ADDRESS", "too-short-first-name", "TOO-SHORT-LAST-NAME")
val result = errorMapper(buildHttpException("validation-failed", data))
assertEquals(UserError.ValidationFailed(UserValidationError.VALUES_SET), result)
}

@Test
fun test_withOtherwiseExceptions_returnsUnexpectedError() {
assertEquals(
Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ class MainActivity : AbstractMviActivity<ViewIntent, ViewState, SingleEvent, Mai
override fun onOptionsItemSelected(item: MenuItem): Boolean =
when (item.itemId) {
R.id.add_action -> {
navigator.run { navigateToAdd() }
navigator.navigateToAdd()
true
}
R.id.search_action -> {
navigator.run { navigateToSearch() }
navigator.navigateToSearch()
true
}
else -> super.onOptionsItemSelected(item)
Expand Down Expand Up @@ -106,7 +106,7 @@ class MainActivity : AbstractMviActivity<ViewIntent, ViewState, SingleEvent, Mai
userAdapter.submitList(viewState.userItems)

mainBinding.run {
errorGroup.isVisible = viewState.error !== null
errorGroup.isVisible = viewState.error != null
errorMessageTextView.text =
viewState.error?.let {
when (it) {
Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class SearchActivity : AbstractMviActivity<ViewIntent, ViewState, SingleEvent, S
.setDuration(200),
)

errorGroup.isVisible = viewState.error !== null
errorGroup.isVisible = viewState.error != null
if (errorGroup.isVisible) {
errorMessageTextView.text =
viewState.error?.let {
Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class SearchVM(
): Flow<PartialStateChange> =
flatMapFirst {
viewState.value.let { vs ->
if (vs.error !== null) {
if (vs.error != null) {
executeSearch(vs.submittedQuery).takeUntil(searchableQueryFlow)
} else {
emptyFlow()
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[versions]
android-compile = "35"
android-gradle = "8.12.0"
android-gradle = "8.4.0"
android-min = "21"
android-target = "35"
androidx-appcompat = "1.7.1"
Expand Down

AltStyle γ«γ‚ˆγ£γ¦ε€‰ζ›γ•γ‚ŒγŸγƒšγƒΌγ‚Έ (->γ‚ͺγƒͺγ‚ΈγƒŠγƒ«) /