Skip to content

Feature/profile image #75 #76

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
RaulRinconX merged 10 commits into from
Dec 3, 2024
Merged

Feature/profile image #75 #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c1b9073
remove commented code for ticket submission in SupportBloc
RaulRinconX Dec 3, 2024
902ba0f
feat: add profile image upload and caching functionality
RaulRinconX Dec 3, 2024
07ec2f7
feat: implement caching for profile image in ProfileBloc
RaulRinconX Dec 3, 2024
932fec7
feat: add permissions for external storage and camera access in Andro…
RaulRinconX Dec 3, 2024
0b2e83b
feat: add permissions for media access and camera in AndroidManifest
RaulRinconX Dec 3, 2024
deaa694
feat: update pubspec.lock to include new camera and file selector dep…
RaulRinconX Dec 3, 2024
0016a68
feat: update pubspec.yaml to include new camera, image_picker, path_p…
RaulRinconX Dec 3, 2024
09a4767
fix: handle storage permission errors and improve error logging in pr…
RaulRinconX Dec 3, 2024
efc785e
fix: remove unused local variable in rating submission logic
RaulRinconX Dec 3, 2024
16a8e61
fix: correct import statement for RatingScreen in ForYouTab widget
RaulRinconX Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions android/app/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@
<uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
<uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.CAMERA" />

<application
android:requestLegacyExternalStorage="true"
android:label="eco_bites"
android:name="${applicationName}"
android:icon="@mipmap/ic_launcher">
Expand Down
3 changes: 3 additions & 0 deletions android/app/src/profile/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@
to allow setting breakpoints, to provide hot reload, etc.
-->
<uses-permission android:name="android.permission.INTERNET"/>
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.CAMERA" />
Comment on lines +7 to +9
Copy link

@coderabbitai coderabbitai bot Dec 3, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Storage permissions need to be updated for modern Android compatibility

The verification reveals several issues that need to be addressed:

  • The main AndroidManifest.xml already includes both legacy storage permissions and the modern READ_MEDIA_IMAGES permission, causing redundancy
  • The app uses android:requestLegacyExternalStorage="true" which forces the legacy storage model
  • The permission handling code in profile_screen.dart still uses the deprecated Permission.storage API

Recommendations:

  • Remove READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE from profile/AndroidManifest.xml as they're redundant
  • Remove android:requestLegacyExternalStorage="true" from the application tag
  • Update the permission request in profile_screen.dart to use Permission.photos instead of Permission.storage
🔗 Analysis chain

Consider using scoped storage and modern permission model

While the permissions are correctly declared, consider the following security recommendations:

  1. For Android 13+ (API 33+), prefer using READ_MEDIA_IMAGES instead of broad storage permissions
  2. For Android 10+ (API 29+), consider implementing scoped storage instead of using legacy storage permissions
  3. Ensure runtime permission requests are properly implemented in the Flutter code

Let's verify the target SDK version and storage implementation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check build.gradle for targetSdkVersion and storage-related configurations
rg "targetSdkVersion|compileSdkVersion|requestLegacyExternalStorage" android/app/build.gradle

Length of output: 183

Script:

#!/bin/bash
# Check flutter.properties or local.properties for SDK version values
rg -l "compileSdkVersion|targetSdkVersion" android/

# Check main AndroidManifest.xml for storage-related configurations
cat android/app/src/main/AndroidManifest.xml

# Search for storage permission handling in Dart code
rg "requestPermission|getPermission|storage|imagePicker" lib/

Length of output: 3996

</manifest>
1 change: 1 addition & 0 deletions lib/features/home/presentation/screens/rating_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class _RatingScreenState extends State<RatingScreen> {
}

