Fix/sprint3 adjustments #27
Conversation
…-#25 Feature/add cart api #25
…oCarrascalDiaz/kotlin_app into feature/add-cart-api-#25
Feature/restaurant map
multithreading added
Develop/local storage
…ategy Develop/caching strategy
…ategy campo de fecha de cumpleaños para los perfiles eliminado
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes various updates across multiple files in an Android project. Key changes involve updates to configuration files in the Changes
Possibly related PRs
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: 48
🧹 Outside diff range and nitpick comments (65)
app/src/main/java/com/uniandes/ecobites/ui/data/MenuItem.kt (1)
1-1: Consider relocating the MenuItem entity to a more appropriate package.The current package
com.uniandes.ecobites.ui.datasuggests this entity is part of the UI layer. However, database entities should typically reside in the data or domain layer following clean architecture principles. Consider moving this class to a more appropriate package such as:
com.uniandes.ecobites.data.entityorcom.uniandes.ecobites.domain.modelapp/src/main/res/layout/activity_map.xml (2)
1-4: LGTM! Consider adding content description.The FrameLayout is appropriately used as a container for the map fragment. However, consider adding a content description for accessibility support.
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android" android:layout_width="match_parent" - android:layout_height="match_parent"> + android:layout_height="match_parent" + android:contentDescription="@string/map_screen_description">Also applies to: 13-13
6-6: Use English comments for better maintainability.Consider using English for comments to maintain consistency and improve code maintainability across international teams.
- <!-- Fragmento del mapa --> + <!-- Map fragment -->app/src/main/java/com/uniandes/ecobites/ui/components/CachedImage.kt (4)
2-3: Remove extra empty lineThere's an unnecessary double empty line after the package declaration.
package com.uniandes.ecobites.ui.components - import androidx.compose.foundation.Image
21-28: Enhance image loading with error handling and optimizationThe current implementation could benefit from:
- Error handling for failed image loads
- Loading placeholder
- Memory-efficient scaling
- Error state placeholder
val painter = rememberAsyncImagePainter( model = ImageRequest.Builder(LocalContext.current) .data(imageUrl) .crossfade(true) .transformations(RoundedCornersTransformation(cornerRadius)) - .scale(Scale.FILL) + .scale(Scale.FIT) + .placeholder(R.drawable.placeholder_image) + .error(R.drawable.error_image) + .fallback(R.drawable.error_image) + .size(coil.size.Size.ORIGINAL) .build() )
30-35: Add loading state and content scale configurationConsider enhancing the Image composable with:
- A loading state indicator
- Configurable content scale parameter
- Alpha animation for smooth loading transitions
+ val isLoading = painter.state is AsyncImagePainter.State.Loading + + Box(modifier = modifier) { + if (isLoading) { + CircularProgressIndicator( + modifier = Modifier.align(Alignment.Center) + ) + } Image( painter = painter, contentDescription = contentDescription, - modifier = modifier, + modifier = Modifier + .fillMaxSize() + .alpha(if (isLoading) 0f else 1f) + .animateContentSize(), contentScale = ContentScale.Crop ) + }
14-36: Add KDoc documentation and memory optimization notesConsider adding comprehensive documentation to help other developers understand:
- The purpose and usage of the component
- Memory considerations when using cached images
- Recommended image sizes and formats
+/** + * A composable that loads and displays an image from a URL with caching support. + * + * @param imageUrl The URL of the image to load + * @param contentDescription Content description for accessibility + * @param modifier Optional modifier for customizing the layout + * @param cornerRadius Optional radius for rounded corners (default: 8f) + * + * Note: For optimal performance: + * - Use appropriately sized images (max 1024x1024) + * - Prefer WebP format for better compression + * - Consider using thumbnail URLs for lists + */ @Composable fun CachedImage(app/src/main/java/com/uniandes/ecobites/ui/screens/ImageCacheScreen.kt (3)
1-14: Remove unnecessary empty lines after package declaration.Multiple consecutive empty lines affect code readability.
package com.uniandes.ecobites.ui.screens - - - import androidx.compose.foundation.layout.*
24-49: Enhance user experience and robustness.Consider the following improvements:
- Add error handling for failed image loads
- Include loading indicators
- Handle image aspect ratios properly
- Implement internationalization
Here's a suggested implementation:
@Composable fun ImageCacheScreen() { Column( modifier = Modifier .fillMaxSize() .padding(16.dp) ) { Text( - text = "Imágenes cargadas en caché", + text = stringResource(R.string.cached_images_title), style = MaterialTheme.typography.headlineMedium, modifier = Modifier.padding(bottom = 16.dp) ) LazyColumn(verticalArrangement = Arrangement.spacedBy(8.dp)) { items(imageUrls) { imageUrl -> CachedImage( imageUrl = imageUrl, - contentDescription = "Imagen en caché", + contentDescription = stringResource(R.string.cached_image_description), modifier = Modifier .fillMaxWidth() - .height(200.dp) + .aspectRatio(16f/9f), + onLoading = { CircularProgressIndicator() }, + onError = { ErrorMessage() } ) } } } } +@Composable +private fun ErrorMessage() { + Text( + text = stringResource(R.string.image_load_error), + color = MaterialTheme.colorScheme.error + ) +}Don't forget to add these string resources to your
strings.xml:<resources> <string name="cached_images_title">Cached Images</string> <string name="cached_image_description">Cached image</string> <string name="image_load_error">Failed to load image</string> </resources>
24-49: Consider implementing proper architecture patterns.The current implementation could benefit from:
- Adding a ViewModel to manage image loading state and cache
- Implementing pull-to-refresh functionality
- Adding cache management and cleanup strategies
- Using proper error boundaries
Consider implementing a ViewModel:
class ImageCacheViewModel : ViewModel() { private val _images = MutableStateFlow<List<CachedImage>>(emptyList()) val images: StateFlow<List<CachedImage>> = _images.asStateFlow() fun loadImages() { viewModelScope.launch { // Load images with proper error handling } } fun clearCache() { viewModelScope.launch { // Implement cache cleanup } } }app/src/main/java/com/uniandes/ecobites/ui/viewmodel/RestaurantViewModel.kt (2)
1-13: Consider implementing dependency injection for better testability.The ViewModel could benefit from dependency injection to manage data sources. This would improve testability and follow clean architecture principles.
Consider:
- Creating a repository interface and implementation
- Injecting the repository through the constructor using Hilt/Dagger
- Moving the data source logic to the repository
Example structure:
@HiltViewModel class RestaurantViewModel @Inject constructor( private val restaurantRepository: RestaurantRepository ) : ViewModel()
15-21: Consider translating comments to English for consistency.The code comments are in Spanish ("para la lista de restaurantes, que será observada por la UI"). Consider translating them to English to maintain consistency across the codebase and improve international collaboration.
- // StateFlow para la lista de restaurantes, que será observada por la UI + // StateFlow for the restaurant list, to be observed by the UIapp/src/main/java/com/uniandes/ecobites/ui/screens/ProfileScreen.kt (3)
26-26: Consider adding phone number validation.The phone number is currently accepting any numeric string. Consider adding validation to ensure proper phone number format and length.
- var phone by remember { mutableStateOf("1234567890") } + var phone by remember { mutableStateOf("") } + var isPhoneValid by remember { mutableStateOf(true) } + + fun validatePhone(input: String): Boolean { + return input.matches(Regex("^\\+?[1-9]\\d{1,14}$")) + }
31-32: Consider hoisting scroll state.The scroll state could be hoisted to allow controlling the scroll position programmatically, especially useful for error states or form submission.
+ val scrollState = rememberScrollState() Column( modifier = Modifier .fillMaxSize() .padding(16.dp) - .verticalScroll(rememberScrollState()) + .verticalScroll(scrollState)
87-87: Enhance TextField accessibility and validation.Consider adding input validation, error states, and content description for better accessibility.
TextField( value = value, onValueChange = onValueChange, label = { Text(label) }, modifier = Modifier .fillMaxWidth() .padding(vertical = 8.dp) - singleLine = true + singleLine = true, + isError = !isValid, + supportingText = if (!isValid) { + { Text("Invalid $label format") } + } else null, + semantics { + contentDescription = "$label input field" + } )app/src/main/java/com/uniandes/ecobites/ui/screens/SignUpScreen.kt (2)
21-21: Remove unnecessary import commentThe inline comment on the import statement is redundant and doesn't follow Kotlin conventions. The purpose of the import is self-explanatory from its name.
-import com.uniandes.ecobites.ui.data.signUpWithEmailAndName // Import the sign-up function +import com.uniandes.ecobites.ui.data.signUpWithEmailAndName
55-62: Improve layout responsivenessThe fixed image dimensions (350x350dp) might be too large for smaller screens. Consider:
- Using relative sizing
- Implementing different dimensions for different screen sizes
- .width(350.dp) - .height(350.dp) + .fillMaxWidth(0.8f) + .aspectRatio(1f)app/src/main/res/drawable/icon.xml (1)
Line range hint
8-22: Consider accessibility and theme support for the icon.The icon appears to be text converted to vectors with a hardcoded color (#0e6b58).
Consider these improvements:
- Use a color resource reference instead of hardcoded color to support themes:
- <path android:fillColor="#0e6b58" ... + <path android:fillColor="@color/icon_color" ...
- If this represents text, consider adding a
contentDescriptionto the vector for accessibility:<vector xmlns:android="http://schemas.android.com/apk/res/android" + android:contentDescription="@string/icon_description" android:height="100dp"app/src/main/java/com/uniandes/ecobites/MainActivity.kt (3)
35-37: Consider using Android's SplashScreen API for better splash screen managementUsing
delay(1500)withinLaunchedEffectis a simple approach to display the splash screen, but Android provides a dedicated SplashScreen API that offers more control and a smoother transition. Leveraging the SplashScreen API can enhance user experience and align with modern best practices.
35-36: Extract splash screen delay duration into a constantHardcoding the delay duration can make future adjustments less convenient. Consider defining a constant for the splash screen duration to improve maintainability.
Apply this diff to extract the duration into a constant:
+private const val SPLASH_SCREEN_DURATION = 1500L LaunchedEffect(Unit) { - delay(1500) + delay(SPLASH_SCREEN_DURATION) showSplashScreen = false }
30-41: Consider using a ViewModel or Dependency Injection for managing biometric authenticationPassing
biometricAuththrough multiple composable functions can lead to tight coupling and reduced scalability. Using aViewModelor a Dependency Injection framework can help manage dependencies more efficiently and improve code maintainability.app/src/main/java/com/uniandes/ecobites/ui/components/NavBar.kt (2)
23-23: Prevent multiple back stack entries by usinglaunchSingleTopin navigationNavigating without specifying navigation options can lead to multiple instances of the same destination on the back stack. To avoid this and ensure only a single instance exists, use
launchSingleTop = true.Apply this change to the
onClickhandlers:- onClick = { navController.navigate("home") } + onClick = { + navController.navigate("home") { + launchSingleTop = true + } + }Repeat this modification for "Cart", "Orders", "Profile", and "Location" navigation items.
Also applies to: 29-29, 35-35, 41-41, 48-48
21-21: Externalize hardcoded strings to string resources for localizationThe labels for the navigation items are hardcoded. To support localization and improve maintainability, consider moving these strings to
strings.xml.Update the label assignments:
- label = { Text("Home") } + label = { Text(stringResource(R.string.navbar_home)) }Add the corresponding entries to your
strings.xml:<string name="navbar_home">Home</string> <string name="navbar_cart">Cart</string> <string name="navbar_orders">Orders</string> <string name="navbar_profile">Profile</string> <string name="navbar_location">Location</string>Ensure you have the necessary import:
import androidx.compose.ui.res.stringResourceAlso applies to: 27-27, 33-33, 39-39, 46-46
app/build.gradle.kts (4)
6-6: Ensure code comments are in a consistent languageThe comment on line 6 is in Spanish. For consistency across the codebase, consider translating it to English.
Suggested change:
id("kotlin-kapt") // Asegúrate de que kapt esté habilitado +id("kotlin-kapt") // Ensure that kapt is enabled
84-84: Verify and update Accompanist FlowLayout to the latest versionThe comment suggests using the latest version available. Please confirm if 0.28.0 is the most recent version of
accompanist-flowlayoutand update if necessary.
87-87: Ensure code comments are in a consistent languageComments on lines 87 and 90 are in Spanish. For consistency, consider translating them to English.
Suggested changes:
Line 87:
-kapt("androidx.room:room-compiler:2.5.1") // Procesador de anotaciones para Room +kapt("androidx.room:room-compiler:2.5.1") // Annotation processor for RoomLine 90:
-kapt("com.github.bumptech.glide:compiler:4.12.0") // Para anotar métodos de Glide +kapt("com.github.bumptech.glide:compiler:4.12.0") // For annotating Glide methodsAlso applies to: 90-90
100-100: Update MockK dependency to the latest versionThe comment suggests checking for the latest version of MockK. Please verify if 1.13.3 is the most recent version and update accordingly to incorporate the latest features and fixes.
app/src/main/java/com/uniandes/ecobites/ui/screens/store/StoreDetailsScreen.kt (3)
89-118: EnhanceProductRowto improve code reusability and clarityThe
ProductRowcomposable handles two states: whenquantity > 0and whenquantity == 0. Consider breaking down the UI components into smaller composables for better readability and reusability.For example, create separate composables for the quantity controls and the "Add to Cart" button:
@Composable fun QuantityControls( quantity: Int, onIncrease: () -> Unit, onDecrease: () -> Unit ) { Row { IconButton(onClick = onDecrease) { Icon(imageVector = Icons.Rounded.KeyboardArrowLeft, contentDescription = "Decrease quantity") } Text(text = quantity.toString(), modifier = Modifier.alignByBaseline()) IconButton(onClick = onIncrease) { Icon(imageVector = Icons.Rounded.KeyboardArrowRight, contentDescription = "Increase quantity") } } } @Composable fun AddToCartButton(onAddToCart: () -> Unit) { Button(onClick = onAddToCart) { Text(text = "Add to Cart") } }Then modify
ProductRowaccordingly:if (quantity > 0) { - Row { - IconButton(onClick = onDecreaseQuantity) { - Icon(imageVector = Icons.Rounded.KeyboardArrowLeft, contentDescription = "Decrease quantity") - } - Text(text = quantity.toString(), modifier = Modifier.alignByBaseline()) - IconButton(onClick = onIncreaseQuantity) { - Icon(imageVector = Icons.Rounded.KeyboardArrowRight, contentDescription = "Increase quantity") - } - } + QuantityControls( + quantity = quantity, + onIncrease = onIncreaseQuantity, + onDecrease = onDecreaseQuantity + ) } else { - Button(onClick = onAddToCart) { - Text(text = "Add to Cart") - } + AddToCartButton(onAddToCart) }
26-26: UserememberSaveablefor state preservation across configuration changesCurrently,
cartStateusesremember, which doesn't preserve state across configuration changes (like screen rotations). Consider usingrememberSaveableto retain the cart state.Apply this change to use
rememberSaveable:-val cartState = remember { mutableStateMapOf<Int, Int>() } +val cartState = rememberSaveable { mutableStateMapOf<Int, Int>() }
97-117: Improve accessibility by adding content descriptions to images and iconsFor better accessibility support, provide meaningful
contentDescriptionfor UI elements that convey information or allow user interaction, especially icons and images.Apply this diff to add content descriptions:
IconButton(onClick = onDecreaseQuantity) { - Icon(imageVector = Icons.Rounded.KeyboardArrowLeft, contentDescription = "Decrease quantity") + Icon( + imageVector = Icons.Rounded.KeyboardArrowLeft, + contentDescription = "Decrease quantity for ${product.name}" + ) } IconButton(onClick = onIncreaseQuantity) { - Icon(imageVector = Icons.Rounded.KeyboardArrowRight, contentDescription = "Increase quantity") + Icon( + imageVector = Icons.Rounded.KeyboardArrowRight, + contentDescription = "Increase quantity for ${product.name}" + ) }app/src/main/java/com/uniandes/ecobites/ui/screens/LoginScreen.kt (2)
33-33: Inconsistent language in code commentsSome code comments are in Spanish, while the rest of the code is in English. For consistency and maintainability, consider using English for all code comments.
You can update the comments as follows:
-// Verificar si la autenticación biométrica está soportada +// Verify if biometric authentication is supported -//Mostrar un Toast para verificar si la autenticación biométrica está soportada +// Show a Toast message to verify if biometric authentication is supported - if(it.length<=30) { // Limitar a 30 caracteres + if(it.length<=30) { // Limit to 30 characters - biometricAuth.authenticate(navController) // Inicia el proceso de autenticación biométrica + biometricAuth.authenticate(navController) // Start biometric authentication processAlso applies to: 72-72, 116-119
125-125: Inconsistent language in UI textThe button text "Iniciar sesión con huella" is in Spanish, while other UI text is in English. For a consistent user experience, consider translating it to English.
You can update the button text as follows:
Button( onClick = { biometricAuth.authenticate(navController) // Start biometric authentication process }, modifier = Modifier .width(200.dp) .height(48.dp) ) { - Text("Iniciar sesión con huella") + Text("Sign in with fingerprint") }app/src/main/java/com/uniandes/ecobites/ui/screens/OrdersScreen.kt (2)
87-93: Clarify the button label in the bottom bar.The button label "local storage" may not be intuitive for users. Consider using a more descriptive label that reflects the destination or action.
Apply this diff to update the button text:
Button( onClick = { navController.navigate("storage") }, colors = ButtonDefaults.buttonColors( containerColor = Color(0xFF907B00) ) ) { - Text(text = "local storage") + Text(text = "View Storage") }
50-55: Consider externalizing sample data for past orders.The
pastOrderslist contains hardcoded sample data within the UI code. For better maintainability and separation of concerns, consider moving this data to a ViewModel or repository.This allows for easier data management and prepares the codebase for integrating real data sources in the future.
app/src/main/java/com/uniandes/ecobites/ui/screens/StorageScreen.kt (7)
41-50: Simplify LiveData observation without using coroutinesIn the
loadMenuItems()function, you are launching a coroutine and switching to the main thread to add a source to_menuItems. This is unnecessary because adding a source toMediatorLiveDatacan be done directly without coroutines. Additionally, sincemenuDao.getAllMenuItems()returns aLiveData, you can assign it directly to_menuItems.You can refactor the
loadMenuItems()function as follows:-private fun loadMenuItems() { - // Configuración de MediatorLiveData para observar cambios en el hilo principal - viewModelScope.launch { - withContext(Dispatchers.Main) { - _menuItems.addSource(menuDao.getAllMenuItems()) { items -> - _menuItems.postValue(items) // Asegura la actualización en el hilo principal - } - } - } -} +private fun loadMenuItems() { + _menuItems.addSource(menuDao.getAllMenuItems()) { items -> + _menuItems.value = items + } +}Alternatively, if you're only observing a single
LiveDatasource, you might not needMediatorLiveDataat all. You can directly expose theLiveDatafrom the DAO:val menuItems: LiveData<List<MenuItem>> = menuDao.getAllMenuItems()
52-58: Avoid unnecessary reloads after database operationsIn the
addMenuItemanddeleteMenuItemfunctions, you're callingloadMenuItems()after modifying the database. SincemenuItemsis already observing changes from the DAO, reloading is redundant. TheLiveDatawill automatically update when the underlying data changes.Consider removing the calls to
loadMenuItems():fun addMenuItem(dishName: String, price: Double) { val newMenuItem = MenuItem(id = UUID.randomUUID().toString(), name = dishName, price = price) viewModelScope.launch(Dispatchers.IO) { menuDao.saveMenuItem(newMenuItem) - loadMenuItems() // Recargar los datos después de añadir un elemento } } fun deleteMenuItem(menuItem: MenuItem) { viewModelScope.launch(Dispatchers.IO) { menuDao.deleteMenuItem(menuItem) - loadMenuItems() // Recargar los datos después de eliminar un elemento } }Also applies to: 60-65
85-89: Restrict price input to valid numerical valuesTo enhance user experience and prevent invalid inputs, restrict the
priceInputTextFieldto accept only numerical values.You can use
keyboardOptionsto specify a numeric keyboard and validate input:TextField( value = priceInput, onValueChange = { if (it.all { char -> char.isDigit() || char == '.' }) { priceInput = it } }, label = { Text("Price") }, keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number) )
153-161: Simplify the factory class declarationIn your
MenuViewModelFactory, you're fully qualifyingViewModelProvider.Factory. Since you've already importedandroidx.lifecycle.*, you can simplify the code.Update the class declaration:
-class MenuViewModelFactory(private val database: MenuDatabase) : androidx.lifecycle.ViewModelProvider.Factory { +class MenuViewModelFactory(private val database: MenuDatabase) : ViewModelProvider.Factory {Make sure to import
ViewModelProvider:import androidx.lifecycle.ViewModelProvider
117-121: Center the "The menu is empty" message verticallyWhen the menu is empty, the message appears at the top of the screen, which might not be visually appealing.
Consider centering the text vertically:
if (menuItems.isEmpty()) { Text( modifier = Modifier - .fillMaxHeight() + .fillMaxSize(), text = "The menu is empty", + textAlign = TextAlign.Center ) } else {
137-138: Use locale-aware number formattingWhen displaying the price, use locale-aware formatting to ensure the correct decimal separator and grouping.
Update the price formatting:
Text( textAlign = TextAlign.Right, - text = "%.2f".format(menuItem.price) + text = NumberFormat.getCurrencyInstance().format(menuItem.price) )Don't forget to import
java.text.NumberFormat:import java.text.NumberFormat
29-66: Consider using Kotlin Flows instead of LiveDataSince you're working with Room and coroutines, you might benefit from using Kotlin Flows for reactive data handling.
Flows provide a more flexible and powerful way to handle asynchronous data streams. Consider refactoring your
MenuDaoto returnFlow<List<MenuItem>>and update yourViewModelaccordingly.app/src/main/java/com/uniandes/ecobites/ui/data/SupabaseClient.kt (6)
50-58: Add Error Handling infetchStoreCurrently,
fetchStoreassumes that the store with the given name exists. Consider adding error handling to manage cases where the store is not found or the network request fails. This will improve the robustness of your application.
69-77: Add Error Handling infetchStoreProductsSimilar to
fetchStore,fetchStoreProductsdoes not handle cases where the store ID might be invalid or the data retrieval fails. Implementing error handling will enhance the user experience and prevent potential crashes.
96-108: Enhance Exception Handling insignInWithEmailWhile exceptions are caught, consider providing user-friendly error messages to inform users why the sign-in failed. Additionally, logging the exception details can aid in debugging.
110-125: Enhance Exception Handling insignUpWithEmailAndNameImproving error handling by providing feedback to the user on registration failures will enhance usability. Logging exceptions can also help diagnose issues during the sign-up process.
180-189: Use Consistent Language in CommentsThe comment in
createSupabaseClientForTestingis in Spanish, while the rest of the code is in English. For consistency and maintainability, consider using a single language throughout the codebase.Apply this diff to update the comment:
-// Creación del cliente Supabase sin autenticación para pruebas +// Create Supabase client without authentication for testing purposes
204-213: Ensure Robust Error Handling infetchCartItemsWithDetailsConsider adding error handling for cases where data retrieval fails. This function currently assumes successful execution, but network issues or data inconsistencies could lead to exceptions. Implementing try-catch blocks and providing fallback mechanisms will improve reliability.
app/src/main/java/com/uniandes/ecobites/ui/navigation/NavigationHost.kt (3)
97-99: Handle the case whenuserIdis null inCartScreenIn the
cartcomposable, theCartScreenis only displayed ifuserIdis not null. However, no action is taken ifuserIdis null, which might leave the screen empty.Consider providing feedback to the user or redirecting them to the login screen:
if (userId != null) { CartScreen(userId = userId) } else { + // Handle the null case + navController.navigate("login") { + popUpTo("cart") { inclusive = true } + } }
85-85: Redundant usage ofLocalContext.currentmultiple timesYou have multiple instances where
val context = LocalContext.currentis declared. Consider defining it once at a higher scope to avoid redundancy.For example, define
contextat the beginning of theNavigationHostcomposable:@Composable fun NavigationHost(navController: NavHostController, biometricAuth: BiometricAuth) { val context = LocalContext.current NavHost(navController = navController, startDestination = "login") { // ... existing routes ... } }
178-189: Consistent comments and code formattingThere are inconsistent comments and code formatting in this section. Ensure that comments are clear and code is properly formatted.
- Remove unnecessary comments in different languages.
- Ensure indentation is consistent.
For example:
-// Pantalla de caching -composable("caching") { // Agregamos la nueva ruta "caching" +// Caching screen +composable("caching") {app/src/main/java/com/uniandes/ecobites/ui/screens/CartScreen.kt (6)
161-165: Display actual product images instead of placeholder.Currently,
CartItemCarduses a placeholder image (R.drawable.def). To enhance the user experience, consider displaying the actual product image if it's available.Apply this diff to load the product image:
- Image( - painter = painterResource(R.drawable.def), // Replace with product image if available + val imagePainter = if (product.products.imageUrl != null) { + rememberAsyncImagePainter(product.products.imageUrl) + } else { + painterResource(R.drawable.def) + } + Image( + painter = imagePainter, contentDescription = "Product Image", modifier = Modifier.size(100.dp).clip(MaterialTheme.shapes.medium) )Make sure to include the necessary imports:
import coil.compose.rememberAsyncImagePainter
148-148: Consider renaming properties to improve code readability.Accessing
product.products.namecan be confusing due to the repetitive terminology. Consider renaming theproductsproperty withinCartProducttoproductoritemto improve readability.For example, update the
CartProductdata class:data class CartProduct( val id: Int, - val products: Product, + val product: Product, val quantity: Int )Then, adjust the usages accordingly:
- text = product.products.name, + text = product.product.name,
198-201: Format subtotal as currency for better user experience.Currently, the subtotal is returned as a plain string. Formatting it as currency enhances clarity and professionalism in the UI.
Apply this diff to format the subtotal:
fun calculateSubtotal(cartItems: List<CartProduct>): String { val subtotal = cartItems.sumOf { (it.products.price) * it.quantity } - return subtotal.toString() // Format as currency without decimals + val formatter = NumberFormat.getCurrencyInstance(Locale("es", "CO")) + return formatter.format(subtotal) }Add the necessary imports:
import java.text.NumberFormat import java.util.Locale
119-125: Center the loading indicator and empty cart message.For improved user experience, consider centering the
CircularProgressIndicatorand the "Your cart is empty" message within the available space.Apply this diff to adjust the layouts:
if (isLoading) { - CircularProgressIndicator(modifier = Modifier.padding(16.dp)) + Box( + contentAlignment = Alignment.Center, + modifier = Modifier.fillMaxSize() + ) { + CircularProgressIndicator() + } } else if (cartItems.isEmpty()) { - Text( - text = "Your cart is empty", - style = MaterialTheme.typography.bodyLarge, - modifier = Modifier.padding(16.dp) - ) + Box( + contentAlignment = Alignment.Center, + modifier = Modifier.fillMaxSize() + ) { + Text( + text = "Your cart is empty", + style = MaterialTheme.typography.bodyLarge + ) + }
92-113: Extract hardcoded strings for localization support.Strings like "Clear", "Subtotal", and "Checkout" are hardcoded. Extracting these into string resources facilitates localization and future maintenance.
For example, in your
res/values/strings.xml:<string name="clear">Clear</string> <string name="subtotal">Subtotal</string> <string name="checkout">Checkout</string>Update the code to use the string resources:
- Text(text = "Clear", color = Color.Red) + Text(text = stringResource(R.string.clear), color = Color.Red)Remember to include the necessary import:
import androidx.compose.ui.res.stringResource
29-67: Handle possible coroutine cancellation inLaunchedEffect.When using
LaunchedEffectalong withrememberCoroutineScope(), there's a possibility of creating overlapping coroutines or not properly handling coroutine cancellation. Consider simplifying by removingrememberCoroutineScope()and directly launching coroutines withinLaunchedEffect.Apply this diff to adjust the coroutine usage:
fun CartScreen(userId: String) { - val scope = rememberCoroutineScope() var cartItems by remember { mutableStateOf<List<CartProduct>>(emptyList()) } var isLoading by remember { mutableStateOf(true) } // Fetch cart items when the screen loads LaunchedEffect(userId) { - scope.launch { isLoading = true cartItems = fetchCartItemsWithDetails(userId) isLoading = false - } } // Clear cart functionality fun handleClearCart() { - scope.launch { + CoroutineScope(Dispatchers.IO).launch { clearCart(userId) cartItems = emptyList() // Clear the local state as well } } // Function to update quantity fun updateQuantity(cartItemId: Int, newQuantity: Int, productId: Int) { - scope.launch { + CoroutineScope(Dispatchers.IO).launch { updateCartItem(productId, newQuantity, userId) // Update in the database // Rest of the code... } }Ensure you have the necessary imports:
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.DispatchersThis adjustment helps manage coroutines more effectively within the composable scope.
app/src/main/java/com/uniandes/ecobites/ui/screens/home/HomeScreen.kt (6)
37-37: Add anidproperty to theStoredata class for unique identificationCurrently, the
Storedata class lacks a unique identifier, which is essential for navigation and data management purposes.Apply this diff to include an
idproperty:data class Store( + val id: String, val name: String, val imageResId: Int )
84-104: Implement search functionality using thequerystateThe
querystate inTopSectionis being updated but not utilized. To enhance user experience, implement functionality to filter the list of stores based on the search query.Consider modifying the
RestaurantViewModelto filter stores and passing thequeryparameter:// In HomeScreen StoresGrid( navController = navController, restaurants = stores.filter { it.name.contains(query, ignoreCase = true) } )Ensure that the
querystate is lifted toHomeScreenor managed appropriately for state hoisting.
154-167: UtilizeselectedTabIndexto filter stores by categoryThe
selectedTabIndexis updated when a category is selected, but it's not currently affecting the displayed content. Implement category-based filtering to enhance functionality.Example modification:
// In HomeScreen or StoresGrid val filteredStores = stores.filter { it.category == categories[selectedTabIndex].name } StoresGrid( navController = navController, restaurants = filteredStores )Ensure that the
Storedata class includes acategoryproperty to support this filtering.
195-195: Remove unnecessary paddingThe padding of
0.dpdoes not affect layout and can be omitted for cleaner code.Apply this diff to remove the redundant padding:
.fillMaxWidth() - .padding(0.dp) // Remueve padding adicional
195-195: Maintain consistency in comment languageThe comment is in Spanish, whereas the rest of the code is in English. For consistency, translate it to English.
Apply this diff to update the comment:
-// Remueve padding adicional +// Removes additional padding
154-154: Consider usingIntforselectedTabIndexstateUsing
Intfor tracking selected tab index is acceptable, but an enum or sealed class might offer better type safety.This is an optional improvement to enhance code readability and maintainability.
app/src/main/java/com/uniandes/ecobites/ui/screens/maps/RestaurantMapScreen.kt (3)
130-147: ReusePlacesClientinstance to improve performanceCreating multiple instances of
PlacesClientcan be inefficient. It's better to initialize it once and reuse it throughout your composable.Initialize
placesClientonce usingremember:// At the beginning of your composable val placesClient = remember { Places.createClient(context) }Then, remove any other instances of
Places.createClient(context)within the composable.
176-182: ExtractisNetworkAvailableto a utility class for reusabilityThe
isNetworkAvailablefunction may be useful in other parts of the application. Consider moving it to a utility class or file to promote code reuse and maintainability.
154-156: Provide a more prominent loading indicatorThe
CircularProgressIndicatoris only shown during search loading but might not be noticeable to users. Consider displaying it in a more prominent position or overlay to improve user awareness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (5)
.kotlin/errors/errors-1729205467831.logis excluded by!**/*.log.kotlin/errors/errors-1730660052865.logis excluded by!**/*.log.kotlin/errors/errors-1730754361219.logis excluded by!**/*.logapp/src/main/res/drawable/def.pngis excluded by!**/*.pngapp/src/main/res/drawable/img.pngis excluded by!**/*.png
📒 Files selected for processing (40)
.idea/deploymentTargetSelector.xml(1 hunks).idea/inspectionProfiles/profiles_settings.xml(1 hunks).idea/kotlinc.xml(1 hunks).idea/misc.xml(1 hunks).idea/vcs.xml(0 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/CachedImage.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/ImageCacheScreen.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/Carousel.kt(0 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/home/CategoriesRow.kt(0 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(0 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/home/TopSection.kt(0 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/maps/RestaurantMapScreen.kt(1 hunks)app/src/main/java/com/uniandes/ecobites/ui/screens/restaurants/PanPaYaScreen.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 with no reviewable changes (5)
- .idea/vcs.xml
- app/src/main/java/com/uniandes/ecobites/ui/screens/home/Carousel.kt
- app/src/main/java/com/uniandes/ecobites/ui/screens/home/CategoriesRow.kt
- app/src/main/java/com/uniandes/ecobites/ui/screens/home/StoresGrid.kt
- app/src/main/java/com/uniandes/ecobites/ui/screens/home/TopSection.kt
✅ Files skipped from review due to trivial changes (8)
- .idea/deploymentTargetSelector.xml
- .idea/inspectionProfiles/profiles_settings.xml
- .idea/misc.xml
- app/src/main/java/com/uniandes/ecobites/ui/screens/restaurants/PanPaYaScreen.kt
- 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)
🪛 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 (20)
.idea/kotlinc.xml (1)
4-4: Significant Kotlin version upgrade - verify compatibility.
The upgrade from Kotlin 1.9.0 to 2.0.20 is a major version change that could introduce breaking changes. While this change aligns with the updates in build.gradle.kts files, please ensure:
- All project dependencies are compatible with Kotlin 2.0
- The codebase has been tested with the new version
- CI/CD pipelines are updated accordingly
Let's verify the Kotlin version consistency across the project:
✅ Verification successful
Let's gather more specific information about the Kotlin versions and compiler settings:
Kotlin version upgrade is consistently applied across the project
The version update to 2.0.20 in .idea/kotlinc.xml is correctly aligned with all other Kotlin configurations:
- Kotlin Android plugin: 2.0.20
- Kotlin Serialization plugin: 2.0.20
- Kotlin Compose plugin: 2.0.20
The project is properly configured with:
- Official Kotlin code style
- JVM target set to 1.8
- AndroidX and other standard Android configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any mismatched Kotlin versions in build files
# and potential compatibility issues.
echo "Checking Kotlin version references in build files..."
rg -l "kotlin|kotlinVersion" --glob "*.{gradle,kts,properties,xml}"
echo -e "\nChecking for Kotlin compiler arguments that might need updates..."
rg "kotlinOptions|freeCompilerArgs" --glob "*.gradle.kts"
Length of output: 488
Script:
#!/bin/bash
# Check specific Kotlin version references and compiler settings
echo "Checking Kotlin version in build.gradle.kts:"
rg "kotlin.*version|kotlinVersion" build.gradle.kts -A 2
echo -e "\nChecking Kotlin version in app/build.gradle.kts:"
rg "kotlin.*version|kotlinVersion" app/build.gradle.kts -A 2
echo -e "\nChecking Kotlin version in gradle.properties:"
cat gradle.properties
echo -e "\nChecking compiler settings in app/build.gradle.kts:"
rg "kotlinOptions" app/build.gradle.kts -A 5
Length of output: 2477
app/src/main/java/com/uniandes/ecobites/ui/data/MenuItem.kt (1)
1-5: LGTM! Package and imports are properly structured.
The imports are minimal and include only the necessary Room annotations.
app/src/main/java/com/uniandes/ecobites/ui/data/MenuDatabase.kt (2)
1-4: LGTM! Package structure and imports are well organized.
The package naming and imports follow Android conventions and include the necessary Room database components.
6-6: Verify the MenuItem entity implementation.
Since this database references the MenuItem entity, let's verify its proper implementation with Room annotations.
✅ Verification successful
Let me gather more information about the MenuItem entity since it wasn't found in the previous search.
Let me do one final check to verify the Entity annotation.
Room database implementation is properly configured
The verification confirms that:
- MenuItem is correctly annotated with
@Entityand has@PrimaryKey - MenuDao interface is properly implemented with standard Room operations (Query, Insert, Delete)
- Database configuration with version 1 matches the current entity setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify MenuItem entity implementation and MenuDao interface
# Check MenuItem entity implementation
echo "Checking MenuItem entity implementation..."
ast-grep --pattern '@Entity
class MenuItem {
$$$
}'
# Check MenuDao implementation
echo "Checking MenuDao implementation..."
ast-grep --pattern '@Dao
interface MenuDao {
$$$
}'
Length of output: 1192
Script:
#!/bin/bash
# Search for MenuItem class implementation with a broader pattern
echo "Searching for MenuItem class..."
rg -A 10 "class MenuItem"
# Search specifically for Room annotations in files containing MenuItem
echo "Searching for Room annotations in MenuItem..."
rg "@(Entity|PrimaryKey|ColumnInfo)" -C 2 "MenuItem"
Length of output: 702
Script:
#!/bin/bash
# Search for Entity annotation in the MenuItem file
echo "Searching for Entity annotation..."
rg -B 2 "data class MenuItem" app/src/main/java/com/uniandes/ecobites/ui/data/MenuItem.kt
Length of output: 200
build.gradle.kts (1)
4-4:
The upgrade from Kotlin 1.9.0 to 2.0.20 is a significant change that introduces the K2 compiler by default and may contain breaking changes.
Please verify:
-
Run this script to check for potential compatibility issues:
-
Consider documenting the upgrade impact in README.md
-
Ensure all team members are aware of this major version change
References:
- Kotlin 2.0 release notes: https://kotlinlang.org/docs/whatsnew-2.0.html
✅ Verification successful
Based on the script results, I can now provide a final response:
✅ Kotlin 2.0.20 upgrade appears compatible with project setup
The verification shows:
- All Kotlin plugins (android, serialization, compose) are consistently using version 2.0.20
- Compose compiler extension version 1.5.1 is compatible with Kotlin 2.0
- No deprecated companion object patterns that might be affected by K2 compiler were found
- Only safe @OptIn usages for Material3 experimental APIs are present
The upgrade should proceed smoothly, but consider:
- Testing the app thoroughly as K2 is now the default compiler
- Running the test suite to verify no runtime issues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential compatibility issues with Kotlin 2.0
# Check build files for potentially incompatible plugin versions
echo "Checking Gradle build files for plugin versions..."
rg -g "*.gradle*" "kotlin|compose|serialization"
# Check for usage of deprecated Kotlin features
echo "Checking for potentially affected Kotlin features..."
fd -e kt -e java --exec ast-grep --pattern 'companion object {
$$$
}'
# Check for experimental API usage
echo "Checking for experimental API usage..."
rg "@OptIn|@Experimental"
Length of output: 3093
app/src/main/java/com/uniandes/ecobites/ui/data/MenuDao.kt (2)
1-8: LGTM: Package structure and imports are well organized.
The imports cover all necessary Room and LiveData components required for the DAO implementation.
9-10: Verify MenuDao integration with MenuDatabase.
The DAO interface is correctly defined. Let's verify its integration with the database.
✅ Verification successful
MenuDao is properly integrated with the database architecture
The verification confirms that:
- MenuDatabase correctly declares the abstract menuDao() function
- MenuItem entity is properly defined with @entity annotation and required fields
- MenuDatabase is correctly used in StorageScreen with proper ViewModel integration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify MenuDao is properly integrated with MenuDatabase
# and that MenuItem entity exists with correct table name
# Check if MenuDatabase properly references this DAO
ast-grep --pattern 'abstract class MenuDatabase : RoomDatabase {
$$$
abstract fun menuDao(): MenuDao
$$$
}'
# Verify MenuItem entity exists with correct table name annotation
ast-grep --pattern '@Entity(tableName = "MenuItem")
class MenuItem'
Length of output: 185
Script:
#!/bin/bash
# Let's try a broader search to find database and entity files
# Search for MenuDatabase class
rg -l "class.*MenuDatabase" --type kotlin
# Search for any Room database class
ast-grep --pattern 'abstract class $_ : RoomDatabase'
# Search for MenuItem class and Entity annotation
rg -l "class.*MenuItem" --type kotlin
rg -l "@Entity" --type kotlin
# Search for any Room DAO references
rg -l "@Dao" --type kotlin
Length of output: 507
Script:
#!/bin/bash
# Let's examine the content of these files to verify the integration
# Check MenuDatabase.kt content
rg -A 5 "class.*MenuDatabase" app/src/main/java/com/uniandes/ecobites/ui/data/MenuDatabase.kt
# Check MenuItem.kt content
cat app/src/main/java/com/uniandes/ecobites/ui/data/MenuItem.kt
# Check for database builder pattern usage
rg "MenuDatabase" app/src/main/java/com/uniandes/ecobites/ui/screens/StorageScreen.kt
Length of output: 970
app/src/main/res/layout/activity_map.xml (1)
7-11: 🛠️ Refactor suggestion
Consider using FragmentContainerView instead of fragment tag.
The <fragment> tag is deprecated. Consider using FragmentContainerView for better lifecycle handling and fragment management.
- <fragment
+ <androidx.fragment.app.FragmentContainerView
android:id="@+id/mapFragment"
android:name="com.google.android.gms.maps.SupportMapFragment"
android:layout_width="match_parent"
android:layout_height="match_parent" />Also, verify that the Google Maps setup is complete:
✅ Verification successful
FragmentContainerView migration recommended, Google Maps setup verified
The Google Maps setup is properly configured with:
- API key declared in AndroidManifest.xml
- Required location permissions present in AndroidManifest.xml
- Maps SDK dependency included in app/build.gradle.kts
Proceed with migrating from <fragment> to FragmentContainerView as suggested in the diff.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Google Maps setup in the project
# Test 1: Check if Google Maps API key is properly configured
echo "Checking for Google Maps API key configuration..."
rg -l "com.google.android.geo.API_KEY" app/src/main/AndroidManifest.xml
# Test 2: Check if required permissions are declared
echo "Checking for location permissions..."
rg -l "android.permission.ACCESS_FINE_LOCATION|android.permission.ACCESS_COARSE_LOCATION" app/src/main/AndroidManifest.xml
# Test 3: Verify Maps SDK dependency
echo "Checking for Google Maps dependency..."
rg -l "com.google.android.gms:play-services-maps" build.gradle.kts app/build.gradle.kts
Length of output: 650
app/src/main/AndroidManifest.xml (1)
8-9: Verify location permission requirements.
While requesting both permissions is valid, ACCESS_FINE_LOCATION automatically includes ACCESS_COARSE_LOCATION. Consider if your Maps implementation actually requires precise location or if coarse location would suffice for your use case.
For privacy best practices:
- If you only need approximate location (accuracy ~10km), remove
ACCESS_FINE_LOCATION - If precise location is required, document the reason in a code comment
app/src/main/java/com/uniandes/ecobites/ui/components/BiometricAuth.kt (1)
1-67: Verify manifest permissions for biometric authentication.
Ensure the following permissions are properly declared in AndroidManifest.xml:
✅ Verification successful
Required biometric permissions are properly declared in AndroidManifest.xml
Both USE_BIOMETRIC and USE_FINGERPRINT permissions are correctly declared in the manifest file, which aligns with the permissions being requested in the BiometricAuth implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the required permissions are declared in AndroidManifest.xml
echo "Checking AndroidManifest.xml for biometric permissions..."
rg -A 1 "android.permission.(USE_BIOMETRIC|USE_FINGERPRINT)" "app/src/main/AndroidManifest.xml"
Length of output: 446
app/src/main/java/com/uniandes/ecobites/ui/screens/ProfileScreen.kt (2)
6-8: LGTM! Navigation and scrolling capabilities properly integrated.
The NavController parameter addition and new imports align well with the navigation requirements.
Also applies to: 17-17, 21-21
70-70: Verify navigation route definition.
Ensure that the "caching" route is properly defined in the navigation graph.
✅ Verification successful
Navigation route "caching" is properly defined
The "caching" route is correctly defined in app/src/main/java/com/uniandes/ecobites/ui/navigation/NavigationHost.kt with its corresponding composable and screen implementation (ImageCacheScreen).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the "caching" route definition in navigation files
rg -l "caching.*Route|route.*caching" app/src/main/java/com/uniandes/ecobites/
Length of output: 80
Script:
#!/bin/bash
# Let's try a broader search for navigation-related files and "caching" route
rg -l "NavHost|composable.*navigation" app/src/main/java/com/uniandes/ecobites/
Length of output: 153
Script:
#!/bin/bash
# Search for any occurrence of "caching" string in the codebase
rg "caching" app/src/main/java/com/uniandes/ecobites/
Length of output: 862
Script:
#!/bin/bash
# Look for navigation graph definitions
fd "Nav.*\.kt" app/src/main/java/com/uniandes/ecobites/
Length of output: 193
app/src/main/res/drawable/icon.xml (2)
1-5: LGTM: Vector dimensions and viewport are well-defined.
The vector drawable is properly configured with matching aspect ratios for both dimensions and viewport.
7-7: Verify icon alignment and optimize group transformation.
The group transformation has the following concerns:
- The scale values of 1 are redundant and can be removed
- The translation values (5.2, 51.2) might cause alignment issues
Please verify the icon's alignment in different contexts:
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">app/src/main/java/com/uniandes/ecobites/ui/components/NavBar.kt (1)
15-15: Ensure all usages of NavBar are updated to the new signature
The NavBar function signature has changed to accept a NavController. Please verify that all components using NavBar have been updated to match the new signature to prevent any compilation errors or unexpected behavior.
Run the following script to identify all usages of NavBar:
✅ Verification successful
All usages of NavBar are correctly updated with the new signature
The verification shows that all calls to NavBar in the codebase are passing the navController parameter as required by the new signature. Each usage in NavigationHost.kt explicitly provides the navController argument using named parameter syntax.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all calls to `NavBar` are using the new signature with `NavController`.
# Test: Search for usages of `NavBar`. Expect: All calls pass a `NavController` instance.
rg 'NavBar\s*\('
Length of output: 1169
app/build.gradle.kts (1)
3-6:
Verify compatibility of Kotlin version 2.0.20 with project configuration and dependencies
The project has been updated to Kotlin version 2.0.20. Please ensure that all plugins, dependencies, and compiler options are compatible with this version, as some libraries might not yet support Kotlin 2.x. Notably, the kotlinCompilerExtensionVersion is set to "1.5.1", which may not be compatible with Kotlin 2.0.20.
Consider updating the kotlinCompilerExtensionVersion to match the Kotlin version:
composeOptions {
- kotlinCompilerExtensionVersion = "1.5.1"
+ kotlinCompilerExtensionVersion = "2.0.20"
}app/src/main/java/com/uniandes/ecobites/ui/screens/StorageScreen.kt (1)
141-144: Verify the existence of delete_icon drawable resource
You are using R.drawable.delete_icon in your Icon composable. Ensure that the delete_icon resource exists in your drawable directories to prevent runtime crashes.
Run the following script to check for the delete_icon resource:
✅ Verification successful
Resource delete_icon exists and is properly referenced
The delete_icon drawable resource exists as a vector drawable XML file at app/src/main/res/drawable/delete_icon.xml, so the usage of R.drawable.delete_icon in the Icon composable is correct and safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that delete_icon exists in drawable resources
# Expected result: Should find at least one file named delete_icon.*
fd delete_icon res/drawable*
Length of output: 133
Script:
#!/bin/bash
# Let's first find the correct location of resource directories
fd -t d -g "*res*" -g "*drawable*"
Length of output: 134
Script:
#!/bin/bash
# Let's try a different approach to locate the resource files
fd -t f -e xml -e png -e jpg -e webp delete_icon app/
Length of output: 97
app/src/main/java/com/uniandes/ecobites/ui/screens/CartScreen.kt (2)
29-29: Ensure all invocations of CartScreen pass the userId parameter.
The CartScreen function signature now includes userId: String. Please verify that all calls to CartScreen have been updated to pass the required userId parameter, to prevent runtime errors.
You can run the following script to identify any invocations missing the userId argument:
✅ Verification successful
Let me search for the actual CartScreen invocations with more context to verify this properly.
All CartScreen invocations are properly updated with the userId parameter
The search results show that the only invocation of CartScreen in the codebase (outside of its definition) is in NavigationHost.kt where it's correctly called with CartScreen(userId = userId). The parameter is properly passed, and there are no missing or incorrect invocations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all invocations of CartScreen without the userId parameter.
rg -A 2 'CartScreen\(' --glob '!.*/CartScreen.kt' | grep -v 'userId'
Length of output: 515
Script:
#!/bin/bash
# Search for CartScreen invocations with more context
rg -B 2 -A 5 'CartScreen\(' --glob '!.*/CartScreen.kt'
Length of output: 1745
136-139: Ensure correct IDs are used when calling updateQuantity.
When invoking updateQuantity, you're passing cartItem.id and cartItem.products.id as cartItemId and productId respectively. Please verify that these IDs correspond correctly to what's expected by updateQuantity and underlying database operations.
To assist with verification, you can run the following script to inspect the definitions and usages of CartProduct, cartItem.id, and cartItem.products.id:
✅ Verification successful
Let me gather more information about the updateQuantity function to verify the ID usage.
The IDs are being used correctly in the updateQuantity calls
The verification shows that:
CartProductdata class has anidfield and a nestedproductsobject with its ownid- The local
updateQuantityfunction correctly uses:cartItemIdto update the local state (cartItems.map { if (it.id == cartItemId)...)productIdto update the database viaupdateCartItem(productId, newQuantity, userId)
- The database operation in
SupabaseClient.ktuses these parameters appropriately
Therefore, the current implementation is correct and the IDs are being passed in the right order.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find definitions and usages of CartProduct and related IDs.
rg -A 5 'data class CartProduct'
rg -A 3 'cartItem\.'
Length of output: 1175
Script:
#!/bin/bash
# Find the definition and implementation of updateQuantity function
ast-grep --pattern 'fun updateQuantity'
# Find any database operations or queries related to cart updates
rg -A 5 'updateCart|updateQuantity'
Length of output: 5317
app/src/main/java/com/uniandes/ecobites/ui/screens/maps/RestaurantMapScreen.kt (1)
38-174: Handle potential null values for selectedLocation
When selectedLocation is null, the map centers on a default location. Ensure this default location is appropriate for your user base or consider prompting the user to select a location.
Confirm that the default location aligns with the intended user experience.
🧰 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)
| @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 definition with additional specifications and validations.
While the basic structure is correct, consider the following improvements for better database management and data integrity:
- Specify a table name to avoid potential conflicts
- Add column name specifications to prevent SQL keyword conflicts
- Add validation for the price field
- Consider adding indexes for better query performance
Here's the suggested implementation:
-@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")
+ val name: String,
+
+ @ColumnInfo(name = "price")
+ val price: Double
) {
+ init {
+ require(price >= 0) { "Price must be non-negative" }
+ require(name.isNotBlank()) { "Name cannot be blank" }
+ require(id.isNotBlank()) { "ID cannot be blank" }
+ }
}Don't forget to add the following import:
import androidx.room.ColumnInfo
import androidx.room.Index| @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.
🛠️ Refactor suggestion
Essential improvements needed for database configuration.
While the basic setup is correct, there are several important considerations for production-ready Room database implementation:
- Add exportSchema parameter to preserve schema version history
- Implement singleton pattern to prevent multiple database instances
- Define migration strategy
- Use constant for version number
Here's the recommended implementation:
+private const val DATABASE_VERSION = 1
+
-@Database(entities = [MenuItem::class], version = 1)
+@Database(
+ entities = [MenuItem::class],
+ version = DATABASE_VERSION,
+ exportSchema = true
+)
abstract class MenuDatabase : RoomDatabase() {
abstract fun menuDao(): MenuDao
+
+ companion object {
+ @Volatile
+ private var INSTANCE: MenuDatabase? = null
+
+ fun getDatabase(context: Context): MenuDatabase {
+ return INSTANCE ?: synchronized(this) {
+ val instance = Room.databaseBuilder(
+ context.applicationContext,
+ MenuDatabase::class.java,
+ "menu_database"
+ )
+ .fallbackToDestructiveMigration() // Replace with proper migration strategy in production
+ .build()
+ INSTANCE = instance
+ instance
+ }
+ }
+ }
}Don't forget to add the following import:
import android.content.Context
import androidx.room.Room| @Query("SELECT * FROM MenuItem") | ||
| fun getAllMenuItems(): LiveData<List<MenuItem>> | ||
|
|
||
| @Insert | ||
| fun saveMenuItem(menuItem: MenuItem) | ||
|
|
||
| @Delete | ||
| fun deleteMenuItem(menuItem: MenuItem) | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance DAO with additional features for production readiness.
While the basic operations are correctly implemented, consider the following improvements:
- Add conflict resolution strategy for insert operations
- Support batch operations for better performance
- Add coroutines support using suspend functions
- Add sorting options to the query
Apply these enhancements:
@Dao
interface MenuDao {
@Query("SELECT * FROM MenuItem")
fun getAllMenuItems(): LiveData<List<MenuItem>>
+
+ @Query("SELECT * FROM MenuItem ORDER BY name ASC")
+ suspend fun getAllMenuItemsSorted(): List<MenuItem>
- @Insert
+ @Insert(onConflict = OnConflictStrategy.REPLACE)
fun saveMenuItem(menuItem: MenuItem)
+ @Insert(onConflict = OnConflictStrategy.REPLACE)
+ suspend fun saveMenuItems(vararg menuItems: MenuItem)
+
@Delete
fun deleteMenuItem(menuItem: MenuItem)
+
+ @Delete
+ suspend fun deleteMenuItems(vararg menuItems: MenuItem)
}Consider implementing a repository pattern on top of this DAO to handle the business logic and provide a clean API to the ViewModels.
📝 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.
| @Query("SELECT * FROM MenuItem") | |
| fun getAllMenuItems(): LiveData<List<MenuItem>> | |
| @Insert | |
| fun saveMenuItem(menuItem: MenuItem) | |
| @Delete | |
| fun deleteMenuItem(menuItem: MenuItem) | |
| } | |
| @Query("SELECT * FROM MenuItem") | |
| fun getAllMenuItems(): LiveData<List<MenuItem>> | |
| @Query("SELECT * FROM MenuItem ORDER BY name ASC") | |
| suspend fun getAllMenuItemsSorted(): List<MenuItem> | |
| @Insert(onConflict = OnConflictStrategy.REPLACE) | |
| fun saveMenuItem(menuItem: MenuItem) | |
| @Insert(onConflict = OnConflictStrategy.REPLACE) | |
| suspend fun saveMenuItems(vararg menuItems: MenuItem) | |
| @Delete | |
| fun deleteMenuItem(menuItem: MenuItem) | |
| @Delete | |
| suspend fun deleteMenuItems(vararg menuItems: MenuItem) | |
| } |
| @Composable | ||
| fun CachedImage( | ||
| imageUrl: String, | ||
| contentDescription: String?, | ||
| modifier: Modifier = Modifier, | ||
| cornerRadius: Float = 8f | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for imageUrl parameter
The imageUrl parameter should be validated to ensure it's not empty and follows a valid URL format. This would prevent potential issues with invalid URLs being passed to the Coil image loader.
@Composable
fun CachedImage(
imageUrl: String,
contentDescription: String?,
modifier: Modifier = Modifier,
cornerRadius: Float = 8f
+) {
+ require(imageUrl.isNotBlank()) { "imageUrl cannot be blank" }
+ require(imageUrl.startsWith("http", ignoreCase = true)) { "imageUrl must be a valid HTTP/HTTPS URL" }
+📝 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.
| @Composable | |
| fun CachedImage( | |
| imageUrl: String, | |
| contentDescription: String?, | |
| modifier: Modifier = Modifier, | |
| cornerRadius: Float = 8f | |
| ) { | |
| @Composable | |
| fun CachedImage( | |
| imageUrl: String, | |
| contentDescription: String?, | |
| modifier: Modifier = Modifier, | |
| cornerRadius: Float = 8f | |
| ) { | |
| require(imageUrl.isNotBlank()) { "imageUrl cannot be blank" } | |
| require(imageUrl.startsWith("http", ignoreCase = true)) { "imageUrl must be a valid HTTP/HTTPS URL" } | |
| <!-- Clave de la API de Google Maps --> | ||
| <meta-data | ||
| android:name="com.google.android.geo.API_KEY" | ||
| android:value="AIzaSyCTHkA6PG3Zr_nhxq8N7dlX-vrmEM4mltY"/> |
There was a problem hiding this comment.
SECURITY RISK: Google Maps API key exposed.
The Google Maps API key is currently exposed in plain text in the manifest file. This is a critical security risk as:
- The key is now in version control and publicly accessible
- It could be misused, leading to unauthorized access and potential billing issues
Recommended fixes:
-
Immediately rotate (replace) this API key in Google Cloud Console
-
Store the new key securely using one of these methods:
// Option 1: BuildConfig (preferred) android { buildTypes { debug { buildConfigField("String", "MAPS_API_KEY", "\"your_debug_key\"") } release { buildConfigField("String", "MAPS_API_KEY", "\"your_release_key\"") } } }
Then in manifest:
<meta-data android:name="com.google.android.geo.API_KEY" android:value="${MAPS_API_KEY}"/>
Option 2: Create a
secrets.propertiesfile (add to .gitignore):MAPS_API_KEY=your_api_keyAnd load it in build.gradle.kts:
val secretsFile = rootProject.file("secrets.properties") val secrets = Properties() secrets.load(FileInputStream(secretsFile))
Would you like me to help set up either of these secure configurations?
🧰 Tools
🪛 Gitleaks
25-25: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement navigation action in the onClick handler
The onClick handler is currently empty. To enable navigation to the store details, implement navigation using navController.
Apply this diff:
onClick = {
-
+ navController.navigate("storeDetail/${store.id}")
}Ensure that the navigation route storeDetail/{storeId} is defined in your navigation graph.
Committable suggestion skipped: line range outside the PR's diff.
| TextField( | ||
| value = searchText, | ||
| onValueChange = { newText -> | ||
| searchText = newText | ||
| coroutineScope.launch { | ||
| if (!isNetworkAvailable(context)) { | ||
| Toast.makeText(context, "No hay conexión, intente más tarde", Toast.LENGTH_SHORT).show() | ||
| } else if (newText.text.isNotBlank()) { | ||
| isLoading = true | ||
| val placesClient = Places.createClient(context) | ||
| val request = FindAutocompletePredictionsRequest.builder() | ||
| .setQuery(newText.text) | ||
| .build() | ||
| placesClient.findAutocompletePredictions(request) | ||
| .addOnSuccessListener { response -> | ||
| predictions = response.autocompletePredictions.map { | ||
| it.placeId to it.getFullText(null).toString() | ||
| } | ||
| isLoading = false | ||
| } | ||
| .addOnFailureListener { | ||
| isLoading = false | ||
| Toast.makeText(context, "Error al buscar lugares", Toast.LENGTH_LONG).show() | ||
| } | ||
| } else { | ||
| predictions = emptyList() | ||
| } | ||
| } | ||
| }, | ||
| placeholder = { Text("Buscar lugar...") }, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(16.dp), | ||
| singleLine = true, | ||
| leadingIcon = { | ||
| Icon(Icons.Default.Search, contentDescription = "Buscar") | ||
| } | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement input debouncing to reduce unnecessary API calls
Currently, the search function sends a request to the Places API on every text change, which can lead to excessive network calls and potential rate limiting. Implementing a debounce mechanism will improve performance and user experience.
Consider applying this change to debounce the search input:
// Add at the top of your composable
var debounceJob by remember { mutableStateOf<Job?>(null) }
// Update the onValueChange function
onValueChange = { newText ->
searchText = newText
debounceJob?.cancel()
debounceJob = coroutineScope.launch {
delay(500) // Adjust the delay as needed
if (!isNetworkAvailable(context)) {
Toast.makeText(context, "No hay conexión, intente más tarde", Toast.LENGTH_SHORT).show()
} else if (newText.text.isNotBlank()) {
isLoading = true
// Existing code to fetch predictions
// ...
} else {
predictions = emptyList()
isLoading = false
}
}
}This will delay the API call until the user has stopped typing for the specified delay period.
| fun RestaurantMapScreen() { | ||
| val context = LocalContext.current | ||
| val fusedLocationClient = remember { LocationServices.getFusedLocationProviderClient(context) } | ||
| var permissionGranted by remember { mutableStateOf(false) } | ||
| var searchText by remember { mutableStateOf(TextFieldValue("")) } | ||
| var predictions by remember { mutableStateOf(listOf<Pair<String, String>>()) } | ||
| var selectedLocation by remember { mutableStateOf<LatLng?>(null) } | ||
| var isLoading by remember { mutableStateOf(false) } | ||
| val coroutineScope = rememberCoroutineScope() | ||
|
|
||
| // Inicializar Google Places API | ||
| LaunchedEffect(Unit) { | ||
| 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") | ||
| val placesClient = Places.createClient(context) | ||
|
|
||
| // Verificar permisos de ubicación | ||
| permissionGranted = ActivityCompat.checkSelfPermission( | ||
| context, | ||
| Manifest.permission.ACCESS_FINE_LOCATION | ||
| ) == PackageManager.PERMISSION_GRANTED | ||
|
|
||
| // Obtener ubicación actual si los permisos están concedidos | ||
| if (permissionGranted) { | ||
| getUserLocation(context, fusedLocationClient) { location -> | ||
| selectedLocation = LatLng(location.latitude, location.longitude) | ||
| } | ||
| } else { | ||
| Toast.makeText(context, "Permiso de ubicación denegado", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| val cameraPositionState = rememberCameraPositionState { | ||
| position = CameraPosition.fromLatLngZoom(selectedLocation ?: LatLng(4.60971, -74.08175), 15f) | ||
| } | ||
|
|
||
| Column { | ||
| // Barra de búsqueda | ||
| TextField( | ||
| value = searchText, | ||
| onValueChange = { newText -> | ||
| searchText = newText | ||
| coroutineScope.launch { | ||
| if (!isNetworkAvailable(context)) { | ||
| Toast.makeText(context, "No hay conexión, intente más tarde", Toast.LENGTH_SHORT).show() | ||
| } else if (newText.text.isNotBlank()) { | ||
| isLoading = true | ||
| val placesClient = Places.createClient(context) | ||
| val request = FindAutocompletePredictionsRequest.builder() | ||
| .setQuery(newText.text) | ||
| .build() | ||
| placesClient.findAutocompletePredictions(request) | ||
| .addOnSuccessListener { response -> | ||
| predictions = response.autocompletePredictions.map { | ||
| it.placeId to it.getFullText(null).toString() | ||
| } | ||
| isLoading = false | ||
| } | ||
| .addOnFailureListener { | ||
| isLoading = false | ||
| Toast.makeText(context, "Error al buscar lugares", Toast.LENGTH_LONG).show() | ||
| } | ||
| } else { | ||
| predictions = emptyList() | ||
| } | ||
| } | ||
| }, | ||
| placeholder = { Text("Buscar lugar...") }, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(16.dp), | ||
| singleLine = true, | ||
| leadingIcon = { | ||
| Icon(Icons.Default.Search, contentDescription = "Buscar") | ||
| } | ||
| ) | ||
|
|
||
| // Mostrar sugerencias basadas en autocompletar | ||
| LazyColumn(modifier = Modifier.fillMaxWidth()) { | ||
| items(predictions) { (placeId, description) -> | ||
| Text( | ||
| text = description, | ||
| modifier = Modifier | ||
| .padding(8.dp) | ||
| .clickable { | ||
| coroutineScope.launch { | ||
| if (!isNetworkAvailable(context)) { | ||
| Toast.makeText(context, "No hay conexión, intente más tarde", Toast.LENGTH_SHORT).show() | ||
| } else { | ||
| val placesClient = Places.createClient(context) | ||
| val placeFields = listOf(Place.Field.LAT_LNG, Place.Field.NAME) | ||
| val fetchPlaceRequest = FetchPlaceRequest.builder(placeId, placeFields).build() | ||
|
|
||
| placesClient.fetchPlace(fetchPlaceRequest) | ||
| .addOnSuccessListener { fetchPlaceResponse -> | ||
| val place = fetchPlaceResponse.place | ||
| selectedLocation = place.latLng | ||
| place.latLng?.let { location -> | ||
| cameraPositionState.move( | ||
| CameraUpdateFactory.newLatLngZoom(location, 15f) | ||
| ) | ||
| } | ||
| } | ||
| .addOnFailureListener { | ||
| Toast.makeText(context, "Error al obtener detalles del lugar", Toast.LENGTH_LONG).show() | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| if (isLoading) { | ||
| CircularProgressIndicator(modifier = Modifier.padding(16.dp)) | ||
| } | ||
|
|
||
| if (permissionGranted) { | ||
| GoogleMap( | ||
| modifier = Modifier.weight(1f), | ||
| cameraPositionState = cameraPositionState, | ||
| ) { | ||
| selectedLocation?.let { location -> | ||
| Marker( | ||
| state = rememberMarkerState(position = location), | ||
| title = "Mi ubicación" | ||
| ) | ||
| } | ||
| } | ||
| } else { | ||
| Text("Se requieren permisos de ubicación para mostrar el mapa.") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance permission handling for better user experience
When location permission is denied, the user only receives a Toast message. Consider providing a more interactive prompt or guiding them to enable permissions in the app settings.
Would you like assistance in implementing a dialog that directs users to the app settings to grant location permissions?
🧰 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)
| 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.
Remove hardcoded API key to secure the application
Storing API keys directly in the code is a security risk and can lead to unauthorized access if the code is exposed. It's recommended to store API keys securely, such as in environment variables or encrypted configuration files, and exclude them from version control.
Apply this diff to remove the hardcoded API key and use a secure 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 local.properties file or secured build configuration and is not checked into version control.
📝 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.
| Places.initialize(context.applicationContext, "AIzaSyCTHkA6PG3Zr_nhxq8N7dlX-vrmEM4mltY") | |
| val apiKey = BuildConfig.MAPS_API_KEY | |
| Places.initialize(context.applicationContext, apiKey) |
🧰 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)
| if (ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) { | ||
| fusedLocationClient.lastLocation.addOnSuccessListener { location: Location? -> | ||
| location?.let { | ||
| onLocationResult(it) | ||
| } ?: Toast.makeText(context, "No se pudo obtener la ubicación", Toast.LENGTH_SHORT).show() | ||
| }.addOnFailureListener { | ||
| Toast.makeText(context, "Error al obtener la ubicación", Toast.LENGTH_SHORT).show() | ||
| } | ||
| } else { | ||
| Toast.makeText(context, "Permiso de ubicación no concedido", Toast.LENGTH_SHORT).show() | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Both location permissions are declared but not properly checked in code
The AndroidManifest.xml correctly declares both ACCESS_FINE_LOCATION and ACCESS_COARSE_LOCATION permissions, but the code only checks for ACCESS_FINE_LOCATION. Update the permission check to handle both:
- In
RestaurantMapScreen.kt, modify the permission check to:if (ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED || ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_COARSE_LOCATION) == PackageManager.PERMISSION_GRANTED) {
🔗 Analysis chain
Check and handle both fine and coarse location permissions
The getUserLocation function only checks for ACCESS_FINE_LOCATION. To ensure broader compatibility, also handle ACCESS_COARSE_LOCATION, and provide appropriate fallbacks if only coarse location is available.
Run the following script to verify declared location permissions:
This will help confirm which permissions are declared and if any adjustments are needed in the code.
This will help verify how location permissions are being checked across the codebase and if there are any existing patterns for handling both permissions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for location permissions in AndroidManifest.xml
# Expected: List of location permissions declared in the manifest
grep -E 'ACCESS_FINE_LOCATION|ACCESS_COARSE_LOCATION' app/src/main/AndroidManifest.xml
Length of output: 248
Script:
#!/bin/bash
# Check for location permission checks in Kotlin files
rg -U "checkSelfPermission.*ACCESS_(FINE|COARSE)_LOCATION" --type kotlin -A 3
Length of output: 680
…CarrascalDiaz/kotlin_app into fix/sprint3_adjustments
2 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Style
Chores