refactor(fitness-start): remove guest tests flow from onboarding - #66
Merged
Merged
Conversation
CowboyGH
added
type: refactor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis pull request removes the guest fitness-start testing flow end-to-end: tests catalog and attempt pages/builders, guest test API methods, guest test repository implementation and guest-specific domain interfaces, and guest session cookie storage. DI registrations were updated to drop cookie-jar and GuestSessionStorage wiring and to register a single TestAttemptRepositoryImpl. Router paths and fitness-start UI were simplified so the quiz collects fitness parameters and completes via AuthSessionCubit without an intermediate guest test step. Strings, README, CHANGELOG, and unit tests were adjusted accordingly. Sequence Diagram(s)sequenceDiagram
participant FitnessStartQuizPage
participant AuthSessionCubit
participant TestAttemptRepository
participant SignUpRoute
FitnessStartQuizPage->>AuthSessionCubit: submit collected fitness parameters
AuthSessionCubit->>TestAttemptRepository: (no guest attempt) -- repository registered for authenticated attempts
AuthSessionCubit->>SignUpRoute: trigger navigation to sign-up (completeGuestFitnessStart)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
101-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate
features/testsdescription to remove guest-flow wording.The project-structure block still documents a “guest test attempt flow,” which conflicts with this PR’s stated removal of guest testing artifacts. Please update this line to authenticated-only wording to keep docs accurate.
Suggested README diff
-│ ├── tests/ # Shared tests catalog and guest test attempt flow +│ ├── tests/ # Authenticated tests catalog and test attempt flow🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 101 - 105, Update the README entry for the tests/ directory: change the description that currently reads "Shared tests catalog and guest test attempt flow" to reflect authenticated-only behavior (e.g., "Shared tests catalog and authenticated-only test attempt flow" or "Shared tests catalog and authenticated test attempt flow") so the docs match the PR's removal of guest testing artifacts; edit the line describing tests/ in README.md accordingly.
🧹 Nitpick comments (1)
lib/features/tests/attempt/data/repositories/test_attempt_repository_impl.dart (1)
17-23: ⚡ Quick winUpdate stale repository docs after the type rename.
The comments still reference
AuthenticatedTestAttemptRepositoryand “authenticated” payload wording, but this class is now the unifiedTestAttemptRepositoryimplementation. Please align these doc/error strings to avoid stale references.✏️ Suggested update
-/// Authenticated implementation of [AuthenticatedTestAttemptRepository]. +/// Implementation of [TestAttemptRepository]. final class TestAttemptRepositoryImpl implements TestAttemptRepository { - /// Logger for tracking authenticated test attempt operations. + /// Logger for tracking test attempt operations. final AppLogger _logger; - /// API client for tests catalog and attempts. + /// API client for tests catalog and attempts. final TestsApiClient _apiClient; @@ - final exception = StateError('Malformed authenticated test result payload.'); + final exception = StateError('Malformed test result payload.');Also applies to: 56-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/tests/attempt/data/repositories/test_attempt_repository_impl.dart` around lines 17 - 23, Update stale documentation and error messages that still reference AuthenticatedTestAttemptRepository and “authenticated” wording to reflect the unified TestAttemptRepository API: change the class doc comment above TestAttemptRepositoryImpl to describe it as the concrete implementation of TestAttemptRepository (remove “Authenticated…”), and update any inline comments or error/log strings in TestAttemptRepositoryImpl (and other occurrences around the class, e.g., where “authenticated” payload or repository is mentioned) to use the unified naming and neutral wording; ensure all references now mention TestAttemptRepository and non-authenticated/neutral payload descriptions so docs and logs stay accurate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@README.md`:
- Around line 101-105: Update the README entry for the tests/ directory: change
the description that currently reads "Shared tests catalog and guest test
attempt flow" to reflect authenticated-only behavior (e.g., "Shared tests
catalog and authenticated-only test attempt flow" or "Shared tests catalog and
authenticated test attempt flow") so the docs match the PR's removal of guest
testing artifacts; edit the line describing tests/ in README.md accordingly.
---
Nitpick comments:
In
`@lib/features/tests/attempt/data/repositories/test_attempt_repository_impl.dart`:
- Around line 17-23: Update stale documentation and error messages that still
reference AuthenticatedTestAttemptRepository and “authenticated” wording to
reflect the unified TestAttemptRepository API: change the class doc comment
above TestAttemptRepositoryImpl to describe it as the concrete implementation of
TestAttemptRepository (remove “Authenticated…”), and update any inline comments
or error/log strings in TestAttemptRepositoryImpl (and other occurrences around
the class, e.g., where “authenticated” payload or repository is mentioned) to
use the unified naming and neutral wording; ensure all references now mention
TestAttemptRepository and non-authenticated/neutral payload descriptions so docs
and logs stay accurate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8b0a26c0-b56e-4c04-94be-111040f57ec3
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
CHANGELOG.mdREADME.mdlib/core/constants/app_strings.dartlib/core/di/di.dartlib/core/network/api_paths.dartlib/core/network/dio_setup.dartlib/core/router/router.dartlib/core/router/router_paths.dartlib/core/services/guest_session_storage/cookie_jar_guest_session_storage.dartlib/core/services/guest_session_storage/guest_session_storage.dartlib/features/auth/presentation/cubits/auth_session_cubit.dartlib/features/fitness_start/presentation/pages/fitness_start_quiz_page.dartlib/features/fitness_start/presentation/pages/fitness_start_test_attempt_page.dartlib/features/fitness_start/presentation/pages/fitness_start_test_attempt_page_builder.dartlib/features/fitness_start/presentation/pages/fitness_start_tests_page.dartlib/features/fitness_start/presentation/pages/fitness_start_tests_page_builder.dartlib/features/tests/attempt/data/repositories/guest_test_attempt_repository_impl.dartlib/features/tests/attempt/data/repositories/test_attempt_repository_impl.dartlib/features/tests/attempt/domain/repositories/test_attempt_repository.dartlib/features/tests/attempt/presentation/pages/tests_attempt_page_builder.dartlib/features/tests/data/remote/tests_api_client.dartpubspec.yamltest/features/auth/presentation/cubits/auth_session_cubit_test.darttest/features/tests/attempt/data/repositories/guest_test_attempt_repository_impl_test.darttest/features/tests/attempt/data/repositories/test_attempt_repository_impl_test.dart
💤 Files with no reviewable changes (15)
- pubspec.yaml
- lib/features/fitness_start/presentation/pages/fitness_start_tests_page_builder.dart
- lib/core/services/guest_session_storage/guest_session_storage.dart
- lib/features/tests/attempt/data/repositories/guest_test_attempt_repository_impl.dart
- lib/features/fitness_start/presentation/pages/fitness_start_test_attempt_page_builder.dart
- lib/core/router/router_paths.dart
- test/features/tests/attempt/data/repositories/guest_test_attempt_repository_impl_test.dart
- lib/features/fitness_start/presentation/pages/fitness_start_test_attempt_page.dart
- lib/features/fitness_start/presentation/pages/fitness_start_tests_page.dart
- lib/core/services/guest_session_storage/cookie_jar_guest_session_storage.dart
- lib/core/router/router.dart
- lib/features/tests/data/remote/tests_api_client.dart
- lib/core/network/dio_setup.dart
- lib/features/tests/attempt/domain/repositories/test_attempt_repository.dart
- lib/core/network/api_paths.dart
CowboyGH
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Summary
Moved onboarding tests to authenticated users only. Simplified guest fitness start flow by removing test catalog and test attempts, making testing available as a dedicated tab post-authentication.