void _submitRating(BuildContext context) {
// ignore: unused_local_variable
final String comment = _commentController.text.trim();

// Perform the rating submission logic here
Expand Down
4 changes: 2 additions & 2 deletions lib/features/home/presentation/widgets/for_you_tab.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import 'package:eco_bites/features/food/domain/entities/offer.dart';
import 'package:eco_bites/features/food/presentation/bloc/food_business_bloc.dart';
import 'package:eco_bites/features/food/presentation/bloc/food_business_event.dart';
import 'package:eco_bites/features/food/presentation/bloc/food_business_state.dart';
import 'package:eco_bites/features/home/presentation/screens/rating_screen.dart';
import 'package:eco_bites/features/profile/presentation/bloc/profile_bloc.dart';
import 'package:eco_bites/features/profile/presentation/bloc/profile_state.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:fluttertoast/fluttertoast.dart';
import 'package:eco_bites/features/home/presentation/screens/rating_screen.dart';

class ForYouTab extends StatelessWidget {
const ForYouTab({super.key});
Expand Down Expand Up @@ -70,7 +70,7 @@ class ForYouTab extends StatelessWidget {
Navigator.of(context).push(
MaterialPageRoute<void>(
builder: (BuildContext context) => RatingScreen(
restaurantName: foodBusiness.name),
restaurantName: foodBusiness.name,),
),
);
},
Expand Down
9 changes: 9 additions & 0 deletions lib/features/profile/presentation/bloc/profile_bloc.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// ignore_for_file: unused_element

import 'dart:convert';
import 'dart:io';
import 'package:eco_bites/core/blocs/internet_connection/internet_connection_bloc.dart';
import 'package:eco_bites/core/blocs/resettable_mixin.dart';
import 'package:eco_bites/core/constants/storage_keys.dart';
Expand All @@ -11,6 +12,7 @@ import 'package:eco_bites/features/profile/presentation/bloc/profile_event.dart'
import 'package:eco_bites/features/profile/presentation/bloc/profile_state.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:logger/logger.dart';
import 'package:path_provider/path_provider.dart';
import 'package:shared_preferences/shared_preferences.dart';

class ProfileBloc extends Bloc<ProfileEvent, ProfileState>
Expand Down Expand Up @@ -50,6 +52,13 @@ class ProfileBloc extends Bloc<ProfileEvent, ProfileState>
);
Logger().d('Cached profile: ${event.updatedProfile.toMap()}');
Logger().d('Cached profile: ${prefs.getString('cachedProfile')}');

final Directory cacheDir = await getTemporaryDirectory();
final String imagePath = '${cacheDir.path}/profile_image.png';
if (File(imagePath).existsSync()) {
// Handle cached profile image
}

Comment on lines +55 to +61
Copy link

@coderabbitai coderabbitai bot Dec 3, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incomplete implementation of cached profile image handling

The code checks for the existence of a cached profile image but doesn't implement any handling logic. This could lead to inconsistent state management.

Consider implementing the image handling logic:

 final Directory cacheDir = await getTemporaryDirectory();
 final String imagePath = '${cacheDir.path}/profile_image.png';
 if (File(imagePath).existsSync()) {
+  try {
+    final File cachedImage = File(imagePath);
+    // Handle the cached image, e.g., emit a state with the cached image
+    emit(ProfileLoaded(event.updatedProfile, cachedImagePath: imagePath));
+  } catch (e) {
+    Logger().e('Failed to handle cached profile image: $e');
+  }
-  // Handle cached profile image
 }

Committable suggestion skipped: line range outside the PR's diff.

