Skip to content

Conversation

@RaulRinconX
Copy link
Contributor

@RaulRinconX RaulRinconX commented Dec 2, 2024

This pull request introduces a new support feature to the EcoBites application, including a SupportBloc for managing support tickets, a new support screen, and integration with the existing application structure. The most important changes include adding the SupportBloc, creating a new support screen, and updating the dependency injection setup.

New Support Feature:

Integration with Existing Application:

Enhancements to Profile Screen:

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a "Need help?" button in the profile screen to navigate to a support screen.
    • Added a structured support ticket submission process with error handling and offline caching.
    • Implemented a new support screen displaying categories for user assistance.
  • Bug Fixes

    • Enhanced profile saving logic to check internet connectivity and cache data when offline.
  • Documentation

    • Updated routing to include support and profile screens.

These updates improve user assistance and profile management, enhancing overall user experience.

@RaulRinconX RaulRinconX added the enhancement New feature or request label Dec 2, 2024
@RaulRinconX RaulRinconX added this to the Sprint 4 milestone Dec 2, 2024
@RaulRinconX RaulRinconX self-assigned this Dec 2, 2024
@RaulRinconX RaulRinconX linked an issue Dec 2, 2024 that may be closed by this pull request
@coderabbitai
Copy link

coderabbitai bot commented Dec 2, 2024

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request introduces a new support feature in the application, which includes the implementation of a SupportBloc for managing support ticket submissions and retries. The ProfileScreen is updated to include a button for user assistance that navigates to the new SupportScreen. The support functionality is integrated into the app structure via the service locator and routing mechanisms. Additionally, error handling related to internet connectivity during profile updates is enhanced, ensuring a smoother user experience when submitting support tickets.

Changes

File Path Change Summary
lib/app.dart - Added import for SupportBloc.
- Included BlocProvider for SupportBloc in MultiBlocProvider.
lib/features/profile/presentation/screens/profile_screen.dart - Added "Need help?" button to navigate to support screen.
- Updated _saveProfile method for internet connectivity checks.
lib/features/support/presentation/bloc/support_bloc.dart - Introduced SupportBloc class with methods for ticket submission and retrying cached tickets.
lib/features/support/presentation/bloc/support_event.dart - Added SupportEvent abstract class with SubmitTicketEvent and RetryCachedTicketsEvent classes.
lib/features/support/presentation/bloc/support_state.dart - Created SupportState abstract class with various concrete states for support functionality.
lib/features/support/presentation/screens/support_screen.dart - Introduced SupportScreen class for displaying support categories and handling ticket submissions.
lib/features/support/presentation/widgets/ticket_form.dart - Added TicketForm class for submitting support tickets with user input handling.
lib/injection_container.dart - Added _setupSupportFeature method to register SupportBloc with service locator.
lib/route_generator.dart - Defined new routes for support and profile screens in RouteGenerator.

Possibly related PRs

🐰 In the garden of code, we hop with glee,
A button for help, as bright as can be!
With tickets and support, all snug and tight,
Our users find solace, day or night.
So let’s cheer for the changes, let’s dance and play,
For a better experience is here to stay! 🎉


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ma-r-s ma-r-s self-requested a review December 2, 2024 22:03
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Outside diff range and nitpick comments (7)
lib/features/support/presentation/widgets/ticket_form.dart (2)

17-33: Refactor to reduce code duplication in state listeners

In the listener function, the handling for SupportSuccess and SupportCached states is similar. Consider refactoring to reduce code duplication by handling these states together.

Here's how you can refactor the code:

listener: (BuildContext context, SupportState state) {
  if (state is SupportSuccess || state is SupportCached) {
+   final String message = state is SupportSuccess
+       ? 'Ticket submitted successfully'
+       : state.message;
    ScaffoldMessenger.of(context).showSnackBar(
-     const SnackBar(content: Text('Ticket submitted successfully')),
+     SnackBar(content: Text(message)),
    );
    Navigator.pop(context);
  } else if (state is SupportFailure) {
    ScaffoldMessenger.of(context).showSnackBar(
      SnackBar(content: Text('Failed to submit ticket: ${state.error}')),
    );
  }
},

