Feat/jackson cuevas test #13
Conversation
…G CHANGE: I need to remove the homePage with the Fetch button because I reached the limit from Yelp GraphQL API.
…of the restaurants. I’m using MVVM.
… strings BREAKING CHANGE: I removed the default theme design from the project
…Restaurant Detail for testing widgets
… I didn’t know the yelp had limit request.
…the restaurant name space
…rieval each time the user switches tabs. Additionally, a RefreshIndicator has been integrated to facilitate the updating of in-memory data.
lib/common/utils.dart
Outdated
| import 'dart:io'; | ||
| import 'package:path_provider/path_provider.dart'; | ||
|
|
||
| import '../../../models/restaurant.dart'; |
There was a problem hiding this comment.
suggestion: avoid_relative_lib_imports
lib/widgets/review_item_widget.dart
Outdated
| crossAxisAlignment: CrossAxisAlignment.start, | ||
| children: [ | ||
| const Text( | ||
| "Review text goes here. Review text goes here. Review text goes here. This is a review. This is a review. This is a review that is 4 lines long."), |
There was a problem hiding this comment.
issue: you're not using the reviewText
There was a problem hiding this comment.
Yes, you are right. The property doesn't exist, which explains the current situation. However, let me explore what options I have to circumvent the use of hard-coded text.
| itemBuilder: (context, index) { | ||
| var review = widget.restaurant.reviews![index]; | ||
| return ReviewListTile( | ||
| stars: review.rating!, |
There was a problem hiding this comment.
you're force unwrapping here, not properly cheking for nullable values
pubspec.yaml
Outdated
| @@ -10,13 +10,19 @@ environment: | |||
| sdk: ">=2.12.0 <3.0.0" | |||
|
|
|||
| dependencies: | |||
| integration_test: | |||
There was a problem hiding this comment.
this is a dev dependency
pubspec.yaml
Outdated
|
|
||
| gap: ^3.0.1 | ||
| stacked: ^3.4.2 | ||
| mockito: ^5.4.4 |
There was a problem hiding this comment.
mockito is a dev dependency
test/MockYelpRepository.dart
Outdated
| import 'package:restaurantour/models/restaurant.dart'; | ||
| import 'package:restaurantour/repositories/yelp_repository.dart'; | ||
|
|
||
| class MockYelpRepository extends Mock implements YelpRepository { |
There was a problem hiding this comment.
snake_case naming convention also applies to tests
| } | ||
|
|
||
| ready() async { | ||
| isLoading = true; |
There was a problem hiding this comment.
thought: seems like this viewmodel is not needed, what do you think?
| final result = await yelpRepo.getRestaurants(); | ||
| return result!.restaurants!; | ||
| } catch (e) { | ||
| throw Exception(e); |
There was a problem hiding this comment.
question: who catches this exception?
| onViewModelReady: (RestaurantViewModel viewModel) => viewModel.ready(), | ||
| builder: (context, RestaurantViewModel viewModel, child) => | ||
| FutureBuilder<List<Restaurant>>( | ||
| future: viewModel.fetchData(), |
There was a problem hiding this comment.
we're making new reuqests each time we come back from the favorites tab, any idea how to persist these?
Hello everyone,
I wanted to inform you about an update to the test. Due to my oversight regarding Yelp's API daily request limits, I had to make a slight modification to the original plan. Additionally, for a bit of enjoyment, I decided to utilize
path_providerfor storing favorite restaurants. Although I initially thought thatshared_preferenceswould be simple to implement, I opted for using a JSON file for data storage as a more engaging approach.Should you require any clarification or have questions about these changes, please don't hesitate to reach out.
Thank you.