Develop/caching strategy #26
Conversation
…-#25 Feature/add cart api #25
…oCarrascalDiaz/kotlin_app into feature/add-cart-api-#25
Feature/restaurant map
multithreading added
Develop/local storage
📝 WalkthroughWalkthroughThis pull request introduces multiple changes across various configuration and source files in an Android application. Key modifications include updates to deployment target settings, the introduction of new components for managing inspection profiles, and significant enhancements to the build configuration, particularly concerning Kotlin plugin versions. New features include a biometric authentication system, a structured navigation setup, and a comprehensive menu management system through Room database integration. Additionally, several UI components have been added or modified to improve user experience and functionality. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 52
🧹 Outside diff range and nitpick comments (51)
app/src/main/java/com/uniandes/ecobites/ui/data/MenuItem.kt (3)
1-2: Consider relocating to a proper data/domain package.The entity is currently in the UI package (
com.uniandes.ecobites.ui.data), which violates separation of concerns. Database entities should reside in a dedicated data or domain layer package.Consider moving this class to a more appropriate package like:
com.uniandes.ecobites.data.entitycom.uniandes.ecobites.domain.model
8-8: Consider using auto-generated UUID for the primary key.The current implementation uses a String ID without specifying how it's generated. Consider using UUID or auto-increment for better ID management.
Here's a suggested implementation:
- @PrimaryKey val id: String, + @PrimaryKey + @ColumnInfo(name = "id") + val id: String = UUID.randomUUID().toString(),Add this import:
import java.util.UUID
1-11: Add documentation for the entity class.Consider adding KDoc documentation to describe the entity's purpose and property constraints.
Add documentation like this:
+/** + * Represents a menu item in the database. + * + * @property id Unique identifier for the menu item + * @property name Name of the menu item (non-blank) + * @property price Price of the menu item (non-negative) + */ @Entity data class MenuItem( // ... properties )build.gradle.kts (1)
4-4: Consider caching implications of Kotlin 2.0.Since this PR is about caching strategy, note that Kotlin 2.0 introduces changes to incremental compilation and build caching behavior.
Consider the following optimizations:
- Enable Gradle build cache
- Configure Kotlin compiler caching
- Review compiler arguments for optimal performance
Would you like assistance in implementing these caching optimizations?
app/src/main/java/com/uniandes/ecobites/ui/data/MenuDao.kt (2)
11-12: Enhance query with ordering and filtering options.The current query might lead to inconsistent ordering in the UI. Consider:
- Adding ORDER BY clause for consistent display
- Implementing filtered queries for better usability
-@Query("SELECT * FROM MenuItem") +@Query("SELECT * FROM MenuItem ORDER BY name ASC") fun getAllMenuItems(): LiveData<List<MenuItem>> + +@Query("SELECT * FROM MenuItem WHERE price <= :maxPrice ORDER BY price ASC") +fun getMenuItemsUnderPrice(maxPrice: Double): LiveData<List<MenuItem>>
9-19: Add documentation and error handling.The interface would benefit from:
- KDoc documentation for each method
- Error handling for failed operations
- Validation annotations for MenuItem properties
Example documentation:
/** * Data Access Object for managing menu items in the database. */ @Dao interface MenuDao { /** * Retrieves all menu items from the database. * @return LiveData list of all menu items */ @Query("SELECT * FROM MenuItem") fun getAllMenuItems(): LiveData<List<MenuItem>> // ... similar documentation for other methods }app/src/main/res/layout/activity_map.xml (2)
6-6: Use English for code commentsFor better maintainability and collaboration, comments should be in English.
- <!-- Fragmento del mapa --> + <!-- Map Fragment -->
2-12: Consider adding accessibility support for the map viewThe map view should include appropriate content descriptions and accessibility features to support users with screen readers. Consider adding:
- A descriptive content description for the map fragment
- Accessibility actions for common map interactions
Add these attributes to the fragment:
android:id="@+id/mapFragment" android:name="com.google.android.gms.maps.SupportMapFragment" android:layout_width="match_parent" - android:layout_height="match_parent" /> + android:layout_height="match_parent" + android:contentDescription="@string/map_content_description" + android:importantForAccessibility="yes" />Note: You'll need to add the
map_content_descriptionstring resource to yourstrings.xmlfile.app/src/main/java/com/uniandes/ecobites/ui/viewmodel/RestaurantViewModel.kt (2)
13-17: Consider using StateFlow for better state management.While
mutableStateOfworks, consider usingStateFlowfor better state management and lifecycle awareness, especially for lists that might grow large. Also, consider translating comments to English for better international collaboration.- // Lista de restaurantes que será observada por la UI - val restaurants = mutableStateOf<List<Store>>(emptyList()) - - // Estado de carga para mostrar un indicador de progreso - val isLoading = mutableStateOf(false) + // List of restaurants to be observed by the UI + private val _restaurants = MutableStateFlow<List<Store>>(emptyList()) + val restaurants = _restaurants.asStateFlow() + + // Loading state for progress indicator + private val _isLoading = MutableStateFlow(false) + val isLoading = _isLoading.asStateFlow()
1-48: Consider implementing dependency injection and proper layering.To improve the architecture:
- Use dependency injection (e.g., Hilt) to inject a repository that will handle data operations
- Move the Store model to a domain layer
- Consider implementing a use case layer between the ViewModel and repository
This will improve testability and maintainability as the app grows.
app/src/main/java/com/uniandes/ecobites/ui/components/NavBar.kt (2)
16-16: Enhance null safety handling for navigation state.The current route extraction could be more robust by providing a default route when the back stack entry is null.
- val currentRoute = navController.currentBackStackEntryAsState()?.value?.destination?.route + val currentRoute = navController.currentBackStackEntryAsState().value?.destination?.route ?: "home"
43-48: Translate Spanish comments to English for consistency.The implementation is correct, but the comments should be in English to maintain consistency across the codebase.
- // Nuevo ítem de Location + // New Location item NavigationBarItem( - icon = { Icon(Icons.Rounded.LocationOn, contentDescription = "Location") }, // Ícono de ubicación + icon = { Icon(Icons.Rounded.LocationOn, contentDescription = "Location") }, // Location icon label = { Text("Location") }, selected = currentRoute == "location", onClick = { navController.navigate("location") } )app/src/main/java/com/uniandes/ecobites/ui/screens/ProfileScreen.kt (5)
22-24: Remove unnecessary empty linesThese consecutive empty lines don't serve any purpose and can be reduced to a single line for better readability.
import com.uniandes.ecobites.R - - - @Composable
31-31: Replace hardcoded phone number with empty stringUsing a placeholder phone number as the initial state could lead to confusion or accidentally shipping with test data. Consider using an empty string instead.
- var phone by remember { mutableStateOf("1234567890") } + var phone by remember { mutableStateOf("") }
37-37: Translate comment to English for consistencyThe codebase appears to be in English, so this Spanish comment should be translated for consistency.
- .verticalScroll(rememberScrollState()), // Habilitar scroll vertical + .verticalScroll(rememberScrollState()), // Enable vertical scroll
53-53: Translate comment to English for consistencyKeep comments in English to maintain consistency throughout the codebase.
- // Imagen de perfil + // Profile image
Line range hint
75-86: Enhance ProfileTextField with validation and accessibility featuresConsider enhancing the TextField component with:
- Error state support
- Validation feedback
- Content description for accessibility
- Input type specifications (e.g., phone, email)
@Composable -fun ProfileTextField(label: String, value: String, onValueChange: (String) -> Unit) { +fun ProfileTextField( + label: String, + value: String, + onValueChange: (String) -> Unit, + isError: Boolean = false, + errorMessage: String? = null, + keyboardType: KeyboardType = KeyboardType.Text +) { TextField( value = value, onValueChange = onValueChange, label = { Text(label) }, modifier = Modifier .fillMaxWidth() .padding(vertical = 8.dp), - singleLine = true + singleLine = true, + isError = isError, + supportingText = errorMessage?.let { { Text(it) } }, + keyboardOptions = KeyboardOptions(keyboardType = keyboardType) ) }app/src/main/java/com/uniandes/ecobites/ui/screens/SignUpScreen.kt (2)
16-16: Add a preview function for the SignUpScreen.The Preview annotation is imported but no preview function is implemented. Adding a preview would help with UI development and testing.
Add this preview function:
@Preview(showBackground = true) @Composable fun SignUpScreenPreview() { SignUpScreen( onSignUpSuccess = {}, navController = rememberNavController() ) }
118-124: Improve navigation implementation.The current implementation uses hard-coded navigation routes and lacks proper navigation handling.
Consider these improvements:
object Screen { const val LOGIN = "login" const val SIGNUP = "signup" // ... other routes } // In SignUpScreen TextButton( onClick = { // Handle back stack navController.navigate(Screen.LOGIN) { popUpTo(Screen.SIGNUP) { inclusive = true } } } ) { Text("Already have an account? Log In") }app/src/main/java/com/uniandes/ecobites/ui/screens/store/StoreDetailsScreen.kt (3)
3-20: Consider adding error handling importsThe current imports lack utilities for handling network errors and loading states. Consider adding:
import kotlinx.coroutines.flow.catch import androidx.compose.runtime.getValue import androidx.compose.runtime.setValue
47-84: Enhance product listing accessibility and UXThe current implementation needs improvements in:
- Accessibility labels for screen readers
- Empty state handling
- Product images display
Consider adding:
LazyColumn { + if (products.isEmpty()) { + item { + Text( + text = "No products available", + modifier = Modifier.fillMaxWidth(), + textAlign = TextAlign.Center + ) + } + } items(products) { product -> ProductRow( product = product, quantity = cartState[product.id] ?: 0, + modifier = Modifier.semantics { + contentDescription = "Product ${product.name}, Price $${product.price}" + }, // ... rest of the parameters ) } }
1-118: Consider implementing ViewModel patternThe current implementation mixes UI and business logic in the composable. Consider implementing a ViewModel to:
- Handle state management
- Process cart operations
- Manage data fetching
- Handle error states
Would you like assistance in creating a ViewModel implementation for this screen?
app/src/main/java/com/uniandes/ecobites/ui/screens/StorageScreen.kt (2)
69-110: Enhance form validation and user feedback.The current implementation lacks proper input validation feedback and keyboard handling. Consider these improvements:
@Composable fun StorageScreen(menuDatabase: MenuDatabase, menuViewModel: MenuViewModel = viewModel(factory = MenuViewModelFactory(menuDatabase))) { var dishName by remember { mutableStateOf("") } var priceInput by remember { mutableStateOf("") } + var showError by remember { mutableStateOf(false) } val menuItems by menuViewModel.menuItems.observeAsState(emptyList()) Column(modifier = Modifier.fillMaxSize().padding(16.dp)) { Row(modifier = Modifier.padding(16.dp)) { TextField( modifier = Modifier.weight(0.6f), value = dishName, onValueChange = { dishName = it }, - label = { Text("Dish name") } + label = { Text("Dish name") }, + isError = showError && dishName.isBlank(), + keyboardActions = KeyboardActions(onNext = { /* Focus price field */ }) ) Spacer(modifier = Modifier.width(16.dp)) TextField( modifier = Modifier.weight(0.4f), value = priceInput, onValueChange = { priceInput = it }, - label = { Text("Price") } + label = { Text("Price") }, + isError = showError && (priceInput.toDoubleOrNull() ?: 0.0) <= 0, + keyboardActions = KeyboardActions(onDone = { /* Submit form */ }) ) } + if (showError) { + Text( + "Please fill all fields correctly", + color = MaterialTheme.colorScheme.error, + modifier = Modifier.padding(horizontal = 16.dp) + ) + }
113-150: Add delete confirmation and improve price formatting.Consider adding a delete confirmation dialog and improving the price formatting with proper currency symbol.
@Composable fun ItemsList(menuItems: List<MenuItem>, menuViewModel: MenuViewModel) { + var showDeleteDialog by remember { mutableStateOf<MenuItem?>(null) } + + showDeleteDialog?.let { menuItem -> + AlertDialog( + onDismissRequest = { showDeleteDialog = null }, + title = { Text("Confirm Delete") }, + text = { Text("Are you sure you want to delete ${menuItem.name}?") }, + confirmButton = { + Button( + onClick = { + menuViewModel.deleteMenuItem(menuItem) + showDeleteDialog = null + } + ) { Text("Delete") } + }, + dismissButton = { + Button(onClick = { showDeleteDialog = null }) { + Text("Cancel") + } + } + ) + } + if (menuItems.isEmpty()) { Text( modifier = Modifier .fillMaxHeight() .padding(16.dp), - text = "The menu is empty" + text = "The menu is empty", + style = MaterialTheme.typography.bodyLarge, + textAlign = TextAlign.Center ) } else {app/src/main/res/drawable/icon.xml (1)
Line range hint
8-22: Consider optimizing vector representation.The current implementation has several potential optimizations:
- Multiple paths use the same color
#0e6b58. This could be moved to the group level.- The complex path data suggests this might be text. Consider using actual text views with custom fonts for better maintainability and performance.
If these paths must remain as vectors, consider this optimization:
- <group android:scaleX="1" android:scaleY="1" android:translateX="5.2" android:translateY="51.2"> + <group android:translateX="5.2" android:translateY="51.2" + android:fillColor="#0e6b58"> - <path android:fillColor="#0e6b58" android:pathData="M108.65,290.76c7.69..."/> + <path android:pathData="M108.65,290.76c7.69..."/> <!-- Similar changes for other paths -->app/src/main/java/com/uniandes/ecobites/MainActivity.kt (2)
36-39: Use a constant for the splash screen delay for maintainabilityHardcoding the splash screen delay can make future adjustments less flexible and introduce magic numbers into your code. Consider defining the delay duration as a constant for better maintainability.
Apply this diff to define a constant for the delay:
+private const val SPLASH_SCREEN_DELAY = 1500L @Composable fun MyApp() { // ... LaunchedEffect(Unit) { - delay(1500) + delay(SPLASH_SCREEN_DELAY) showSplashScreen = false } // ...
45-45: Consider using CompositionLocal to provide biometricAuth to ComposablesPassing
biometricAuthas a parameter through multiple Composables can become cumbersome as the app evolves. UtilizingCompositionLocalallows you to accessbiometricAuthanywhere in the Composable hierarchy without explicitly passing it as a parameter.Here's how you might implement it:
Define a CompositionLocal:
val LocalBiometricAuth = compositionLocalOf<BiometricAuth> { error("No BiometricAuth provided") }Provide it in
MyApp:@Composable fun MyApp() { val context = LocalContext.current val biometricAuth = remember { BiometricAuth(context) } val navController = rememberNavController() CompositionLocalProvider(LocalBiometricAuth provides biometricAuth) { if (showSplashScreen) { SplashScreen() } else { NavigationHost(navController = navController) } } }Access it in Composables:
@Composable fun SomeComposable() { val biometricAuth = LocalBiometricAuth.current // Use biometricAuth here }By doing this, you make
biometricAuthaccessible to any Composable that needs it without parameter passing.app/src/main/java/com/uniandes/ecobites/ui/screens/home/StoresGrid.kt (2)
44-44: Ensure the clickable area covers the entireStoreItemThe order of modifiers matters in Compose. Applying
modifier.clickable { onClick() }may not make the entireStoreItemclickable if the passedmodifierhas size constraints. Consider reordering the modifiers to ensure the whole component is clickable.Apply this diff to reorder the modifiers:
fun StoreItem(store: Store, modifier: Modifier = Modifier, onClick: () -> Unit) { Column( - horizontalAlignment = Alignment.Start, - modifier = modifier.clickable { onClick() } + modifier = Modifier.clickable { onClick() }.then(modifier), + horizontalAlignment = Alignment.Start ) {
1-1: Review the package declaration for consistencyThe file is located in
ui/screens/home, but the package is declared ascom.uniandes.ecobites.ui.components. For consistency and maintainability, consider updating the package declaration to match the directory structure or moving the file to the appropriate package.app/src/main/java/com/uniandes/ecobites/ui/screens/home/HomeScreen.kt (2)
33-33: Maintain consistency in code comment languageThe code contains comments in Spanish:
- Line 33:
// Cargar los datos de las tiendas al montar la pantalla- Line 48:
// Mostrar indicador de carga mientras se obtienen los datos- Line 56:
// Mostrar los datos una vez que estén listosConsider using a consistent language (preferably English) for code comments to enhance readability and maintain consistency throughout the codebase.
Also applies to: 48-48, 56-56
29-29: Consider injectingRestaurantViewModelas a parameter for better testabilityPassing
RestaurantViewModelas a parameter toHomeScreenpromotes separation of concerns and improves testability. It allows you to supply mock implementations during unit testing and enhances reusability.app/build.gradle.kts (3)
88-90: Update Glide to the latest stable versionGlide 4.12.0 is an older version. Updating to the latest stable release ensures better performance and access to new features.
Consider updating Glide and its related dependencies:
-implementation ("com.github.bumptech.glide:glide:4.12.0") // Standard Glide -implementation ("com.github.bumptech.glide:compose:1.0.0-alpha.1") // Glide for Jetpack Compose -kapt ("com.github.bumptech.glide:compiler:4.12.0") // For annotating Glide methods +implementation ("com.github.bumptech.glide:glide:4.15.1") // Standard Glide +implementation ("com.github.bumptech.glide:compose:1.0.0-alpha.4") // Glide for Jetpack Compose +kapt ("com.github.bumptech.glide:compiler:4.15.1") // For annotating Glide methods
97-99: Update testing libraries to the latest versionsSome of the testing dependencies are not up-to-date. Updating them can provide bug fixes and new features.
Consider updating the versions as follows:
-testImplementation ("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.0") +testImplementation ("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.3") // Already declared above; ensure only one declaration exists -testImplementation ("junit:junit:4.13.2") -testImplementation ("io.mockk:mockk:1.13.3") // Check the latest version +testImplementation ("io.mockk:mockk:1.13.5") // Updated to the latest version
97-99: Add missing Android test dependencies for MockKTo use MockK in Android tests, include the following dependency:
+androidTestImplementation ("io.mockk:mockk-android:1.13.5")app/src/main/java/com/uniandes/ecobites/ui/screens/LoginScreen.kt (1)
31-36: Remove emptyLaunchedEffectblockThe
LaunchedEffectblock at lines 34-36 is empty and does not perform any action. Consider removing it to clean up the code.app/src/main/java/com/uniandes/ecobites/ui/screens/OrdersScreen.kt (3)
67-70: Implement the navigation action for the back buttonThe
onClickhandler for the back button is currently empty. To enhance user experience, please implement the back navigation action to allow users to return to the previous screen.
89-89: Avoid hard-coded color valuesUsing hard-coded color values like
Color(0xFF907B00)can make the app harder to maintain and reduce theming flexibility. Consider defining this color in your theme or using a color resource for better maintainability and consistency.
145-147: Implement the "View Details" actionThe
onClickhandler for the "View Details" button is currently empty. Implement this action to navigate to the order details screen when the user taps the button.app/src/main/java/com/uniandes/ecobites/ui/data/SupabaseClient.kt (5)
7-21: Remove duplicate import statementsThere's a duplicate import of
io.github.jan.supabase.createSupabaseClientat lines 7 and 21. Removing the redundant import can clean up the code and prevent potential confusion.
31-38: ConsolidateRestaurantandStoredata classes to reduce duplicationThe
RestaurantandStoredata classes have identical fields and structures. If they represent the same entity, consider consolidating them into a single data class to avoid duplication and improve maintainability. This will simplify the codebase and reduce potential inconsistencies.Also applies to: 41-47
180-189: Review placeholder values in testing clientIn the
createSupabaseClientForTestingfunction, thesupabaseUrlandsupabaseKeyare placeholder values. Ensure that these are replaced with appropriate test credentials before running tests, and that sensitive test credentials are also secured and not hardcoded.
96-108: Consider providing detailed error messages in authenticationThe
signInWithEmailfunction catches exceptions but returns a generic failure result. Providing more detailed error messages, such as incorrect password or network issues, can enhance user experience by offering precise feedback.
110-125: Enhance error handling in user registrationSimilarly, in
signUpWithEmailAndName, consider capturing specific exceptions (e.g., email already in use) and returning informative error messages. This will help users understand why registration might have failed and take corrective actions.app/src/main/java/com/uniandes/ecobites/ui/screens/CartScreen.kt (3)
198-201: Format subtotal using proper currency formattingCurrently, the
calculateSubtotalfunction returns the subtotal as a plain string without proper currency formatting. To improve user experience and handle localization, consider formatting the subtotal usingNumberFormat.Here is how you can modify the
calculateSubtotalfunction:import java.text.NumberFormat import java.util.Locale fun calculateSubtotal(cartItems: List<CartProduct>): String { val subtotal = cartItems.sumOf { it.products.price * it.quantity } val format = NumberFormat.getCurrencyInstance(Locale("es", "CO")) return format.format(subtotal) }
162-165: Display actual product images instead of placeholderThe
CartItemCardis currently displaying a placeholder image. To enhance the user experience, display the actual product images if they are available.You can modify the code to load images from URLs provided in
product.products.imageUrl. This may require adding an image loading library like Coil.import coil.compose.rememberAsyncImagePainter Image( painter = rememberAsyncImagePainter(product.products.imageUrl), contentDescription = "Product Image", modifier = Modifier .size(100.dp) .clip(MaterialTheme.shapes.medium) )Ensure you have the Coil dependency added to your project:
implementation("io.coil-kt:coil-compose:2.1.0")
31-32: Avoid unnecessary recompositions by managing state updates appropriatelyThe state variables
cartItemsandisLoadingare being updated insideLaunchedEffect, which might cause unnecessary recompositions. Consider structuring your state management to minimize recompositions for better performance.app/src/main/java/com/uniandes/ecobites/ui/screens/maps/RestaurantMapScreen.kt (3)
51-51: Extract user-facing strings to string resources for localizationHardcoding user-visible strings in the code hinders localization and maintenance. Please move these strings to
strings.xmlresources.For example, apply this diff to extract a string to resources:
- Toast.makeText(context, "No hay conexión, intente más tarde", Toast.LENGTH_SHORT).show() + Toast.makeText(context, context.getString(R.string.no_connection_try_later), Toast.LENGTH_SHORT).show()And add the following to your
strings.xml:<string name="no_connection_try_later">No hay conexión, intente más tarde</string>Repeat this process for the other hardcoded strings in lines 68, 85, 101, 128, 145, 171, 195, 197, and 200.
Also applies to: 68-68, 85-85, 101-101, 128-128, 145-145, 171-171, 195-195, 197-197, 200-200
54-54: InitializeplacesClientonce to avoid redundant creation
placesClientis initialized multiple times within the composable. It's more efficient to initialize it once and reuse it throughout.Apply this diff to initialize
placesClientonce:At line 54:
- val placesClient = Places.createClient(context) + val placesClient = remember { Places.createClient(context) }At line 88, remove the redundant initialization:
- val placesClient = Places.createClient(context)At line 130, remove the redundant initialization:
- val placesClient = Places.createClient(context)This ensures that
placesClientis created only once and reused, improving performance.Also applies to: 88-88, 130-130
57-61: Request location permission when not grantedCurrently, if the location permission is not granted, the app only shows a toast message. Consider requesting the permission from the user to enhance the user experience.
You can use
rememberLauncherForActivityResultto request permissions in Compose. Modify your code as follows:// At the top of your composable val permissionLauncher = rememberLauncherForActivityResult( contract = ActivityResultContracts.RequestPermission() ) { isGranted -> permissionGranted = isGranted if (isGranted) { // Permission granted, proceed to get user location getUserLocation(context, fusedLocationClient) { location -> selectedLocation = LatLng(location.latitude, location.longitude) } } else { Toast.makeText(context, "Permiso de ubicación denegado", Toast.LENGTH_SHORT).show() } } // Replace your existing permission check permissionGranted = ContextCompat.checkSelfPermission( context, Manifest.permission.ACCESS_FINE_LOCATION ) == PackageManager.PERMISSION_GRANTED if (permissionGranted) { // Existing code to get user location } else { // Request the permission permissionLauncher.launch(Manifest.permission.ACCESS_FINE_LOCATION) }This approach prompts the user for permission, improving functionality.
Also applies to: 199-201
app/src/main/java/com/uniandes/ecobites/ui/navigation/NavigationHost.kt (2)
69-74: MoveisNetworkAvailablefunction outside the composable to avoid unnecessary re-creation.Defining
isNetworkAvailablewithin the composable function may lead to performance issues due to re-creation on recomposition. Consider moving it outside the composable scope.
94-95: UseLaunchedEffectto show Toast messages safely in composable functions.Calling
Toast.makeTextdirectly in a composable can cause issues during recomposition. UseLaunchedEffectto display Toast messages without side effects on the UI.Apply this refactor:
LaunchedEffect(Unit) { Toast.makeText(context, "Sin conexión, intente más tarde", Toast.LENGTH_SHORT).show() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
.kotlin/errors/errors-1729205467831.logis excluded by!**/*.log.kotlin/errors/errors-1730660052865.logis excluded by!**/*.logapp/src/main/res/drawable/def.pngis excluded by!**/*.png
📒 Files selected for processing (33)
.idea/deploymentTargetSelector.xml(1 hunks).idea/inspectionProfiles/profiles_settings.xml(1 hunks).idea/kotlinc.xml(1 hunks).idea/misc.xml(1 hunks)README.md(1 hunks)app/build.gradle.kts(2 hunks)app/src/main/AndroidManifest.xml(2 hunks)app/src/main/java/com/uniandes/ecobites/MainActivity.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/components/BiometricAuth.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/components/NavBar.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/data/MenuDao.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/data/MenuDatabase.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/data/MenuItem.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/data/SupabaseClient.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/navigation/NavigationHost.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/CartScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/LoginScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/OrdersScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/ProfileScreen.kt(3 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/SignUpScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/StorageScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/home/HomeScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/home/StoresGrid.kt(2 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/maps/RestaurantMapScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/store/StoreDetailsScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/theme/Color.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/viewmodel/RestaurantViewModel.kt(1 hunks)app/src/main/res/drawable/delete_icon.xml(1 hunks)app/src/main/res/drawable/ic_error.xml(1 hunks)app/src/main/res/drawable/ic_placeholder.xml(1 hunks)app/src/main/res/drawable/icon.xml(2 hunks)app/src/main/res/layout/activity_map.xml(1 hunks)build.gradle.kts(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- .idea/deploymentTargetSelector.xml
- .idea/inspectionProfiles/profiles_settings.xml
- .idea/kotlinc.xml
- .idea/misc.xml
- app/src/main/java/com/uniandes/ecobites/ui/theme/Color.kt
- app/src/main/res/drawable/delete_icon.xml
- app/src/main/res/drawable/ic_error.xml
- app/src/main/res/drawable/ic_placeholder.xml
🧰 Additional context used
🪛 LanguageTool
README.md
[misspelling] ~2-~2: El plural de las siglas no se marca gráficamente.
Context: ...[Compila apps adaptables con las nuevas APIs de Compose, ahora estables](https://dev...
(SIGLAS)
[uncategorized] ~2-~2: Probablemente hay un error. Considere aplicar la sugerencia.
Context: ...s nuevas APIs de Compose, ahora estables](https://developer.android.com/?hl=es-419#:~:text=Discover%20the%20latest%20app%20development%20tools,) no olvides configurar el gradle para una...
(AI_ES_GGEC_REPLACEMENT_OTHER)
[uncategorized] ~3-~3: Probablemente hay un error. Considere aplicar la sugerencia.
Context: ...rar el gradle para una exitosa ejecución..
(AI_ES_GGEC_REPLACEMENT_PUNCTUATION)
🪛 Gitleaks
app/src/main/AndroidManifest.xml
25-25: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
app/src/main/java/com/uniandes/ecobites/ui/data/SupabaseClient.kt
26-26: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
app/src/main/java/com/uniandes/ecobites/ui/screens/maps/RestaurantMapScreen.kt
53-53: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
🔇 Additional comments (14)
app/src/main/java/com/uniandes/ecobites/ui/data/MenuDatabase.kt (1)
1-4: LGTM! Clean and minimal imports.
The package structure and imports are well-organized, including only the necessary Room database dependencies.
app/src/main/java/com/uniandes/ecobites/ui/data/MenuDao.kt (1)
9-10: Consider implementing caching strategy with Room.
Since this PR focuses on caching strategy, consider enhancing the DAO with:
- Room's built-in caching through
@Transactionfor complex operations - Custom caching policy using
@RawQuerywith observability - Implementing offline-first architecture with
@Insert(onConflict = OnConflictStrategy.REPLACE)
Let's check if there are any complex operations that might benefit from transactions:
app/src/main/java/com/uniandes/ecobites/ui/viewmodel/RestaurantViewModel.kt (1)
1-11: LGTM! Clean and well-organized imports.
The imports are concise and the class declaration follows Android architecture best practices.
app/src/main/java/com/uniandes/ecobites/ui/components/NavBar.kt (1)
15-15: Function signature change looks good but needs verification.
The switch to using NavController is a good architectural improvement that better aligns with Jetpack Compose best practices. However, this is a breaking change that requires updates in all places where NavBar is used.
✅ Verification successful
All NavBar usages have been properly updated to use NavController
The verification shows that all instances of NavBar usage in the codebase have been updated to use the new signature with NavController. Each call in NavigationHost.kt correctly passes the navController parameter, matching the new function signature in NavBar.kt. The breaking change has been properly handled across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of NavBar to ensure they've been updated
# to pass NavController instead of the old parameters
# Search for NavBar usage
rg "NavBar\s*\(" --type kotlin
Length of output: 1053
app/src/main/java/com/uniandes/ecobites/ui/screens/StorageScreen.kt (1)
153-161: LGTM!
The ViewModel factory implementation follows best practices with proper type checking and error handling.
app/src/main/res/drawable/icon.xml (2)
1-5: Add accessibility description and consider performance implications.
- Add a content description for accessibility:
<vector xmlns:android="http://schemas.android.com/apk/res/android"
+ android:contentDescription="@string/app_logo_description"
android:height="100dp"
android:viewportHeight="512"
android:viewportWidth="512"
android:width="100dp">- Consider creating different sizes of this drawable for various use cases to optimize performance. The current 100dp size might be excessive for smaller UI elements.
#!/bin/bash
# Check if this drawable is used in layouts and its various use cases
rg -l '@drawable/icon' 'app/src/main/res/layout/*.xml'7-7: Optimize group transformation attributes.
The current group transformation has several potential issues:
- The scale factors of 1 (
scaleX="1" scaleY="1") are redundant as they don't affect the transformation - The translation values seem arbitrary and might cause positioning issues across different screen sizes
Consider these improvements:
- <group android:scaleX="1" android:scaleY="1" android:translateX="5.2" android:translateY="51.2">
+ <group android:translateX="5.2" android:translateY="51.2">Also, verify if these translation values are necessary. If the paths are already properly positioned, you might not need the group wrapper at all.
✅ Verification successful
This script will help us:
- Find all XML files in the drawable directory
- Show the group elements and their surrounding context
- Help understand if the translation pattern is common or unique to this file
This will help us:
- See the complete vector drawable
- Understand if the translations are necessary for proper positioning
- Verify if the paths inside actually need the group wrapper
Keep the translation values but remove redundant scale attributes
The translation values (translateX="5.2" translateY="51.2") appear to be intentional for positioning the text content within the vector drawable, as they help center the text vertically and horizontally in the 512x512 viewport. However, the scale values of 1 (scaleX="1" scaleY="1") are redundant since they don't modify the shape.
- <group android:scaleX="1" android:scaleY="1" android:translateX="5.2" android:translateY="51.2">
+ <group android:translateX="5.2" android:translateY="51.2">🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if similar translation patterns exist in other drawables
rg -l 'android:translateX|android:translateY' 'app/src/main/res/drawable/*.xml'
Length of output: 218
Script:
#!/bin/bash
# Let's try a different approach to check vector drawables
fd -e xml -t f . app/src/main/res/drawable/ -x cat {} \; | grep -A 1 -B 1 "group"
Length of output: 1962
Script:
#!/bin/bash
# Let's check the complete icon.xml file to understand the full context
fd -g "icon.xml" . app/src/main/res/drawable/ -x cat {}
Length of output: 14807
app/src/main/java/com/uniandes/ecobites/ui/screens/home/StoresGrid.kt (1)
31-31: Verify the effect of Modifier.weight(1f) in this context
Modifier.weight(1f) may not have the intended effect inside a Column without specifying height constraints. Ensure that the weight modifier is necessary and that the layout behaves as expected.
app/build.gradle.kts (2)
3-6: Verify compatibility of Kotlin plugin version "2.0.20" with AGP "8.6.1"
The Kotlin plugins have been updated to version "2.0.20". Please ensure that this version is compatible with the Android Gradle Plugin (AGP) version "8.6.1" and the rest of your project dependencies.
76-76: Confirm necessity of androidx.biometric:biometric dependency
The androidx.biometric:biometric:1.1.0 library has been added. Ensure that biometric authentication is fully implemented and required for the project.
If biometric features are utilized, please disregard this comment. If not, consider removing the dependency to minimize app size and reduce potential attack surfaces.
app/src/main/java/com/uniandes/ecobites/ui/screens/LoginScreen.kt (1)
25-25: Ensure all usages of LoginScreen are updated with new parameters
The LoginScreen function signature has been updated to include navController and biometricAuth parameters (line 25). Please verify that all calls to LoginScreen in the codebase are updated accordingly to prevent runtime errors.
Run the following script to find usages of LoginScreen:
✅ Verification successful
All LoginScreen usages are properly updated with the required parameters
The only usage of LoginScreen in the codebase is in NavigationHost.kt, and it correctly includes all three required parameters:
onLoginSuccesslambdanavControllerbiometricAuth
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of LoginScreen to verify the parameters are updated.
# Test: Search for composable calls to LoginScreen. Expect: Calls with the new parameter list.
rg -A 2 $'LoginScreen('
Length of output: 99
Script:
#!/bin/bash
# Search for LoginScreen composable function calls without regex special characters
rg -l "LoginScreen\("
# Get more context around the matches
rg -B 2 -A 2 "LoginScreen\("
Length of output: 1259
Script:
#!/bin/bash
# Get full context of LoginScreen usage in NavigationHost.kt to verify parameters
rg -B 3 -A 6 "LoginScreen\(" app/src/main/java/com/uniandes/ecobites/ui/navigation/NavigationHost.kt
Length of output: 492
Script:
#!/bin/bash
# Get more lines of context to see the complete LoginScreen call
rg -B 3 -A 10 "LoginScreen\(" app/src/main/java/com/uniandes/ecobites/ui/navigation/NavigationHost.kt
Length of output: 564
app/src/main/java/com/uniandes/ecobites/ui/screens/OrdersScreen.kt (1)
43-47: Well-structured and readable code
The implementations of the Order data class, OrdersScreen, and OrderItemCard are well-organized and follow good practices. The use of Jetpack Compose components is appropriate, and the code is readable and maintainable.
Also applies to: 58-109, 112-150
app/src/main/java/com/uniandes/ecobites/ui/screens/CartScreen.kt (2)
62-62: Verify parameter order in removeFromCart function call
Ensure that the parameters passed to removeFromCart are in the correct order. Passing parameters in the wrong order may cause errors.
Please confirm that removeFromCart accepts parameters in the order (cartItemId, userId). Run the following script to verify the function definition:
#!/bin/bash
# Description: Verify the parameter order of removeFromCart function.
# Test: Search for the function definition of removeFromCart.
# Expect: Parameters are in the order (cartItemId, userId).
rg -A 2 "fun removeFromCart\(" --glob "**/*.kt"54-54: Verify parameter order in updateCartItem function call
Ensure that the parameters passed to updateCartItem are in the correct order as expected by the function definition. Misordered parameters might lead to unexpected behavior.
Please confirm that updateCartItem accepts parameters in the order (productId, newQuantity, userId). Run the following script to verify the function definition:
✅ Verification successful
Parameters in updateCartItem function call are correctly ordered
The function definition in SupabaseClient.kt shows:
suspend fun updateCartItem(productId: Int, newQuantity: Int, userId: String)And the call in CartScreen.kt matches this order exactly:
updateCartItem(productId, newQuantity, userId)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the parameter order of updateCartItem function.
# Test: Search for the function definition of updateCartItem.
# Expect: Parameters are in the order (productId, newQuantity, userId).
rg -A 3 "fun updateCartItem\(" --glob "**/*.kt"
Length of output: 449
Script:
#!/bin/bash
# Let's try a more comprehensive search for updateCartItem using ast-grep and ripgrep
# First, search for any function definition or declaration containing updateCartItem
ast-grep --pattern 'fun updateCartItem($$$)'
# Also search for any usage of updateCartItem to see the context
rg "updateCartItem" --glob "**/*.kt" -B 2 -A 2
Length of output: 3660
| @Entity | ||
| data class MenuItem( | ||
| @PrimaryKey val id: String, | ||
| val name: String, | ||
| val price: Double | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance the Room entity with additional annotations and constraints.
The current implementation could benefit from several Room best practices and data validation improvements:
Apply these enhancements:
-@Entity
+@Entity(
+ tableName = "menu_items",
+ indices = [Index(value = ["name"])]
+)
data class MenuItem(
- @PrimaryKey val id: String,
- val name: String,
- val price: Double
+ @PrimaryKey
+ @ColumnInfo(name = "id")
+ val id: String,
+
+ @ColumnInfo(name = "name")
+ @NonNull
+ val name: String,
+
+ @ColumnInfo(name = "price")
+ @NonNull
+ val price: Double
) {
+ init {
+ require(price >= 0) { "Price must be non-negative" }
+ require(name.isNotBlank()) { "Name must not be blank" }
+ }
}Don't forget to add these imports:
import androidx.room.ColumnInfo
import androidx.room.Index
import androidx.annotation.NonNull| @Database(entities = [MenuItem::class], version = 1) | ||
| abstract class MenuDatabase : RoomDatabase() { | ||
| abstract fun menuDao(): MenuDao | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Database implementation needs improvement - issues found in current setup
The verification revealed several issues with the current database implementation:
- Database instance is being created directly in the NavigationHost instead of using a singleton pattern:
// In NavigationHost.kt
val menuDatabase = Room.databaseBuilder(
LocalContext.current,
MenuDatabase::class.java,
"menu.db"
).build()-
Missing configurations:
- No schema export configuration in build.gradle
- No migration strategy
- Database creation is not thread-safe
- Multiple database instances can be created
-
Room dependencies need updating:
- Using inconsistent versions (2.4.3 runtime, 2.5.1 compiler)
- Newer stable versions are available (2.6.1)
The original review comment's suggestions are valid and necessary for a production-ready implementation. Please implement the suggested changes, particularly:
- Move database initialization to the MenuDatabase companion object
- Implement proper thread synchronization
- Update Room dependencies to consistent versions
- Add schema export configuration
🔗 Analysis chain
Essential improvements needed for production-ready database implementation.
While the basic setup is correct, several critical components are missing for a robust Room database implementation:
- Add singleton pattern to prevent multiple database instances
- Configure database name and export schema
- Implement migration strategy
- Add thread synchronization for database operations
Apply these improvements:
+@Database(
+ entities = [MenuItem::class],
+ version = 1,
+ exportSchema = true
+)
-@Database(entities = [MenuItem::class], version = 1)
abstract class MenuDatabase : RoomDatabase() {
abstract fun menuDao(): MenuDao
+
+ companion object {
+ @Volatile
+ private var INSTANCE: MenuDatabase? = null
+ private const val DATABASE_NAME = "menu_database"
+
+ fun getDatabase(context: Context): MenuDatabase {
+ return INSTANCE ?: synchronized(this) {
+ val instance = Room.databaseBuilder(
+ context.applicationContext,
+ MenuDatabase::class.java,
+ DATABASE_NAME
+ )
+ .addMigrations() // Add migrations here when updating version
+ .build()
+ INSTANCE = instance
+ instance
+ }
+ }
+ }
}Don't forget to:
- Create a
migrationspackage to maintain database migrations - Add schema location in your
build.gradle:
android {
defaultConfig {
javaCompileOptions {
annotationProcessorOptions {
arguments += [
"room.schemaLocation": "$projectDir/schemas".toString()
]
}
}
}
}Let's verify if any migrations exist in the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing migration files and schema tracking
# Test 1: Look for migration files
echo "Checking for migration files..."
fd -e kt -e java --full-path ".*[Mm]igration.*"
# Test 2: Check build.gradle for schema location configuration
echo "Checking build.gradle for schema configuration..."
rg "room.schemaLocation" -A 3 -B 3
Length of output: 266
Script:
#!/bin/bash
# Let's verify the current database implementation and usage
# Check for database instance creation and configuration
echo "Checking database instance creation patterns..."
rg -A 10 "RoomDatabase\(\)"
# Look for database builder configurations
echo "Checking database builder patterns..."
rg -A 10 "Room\.databaseBuilder"
# Check if there's any schema directory or configuration
echo "Checking for schema related configurations..."
fd "schemas"
# Check build.gradle files for Room configuration
echo "Checking build.gradle files for Room setup..."
fd "build.gradle" --exec cat {}
Length of output: 6325
| plugins { | ||
| id("com.android.application") version "8.6.1" apply false // AGP version | ||
| id("org.jetbrains.kotlin.android") version "1.9.0" apply false // Kotlin version | ||
| id("org.jetbrains.kotlin.android") version "2.0.20" apply false // Kotlin version |
There was a problem hiding this comment.
💡 Codebase verification
AGP and Kotlin versions are not compatible with current stable releases
The verification reveals multiple compatibility concerns:
- AGP version 8.6.1 is used in both root and app build.gradle.kts, but this version doesn't exist yet (current stable is 8.2.x)
- Kotlin 2.0.20 is used consistently across plugins but this version is not yet released (current stable is 1.9.x)
- Several androidx dependencies also use non-existent future versions:
- androidx.core:core-ktx:1.13.1
- androidx.lifecycle:lifecycle-runtime-ktx:2.8.6
- androidx.navigation:navigation-compose:2.8.2
- androidx.fragment:fragment-ktx:1.8.4
Recommendations:
- Downgrade AGP to latest stable version (8.2.x)
- Use latest stable Kotlin version (1.9.x)
- Update androidx dependencies to their current stable versions
🔗 Analysis chain
Review Kotlin 2.0.20 compatibility and migration requirements.
The upgrade from Kotlin 1.9.0 to 2.0.20 is a major version change that requires careful consideration:
- Kotlin 2.0 introduces breaking changes that might affect your codebase
- The specified AGP version 8.6.1 seems incorrect (latest stable is 8.2.x)
Let's verify the AGP compatibility:
Recommendations:
- Review the Kotlin 2.0 migration guide
- Downgrade AGP to a stable version (e.g., 8.2.x)
- Verify compatibility between AGP and Kotlin versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Kotlin-related configuration and potential version conflicts
# Test 1: Search for Kotlin version declarations
echo "Checking Kotlin version declarations:"
rg --type gradle "kotlin.*version.*2\.0\.20"
# Test 2: Search for AGP version declarations
echo -e "\nChecking AGP version declarations:"
rg --type gradle "com\.android\.application.*version"
# Test 3: Look for kotlin-related dependencies
echo -e "\nChecking Kotlin-related dependencies:"
rg --type gradle "implementation.*org\.jetbrains\.kotlin"
Length of output: 870
Script:
#!/bin/bash
# Let's try a different approach to find gradle files and their content
# First, find all gradle files
echo "Finding all gradle files:"
fd -e gradle -e kts
# Then, let's look at the content of build.gradle.kts
echo -e "\nContent of build.gradle.kts:"
cat build.gradle.kts
# Also check for any other Kotlin or Android related configuration
echo -e "\nSearching for version configurations:"
find . -type f -name "*.gradle*" -o -name "*.kts" | while read -r file; do
echo "=== $file ==="
cat "$file" 2>/dev/null
done
Length of output: 5877
| @Insert | ||
| fun saveMenuItem(menuItem: MenuItem) |
There was a problem hiding this comment.
Add conflict resolution and coroutine support.
The current implementation lacks:
- Conflict resolution strategy for duplicate items
- Coroutine support for background thread execution
-@Insert
-fun saveMenuItem(menuItem: MenuItem)
+@Insert(onConflict = OnConflictStrategy.REPLACE)
+suspend fun saveMenuItem(menuItem: MenuItem)Don't forget to add the import:
import androidx.room.OnConflictStrategy| @Delete | ||
| fun deleteMenuItem(menuItem: MenuItem) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add coroutine support and bulk delete operation.
Enhance the delete functionality with:
- Coroutine support for background thread execution
- Bulk delete operation for better performance
-@Delete
-fun deleteMenuItem(menuItem: MenuItem)
+@Delete
+suspend fun deleteMenuItem(menuItem: MenuItem)
+
+@Delete
+suspend fun deleteMenuItems(menuItems: List<MenuItem>)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Delete | |
| fun deleteMenuItem(menuItem: MenuItem) | |
| @Delete | |
| suspend fun deleteMenuItem(menuItem: MenuItem) | |
| @Delete | |
| suspend fun deleteMenuItems(menuItems: List<MenuItem>) |
| IconButton(onClick = { /* Handle back action */ }) { | ||
| Icon(imageVector = Icons.Rounded.KeyboardArrowLeft, contentDescription = "Back") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement the back navigation action
The onClick handler for the back button is currently empty. Implementing the back navigation will enhance user experience by allowing users to return to the previous screen.
You can pass the NavController to the CartScreen composable and use it to navigate back. Apply the following changes:
+import androidx.navigation.NavController
-fun CartScreen(userId: String) {
+fun CartScreen(userId: String, navController: NavController) {
// Existing code...
Scaffold(
topBar = {
TopAppBar(
title = { Text("Cart") },
navigationIcon = {
- IconButton(onClick = { /* Handle back action */ }) {
+ IconButton(onClick = { navController.popBackStack() }) {
Icon(imageVector = Icons.Rounded.KeyboardArrowLeft, contentDescription = "Back")
}
}
)
},
// Rest of the code...
)
}Committable suggestion skipped: line range outside the PR's diff.
| if (!isNetworkAvailable(context)) { | ||
| Toast.makeText(context, "No hay conexión, intente más tarde", Toast.LENGTH_SHORT).show() | ||
| } else { | ||
| Places.initialize(context.applicationContext, "AIzaSyCTHkA6PG3Zr_nhxq8N7dlX-vrmEM4mltY") |
There was a problem hiding this comment.
Do not hardcode API keys; retrieve them securely from configuration
Hardcoding API keys poses a security risk. Please store the API key securely, such as in a configuration file, environment variable, or using BuildConfig, and retrieve it at runtime.
Apply this diff to replace the hardcoded API key with a secure retrieval method:
- Places.initialize(context.applicationContext, "AIzaSyCTHkA6PG3Zr_nhxq8N7dlX-vrmEM4mltY")
+ val apiKey = BuildConfig.MAPS_API_KEY
+ Places.initialize(context.applicationContext, apiKey)Ensure that MAPS_API_KEY is defined in your build.gradle file and securely imported without exposing it in the codebase.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Gitleaks
53-53: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
| @@ -1,18 +1,168 @@ | |||
| package com.uniandes.ecobites.ui.navigation | |||
|
|
|||
| import RestaurantMapScreen | |||
There was a problem hiding this comment.
Specify the full package name in the import statement.
The import statement import RestaurantMapScreen may cause issues if RestaurantMapScreen is not in the default package. Please provide the fully qualified package name to avoid compilation errors.
| val menuDatabase = Room.databaseBuilder( | ||
| LocalContext.current, | ||
| MenuDatabase::class.java, | ||
| "menu.db" | ||
| ).build() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid creating database instances inside composable functions to prevent unnecessary recreations.
Initializing menuDatabase inside a composable may lead to multiple instances due to recompositions. Move the initialization to a ViewModel or use remember to retain the instance across recompositions.
Apply this refactor using remember:
val menuDatabase = remember {
Room.databaseBuilder(
LocalContext.current,
MenuDatabase::class.java,
"menu.db"
).build()
}Or consider initializing menuDatabase in a ViewModel for better architecture.
| val user = supabase.auth.currentUserOrNull() | ||
| val userId = user?.id | ||
| StoreDetailsScreen(storeName ?: "", userId = userId!!) |
There was a problem hiding this comment.
Avoid using !! on userId to prevent potential NullPointerException.
Using userId!! assumes that userId is not null, which may not be safe. Handle the nullability of userId appropriately to prevent runtime exceptions.
Apply this fix:
-StoreDetailsScreen(storeName ?: "", userId = userId!!)
+if (userId != null) {
+ StoreDetailsScreen(storeName ?: "", userId = userId)
+} else {
+ // Handle the case where userId is null
+}Committable suggestion skipped: line range outside the PR's diff.
2 participants
Summary by CodeRabbit
Release Notes
New Features
OrdersScreenandStorageScreenfor managing orders and menu items.NavBarandNavigationHost.Bug Fixes
Documentation
Style
Chores