Conversation
WalkthroughAdds Rocket detail feature with Bloc, UI widgets, route, utilities, and localizations. Updates rockets list item to navigate to the new detail route. Introduces multiple integration and unit/widget tests, plus test utilities update. CI and Makefile gain integration/screenshot test invocations. Minor nav destination key rename and test data tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant RocketsScreen
participant NavigationService
participant Router as AppRouter
participant RocketScreen
participant Bloc as RocketBloc
participant Repo as RocketRepository
participant UI as RocketDetailUI
User->>RocketsScreen: Tap RocketItem
RocketsScreen->>NavigationService: navigateTo(Routes.rocket, rocket)
NavigationService->>AppRouter: push('rocket', args: rocket)
AppRouter->>RocketScreen: build with rocketId
RocketScreen->>Bloc: add(RocketEvent.load(rocketId))
Bloc->>RocketScreen: state = loading()
Bloc->>Repo: getRocket(rocketId)
Repo-->>Bloc: RocketResource
Bloc->>RocketScreen: state = success(rocket)
RocketScreen->>UI: render details (Overview, Specs, Payload...)
sequenceDiagram
autonumber
participant RocketScreen
participant Bloc as RocketBloc
participant Repo as RocketRepository
RocketScreen->>Bloc: add(load)
Bloc->>Repo: getRocket(rocketId)
Repo-->>Bloc: throw Exception
Bloc->>RocketScreen: state = error()
RocketScreen->>RocketScreen: Show ErrorContent
RocketScreen->>Bloc: Retry tap -> add(load)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #41 +/- ##
==========================================
+ Coverage 83.35% 83.80% +0.44%
==========================================
Files 113 118 +5
Lines 2271 2451 +180
==========================================
+ Hits 1893 2054 +161
- Misses 378 397 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/rockets/widget/rocket_item/rocket_item.dart (1)
41-56: Fix two runtime crash risks (empty images and unsafe substring)
- flickrImages?.first throws on non-null empty lists.
- substring(0, 4) throws if firstFlight length < 4.
- child: Image.network( - rocket.flickrImages?.first ?? '', - fit: BoxFit.cover, - errorBuilder: (context, error, stackTrace) { - return Container( - color: theme.colorScheme.surfaceContainerHighest, - child: Icon( - Icons.rocket_launch, - size: 64, - color: theme.colorScheme.onSurfaceVariant, - ), - ); - }, - ), + child: (rocket.flickrImages != null && + rocket.flickrImages!.isNotEmpty) + ? Image.network( + rocket.flickrImages!.first, + fit: BoxFit.cover, + ) + : Container( + color: theme.colorScheme.surfaceContainerHighest, + child: Icon( + Icons.rocket_launch, + size: 64, + color: theme.colorScheme.onSurfaceVariant, + ), + ), @@ - _InfoChip( - icon: Icons.calendar_today, - label: rocket.firstFlight?.substring(0, 4) ?? '-', - ), + _InfoChip( + icon: Icons.calendar_today, + label: (rocket.firstFlight != null && + rocket.firstFlight!.length >= 4) + ? rocket.firstFlight!.substring(0, 4) + : '-', + ),Also applies to: 105-109
🧹 Nitpick comments (30)
Makefile (1)
1-1: Add screenshot_test to .PHONYAvoids make treating it as a file target.
-.PHONY: gen genAll rebuild check get localize runDev runDevQa runDevStaging runProdRelease \ - release apk lines force_upgrade integration_test +.PHONY: gen genAll rebuild check get localize runDev runDevQa runDevStaging runProdRelease \ + release apk lines force_upgrade integration_test screenshot_testAlso applies to: 66-68
lib/features/rocket/bloc/rocket_event.dart (1)
5-7: Prefer explicit rocketId over a hardcoded defaultDefaulting to 'falcon1' can mask call-site bugs. Make rocketId required and let UI/router supply it.
- const factory RocketEvent.load({ - @Default('falcon1') String rocketId, - }) = RocketLoadEvent; + const factory RocketEvent.load({ + required String rocketId, + }) = RocketLoadEvent;test/features/rockets/widget/rocket_item_test.dart (1)
61-61: Avoid locale-fragile currency assertion.The hardcoded "$50.0M" may break if formatting becomes locale-aware. Prefer deriving the expected via the same formatter used by the widget or assert with a locale-agnostic pattern.
lib/utils/rocket_utils.dart (1)
3-6: Consider locale-aware number formatting (optional).Currently uses toStringAsFixed with a dot; in non-English locales a comma is expected. Add optional locale support without changing defaults.
Apply this diff:
+import 'package:intl/intl.dart'; + extension RocketUtils on num? { /// Convert kg to tons and format as localized string - String toTonsString(String tonsLabel, {int fractionDigits = 0}) { - final valueInTons = (this ?? 0) / 1000; - return '${valueInTons.toStringAsFixed(fractionDigits)} $tonsLabel'; - } + String toTonsString( + String tonsLabel, { + int fractionDigits = 0, + String? locale, + bool trimTrailingZeros = false, + }) { + final valueInTons = (this ?? 0) / 1000; + String formatted; + if (locale == null) { + formatted = valueInTons.toStringAsFixed(fractionDigits); + } else { + final nf = NumberFormat.decimalPattern(locale) + ..minimumFractionDigits = fractionDigits + ..maximumFractionDigits = fractionDigits; + formatted = nf.format(valueInTons); + } + if (trimTrailingZeros && fractionDigits > 0) { + formatted = formatted + .replaceFirst(RegExp(r'([.,]\d*?)0+$'), r'\1') + .replaceFirst(RegExp(r'[.,]$'), ''); + } + return '$formatted $tonsLabel'; + } }test/utils/rocket_utils_test.dart (1)
8-33: Add one locale-format test (optional).If you adopt locale-aware formatting, add a case for a comma-decimal locale (e.g., 'de') to prevent regressions.
lib/l10n/intl_de.arb (1)
217-232: Optional: add @-metadata for new keys.Include descriptions for new entries to aid translators and reviewers.
lib/features/rocket/widget/detail_row_widget.dart (1)
16-29: Potential overflow with long texts.Row + spaceBetween with two Texts can overflow. Consider truncation or Flexible.
Two options:
- Minimal: add overflow handling to both Texts.
Text( label, style: theme.textTheme.bodyMedium?.copyWith( color: theme.colorScheme.onSurfaceVariant, ), + maxLines: 1, + overflow: TextOverflow.ellipsis, ), Text( value, style: theme.textTheme.bodyMedium?.copyWith( fontWeight: FontWeight.w600, ), + maxLines: 1, + overflow: TextOverflow.ellipsis, + textAlign: TextAlign.right, ),
- Stronger: wrap the label with Flexible to allow shrinking if needed.
- Text( + Flexible( + child: Text( label, style: theme.textTheme.bodyMedium?.copyWith( color: theme.colorScheme.onSurfaceVariant, ), - ), + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + ),lib/features/rocket/widget/spec_card_widget.dart (1)
36-47: Add overflow guards for resilience.Prevent layout overflow for long labels/values.
Text( label, style: theme.textTheme.labelSmall?.copyWith( color: theme.colorScheme.onSurfaceVariant, ), + maxLines: 1, + overflow: TextOverflow.ellipsis, ), Text( value, style: theme.textTheme.titleMedium?.copyWith( fontWeight: FontWeight.bold, ), + maxLines: 1, + overflow: TextOverflow.ellipsis, ),test/bloc/utils.dart (1)
51-55: Nit: callback naming.Consider renaming to onContext for brevity; behavior stays the same.
lib/l10n/app_localizations_uk.dart (3)
326-327: Prefer “Вантажопідйомність” for payload capacity.
Avoids confusion with existing “Корисне навантаження” (actual payload).- String get payloadCapacity => 'Корисне навантаження'; + String get payloadCapacity => 'Вантажопідйомність';
329-330: Nit: Consider “Характеристики двигуна”.
Matches the technical tone of “Технічні характеристики”.- String get engineDetails => 'Деталі двигуна'; + String get engineDetails => 'Характеристики двигуна';
359-363: Pluralization for “тонн”.
Ukrainian needs plural forms (one/few/many). Current static “тонн” will be grammatically wrong for some numbers. Recommend migrating this to an Intl.plural key in ARB and exposing an API like tons(n).I can draft ARB entries and the generated getters if you want.
lib/features/rocket/bloc/rocket_bloc.freezed.dart (1)
604-620: Optional: Carry error details in RocketErrorState.
Including an Exception/message enables richer UI/telemetry. Change in source (rocket_bloc.dart), then regenerate.I can propose the source Freezed spec diff if helpful.
test/repository/rocket_repository_test.dart (1)
133-133: Mock external network images in repository tests
Wrap the Flickr‐URL fixture inmockNetworkImagesFor(() => …)(or apply anHttpOverrideshelper) around the test in test/repository/rocket_repository_test.dart at line 133 to avoid real HTTP calls.lib/routes/router.dart (1)
7-7: Ensure RocketScreen consumes Navigator argumentsYou pass a RocketResource as arguments from the list screen, but the builder here returns const RocketScreen() and ignores settings.arguments. Verify RocketScreen reads ModalRoute.of(context)!.settings.arguments (and type-checks it) or thread the argument through the constructor.
Example (if RocketScreen supports constructor args):
- Routes.rocket: (_) => const RocketScreen(), + Routes.rocket: (args) => RocketScreen(rocket: args as RocketResource),At minimum, add a runtime type check and graceful fallback if args are missing/wrong type to avoid crashes.
Also applies to: 15-15, 26-27
integration_test/rocket_screen_screenshot_test.dart (2)
14-16: Use ensureInitialized() to create the binding (single instance)Avoid instantiating the binding directly and then calling ensureInitialized; create it once via ensureInitialized and keep the reference.
- final binding = IntegrationTestWidgetsFlutterBinding(); - IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + final binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized() + as IntegrationTestWidgetsFlutterBinding;
39-45: Wait for settles before asserting/screenshottingUse pumpAndSettle to avoid flakiness due to pending microtasks/animations before expectations and screenshots.
- await tester.pump(); + await tester.pumpAndSettle();lib/features/rockets/rockets_screen.dart (1)
70-75: Verify route argument contract (RocketResource vs ID)You pass the full RocketResource to Routes.rocket. Ensure the route/screen actually consumes the same type; otherwise pass a typed arguments object or just an identifier to avoid heavy object passing and stale data.
If you keep passing the resource, add a runtime check on the receiving side and handle null/mismatch gracefully.
integration_test/rockets_integration_live_test.dart (1)
27-30: Minor: tap the located keyed item for robustnessPrefer tapping the found keyed widget to avoid coupling to list order.
- await tester.tap(find.byType(RocketItemWidget).first); + await tester.tap(firstItem);lib/features/rocket/bloc/rocket_bloc.dart (1)
17-19: Log or surface the exception for diagnosticsSwallowing the exception hampers debugging. Consider logging or attaching a message in the error state (while keeping UI generic).
- } catch (e) { - emit(const RocketState.error()); + } catch (e, s) { + // TODO: hook into logger/Crashlytics + // log('Rocket load failed', error: e, stackTrace: s); + emit(const RocketState.error()); }test/features/rocket/widget/detail_row_widget_test.dart (1)
29-41: Be cautious: overriding onSurfaceVariant in fromSeed across Flutter versionsIf your Flutter channel doesn’t support onSurfaceVariant in ColorScheme.fromSeed’s parameters, use copyWith to avoid compile issues:
- final theme = ThemeData( - colorScheme: ColorScheme.fromSeed( - seedColor: Colors.deepPurple, - onSurfaceVariant: onSurfaceVariant, - ), + final theme = ThemeData( + colorScheme: ColorScheme.fromSeed( + seedColor: Colors.deepPurple, + ).copyWith( + onSurfaceVariant: onSurfaceVariant, ),test/features/rocket/rocket_screen_test.dart (2)
82-94: Also verify retry dispatch on errorAdd a tap on “Try Again” and verify that RocketLoadEvent with the same rocketId is added.
@@ await tester.pumpAndSettle(); expect(find.text('Try Again'), findsOneWidget); + await tester.tap(find.text('Try Again')); + await tester.pump(); + verify(() => rocketBloc.add(const RocketLoadEvent(rocketId: 'falcon1'))).called(1);Tip: if mocktail complains about RocketEvent, register a fallback:
setUpAll(() { registerFallbackValue(const RocketLoadEvent(rocketId: 'falcon1')); });
25-43: Consider exercising a state transition with whenListenInstead of only stubbing state, use whenListen to emit loading → success and assert the UI changes without rebuilding the tree.
test/features/rocket/widget/spec_card_widget_test.dart (1)
37-47: ColorScheme overrides may vary by Flutter versionIf your CI matrix includes older Flutter SDKs,
ColorScheme.fromSeedmight not acceptonSurfaceVariant(orprimary) as named overrides. To be future/older-SDK proof, derive then copyWith.- final theme = ThemeData( - colorScheme: ColorScheme.fromSeed( - seedColor: primaryColor, - primary: primaryColor, - onSurfaceVariant: onSurfaceVariant, - ), - ); + final theme = ThemeData( + colorScheme: ColorScheme.fromSeed(seedColor: primaryColor) + .copyWith(onSurfaceVariant: onSurfaceVariant, primary: primaryColor), + );lib/features/rocket/rocket_screen.dart (2)
44-67: Consider using Freezed’s pattern helpers for clarity
state.whenorstate.mapreduces boilerplate and avoids promotion pitfalls.return state.when( loading: () => const Scaffold(appBar: AppBar(), body: LoadingContent()), success: (rocket) => RocketDetailScreen(rocket: rocket), error: () => Scaffold( appBar: AppBar(), body: ErrorContent(onTryAgainClick: () { context.read<RocketBloc>().add(RocketLoadEvent(rocketId: rocketId)); }), ), );
113-116: SDK compatibility: Color.withValues
withValues(alpha: 0.8)is relatively new. If your lowest supported Flutter version predates it, switch towithOpacity(0.8).lib/l10n/app_localizations_de.dart (1)
357-358: Use “Meereshöhe” for aerospace accuracy.“Sea Level” is typically translated as “Meereshöhe” rather than “Bodenniveau”.
Apply:
- String get thrustSeaLevelLabel => 'Schub (Bodenniveau)'; + String get thrustSeaLevelLabel => 'Schub (Meereshöhe)';lib/l10n/app_localizations_en.dart (1)
360-360: Verify unit standard: “tons” vs “tonnes (t)”.If you display metric values, use “tonnes” (or “t”). Keep “tons” only for US short tons.
Possible change:
- String get tons => 'tons'; + String get tons => 'tonnes';lib/l10n/app_localizations_pt.dart (2)
324-325: Refine “Capacidade de Carga”.More precise is “Capacidade de carga útil”.
Apply:
- String get payloadCapacity => 'Capacidade de Carga'; + String get payloadCapacity => 'Capacidade de carga útil';
348-349: Confirm “Número” vs “Quantidade”.If this is a count (e.g., number of engines), “Quantidade” reads better; keep “Número” for identifiers.
Potential change:
- String get numberLabel => 'Número'; + String get numberLabel => 'Quantidade';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
lib/generated/intl/messages_de.dartis excluded by!**/generated/**lib/generated/intl/messages_en.dartis excluded by!**/generated/**lib/generated/intl/messages_pt.dartis excluded by!**/generated/**lib/generated/intl/messages_uk.dartis excluded by!**/generated/**lib/generated/l10n.dartis excluded by!**/generated/**screenshots/rocket-screenshot.pngis excluded by!**/*.pngscreenshots/settings-screenshot.pngis excluded by!**/*.png
📒 Files selected for processing (35)
.github/workflows/ci.yml(1 hunks)Makefile(1 hunks)integration_test/rocket_screen_screenshot_test.dart(1 hunks)integration_test/rocket_screen_test.dart(1 hunks)integration_test/rockets_integration_live_test.dart(1 hunks)lib/features/main/navigation/destinations.dart(1 hunks)lib/features/rocket/bloc/rocket_bloc.dart(1 hunks)lib/features/rocket/bloc/rocket_bloc.freezed.dart(1 hunks)lib/features/rocket/bloc/rocket_event.dart(1 hunks)lib/features/rocket/bloc/rocket_state.dart(1 hunks)lib/features/rocket/rocket_screen.dart(1 hunks)lib/features/rocket/widget/detail_row_widget.dart(1 hunks)lib/features/rocket/widget/spec_card_widget.dart(1 hunks)lib/features/rockets/rockets_screen.dart(1 hunks)lib/features/rockets/widget/rocket_item/rocket_item.dart(2 hunks)lib/l10n/app_localizations.dart(1 hunks)lib/l10n/app_localizations_de.dart(1 hunks)lib/l10n/app_localizations_en.dart(1 hunks)lib/l10n/app_localizations_pt.dart(1 hunks)lib/l10n/app_localizations_uk.dart(1 hunks)lib/l10n/intl_de.arb(1 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_pt.arb(1 hunks)lib/l10n/intl_uk.arb(1 hunks)lib/routes/router.dart(2 hunks)lib/utils/rocket_utils.dart(1 hunks)test/bloc/utils.dart(2 hunks)test/features/launch/bloc/launch_bloc_test.dart(1 hunks)test/features/rocket/bloc/rocket_bloc_test.dart(1 hunks)test/features/rocket/rocket_screen_test.dart(1 hunks)test/features/rocket/widget/detail_row_widget_test.dart(1 hunks)test/features/rocket/widget/spec_card_widget_test.dart(1 hunks)test/features/rockets/widget/rocket_item_test.dart(2 hunks)test/repository/rocket_repository_test.dart(1 hunks)test/utils/rocket_utils_test.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-11T19:29:38.429Z
Learnt from: ashtanko
PR: ashtanko/flutter_bloc_app_template#30
File: lib/di/app_bloc_providers.dart:21-26
Timestamp: 2025-08-11T19:29:38.429Z
Learning: In the flutter_bloc_app_template project, immediate event dispatch using cascade notation (..add()) in BlocProvider creation is the preferred pattern. The blocs should dispatch initial events immediately upon creation rather than deferring them to widget initialization or post-frame callbacks.
Applied to files:
lib/features/rocket/bloc/rocket_bloc.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: App
- GitHub Check: Flutter Integration Tests (31)
🔇 Additional comments (30)
test/features/launch/bloc/launch_bloc_test.dart (1)
11-11: Rename looks goodGroup name aligns with the bloc type.
lib/features/main/navigation/destinations.dart (1)
20-21: Key rename verification: ensure no stale ‘rockets’ references Confirm all usages (tests, finders, selectors) have been updated to userockets_screenand that nothing still relies on the oldrocketskey..github/workflows/ci.yml (1)
137-139: New integration tests: ensure determinismIf these hit live APIs, they may flake on CI. Consider fixtures/mocks or gating networked tests behind a flag.
lib/l10n/intl_en.arb (1)
353-369: New Rocket strings added — LGTMKeys and formatting look valid JSON and consistent with other locales.
lib/l10n/intl_uk.arb (1)
349-365: UA translations — LGTMTerminology is consistent; pluralization not required here.
lib/features/rockets/widget/rocket_item/rocket_item.dart (1)
14-15: Callback API simplification — LGTMZero-arg onClick and wiring InkWell.onTap to it reads cleanly. Ensure all callers updated.
Also applies to: 33-34
test/features/rockets/widget/rocket_item_test.dart (1)
44-45: LGTM: onClick signature change reflected correctly.No-arg callbacks match RocketItemWidget’s new API.
Also applies to: 76-77
test/utils/rocket_utils_test.dart (1)
8-33: Good coverage and rounding cases.Nulls, large numbers, and varying fractionDigits are well covered.
lib/l10n/intl_de.arb (1)
217-232: German keys/translations look consistent.Keys align with new getters; translation quality is good.
test/bloc/utils.dart (1)
12-18: Nice addition: context capture hook.Enables localized assertions without plumbing context through tests.
lib/l10n/app_localizations.dart (2)
681-769: New getters align with ARB additions.Surface looks consistent across locales.
772-807: All localized getters are present and in sync
Verified thatapp_localizations_{en,de,pt,uk}.dartinclude the required getters.lib/l10n/app_localizations_uk.dart (1)
319-363: LGTM: New UA localization keys added and aligned with base.
Strings are present and wired. No runtime risks spotted.lib/features/rocket/bloc/rocket_bloc.freezed.dart (2)
1-11: Generated file looks consistent.
No manual edits needed; codegen output LGTM.
236-241: No impact from default'falcon1'– explicitrocketIdalways passed
AllRocketLoadEventinvocations in UI and tests specify arocketId(derived from the route argument or test variable), so the default value ('falcon1') is never relied upon. Routes/UI always passrocketId, and no call sites invokeRocketLoadEvent()bare. Tests for the repository also use explicit IDs (including a test that uses'falcon1'). No alignment issues detected.lib/routes/router.dart (1)
33-34: LGTM: added to animated routesIncluding Routes.rocket in _animatedRoutes keeps transitions consistent.
test/features/rocket/bloc/rocket_bloc_test.dart (2)
27-42: LGTM: success path is well coveredState sequence and repository interactions are asserted correctly.
44-58: LGTM: error path is well coveredGood negative case coverage with exact call verification.
integration_test/rockets_integration_live_test.dart (1)
25-33: LGTM: stable E2E nav assertionsFinding by stable keys and verifying title + “Overview” is appropriate for the live flow.
lib/features/rocket/bloc/rocket_bloc.dart (2)
10-21: LGTM: straightforward load → success/error flowHandler emits loading, awaits repository, and maps to success/error cleanly.
11-11: Verify immediate event dispatch at provider creationPer prior project convention, prefer creating the bloc and immediately dispatching RocketLoadEvent via cascade in the provider (..add()). Ensure RocketScreen/DI follows this so the UI doesn’t rely on external triggers.
test/features/rocket/widget/detail_row_widget_test.dart (1)
54-66: LGTM: layout and spacing assertions are crispGood checks on MainAxisAlignment.spaceBetween and vertical padding.
integration_test/rocket_screen_test.dart (1)
53-63: After fix, assertions will actually run per localeOnce the callback is invoked, the localized labels will be asserted as intended across appSupportedLocales.
test/features/rocket/rocket_screen_test.dart (1)
17-23: Solid MockBloc setupStream stub + close teardown look good and keep tests deterministic.
test/features/rocket/widget/spec_card_widget_test.dart (1)
72-77: LGTM: elevation expectation matches widget contractZero-elevation assertion aligns with SpecCardWidget’s design.
lib/l10n/intl_pt.arb (1)
214-219: Remove erroneous JSON syntax suggestion
The ARB file is valid JSON—no missing comma after the metadata block.Likely an incorrect or invalid review comment.
lib/l10n/app_localizations_de.dart (2)
316-360: DE rocket detail keys added — looks good.Keys align with the base interface and existing naming. No API concerns.
348-349: Confirm “Anzahl” vs “Nummer”.If this labels a count (e.g., number of engines), “Anzahl” is correct; if it’s an identifier, prefer “Nummer”.
lib/l10n/app_localizations_en.dart (1)
317-360: EN rocket detail keys added — consistent and clear.Matches base API; capitalization/style is consistent with existing labels.
lib/l10n/app_localizations_pt.dart (1)
317-360: PT rocket detail keys added — translations look solid.Terminology is consistent with aerospace Portuguese.
| Future<void> pumpRocketScreen( | ||
| WidgetTester tester, { | ||
| required Locale locale, | ||
| void Function(BuildContext context)? contextCallback, | ||
| }) async { | ||
| when(() => rocketBloc.state).thenReturn( | ||
| RocketSuccessState( | ||
| rocket: mockRocket, | ||
| ), | ||
| ); | ||
|
|
||
| await tester.pumpLocalizedWidgetWithBloc<RocketBloc>( | ||
| bloc: rocketBloc, | ||
| child: const RocketScreenBlocContent(rocketId: 'falcon9'), | ||
| locale: locale, | ||
| ); | ||
| await tester.pumpAndSettle(); | ||
| } |
There was a problem hiding this comment.
Bug: contextCallback is never invoked (assertions silently skipped)
Call the callback with a BuildContext after pumping to execute the localized assertions.
Future<void> pumpRocketScreen(
WidgetTester tester, {
required Locale locale,
void Function(BuildContext context)? contextCallback,
}) async {
@@
await tester.pumpLocalizedWidgetWithBloc<RocketBloc>(
bloc: rocketBloc,
child: const RocketScreenBlocContent(rocketId: 'falcon9'),
locale: locale,
);
- await tester.pumpAndSettle();
+ await tester.pumpAndSettle();
+ if (contextCallback != null) {
+ final ctx = tester.element(find.byType(RocketScreenBlocContent));
+ contextCallback(ctx);
+ }
}📝 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.
| Future<void> pumpRocketScreen( | |
| WidgetTester tester, { | |
| required Locale locale, | |
| void Function(BuildContext context)? contextCallback, | |
| }) async { | |
| when(() => rocketBloc.state).thenReturn( | |
| RocketSuccessState( | |
| rocket: mockRocket, | |
| ), | |
| ); | |
| await tester.pumpLocalizedWidgetWithBloc<RocketBloc>( | |
| bloc: rocketBloc, | |
| child: const RocketScreenBlocContent(rocketId: 'falcon9'), | |
| locale: locale, | |
| ); | |
| await tester.pumpAndSettle(); | |
| } | |
| Future<void> pumpRocketScreen( | |
| WidgetTester tester, { | |
| required Locale locale, | |
| void Function(BuildContext context)? contextCallback, | |
| }) async { | |
| when(() => rocketBloc.state).thenReturn( | |
| RocketSuccessState( | |
| rocket: mockRocket, | |
| ), | |
| ); | |
| await tester.pumpLocalizedWidgetWithBloc<RocketBloc>( | |
| bloc: rocketBloc, | |
| child: const RocketScreenBlocContent(rocketId: 'falcon9'), | |
| locale: locale, | |
| ); | |
| await tester.pumpAndSettle(); | |
| if (contextCallback != null) { | |
| final ctx = tester.element(find.byType(RocketScreenBlocContent)); | |
| contextCallback(ctx); | |
| } | |
| } |
🤖 Prompt for AI Agents
In integration_test/rocket_screen_test.dart around lines 23 to 40, the provided
contextCallback parameter is never invoked after the widget is pumped so any
localized assertions are skipped; after awaiting tester.pumpAndSettle() retrieve
a BuildContext for the rendered RocketScreen widget (for example via
tester.element on a Finder targeting the RocketScreen content widget) and, if
contextCallback is not null, call contextCallback(context) so the caller's
assertions run against the pumped widget tree.
| @Freezed() | ||
| abstract class RocketState with _$RocketState { | ||
| const factory RocketState.loading() = RocketLoadingState; | ||
|
|
||
| const factory RocketState.success({ | ||
| required RocketResource rocket, | ||
| }) = RocketSuccessState; | ||
|
|
||
| const factory RocketState.error() = RocketErrorState; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Freezed annotation casing causes a build failure
Use @freezed (lowercase) and declare class (not abstract) per Freezed conventions; otherwise code generation fails.
-@Freezed()
-abstract class RocketState with _$RocketState {
+@freezed
+class RocketState with _$RocketState {
const factory RocketState.loading() = RocketLoadingState;
const factory RocketState.success({
required RocketResource rocket,
}) = RocketSuccessState;
const factory RocketState.error() = RocketErrorState;
}Also ensure part 'rocket_bloc.freezed.dart'; is present in rocket_bloc.dart.
🏁 Script executed:
#!/bin/bash
# Verify Freezed annotation casing in RocketState
rg -n "^\s*@Freezed" -C2 lib/features/rocket/bloc/rocket_state.dart || echo "No @Freezed found in rocket_state.dart"
rg -n "^\s*@freezed" -C2 lib/features/rocket/bloc/rocket_state.dart || echo "No @freezed found in rocket_state.dart"
# Verify part directive in rocket_bloc.dart
rg -n "part ['\"]rocket_bloc.freezed.dart['\"]" lib/features/rocket/bloc/rocket_bloc.dart || echo "No part 'rocket_bloc.freezed.dart' directive found in rocket_bloc.dart"Length of output: 231
Use lowercase @freezed and concrete class
-@Freezed()
-abstract class RocketState with _$RocketState {
+@freezed
+class RocketState with _$RocketState {📝 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.
| @Freezed() | |
| abstract class RocketState with _$RocketState { | |
| const factory RocketState.loading() = RocketLoadingState; | |
| const factory RocketState.success({ | |
| required RocketResource rocket, | |
| }) = RocketSuccessState; | |
| const factory RocketState.error() = RocketErrorState; | |
| } | |
| @freezed | |
| class RocketState with _$RocketState { | |
| const factory RocketState.loading() = RocketLoadingState; | |
| const factory RocketState.success({ | |
| required RocketResource rocket, | |
| }) = RocketSuccessState; | |
| const factory RocketState.error() = RocketErrorState; | |
| } |
🤖 Prompt for AI Agents
In lib/features/rocket/bloc/rocket_state.dart around lines 3 to 12, replace the
incorrect @Freezed() annotation and abstract class declaration: change
@Freezed() to the lowercase @freezed (no parentheses) and make the class
concrete by using "class RocketState with _$RocketState" instead of "abstract
class RocketState with _$RocketState", keeping the existing factory constructors
unchanged.
| final rocket = ModalRoute.of(context)?.settings.arguments as RocketResource; | ||
| final rocketId = rocket.rocketId ?? 'falcon1'; | ||
| return BlocProvider( |
There was a problem hiding this comment.
Null-aware chaining bug on route arguments (compile-time error)
ModalRoute.of(context)?.settings yields a nullable RouteSettings; accessing .arguments without ?. breaks NNBD. Fix by casting to a nullable RocketResource and using a safe fallback.
- final rocket = ModalRoute.of(context)?.settings.arguments as RocketResource;
- final rocketId = rocket.rocketId ?? 'falcon1';
+ final rocket = ModalRoute.of(context)?.settings.arguments as RocketResource?;
+ final rocketId = rocket?.rocketId ?? 'falcon1';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/features/rocket/rocket_screen.dart around lines 19 to 21, the code
force-casts ModalRoute.of(context)?.settings.arguments to RocketResource causing
a NNBD compile error when settings is null; change the cast to a nullable type
(cast to RocketResource?) and then use null-aware access for rocketId (e.g.,
rocket?.rocketId ?? 'falcon1') so you safely handle a missing route/arguments
and provide the 'falcon1' fallback.
| label: l10n.heightLabel, | ||
| value: '${rocket.height?.meters} m', | ||
| icon: Icons.height, | ||
| ), | ||
| SpecCardWidget( | ||
| label: l10n.diameterLabel, | ||
| value: '${rocket.diameter?.meters} m', | ||
| icon: Icons.circle_outlined, | ||
| ), | ||
| SpecCardWidget( | ||
| label: l10n.massLabel, | ||
| value: (rocket.mass?.kg) | ||
| .toTonsString(l10n.tons, fractionDigits: 0), | ||
| icon: Icons.fitness_center, | ||
| ), | ||
| SpecCardWidget( | ||
| label: l10n.stagesLabel, | ||
| value: '${rocket.stages}', | ||
| icon: Icons.layers, | ||
| ), | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid “null m”/“null” in specs; show N/A when data is missing
Guard nulls and keep units readable.
- SpecCardWidget(
- label: l10n.heightLabel,
- value: '${rocket.height?.meters} m',
- icon: Icons.height,
- ),
+ SpecCardWidget(
+ label: l10n.heightLabel,
+ value: (rocket.height?.meters != null)
+ ? '${rocket.height!.meters} m'
+ : l10n.notAvailable,
+ icon: Icons.height,
+ ),
@@
- SpecCardWidget(
- label: l10n.diameterLabel,
- value: '${rocket.diameter?.meters} m',
- icon: Icons.circle_outlined,
- ),
+ SpecCardWidget(
+ label: l10n.diameterLabel,
+ value: (rocket.diameter?.meters != null)
+ ? '${rocket.diameter!.meters} m'
+ : l10n.notAvailable,
+ icon: Icons.circle_outlined,
+ ),
@@
- SpecCardWidget(
- label: l10n.massLabel,
- value: (rocket.mass?.kg)
- .toTonsString(l10n.tons, fractionDigits: 0),
- icon: Icons.fitness_center,
- ),
+ SpecCardWidget(
+ label: l10n.massLabel,
+ value: (rocket.mass?.kg)
+ ?.toTonsString(l10n.tons, fractionDigits: 0) ??
+ l10n.notAvailable,
+ icon: Icons.fitness_center,
+ ),
@@
- SpecCardWidget(
- label: l10n.stagesLabel,
- value: '${rocket.stages}',
- icon: Icons.layers,
- ),
+ SpecCardWidget(
+ label: l10n.stagesLabel,
+ value: rocket.stages?.toString() ?? l10n.notAvailable,
+ icon: Icons.layers,
+ ),📝 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.
| label: l10n.heightLabel, | |
| value: '${rocket.height?.meters} m', | |
| icon: Icons.height, | |
| ), | |
| SpecCardWidget( | |
| label: l10n.diameterLabel, | |
| value: '${rocket.diameter?.meters} m', | |
| icon: Icons.circle_outlined, | |
| ), | |
| SpecCardWidget( | |
| label: l10n.massLabel, | |
| value: (rocket.mass?.kg) | |
| .toTonsString(l10n.tons, fractionDigits: 0), | |
| icon: Icons.fitness_center, | |
| ), | |
| SpecCardWidget( | |
| label: l10n.stagesLabel, | |
| value: '${rocket.stages}', | |
| icon: Icons.layers, | |
| ), | |
| ], | |
| SpecCardWidget( | |
| label: l10n.heightLabel, | |
| value: (rocket.height?.meters != null) | |
| ? '${rocket.height!.meters} m' | |
| : l10n.notAvailable, | |
| icon: Icons.height, | |
| ), | |
| SpecCardWidget( | |
| label: l10n.diameterLabel, | |
| value: (rocket.diameter?.meters != null) | |
| ? '${rocket.diameter!.meters} m' | |
| : l10n.notAvailable, | |
| icon: Icons.circle_outlined, | |
| ), | |
| SpecCardWidget( | |
| label: l10n.massLabel, | |
| value: (rocket.mass?.kg | |
| ?.toTonsString(l10n.tons, fractionDigits: 0)) | |
| ?? l10n.notAvailable, | |
| icon: Icons.fitness_center, | |
| ), | |
| SpecCardWidget( | |
| label: l10n.stagesLabel, | |
| value: rocket.stages?.toString() ?? l10n.notAvailable, | |
| icon: Icons.layers, | |
| ), |
🤖 Prompt for AI Agents
In lib/features/rocket/rocket_screen.dart around lines 181 to 201, the
SpecCardWidget value expressions can produce "null m" or "null" when rocket
fields are missing; update each value to guard against nulls and display a
localized "N/A" (e.g. l10n.notAvailable or fallback "N/A") while keeping units
when a numeric value exists — for example, check rocket.height?.meters and
rocket.diameter?.meters and render '${value} m' only if value != null else
l10n.notAvailable, format mass similarly using mass?.kg ? mass.toTonsString(...)
: l10n.notAvailable, and show stages as '${rocket.stages}' only if rocket.stages
!= null else l10n.notAvailable.
| DetailRowWidget( | ||
| label: 'Type', | ||
| value: | ||
| rocket.engines?.type.toString().toUpperCase() ?? | ||
| 'N/A', | ||
| ), | ||
| DetailRowWidget( | ||
| label: l10n.typeLabel, | ||
| value: | ||
| rocket.engines?.type.toString().toUpperCase() ?? | ||
| 'N/A', | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate “Type” row in Engine Details
You render engine “Type” twice (one hardcoded, one localized). Remove the hardcoded row to avoid duplication.
- DetailRowWidget(
- label: 'Type',
- value:
- rocket.engines?.type.toString().toUpperCase() ??
- 'N/A',
- ),🤖 Prompt for AI Agents
In lib/features/rocket/rocket_screen.dart around lines 256 to 267, there are two
DetailRowWidget entries rendering the engine "Type" twice (one hardcoded label
'Type' and one using l10n.typeLabel); remove the hardcoded DetailRowWidget (the
one with label: 'Type') so only the localized l10n.typeLabel row remains,
ensuring value logic stays the same
(rocket.engines?.type.toString().toUpperCase() ?? 'N/A').
| label: l10n.thrustSeaLevelLabel, | ||
| value: '${rocket.engines?.thrustSeaLevel?.kN} kN', | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle null thrust gracefully
Prevent “null kN”; show N/A if thrust is absent.
- DetailRowWidget(
- label: l10n.thrustSeaLevelLabel,
- value: '${rocket.engines?.thrustSeaLevel?.kN} kN',
- ),
+ DetailRowWidget(
+ label: l10n.thrustSeaLevelLabel,
+ value: (rocket.engines?.thrustSeaLevel?.kN != null)
+ ? '${rocket.engines!.thrustSeaLevel!.kN} kN'
+ : l10n.notAvailable,
+ ),📝 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.
| label: l10n.thrustSeaLevelLabel, | |
| value: '${rocket.engines?.thrustSeaLevel?.kN} kN', | |
| ), | |
| DetailRowWidget( | |
| label: l10n.thrustSeaLevelLabel, | |
| value: (rocket.engines?.thrustSeaLevel?.kN != null) | |
| ? '${rocket.engines!.thrustSeaLevel!.kN} kN' | |
| : l10n.notAvailable, | |
| ), |
🤖 Prompt for AI Agents
In lib/features/rocket/rocket_screen.dart around lines 285-287, the label
currently interpolates thrustSeaLevel directly which can produce "null kN";
update the value construction to check for null and display a fallback like
"N/A" when rocket.engines?.thrustSeaLevel?.kN is null — otherwise format and
append " kN". Ensure the null check uses the existing null-aware accessors and
returns the fallback string so the UI never shows "null kN".
| String get stagesLabel => 'Стадії'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use “Ступені” (rocket stages) instead of “Стадії”.
Domain-accurate term and consistent with “Перша ступінь”.
Apply:
- String get stagesLabel => 'Стадії';
+ String get stagesLabel => 'Ступені';📝 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.
| String get stagesLabel => 'Стадії'; | |
| String get stagesLabel => 'Ступені'; |
🤖 Prompt for AI Agents
In lib/l10n/app_localizations_uk.dart around lines 341-342, the Ukrainian
translation uses "Стадії" for stages; replace that string with the
domain-accurate term "Ступені" so it matches terminology like "Перша ступінь"
and keeps translations consistent across the app.
1 participant
Summary by CodeRabbit
New Features
Tests
Chores