emit(
ProfileError(
'No internet connection. Your data will be saved when you are online.',
Expand Down
90 changes: 75 additions & 15 deletions lib/features/profile/presentation/screens/profile_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:io';

import 'package:eco_bites/core/blocs/internet_connection/internet_connection_bloc.dart';
import 'package:eco_bites/core/utils/analytics_service.dart';
import 'package:eco_bites/features/auth/presentation/bloc/auth_bloc.dart';
Expand All @@ -11,7 +13,10 @@ import 'package:eco_bites/features/profile/presentation/bloc/profile_state.dart'
import 'package:firebase_auth/firebase_auth.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:image_picker/image_picker.dart';
import 'package:intl/intl.dart';
import 'package:path_provider/path_provider.dart';
import 'package:permission_handler/permission_handler.dart';
import 'package:shared_preferences/shared_preferences.dart';

class ProfileScreen extends StatefulWidget {
Expand All @@ -31,10 +36,12 @@ class ProfileScreenState extends State<ProfileScreen> {
CuisineType? _favoriteCuisine;
DietType? _dietType;
bool _isInitialized = false;
File? _profileImage;

@override
void initState() {
super.initState();
_loadProfileImage();
final String? userId = FirebaseAuth.instance.currentUser?.uid;
if (userId != null) {
context.read<ProfileBloc>().add(LoadProfileEvent(userId));
Expand All @@ -45,6 +52,43 @@ class ProfileScreenState extends State<ProfileScreen> {
}
}

Future<void> _pickImage() async {
try {
// Check storage permission
if (await Permission.storage.request().isGranted) {
final ImagePicker picker = ImagePicker();
final XFile? image = await picker.pickImage(source: ImageSource.gallery);

if (image != null) {
setState(() {
_profileImage = File(image.path);
});

// Save the profile image to cache
final Directory cacheDir = await getTemporaryDirectory();
final String imagePath = '${cacheDir.path}/profile_image.png';
await _profileImage!.copy(imagePath);
}
} else {
// ignore: avoid_print
print('Storage permission not granted.');
}
} catch (e) {
// ignore: avoid_print
print('Error picking image: $e');
}
}

Future<void> _loadProfileImage() async {
final Directory cacheDir = await getTemporaryDirectory();
final String imagePath = '${cacheDir.path}/profile_image.png';
if (File(imagePath).existsSync()) {
setState(() {
_profileImage = File(imagePath);
});
}
}
Comment on lines +82 to +90
Copy link

@coderabbitai coderabbitai bot Dec 3, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling to profile image loading

The image loading implementation lacks error handling and proper cleanup.

Consider this improved implementation:

 Future<void> _loadProfileImage() async {
+  try {
     final Directory cacheDir = await getTemporaryDirectory();
     final String imagePath = '${cacheDir.path}/profile_image.png';
-    if (File(imagePath).existsSync()) {
+    final file = File(imagePath);
+    if (await file.exists()) {
       setState(() {
         _profileImage = File(imagePath);
       });
     }
+  } catch (e) {
+    Logger().e('Error loading profile image', error: e, stackTrace: StackTrace.current);
+    // Don't show error to user as this is a silent operation
   }
 }

Committable suggestion skipped: line range outside the PR's diff.


// ignore: unused_element
Future<void> _saveProfile(UserProfile updatedProfile) async {
final InternetConnectionBloc internetConnectionBloc =
Expand Down Expand Up @@ -72,6 +116,12 @@ class ProfileScreenState extends State<ProfileScreen> {
);
}
}
if (_profileImage != null) {
// Save the profile image to cache
final Directory cacheDir = await getTemporaryDirectory();
final String imagePath = '${cacheDir.path}/profile_image.png';
await _profileImage!.copy(imagePath);
}
Comment on lines +119 to +124
Copy link

@coderabbitai coderabbitai bot Dec 3, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consolidate profile saving logic and add cleanup

There's duplicate code for saving profile data to cache, and no cleanup of old cached files.

Consider extracting the caching logic to a separate method and adding cleanup:

+ Future<void> _saveToCache(UserProfile profile, {File? image}) async {
+   try {
+     final Directory cacheDir = await getTemporaryDirectory();
+     
+     // Clean up old cache files
+     final cacheFiles = await cacheDir.list().where((entity) => 
+       entity.path.contains('profile')).toList();
+     for (var file in cacheFiles) {
+       await file.delete();
+     }
+     
+     // Save new profile data
+     final String profilePath = '${cacheDir.path}/profile.json';
+     await File(profilePath).writeAsString(profile.toMap().toString());
+     
+     // Save new image if provided
+     if (image != null) {
+       final String imagePath = '${cacheDir.path}/profile_image.png';
+       await image.copy(imagePath);
+     }
+   } catch (e) {
+     Logger().e('Error saving to cache', error: e, stackTrace: StackTrace.current);
+     throw Exception('Failed to save profile to cache');
+   }
+ }

Update the profile saving logic to use this method:

 if (internetConnectionBloc.state is DisconnectedInternet) {
-  final Directory cacheDir = await getTemporaryDirectory();
-  final String profilePath = '${cacheDir.path}/profile.json';
-  await File(profilePath).writeAsString(updatedProfile.toMap().toString());
+  await _saveToCache(updatedProfile, image: _profileImage);
   // ... show snackbar
 }

Also applies to: 231-251

}

@override
Expand Down Expand Up @@ -120,6 +170,19 @@ class ProfileScreenState extends State<ProfileScreen> {

return Column(
children: <Widget>[
GestureDetector(
onTap: _pickImage,
child: CircleAvatar(
radius: 50,
backgroundImage: _profileImage != null
? FileImage(_profileImage!)
: null,
child: _profileImage == null
? const Icon(Icons.camera_alt, size: 50)
: null,
),
),
const SizedBox(height: 24),
ElevatedButton(
onPressed: () {
Navigator.pushNamed(context, '/support');
Expand Down Expand Up @@ -154,8 +217,7 @@ class ProfileScreenState extends State<ProfileScreen> {
Center(
child: ElevatedButton(
onPressed: () async {
final String? userId =
FirebaseAuth.instance.currentUser?.uid;
final String? userId = FirebaseAuth.instance.currentUser?.uid;
if (userId != null) {
final UserProfile updatedProfile = UserProfile(
userId: userId,
Expand All @@ -164,18 +226,16 @@ class ProfileScreenState extends State<ProfileScreen> {
citizenId: _citizenIdController.text,
email: _emailController.text,
phone: _phoneController.text,
birthDate: DateFormat('MM/dd/yyyy')
.parse(_birthDateController.text),
favoriteCuisine:
_favoriteCuisine ?? CuisineType.other,
birthDate: DateFormat('MM/dd/yyyy').parse(_birthDateController.text),
favoriteCuisine: _favoriteCuisine ?? CuisineType.other,
dietType: _dietType ?? DietType.none,
);

final InternetConnectionBloc internetConnectionBloc =
context.read<InternetConnectionBloc>();
final InternetConnectionBloc internetConnectionBloc = context.read<InternetConnectionBloc>();
if (internetConnectionBloc.state is DisconnectedInternet) {
final SharedPreferences prefs = await SharedPreferences.getInstance();
await prefs.setString('cachedProfile', updatedProfile.toMap().toString());
final Directory cacheDir = await getTemporaryDirectory();
final String profilePath = '${cacheDir.path}/profile.json';
await File(profilePath).writeAsString(updatedProfile.toMap().toString());
if (mounted) {
// ignore: use_build_context_synchronously
ScaffoldMessenger.of(context).showSnackBar(
Expand All @@ -188,11 +248,11 @@ class ProfileScreenState extends State<ProfileScreen> {
}
} else {
context.read<ProfileBloc>().add(
UpdateProfileEvent(
userId: userId,
updatedProfile: updatedProfile,
),
);
UpdateProfileEvent(
userId: userId,
updatedProfile: updatedProfile,
),
);
}
}
},
Expand Down
1 change: 0 additions & 1 deletion lib/features/support/presentation/bloc/support_bloc.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class SupportBloc extends Bloc<SupportEvent, SupportState> {
) async {
emit(SupportLoading());
try {
// Intentar subir el ticket a Firestore
await firestore.collection('support_tickets').add(<String, dynamic>{
'category': event.category,
'subOption': event.subOption,
Expand Down
Loading
Loading