Skip to content

[NOW-689][NOW-690][NOW-691]#475

Merged
AustinChangLinksys merged 2 commits intodev-1.2.8from
peter/now-689
Oct 28, 2025
Merged

[NOW-689][NOW-690][NOW-691]#475
AustinChangLinksys merged 2 commits intodev-1.2.8from
peter/now-689

Conversation

@PeterJhongLinksys
Copy link
Collaborator

@PeterJhongLinksys PeterJhongLinksys commented Oct 27, 2025

User description

  1. NOW-689: Incredible-WiFi: The save button should be disabled when the password field is empty

  2. NOW-690: Incredible-WiFi: After switching the setting mode, the settings in Quick Setup mode are not retained

  3. NOW-691: Incredible-WiFi: Cannot save WPA3 security settings in Quick Setup mode after toggling Quick Setup off/on with modified settings

  • Refactor SimpleModeView to be a stateless widget, with its state now managed by _SimpleModeViewState for improved simplicity and performance.
    • Improve the logic for determining available security types in simple mode by using a priority order for selection, enhancing clarity and user experience.
    • Centralize simple mode WiFi settings within the wifiListProvider to improve state management and remove the need for local text editing controllers in WiFiListView.
    • Update the change detection and data verification logic to accurately handle simple mode, ensuring that changes are tracked correctly and data is validated before
      saving.

PR Type

Bug fix, Enhancement


Description

  • Refactor SimpleModeView to stateless widget with state management in _SimpleModeViewState

  • Centralize simple mode WiFi settings in wifiListProvider for improved state management

  • Implement priority-based security type selection for simple mode WiFi

  • Fix save button validation and state change detection for simple mode

  • Add comprehensive unit tests for new security type selection methods


Diagram Walkthrough

flowchart LR
  A["WiFi List Provider"] -->|manages simple mode state| B["SimpleModeView"]
  A -->|calculates available security types| C["Security Type Selection"]
  C -->|priority order| D["WPA3 → WPA2/3 Mixed → WPA2"]
  B -->|updates via callbacks| A
  E["WiFi List View"] -->|detects state changes| F["Save Button Validation"]
  F -->|validates password| G["Data Verification"]
Loading

File Walkthrough

Relevant files
Enhancement
wifi_list_provider.dart
Centralize simple mode security type logic in provider     

lib/page/wifi_settings/providers/wifi_list_provider.dart

  • Renamed _checkMode() to _setupSimpleMode() for clarity
  • Added getSimpleModeAvailableSecurityTypeList() to calculate
    intersection of security types across all WiFi networks
  • Added getSimpleModeAvailableSecurityType() with priority-based
    selection (WPA3 → WPA2/3 Mixed → WPA2)
  • Updated simple mode WiFi state to include available security types and
    properly selected security type
  • Both new methods marked with @visibleForTesting annotation for unit
    testing
+54/-8   
wifi_list_simple_mode_view.dart
Refactor SimpleModeView to stateless widget architecture 

lib/page/wifi_settings/views/wifi_list_simple_mode_view.dart

  • Converted SimpleModeView from stateful to stateless widget with state
    management in _SimpleModeViewState
  • Removed required parameters for text controllers and security type
    from widget constructor
  • Added local _passwordController in _SimpleModeViewState with proper
    initialization and disposal
  • Updated password field to sync with security type changes (clear for
    open networks)
  • Changed callback parameters from required to optional with
    null-coalescing calls
  • Simplified security type card to use simpleModeWifi data directly
    instead of calculating available types
+96/-80 
Bug fix
wifi_list_view.dart
Remove local state management and improve change detection

lib/page/wifi_settings/views/wifi_list_view.dart

  • Removed local text editing controllers for simple mode WiFi name and
    password
  • Removed _initSimpleModeSettings() method as state is now managed by
    provider
  • Simplified quick setup switch logic by removing manual state
    initialization
  • Added _stateHasChanged() method to properly detect changes based on
    simple/advanced mode
  • Updated _dataVerify() to validate password based on current mode
    (simple vs advanced)
  • Fixed change detection to only check mainWiFi changes in advanced mode
    and simpleModeWifi changes in simple mode
+40/-52 
Tests
wifi_list_provider_test.dart
Add unit tests for security type selection methods             

test/page/wifi_settings/providers/wifi_list_provider_test.dart

  • Added comprehensive unit tests for
    getSimpleModeAvailableSecurityTypeList() method
  • Added comprehensive unit tests for
    getSimpleModeAvailableSecurityType() method
  • Created TestableWifiListNotifier class to enable testing of provider
    methods
  • Tests cover intersection calculation, empty lists, single items, and
    priority-based selection
  • Tests verify priority order: WPA3 → WPA2/3 Mixed → WPA2 → first
    available
+164/-0 

… using quick setup WiFi with WPA2 security mode

Refactor: Extract logic for simple mode WiFi security

Refactored the `_checkMode` method into `_setupSimpleMode` to improve clarity and separation of concerns.

