Skip to content

Conversation

@WAHID-QANDIL
Copy link
Owner

No description provided.

…ist and improve access token retrieval logic
@WAHID-QANDIL WAHID-QANDIL requested a review from Copilot June 20, 2025 21:41
@WAHID-QANDIL WAHID-QANDIL merged commit 52f8ca4 into main Jun 20, 2025
1 check passed
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements user profile support by adding domain use cases, integrating local persistence with Room and DataStore, and updating networking to include authorization headers.

  • Introduces GetUserProfile and UpdateProfile use cases and wires them into Hilt DI
  • Sets up MahdDatabase with StudentEntity and moves SecureTokenStore to encrypted DataStore
  • Updates NetworkModule to fetch a token and apply it via an OkHttp interceptor

Reviewed Changes

Copilot reviewed 20 out of 30 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
MainActivity.kt Switched to lifecycleScope for saving first launch flag
domain/usecase/UpdateProfile.kt Added new use case for updating profile
domain/usecase/Register.kt Fixed import path for RegisterRequest
domain/usecase/ProfileUseCases.kt Bundles profile-related use cases
domain/usecase/Login.kt Minor formatting tweak in constructor
domain/repository/Repository.kt Added getUserProfile and updateUserProfile signatures
domain/model/Student.kt Removed sensitive fields from domain model
di/NetworkModule.kt Injects SecureTokenStore and custom network interceptor
di/DatabaseModule.kt Provides Room database instance
di/AppModule.kt Registers profile use cases in DI
data/source/local/datastore/SecureTokenStore.kt Migrated token store to encrypted DataStore
data/repository/RepositoryImpl.kt Uses local DB and secure store for profile flows
data/api/model/UserProfile.kt Added serialization and mapping to DB entity
data/api/model/RegisterRequest.kt, LoginResponse.kt Updated package paths
data/api/MahdApiService.kt Added profile endpoints and updated imports
Comments suppressed due to low confidence (5)

app/src/main/java/org/mahd_e_learning_platform/domain/usecase/UpdateProfile.kt:7

  • [nitpick] The parameter firstname uses inconsistent camelCase. Rename it to firstName to match lastName and the JSON payload key.
    suspend operator fun invoke(firstname: String, lastName: String) {

app/src/main/java/org/mahd_e_learning_platform/domain/model/Student.kt:5

  • [nitpick] Property lastname should use camelCase lastName to stay consistent with firstName and other mappings.
    val lastname: String,

app/src/main/java/org/mahd_e_learning_platform/data/api/MahdApiService.kt:15

  • The URL is missing a slash between UMS_PORT_NUMBER and api. It should be ...${UMS_PORT_NUMBER}/api/... to form a valid endpoint.
    @POST("${BuildConfig.BASEURL}${BuildConfig.UMS_PORT_NUMBER}api/v1/ums/auth/register")

app/src/main/java/org/mahd_e_learning_platform/data/api/MahdApiService.kt:18

  • [nitpick] Using POST to fetch a user profile is unconventional. Consider changing this to a GET request to align with RESTful semantics.
    @POST("${BuildConfig.BASEURL}${BuildConfig.UMS_PORT_NUMBER}/api/v1/ums/user/profile")

app/src/main/java/org/mahd_e_learning_platform/di/NetworkModule.kt:30

  • There is no getTokenSynchronously() method on SecureTokenStore. This will not compile—consider exposing a synchronous API or retrieving the token via a coroutine.
        var token: String? = secureTokenStore.getTokenSynchronously()

authInterceptor: AuthInterceptor,
secureTokenStore: SecureTokenStore
): OkHttpClient {
var token: String? = secureTokenStore.getTokenSynchronously()
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading the token once at startup means subsequent updates won’t be reflected. Fetch the latest token inside the interceptor for each request.

Suggested change
var token: String? = secureTokenStore.getTokenSynchronously()

Copilot uses AI. Check for mistakes.

val accessFirstLaunchState = context.dataStore.data.map { prefs ->
prefs[IS_FIRST_LAUNCH] ?: true
prefs[IS_FIRST_LAUNCH] == false
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic in accessFirstLaunchState is inverted. == false yields true when not first launch; consider returning a clear flag or renaming the flow accordingly.

Suggested change
prefs[IS_FIRST_LAUNCH] == false
prefs[IS_FIRST_LAUNCH] ?: true

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants