Modularize preferences#4601
Conversation
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Failed
(1 new file with code health below 10.00)
Our agent can fix these. Install it.
Gates Passed
Reason for failure
| New code is healthy | Violations | Code Health Impact | |
|---|---|---|---|
| preference_option.dart | 1 rule | 9.69 | Suppress |
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
| final session = context.session; | ||
| final accountId = context.accountId; | ||
| final current = context.serverOptions; | ||
| if (session == null || accountId == null || current == null) { |
There was a problem hiding this comment.
❌ New issue: Complex Conditional
ServerPreferenceOption.toggle has 1 complex conditionals with 2 branches, threshold = 2
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@lib/features/manage_account/presentation/preferences/model/preference_option.dart`:
- Around line 107-110: The null-check block currently lumps all missing values
into NotFoundAccountIdException; update the logic in preference_option.dart to
check each nullable (session, accountId, current) separately and return a
Stream.value(Left(UpdateServerSettingFailure(...))) with the specific exception
for the missing value (e.g., NotFoundSessionException when session is null,
NotFoundAccountIdException when accountId is null,
NotFoundServerOptionsException or similar when current is null), ensuring the
returned exception type matches the failing validation case and keeping the
existing UpdateServerSettingFailure and Stream.value/Left wrapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e131f433-75aa-46a5-b4d3-6d84f8d8601d
📒 Files selected for processing (10)
lib/features/manage_account/presentation/model/preferences_option_type.dartlib/features/manage_account/presentation/preferences/bindings/preferences_bindings.dartlib/features/manage_account/presentation/preferences/model/preference_option.dartlib/features/manage_account/presentation/preferences/model/preference_option_registry.dartlib/features/manage_account/presentation/preferences/model/preference_options.dartlib/features/manage_account/presentation/preferences/preferences_controller.dartlib/features/manage_account/presentation/preferences/preferences_view.dartlib/features/manage_account/presentation/preferences/widgets/preferences_option_item.darttest/features/manage_account/presentation/model/preferences_option_type_test.darttest/features/manage_account/presentation/preferences/preference_option_registry_test.dart
💤 Files with no reviewable changes (2)
- test/features/manage_account/presentation/model/preferences_option_type_test.dart
- lib/features/manage_account/presentation/model/preferences_option_type.dart
| if (session == null || accountId == null || current == null) { | ||
| return Stream.value( | ||
| Left(UpdateServerSettingFailure(NotFoundAccountIdException())), | ||
| ); |
There was a problem hiding this comment.
Exception name doesn't match all validation cases.
The condition validates three nullable fields (session, accountId, current), but always returns NotFoundAccountIdException() regardless of which is actually null. This makes debugging harder when the real issue is a missing session or server options.
🔍 Suggested fix with specific exceptions
- if (session == null || accountId == null || current == null) {
- return Stream.value(
- Left(UpdateServerSettingFailure(NotFoundAccountIdException())),
- );
- }
+ if (session == null) {
+ return Stream.value(
+ Left(UpdateServerSettingFailure(NotFoundSessionException())),
+ );
+ }
+ if (accountId == null) {
+ return Stream.value(
+ Left(UpdateServerSettingFailure(NotFoundAccountIdException())),
+ );
+ }
+ if (current == null) {
+ return Stream.value(
+ Left(UpdateServerSettingFailure(NotFoundServerSettingsException())),
+ );
+ }🤖 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/manage_account/presentation/preferences/model/preference_option.dart`
around lines 107 - 110, The null-check block currently lumps all missing values
into NotFoundAccountIdException; update the logic in preference_option.dart to
check each nullable (session, accountId, current) separately and return a
Stream.value(Left(UpdateServerSettingFailure(...))) with the specific exception
for the missing value (e.g., NotFoundSessionException when session is null,
NotFoundAccountIdException when accountId is null,
NotFoundServerOptionsException or similar when current is null), ensuring the
returned exception type matches the failing validation case and keeping the
existing UpdateServerSettingFailure and Stream.value/Left wrapping.
|
LGTM but @tddang-linagora should we have E2E test to ensure the preference options are displayed well with our configuration |
3 participants
Summary by CodeRabbit