Extracted the logic for determining the available security types in simple mode into a new method, `getSimpleModeAvailableSecurityTypeList`. This method now calculates the intersection of available security types across all main WiFi networks.

A new method, `getSimpleModeAvailableSecurityType`, has been added to determine the default security type for simple mode. It prioritizes WPA3, then WPA2/WPA3 Mixed, and then WPA2, if available.

Added unit tests for the new methods to ensure their correctness.
1. [NOW-689]: Incredible-WiFi: The save button should be disabled when the password field is empty

2. [NOW-690]: Incredible-WiFi: After switching the setting mode, the settings in Quick Setup mode are not retained

3. [NOW-691]: Incredible-WiFi: Cannot save WPA3 security settings in Quick Setup mode after toggling Quick Setup off/on with modified settings

- Refactor SimpleModeView to be a stateless widget, with its state now managed by _SimpleModeViewState for improved simplicity and performance.
   - Improve the logic for determining available security types in simple mode by using a priority order for selection, enhancing clarity and user experience.
   - Centralize simple mode WiFi settings within the wifiListProvider to improve state management and remove the need for local text editing controllers in WiFiListView.
   - Update the change detection and data verification logic to accurately handle simple mode, ensuring that changes are tracked correctly and data is validated before
     saving.
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

🔴
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Sensitive Data Logged: The debug log prints the entire WiFi state via state.toJson(), which may include SSIDs and
passwords (e.g., simpleModeWifi.password), risking exposure of sensitive data in logs.

Referred Code
state = state.copyWith(
  mainWiFi: wifiItems,
  guestWiFi: guestWiFi,
);
_setupSimpleMode();
logger.d('[State]:[wiFiList]: ${state.toJson()}');
return state;
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Auditing: The new simple mode setup and change-detection flows do not add any audit logging for
critical user actions (e.g., toggling quick setup, changing SSID/security/password),
making it unclear if these actions are recorded for audit trails.

Referred Code
children: [
  _wifiDescription(wifiBands),
  const AppGap.medium(),
  _quickSetupSwitch(state.isSimpleMode),
  const AppGap.medium(),
  Expanded(
    child: state.isSimpleMode
        ? SimpleModeView(
            onWifiNameEdited: (value) {
              setState(() {
                final wifiItem =
                    state.simpleModeWifi.copyWith(ssid: value);
                ref
                    .read(wifiListProvider.notifier)
                    .setSimpleModeWifi(wifiItem);
              });
            },
            onWifiPasswordEdited: (value) {
              setState(() {
                final wifiItem =
                    state.simpleModeWifi.copyWith(password: value);


 ... (clipped 208 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Edge Cases Unhandled: Methods like getSimpleModeAvailableSecurityTypeList and _setupSimpleMode do not handle
null/empty intersections beyond returning empty lists, which may lead to inconsistent UI
states without explicit error handling or user feedback.

Referred Code
void _setupSimpleMode() {
  final availableSecurityTypeList =
      getSimpleModeAvailableSecurityTypeList(state.mainWiFi);
  final firstEnabledWifi =
      state.mainWiFi.firstWhereOrNull((e) => e.isEnabled) ??
          state.mainWiFi.first;

  final isSimple = state.mainWiFi.every((wifi) =>
      wifi.isEnabled == firstEnabledWifi.isEnabled &&
      wifi.ssid == firstEnabledWifi.ssid &&
      wifi.password == firstEnabledWifi.password &&
      wifi.securityType == firstEnabledWifi.securityType);
  final simpleModeWifi = firstEnabledWifi.copyWith(
    securityType: getSimpleModeAvailableSecurityType(
        firstEnabledWifi.securityType, availableSecurityTypeList),
    availableSecurityTypes: availableSecurityTypeList,
  );
  state =
      state.copyWith(isSimpleMode: isSimple, simpleModeWifi: simpleModeWifi);
}



 ... (clipped 46 lines)
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Password Validation: While _dataVerify prevents empty passwords for secured modes, there is no visible
validation for password strength/length or sanitization when editing SSID/password, which
may be required for security compliance.

Referred Code
bool _dataVerify(WiFiState state) {
  // Password verify
  if (state.isSimpleMode) {
    final emptyPassword = !state.simpleModeWifi.securityType.isOpenVariant &&
        state.simpleModeWifi.password.isEmpty;
    return !emptyPassword;
  } else {
    final hasEmptyPassword = state.mainWiFi
        .any((e) => !e.securityType.isOpenVariant && e.password.isEmpty);
    return !hasEmptyPassword;
  }
}
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Oct 27, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Refactor state management in SimpleModeView

The local _passwordController in SimpleModeView is not synchronized with the
wifiListProvider state, potentially showing stale data. Update the controller's
text within the build method to reflect the current application state.

Examples:

lib/page/wifi_settings/views/wifi_list_simple_mode_view.dart [35-54]
  final TextEditingController _passwordController = TextEditingController();

  @override
  void initState() {
    super.initState();

    final simpleWifi = ref.read(wifiListProvider).simpleModeWifi;
    _passwordController.text =
        simpleWifi.securityType.isOpenVariant ? '' : simpleWifi.password;
  }

 ... (clipped 10 lines)

Solution Walkthrough:

Before:

class _SimpleModeViewState extends ConsumerState<SimpleModeView> {
  final TextEditingController _passwordController = TextEditingController();

  @override
  void initState() {
    super.initState();
    // Controller is initialized only once from the provider.
    final simpleWifi = ref.read(wifiListProvider).simpleModeWifi;
    _passwordController.text = simpleWifi.password;
  }

  @override
  Widget build(BuildContext context) {
    // The build method uses the latest state from the provider...
    final simpleWifi = ref.watch(wifiListProvider).simpleModeWifi;
    // ...but the password field uses the local controller, which is not
    // updated if the provider state changes from an external source.
    return _simpleWiFiPasswordCard(simpleWifi.password, ...);
  }
}

After:

class _SimpleModeViewState extends ConsumerState<SimpleModeView> {
  final TextEditingController _passwordController = TextEditingController();

  @override
  void initState() {
    super.initState();
    // ...
  }

  @override
  Widget build(BuildContext context) {
    final simpleWifi = ref.watch(wifiListProvider).simpleModeWifi;

    // Synchronize the local controller with the provider state on every build.
    final newPassword = simpleWifi.securityType.isOpenVariant ? '' : simpleWifi.password;
    if (_passwordController.text != newPassword) {
      _passwordController.text = newPassword;
    }

    return _simpleWiFiPasswordCard(simpleWifi.password, ...);
  }
}
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a state synchronization bug where the local _passwordController can become out of sync with the central wifiListProvider state, undermining the PR's goal of centralized state management.

Medium
Possible issue
Avoid modifying controller state in build

In _simpleWiFiPasswordCard, remove the direct modification of
_passwordController.text to prevent potential runtime errors and adhere to
Flutter best practices, as this logic is already handled elsewhere.

lib/page/wifi_settings/views/wifi_list_simple_mode_view.dart [139-196]

 Widget _simpleWiFiPasswordCard(
     String password, WifiSecurityType securityType) {
-  if (securityType.isOpenVariant) {
-    _passwordController.text = '';
-  }
   return Opacity(
     opacity: securityType.isOpenVariant ? .5 : 1,
     child: IgnorePointer(
       ignoring: securityType.isOpenVariant ? true : false,
       child: AppListCard(
         showBorder: false,
         padding: const EdgeInsets.symmetric(vertical: 8.0),
         title: AppText.bodyMedium(
           loc(context).wifiPassword,
           color: !securityType.isOpenVariant && password.isEmpty
               ? Theme.of(context).colorScheme.error
               : Theme.of(context).colorScheme.onSurface,
         ),
         description: IntrinsicWidth(
           child: Theme(
             data: Theme.of(context).copyWith(
                 inputDecorationTheme: const InputDecorationTheme(
                     isDense: true, contentPadding: EdgeInsets.zero)),
             child: securityType.isOpenVariant
                 ? Semantics(
                     textField: false,
                     explicitChildNodes: true,
                     child: AppTextField(
                       readOnly: true,
                       border: InputBorder.none,
                       controller: _passwordController,
                     ),
                   )
                 : Semantics(
                     textField: false,
                     explicitChildNodes: true,
                     child: AppPasswordField(
                       readOnly: true,
                       border: InputBorder.none,
                       controller: _passwordController,
                       suffixIconConstraints: const BoxConstraints(),
                     ),
                   ),
           ),
         ),
         trailing: const Icon(LinksysIcons.edit),
         onTap: () {
           showWifiPasswordModal(password, (value) {
             widget.onWifiPasswordEdited?.call(value);
             setState(() {
               _passwordController.text = value;
             });
           });
         },
       ),
     ),
   );
 }

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a Flutter anti-pattern of modifying a controller within a build method, which can cause runtime issues, and the proposed fix is valid.

Medium
  • More

Comment on lines +140 to +144
String password, WifiSecurityType securityType) {
if (securityType.isOpenVariant) {
_passwordController.text = '';
}
return Opacity(

Choose a reason for hiding this comment

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

Suggestion: Avoid modifying controller state in build

Suggested change
String password, WifiSecurityType securityType) {
if (securityType.isOpenVariant) {
_passwordController.text = '';
}
return Opacity(
String password, WifiSecurityType securityType) {
return Opacity(

Copy link
Collaborator

@AustinChangLinksys AustinChangLinksys left a comment

Choose a reason for hiding this comment

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

Looks good to me

@AustinChangLinksys AustinChangLinksys merged commit 92f5377 into dev-1.2.8 Oct 28, 2025
@PeterJhongLinksys PeterJhongLinksys deleted the peter/now-689 branch November 6, 2025 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants