[MS-1292] Refactor feature/login to use Kotlin Serialization instead of Jackson#1526
Conversation
1902137 to
c5c9f7d
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the feature/login module to use Kotlin Serialization instead of Jackson for parsing QR code JSON results. This is part of a broader migration effort to standardize on Kotlin Serialization across the codebase.
Changes:
- Replaced Jackson annotations with Kotlin Serialization annotations in
QrCodeContentdata class - Updated
LoginFormViewModelto useJsonHelper.json.decodeFromString()instead ofJsonHelper.fromJson() - Removed Jackson dependencies from build.gradle.kts files
- Updated tests to use actual JSON strings instead of mocking the JSON parsing
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/ui-base/build.gradle.kts | Removed Jackson dependency from ui-base module |
| feature/login/build.gradle.kts | Removed Jackson dependency and added Kotlin Serialization plugin |
| feature/login/src/main/java/com/simprints/feature/login/screens/qrscanner/QrCodeContent.kt | Replaced Jackson @JsonProperty with Kotlin Serialization @SerialName annotation and added @Serializable |
| feature/login/src/main/java/com/simprints/feature/login/screens/form/LoginFormViewModel.kt | Updated JSON parsing to use Kotlin Serialization API; added blank lines in when expression |
| feature/login/src/test/java/com/simprints/feature/login/screens/form/LoginFormViewModelTest.kt | Updated tests to use actual JSON strings and removed JsonHelper mock; changed to wildcard imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import androidx.arch.core.executor.testing.InstantTaskExecutorRule | ||
| import androidx.lifecycle.Observer | ||
| import com.google.common.truth.Truth.assertThat | ||
| import com.google.common.truth.Truth.* |
There was a problem hiding this comment.
Wildcard imports should be avoided. Instead of using import com.google.common.truth.Truth.* and import io.mockk.*, explicitly import only the functions and classes that are used in this file. This makes dependencies clearer and avoids potential naming conflicts.
There was a problem hiding this comment.
Explicitly allowed in tests.
| import io.mockk.clearMocks | ||
| import io.mockk.coEvery | ||
| import io.mockk.every | ||
| import io.mockk.* |
There was a problem hiding this comment.
Wildcard imports should be avoided. Instead of using import io.mockk.*, explicitly import only the functions and classes that are used in this file (e.g., MockKAnnotations, clearMocks, coEvery, every, mockk, slot, verify). This makes dependencies clearer and avoids potential naming conflicts.
| import io.mockk.* | |
| import io.mockk.MockKAnnotations | |
| import io.mockk.clearMocks | |
| import io.mockk.coEvery | |
| import io.mockk.every | |
| import io.mockk.mockk | |
| import io.mockk.slot | |
| import io.mockk.verify |
|
|
||
| AuthenticateDataResult.BadCredentials -> SignInState.BadCredentials | ||
|
|
||
| AuthenticateDataResult.IntegrityException -> SignInState.IntegrityException | ||
|
|
||
| AuthenticateDataResult.IntegrityServiceTemporaryDown -> SignInState.IntegrityServiceTemporaryDown | ||
|
|
||
| AuthenticateDataResult.MissingOrOutdatedGooglePlayStoreApp -> SignInState.MissingOrOutdatedGooglePlayStoreApp | ||
|
|
||
| AuthenticateDataResult.Offline -> SignInState.Offline | ||
|
|
||
| AuthenticateDataResult.TechnicalFailure -> SignInState.TechnicalFailure | ||
|
|
||
| AuthenticateDataResult.Unknown -> SignInState.Unknown | ||
|
|
There was a problem hiding this comment.
The blank lines between each branch of the when expression are unnecessary and inconsistent with typical Kotlin style conventions. Consider removing these extra blank lines to make the code more compact and readable.
| AuthenticateDataResult.BadCredentials -> SignInState.BadCredentials | |
| AuthenticateDataResult.IntegrityException -> SignInState.IntegrityException | |
| AuthenticateDataResult.IntegrityServiceTemporaryDown -> SignInState.IntegrityServiceTemporaryDown | |
| AuthenticateDataResult.MissingOrOutdatedGooglePlayStoreApp -> SignInState.MissingOrOutdatedGooglePlayStoreApp | |
| AuthenticateDataResult.Offline -> SignInState.Offline | |
| AuthenticateDataResult.TechnicalFailure -> SignInState.TechnicalFailure | |
| AuthenticateDataResult.Unknown -> SignInState.Unknown | |
| AuthenticateDataResult.BadCredentials -> SignInState.BadCredentials | |
| AuthenticateDataResult.IntegrityException -> SignInState.IntegrityException | |
| AuthenticateDataResult.IntegrityServiceTemporaryDown -> SignInState.IntegrityServiceTemporaryDown | |
| AuthenticateDataResult.MissingOrOutdatedGooglePlayStoreApp -> SignInState.MissingOrOutdatedGooglePlayStoreApp | |
| AuthenticateDataResult.Offline -> SignInState.Offline | |
| AuthenticateDataResult.TechnicalFailure -> SignInState.TechnicalFailure | |
| AuthenticateDataResult.Unknown -> SignInState.Unknown |
There was a problem hiding this comment.
I will look into disabling those rules.
|



JIRA ticket
Will be released in: 2026.1.0
Notable changes
Testing guidance
Additional work checklist