38-76: Prevent overflow when keyboard appears by wrapping content in a SingleChildScrollView

To avoid overflow issues when the keyboard appears, especially on smaller screens, wrap the form content in a SingleChildScrollView.

Apply this change:

return Padding(
  padding: const EdgeInsets.all(16.0),
- child: Column(
+ child: SingleChildScrollView(
+   child: Column(
      children: <Widget>[
        // ... existing widgets ...
      ],
+   ),
+ ),
);
lib/features/support/presentation/screens/support_screen.dart (2)

8-39: Add rotation handling to modal bottom sheet

The modal bottom sheet might not handle device rotation gracefully. Consider adding useRootNavigator: true and handling orientation changes.

Apply this diff:

    showModalBottomSheet<void>(
      context: context,
      isScrollControlled: true,
+     useRootNavigator: true,
      builder: (BuildContext context) {

76-159: Enhance accessibility support

Add semantic labels and improve accessibility for support categories.

Apply this pattern to each ListTile:

                  ListTile(
                    leading: const Icon(Icons.shopping_cart),
                    title: const Text('Order Management'),
                    trailing: const Icon(Icons.chevron_right),
+                   semanticLabel: 'Order Management support options',
lib/injection_container.dart (1)

222-230: Consider implementing complete feature architecture

The support feature setup is missing important architectural layers compared to other features:

  1. Repository layer for data management
  2. Use cases for business logic
  3. Data sources for local/remote data handling

This could make the feature harder to test and maintain.

Consider implementing these layers following the pattern used in other features like the profile feature. Example structure:

// Repository
abstract class SupportRepository {
  Future<void> submitTicket(SupportTicket ticket);
  Future<List<SupportTicket>> getCachedTickets();
}

// Use cases
class SubmitTicketUseCase {
  final SupportRepository repository;
  Future<void> call(SupportTicket ticket) => repository.submitTicket(ticket);
}

// Data sources
abstract class SupportRemoteDataSource {
  Future<void> submitTicket(SupportTicket ticket);
}
lib/features/profile/presentation/screens/profile_screen.dart (2)

122-131: Add error handling for navigation

While the help button implementation is clean, consider adding error handling for navigation failures and a loading state during transition.

 ElevatedButton(
   onPressed: () {
-    Navigator.pushNamed(context, '/support');
+    try {
+      setState(() => _isNavigating = true);
+      Navigator.pushNamed(context, '/support').catchError((error) {
+        ScaffoldMessenger.of(context).showSnackBar(
+          const SnackBar(content: Text('Unable to access support screen')),
+        );
+      }).whenComplete(() {
+        if (mounted) setState(() => _isNavigating = false);
+      });
+    } catch (e) {
+      ScaffoldMessenger.of(context).showSnackBar(
+        const SnackBar(content: Text('Navigation error occurred')),
+      );
+    }
   },
   style: ElevatedButton.styleFrom(
     padding: const EdgeInsets.symmetric(horizontal: 32, vertical: 16),
     textStyle: const TextStyle(fontSize: 18),
   ),
-  child: const Text('Need help?'),
+  child: _isNavigating
+      ? const CircularProgressIndicator()
+      : const Text('Need help?'),
 ),

Line range hint 155-194: Refactor save profile logic to avoid code duplication

The profile saving logic is duplicated between the onPressed handler and _saveProfile method. Consider extracting the profile creation logic to a separate method.

+ Future<UserProfile?> _createUpdatedProfile() async {
+   final String? userId = FirebaseAuth.instance.currentUser?.uid;
+   if (userId == null) return null;
+   
+   return UserProfile(
+     userId: userId,
+     name: _nameController.text,
+     surname: _surnameController.text,
+     citizenId: _citizenIdController.text,
+     email: _emailController.text,
+     phone: _phoneController.text,
+     birthDate: DateFormat('MM/dd/yyyy').parse(_birthDateController.text),
+     favoriteCuisine: _favoriteCuisine ?? CuisineType.other,
+     dietType: _dietType ?? DietType.none,
+   );
+ }

  ElevatedButton(
-   onPressed: () async {
-     final String? userId = FirebaseAuth.instance.currentUser?.uid;
-     if (userId != null) {
-       final UserProfile updatedProfile = UserProfile(...)
-       final InternetConnectionBloc internetConnectionBloc = ...
+   onPressed: () async {
+     final profile = await _createUpdatedProfile();
+     if (profile != null) {
+       await _saveProfile(profile);
+     }
    },
    child: const Text('Save'),
  ),
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ac1c5ed and f0a185c.

📒 Files selected for processing (9)
  • lib/app.dart (2 hunks)
  • lib/features/profile/presentation/screens/profile_screen.dart (3 hunks)
  • lib/features/support/presentation/bloc/support_bloc.dart (1 hunks)
  • lib/features/support/presentation/bloc/support_event.dart (1 hunks)
  • lib/features/support/presentation/bloc/support_state.dart (1 hunks)
  • lib/features/support/presentation/screens/support_screen.dart (1 hunks)
  • lib/features/support/presentation/widgets/ticket_form.dart (1 hunks)
  • lib/injection_container.dart (3 hunks)
  • lib/route_generator.dart (2 hunks)
🔇 Additional comments (12)
lib/features/support/presentation/bloc/support_event.dart (3)

3-8: Base event class is appropriately defined

The SupportEvent class correctly extends Equatable and provides an empty props list.


10-24: SubmitTicketEvent class is well-defined

The class properly extends SupportEvent, includes the required fields, and overrides props correctly.


26-26: RetryCachedTicketsEvent is correctly defined

An event to trigger the retry of cached tickets.

lib/features/support/presentation/bloc/support_state.dart (3)

3-8: Base state class correctly defined

The SupportState class properly extends Equatable and provides an empty props list.


16-22: SupportFailure state properly handles error information

The SupportFailure class includes an error field and overrides props accordingly.


24-31: SupportCached state correctly defined with message

The SupportCached class includes a message field and overrides props.

lib/route_generator.dart (3)

5-7: Imports added appropriately

The import statements for ProfileScreen and SupportScreen are correctly added.


15-16: Route constants defined correctly

The route constants supportScreen and profileScreen are appropriately added.


39-42: New routes handled correctly in generateRoute

The supportScreen and profileScreen routes are correctly added to the generateRoute method.

lib/app.dart (2)

72-72: LGTM: SupportBloc integration looks good

The SupportBloc is properly integrated into the MultiBlocProvider.


69-72: ⚠️ Potential issue

Remove duplicate ProfileBloc provider

There are two identical ProfileBloc providers which could cause unexpected behavior with state management.

Apply this diff to fix the issue:

        BlocProvider<ProfileBloc>(
          create: (_) => serviceLocator<ProfileBloc>(),
        ),
-       BlocProvider<ProfileBloc>(
-         create: (_) => serviceLocator<ProfileBloc>(),
-       ),
        BlocProvider<SupportBloc>(create: (_) => serviceLocator<SupportBloc>()),

Likely invalid or redundant comment.

lib/features/profile/presentation/screens/profile_screen.dart (1)

122-124: Verify route and bloc registration

Ensure that the support route and required blocs are properly registered.

✅ Verification successful

Route and bloc setup verified successfully

The verification confirms:

  • Support route is properly registered in lib/route_generator.dart
  • SupportBloc is correctly provided in the app's widget tree via BlocProvider
  • Support screen exists at the expected location lib/features/support/presentation/screens/support_screen.dart
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check route registration
echo "Checking route registration..."
rg -A 5 "'/support'" lib/route_generator.dart

# Check bloc provider setup
echo "Checking bloc provider setup..."
rg -A 5 "SupportBloc" lib/app.dart

# Verify support screen exists
echo "Verifying support screen..."
fd -e dart -p "support_screen.dart$"

Length of output: 862

Comment on lines +71 to +85
for (final String ticket in cachedTickets) {
final Map<String, dynamic> ticketData = jsonDecode(ticket);
try {
await firestore.collection('support_tickets').add(<String, dynamic>{
'category': ticketData['category'],
'subOption': ticketData['subOption'],
'reason': ticketData['reason'],
'description': ticketData['description'],
'timestamp': FieldValue.serverTimestamp(),
});
} catch (e) {
emit(SupportFailure(error: e.toString()));
return;
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure all cached tickets are retried even if some submissions fail

Currently, if submitting a cached ticket fails, the method emits SupportFailure and returns, which stops the submission of any remaining cached tickets. It's better to attempt to submit all cached tickets, even if some fail, and handle failures accordingly.

Here's how you can modify the code:

+ final List<String> failedTickets = [];
  for (final String ticket in cachedTickets) {
    final Map<String, dynamic> ticketData = jsonDecode(ticket);
    try {
      await firestore.collection('support_tickets').add(<String, dynamic>{
        'category': ticketData['category'],
        'subOption': ticketData['subOption'],
        'reason': ticketData['reason'],
        'description': ticketData['description'],
        'timestamp': FieldValue.serverTimestamp(),
      });
    } catch (e) {
+     failedTickets.add(ticket);
    }
  }

- emit(SupportFailure(error: e.toString()));
- return;

+ if (failedTickets.isNotEmpty) {
+   await prefs.setStringList('cachedTickets', failedTickets);
+   emit(SupportFailure(error: 'Some tickets failed to submit.'));
+ } else {
+   await prefs.remove('cachedTickets');
+   emit(SupportSuccess());
+ }

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

Comment on lines +17 to +22
internetConnectionBloc.stream.listen((InternetConnectionState state) {
if (state is ConnectedInternet) {
add(RetryCachedTicketsEvent());
}
});
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Manage the StreamSubscription to prevent memory leaks

Currently, the subscription to internetConnectionBloc.stream is not being canceled when the SupportBloc is closed, which could lead to memory leaks. To prevent this, store the subscription and cancel it in the close method.

Here's how you can modify the code:

+ late final StreamSubscription<InternetConnectionState> _internetConnectionSubscription;

  SupportBloc({required this.firestore, required this.internetConnectionBloc})
      : super(SupportInitial()) {
-   internetConnectionBloc.stream.listen((InternetConnectionState state) {
+   _internetConnectionSubscription = internetConnectionBloc.stream.listen((InternetConnectionState state) {
      if (state is ConnectedInternet) {
        add(RetryCachedTicketsEvent());
      }
    });
  }

+ @override
+ Future<void> close() {
+   _internetConnectionSubscription.cancel();
+   return super.close();
+ }
📝 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.

Suggested change
internetConnectionBloc.stream.listen((InternetConnectionState state) {
if (state is ConnectedInternet) {
add(RetryCachedTicketsEvent());
}
});
}
late final StreamSubscription<InternetConnectionState> _internetConnectionSubscription;
SupportBloc({required this.firestore, required this.internetConnectionBloc})
: super(SupportInitial()) {
_internetConnectionSubscription = internetConnectionBloc.stream.listen((InternetConnectionState state) {
if (state is ConnectedInternet) {
add(RetryCachedTicketsEvent());
}
});
}
@override
Future<void> close() {
_internetConnectionSubscription.cancel();
return super.close();
}

Comment on lines +5 to +12
class TicketForm extends StatelessWidget {
TicketForm({super.key, required this.category, required this.subOption});
final String category;
final String subOption;

final TextEditingController reasonController = TextEditingController();
final TextEditingController descriptionController = TextEditingController();

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Convert TicketForm to a StatefulWidget to manage controllers

Currently, TicketForm is a StatelessWidget but holds TextEditingController instances. Controllers are mutable and should be disposed to prevent memory leaks. Therefore, TicketForm should be converted to a StatefulWidget to properly manage the controllers and dispose of them in the dispose method.

Here's how you can modify the code:

- class TicketForm extends StatelessWidget {
+ class TicketForm extends StatefulWidget {
+   const TicketForm({super.key, required this.category, required this.subOption});

    final String category;
    final String subOption;

+   @override
+   _TicketFormState createState() => _TicketFormState();
+ }

+ class _TicketFormState extends State<TicketForm> {
+   late final TextEditingController reasonController;
+   late final TextEditingController descriptionController;

+   @override
+   void initState() {
+     super.initState();
+     reasonController = TextEditingController();
+     descriptionController = TextEditingController();
+   }

+   @override
+   void dispose() {
+     reasonController.dispose();
+     descriptionController.dispose();
+     super.dispose();
+   }

    @override
    Widget build(BuildContext context) {
      // ...existing build method...
    }
+ }

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

Comment on lines +60 to +69
: () {
context.read<SupportBloc>().add(
SubmitTicketEvent(
category: category,
subOption: subOption,
reason: reasonController.text,
description: descriptionController.text,
),
);
},
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation before submitting the ticket

Currently, the form does not validate user input before submitting the ticket. To enhance user experience and prevent empty submissions, validate that the reason and description fields are not empty.

Here's how you can implement basic validation:

onPressed: state is SupportLoading
    ? null
    : () {
+       if (reasonController.text.isEmpty || descriptionController.text.isEmpty) {
+         ScaffoldMessenger.of(context).showSnackBar(
+           const SnackBar(content: Text('Please fill in all fields')),
+         );
+         return;
+       }
        context.read<SupportBloc>().add(
          SubmitTicketEvent(
            category: widget.category,
            subOption: widget.subOption,
            reason: reasonController.text,
            description: descriptionController.text,
          ),
        );
      },

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

Comment on lines +76 to +159
ListTile(
leading: const Icon(Icons.shopping_cart),
title: const Text('Order Management'),
trailing: const Icon(Icons.chevron_right),
onTap: () {
_showOptions(context, 'Order Management', <String>[
'Track Order',
'Cancel Order',
'Return Order',
'Order History',
]);
},
),
const Divider(),
ListTile(
leading: const Icon(Icons.payment),
title: const Text('Payment Management'),
trailing: const Icon(Icons.chevron_right),
onTap: () {
_showOptions(context, 'Payment Management', <String>[
'Payment Methods',
'Refund Status',
'Payment Issues',
'Billing History',
]);
},
),
const Divider(),
ListTile(
leading: const Icon(Icons.account_circle),
title: const Text('Account and Profile Management'),
trailing: const Icon(Icons.chevron_right),
onTap: () {
_showOptions(context, 'Account and Profile Management', <String>[
'Update Profile',
'Change Password',
'Privacy Settings',
'Delete Account',
]);
},
),
const Divider(),
ListTile(
leading: const Icon(Icons.info),
title: const Text('About EcoBites'),
trailing: const Icon(Icons.chevron_right),
onTap: () {
_showOptions(context, 'About EcoBites', <String>[
'Company Information',
'Terms of Service',
'Privacy Policy',
'Contact Us',
]);
},
),
const Divider(),
ListTile(
leading: const Icon(Icons.security),
title: const Text('Security'),
trailing: const Icon(Icons.chevron_right),
onTap: () {
_showOptions(context, 'Security', <String>[
'Report a Security Issue',
'Account Security Tips',
'Two-Factor Authentication',
'Security Updates',
]);
},
),
const Divider(),
ListTile(
leading: const Icon(Icons.history),
title: const Text('Message History'),
trailing: const Icon(Icons.chevron_right),
onTap: () {
_showOptions(context, 'Message History', <String>[
'View Messages',
'Delete Messages',
'Archive Messages',
'Message Settings',
]);
},
),
],
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Extract hardcoded strings to localization constants

The support categories and their options should be extracted to localization constants for better maintainability and internationalization support.

Create a new file lib/core/constants/support_strings.dart and move all strings there.

Comment on lines +41 to +59
void _showTicketForm(BuildContext context, String category, String subOption) {
Navigator.push(
context,
MaterialPageRoute<void>(
builder: (BuildContext context) => Scaffold(
appBar: AppBar(
title: const Text('Submit Ticket'),
leading: IconButton(
icon: const Icon(Icons.arrow_back),
onPressed: () {
Navigator.pop(context);
},
),
),
body: TicketForm(category: category, subOption: subOption),
),
),
);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for navigation operations

The navigation operations should be wrapped in try-catch blocks to handle potential errors gracefully.

Apply this diff:

  void _showTicketForm(BuildContext context, String category, String subOption) {
+   try {
      Navigator.push(
        context,
        MaterialPageRoute<void>(
          builder: (BuildContext context) => Scaffold(
            appBar: AppBar(
              title: const Text('Submit Ticket'),
              leading: IconButton(
                icon: const Icon(Icons.arrow_back),
                onPressed: () {
+                 try {
                    Navigator.pop(context);
+                 } catch (e) {
+                   debugPrint('Error navigating back: $e');
+                 }
                },
              ),
            ),
            body: TicketForm(category: category, subOption: subOption),
          ),
        ),
      );
+   } catch (e) {
+     debugPrint('Error showing ticket form: $e');
+   }
  }
📝 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.

Suggested change
void _showTicketForm(BuildContext context, String category, String subOption) {
Navigator.push(
context,
MaterialPageRoute<void>(
builder: (BuildContext context) => Scaffold(
appBar: AppBar(
title: const Text('Submit Ticket'),
leading: IconButton(
icon: const Icon(Icons.arrow_back),
onPressed: () {
Navigator.pop(context);
},
),
),
body: TicketForm(category: category, subOption: subOption),
),
),
);
}
void _showTicketForm(BuildContext context, String category, String subOption) {
try {
Navigator.push(
context,
MaterialPageRoute<void>(
builder: (BuildContext context) => Scaffold(
appBar: AppBar(
title: const Text('Submit Ticket'),
leading: IconButton(
icon: const Icon(Icons.arrow_back),
onPressed: () {
try {
Navigator.pop(context);
} catch (e) {
debugPrint('Error navigating back: $e');
}
},
),
),
body: TicketForm(category: category, subOption: subOption),
),
),
);
} catch (e) {
debugPrint('Error showing ticket form: $e');
}
}

Comment on lines 173 to 194
final InternetConnectionBloc internetConnectionBloc =
context.read<InternetConnectionBloc>();
if (internetConnectionBloc.state is DisconnectedInternet) {
final SharedPreferences prefs = await SharedPreferences.getInstance();
await prefs.setString('cachedProfile', updatedProfile.toMap().toString());
if (mounted) {
ScaffoldMessenger.of(context).showSnackBar(
const SnackBar(
content: Text(
'No internet connection. Your data will be saved when you are online.',
),
),
);
}
} else {
context.read<ProfileBloc>().add(
UpdateProfileEvent(
userId: userId,
updatedProfile: updatedProfile,
),
);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation and retry mechanism for offline saves

The current implementation lacks data validation and a retry mechanism for cached profiles.

  1. Add validation before saving:
bool _validateProfile(UserProfile profile) {
  return profile.name.isNotEmpty && 
         profile.email.isNotEmpty &&
         EmailValidator.validate(profile.email);
}
  1. Implement a retry mechanism for cached profiles:
Future<void> _retryCachedProfile() async {
  final prefs = await SharedPreferences.getInstance();
  final cachedProfile = prefs.getString('cachedProfile');
  if (cachedProfile != null) {
    try {
      final profile = UserProfile.fromMap(cachedProfile);
      await _saveProfile(profile);
      await prefs.remove('cachedProfile');
    } catch (e) {
      // Handle parsing errors
    }
  }
}
  1. Consider using a more robust caching solution like Hive or SQLite for storing multiple pending profiles.

Comment on lines +173 to +176
final InternetConnectionBloc internetConnectionBloc =
context.read<InternetConnectionBloc>();
if (internetConnectionBloc.state is DisconnectedInternet) {
final SharedPreferences prefs = await SharedPreferences.getInstance();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential race conditions in connectivity check

The current implementation might have race conditions between checking connection state and saving the profile.

Consider using a more robust approach:

Future<bool> _isConnected() async {
  try {
    final result = await InternetAddress.lookup('example.com');
    return result.isNotEmpty && result[0].rawAddress.isNotEmpty;
  } on SocketException catch (_) {
    return false;
  }
}

@RaulRinconX RaulRinconX merged commit 669058a into main Dec 2, 2024
1 check passed
This was referenced Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: DONE

Development

Successfully merging this pull request may close these issues.

feature/support-form

3 participants