refactor(pnp): Extract AutoConfigurationUIModel from JNAP layer

[HankYuLinksys]

User description

Summary

• Created AutoConfigurationUIModel and AutoConfigurationMethodUI enum in pnp_ui_models.dart to abstract JNAP data models from the Provider layer
• Added JNAP Model → UI Model conversion methods in pnp_service.dart (_convertToUIModel, _convertMethod)
• Updated pnp_provider.dart to return AutoConfigurationUIModel instead of JNAP AutoConfigurationSettings
• Updated mock_pnp_providers.dart to use new UI model with updated field names
• Updated router_provider.dart to use new UI model field names (isSupported, userAcknowledged, method)
• Updated demo_overrides.dart to use new UI model

This refactoring ensures compliance with the three-layer architecture defined in Constitution Article V Section 5.3, preventing JNAP Data Models from leaking into the Provider layer.

---

PR Type

Enhancement

---

Description

• Extract auto-configuration UI model from JNAP layer

• Create AutoConfigurationUIModel and AutoConfigurationMethodUI enum

• Add conversion methods in PnpService for JNAP to UI model

• Update all providers and services to use new UI model

• Replace JNAP model imports with UI model imports across codebase

---

Diagram Walkthrough

flowchart LR
  JNAP["JNAP Layer<br/>AutoConfigurationSettings"]
  Service["PnpService<br/>Conversion Logic"]
  UIModel["UI Layer<br/>AutoConfigurationUIModel"]
  Providers["Providers & Router<br/>Use UI Model"]
  
  JNAP -->|_convertToUIModel| Service
  Service -->|returns| UIModel
  UIModel -->|consumed by| Providers

Loading

File Walkthrough

Relevant files

Enhancement

pnp_ui_models.dart Add AutoConfigurationUIModel and method enum                          lib/page/instant_setup/models/pnp_ui_models.dart Added AutoConfigurationMethodUI enum with preConfigured and autoParent values Created AutoConfigurationUIModel class with isSupported, userAcknowledged, and method fields Implemented copyWith(), toMap(), fromMap(), toJson(), and fromJson() methods Provides complete abstraction of JNAP auto-configuration data for UI layer +70/-0    pnp_service.dart Add JNAP to UI model conversion logic                                        lib/page/instant_setup/services/pnp_service.dart Updated autoConfigurationCheck() return type to AutoConfigurationUIModel? Added _convertToUIModel() method to convert JNAP model to UI model Added _convertMethod() helper to map JNAP method enum to UI method enum Encapsulates JNAP model conversion within service layer +34/-8    pnp_provider.dart Update provider to use UI model                                                    lib/page/instant_setup/providers/pnp_provider.dart Changed import from JNAP AutoConfigurationSettings to UI AutoConfigurationUIModel Updated BasePnpNotifier.autoConfigurationCheck() return type to AutoConfigurationUIModel? Updated PnpNotifier.autoConfigurationCheck() return type to AutoConfigurationUIModel? +3/-3      mock_pnp_providers.dart Update mock providers with UI model                                            lib/page/instant_setup/providers/mock_pnp_providers.dart Removed JNAP AutoConfigurationSettings import Updated all mock notifier implementations to return AutoConfigurationUIModel Changed field names: isAutoConfigurationSupported → isSupported, userAcknowledgedAutoConfiguration → userAcknowledged, autoConfigurationMethod → method Updated AutoConfigurationMethod enum references to AutoConfigurationMethodUI +20/-21  router_provider.dart Update router provider field references                                    lib/route/router_provider.dart Changed import from JNAP AutoConfigurationSettings to UI AutoConfigurationUIModel Updated field access: isAutoConfigurationSupported → isSupported, userAcknowledgedAutoConfiguration → userAcknowledged, autoConfigurationMethod → method Updated enum comparison from AutoConfigurationMethod.autoParent to AutoConfigurationMethodUI.autoParent Updated comments to reflect new field names +10/-12  demo_overrides.dart Update demo overrides with UI model                                            lib/demo/providers/demo_overrides.dart Changed import from JNAP AutoConfigurationSettings to UI AutoConfigurationUIModel Updated _DemoPnpNotifier.autoConfigurationCheck() return type to AutoConfigurationUIModel? Updated mock return value with new field names: isSupported instead of isAutoConfigurationSupported +3/-3

---

[qodo-code-review]

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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	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 Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Swallowed repository error: The new autoConfigurationCheck() swallows repository errors via .onError((error, stackTrace) => null) without logging or context, preventing actionable diagnosis and masking failure causes. Referred Code final result = await repo .send( JNAPAction.getAutoConfigurationSettings, fetchRemote: true, cacheLevel: CacheLevel.noCache, ) .then<AutoConfigurationUIModel?>((data) { final jnapModel = AutoConfigurationSettings.fromMap(data.output); return _convertToUIModel(jnapModel); }).onError((error, stackTrace) => null); logger.d('[PnP]: Service - Auto Configuration Check result: $result'); Learn more about managing compliance generic rules or creating your own custom rules
⚪	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: Potential sensitive logging: The log line logger.d('[PnP]: Service - Auto Configuration Check result: $result') may serialize and emit fields from the auto-configuration state, and it is unclear from the diff whether this could include sensitive or environment-specific data. Referred Code logger.d('[PnP]: Service - Auto Configuration Check result: $result'); return result; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Enum parsing unchecked: AutoConfigurationUIModel.fromMap() uses AutoConfigurationMethodUI.values.byName(...) on external/map data without guarding against unexpected values, which can throw and potentially crash the flow. Referred Code factory AutoConfigurationUIModel.fromMap(Map<String, dynamic> map) { return AutoConfigurationUIModel( isSupported: map['isSupported'] as bool?, userAcknowledged: map['userAcknowledged'] as bool?, method: map['method'] != null ? AutoConfigurationMethodUI.values.byName(map['method'] as String) : null, ); Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Treat null as unacknowledged In the routing logic, change the condition from config.userAcknowledged == false to config.userAcknowledged != true to correctly handle null values as unacknowledged. lib/route/router_provider.dart [193-195] -return config.userAcknowledged == false +return config.userAcknowledged != true ? LocalWhereToGo.firstTimeLogin : LocalWhereToGo.login; Apply / Chat Suggestion importance[1-10]: 8 __ Why: This is a critical fix for the routing logic, as userAcknowledged can be null. The current code == false would incorrectly route to login for a null value, while the suggested != true correctly handles both false and null as unacknowledged.	Medium
	Use try/catch for conversion In autoConfigurationCheck, replace the .then().onError() chain with a try/catch block to improve readability and add error logging when fetching and converting auto-configuration settings. lib/page/instant_setup/services/pnp_service.dart [219-228] -...await repo - .send( +try { + final data = await repo.send( JNAPAction.getAutoConfigurationSettings, fetchRemote: true, cacheLevel: CacheLevel.noCache, - ) - .then<AutoConfigurationUIModel?>((data) { - final jnapModel = AutoConfigurationSettings.fromMap(data.output); - return _convertToUIModel(jnapModel); - }).onError((error, stackTrace) => null); + ); + final jnapModel = AutoConfigurationSettings.fromMap(data.output); + return _convertToUIModel(jnapModel); +} catch (error, stackTrace) { + logger.e('[PnP]: Service - Auto Configuration Check error', error, stackTrace); + return null; +} [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: The suggestion improves code readability and error handling by replacing a .then().onError() chain with a more conventional try/catch block, which also allows for logging the specific error.	Low
Possible issue	Safely handle enum deserialization In the AutoConfigurationUIModel.fromMap factory, wrap the byName call in a try-catch block to prevent crashes from invalid string values for the method field during deserialization. lib/page/instant_setup/models/pnp_ui_models.dart [149-157] factory AutoConfigurationUIModel.fromMap(Map<String, dynamic> map) { + AutoConfigurationMethodUI? method; + if (map['method'] != null) { + try { + method = AutoConfigurationMethodUI.values.byName(map['method'] as String); + } catch (e) { + // Gracefully handle invalid enum value from cache/map + method = null; + } + } + return AutoConfigurationUIModel( isSupported: map['isSupported'] as bool?, userAcknowledged: map['userAcknowledged'] as bool?, - method: map['method'] != null - ? AutoConfigurationMethodUI.values.byName(map['method'] as String) - : null, + method: method, ); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that deserializing an enum using byName without error handling is unsafe and can lead to a crash. The proposed try-catch block is a robust way to handle corrupted or unexpected data.	Medium
	Add a default case to the switch Add a default case to the switch expression in _convertMethod to gracefully handle unknown AutoConfigurationMethod values from the JNAP API and prevent potential runtime crashes. lib/page/instant_setup/services/pnp_service.dart [244-252] AutoConfigurationMethodUI? _convertMethod( AutoConfigurationMethod? jnapMethod) { if (jnapMethod == null) return null; return switch (jnapMethod) { AutoConfigurationMethod.preConfigured => AutoConfigurationMethodUI.preConfigured, AutoConfigurationMethod.autoParent => AutoConfigurationMethodUI.autoParent, + // It's possible for the JNAP API to return a method not yet + // supported by the app. Return null to handle this gracefully. + _ => null, }; } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that a non-exhaustive switch on an enum from an external source can cause a runtime crash if the source adds new enum members, and proposes a valid fix to improve robustness.	Low
Update

[qodo-code-review]

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: 🛡️ Quality & Logic (PR)

Failed stage: 🔍 Flutter Analyze [❌]

Failed test name: ""

Failure summary: The action failed because the Dart analysis/lint step reported compilation/analysis errors, causing the process to exit with code 1. - invalid_override errors in mock/test code due to a mismatched method return type for autoConfigurationCheck: - test/mocks/pnp_notifier_mocks.dart:364:46: MockPnpNotifier.autoConfigurationCheck returns Future<AutoConfigurationSettings?> but must override PnpNotifier.autoConfigurationCheck returning Future<AutoConfigurationUIModel?>. - test/mocks/pnp_service_mocks.dart:273:46: MockPnpService.autoConfigurationCheck returns Future<AutoConfigurationSettings?> but must override PnpService.autoConfigurationCheck returning Future<AutoConfigurationUIModel?>. - test/page/instant_setup/localizations/pnp_setup_view_test.dart:108:38: FakePnpNotifier.autoConfigurationCheck returns Future<AutoConfigurationSettings?> but must override BasePnpNotifier.autoConfigurationCheck returning Future<AutoConfigurationUIModel?>. Additional warnings/infos were present, but the invalid_override errors are what made the step fail.

Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 469: info • Don't use 'BuildContext's across async gaps, guarded by an unrelated 'mounted' check • lib/page/instant_admin/views/timezone_view.dart:57:17 • use_build_context_synchronously 470: info • Don't use 'BuildContext's across async gaps • lib/page/instant_setup/pnp_setup_view.dart:355:31 • use_build_context_synchronously 471: info • Don't use 'BuildContext's across async gaps • lib/util/get_log_selector/get_log_mobile.dart:11:36 • use_build_context_synchronously 472: info • The imported package 'equatable' isn't a dependency of the importing package • packages/usp_client_core/lib/src/connection/usp_connection_config.dart:6:8 • depend_on_referenced_packages 473: info • The prefix 'Mock' isn't a lower_case_with_underscores identifier • test/common/test_helper.dart:67:46 • library_prefixes 474: info • The type of the right operand ('Provider<RouterRepository>') isn't a subtype or a supertype of the left operand ('ProviderListenable<T>') • test/common/unit_test_helper.dart:48:18 • unrelated_type_equality_checks 475: info • The type of the right operand ('NotifierProviderImpl<DashboardHomeNotifier, DashboardHomeState>') isn't a subtype or a supertype of the left operand ('ProviderListenable<T>') • test/common/unit_test_helper.dart:51:18 • unrelated_type_equality_checks 476: warning • The method doesn't override an inherited method • test/mocks/add_nodes_notifier_mocks.dart:138:30 • override_on_non_overriding_member 477: warning • The method doesn't override an inherited method • test/mocks/add_nodes_notifier_mocks.dart:161:39 • override_on_non_overriding_member 478: warning • The method doesn't override an inherited method • test/mocks/add_nodes_notifier_mocks.dart:176:42 • override_on_non_overriding_member 479: warning • The method doesn't override an inherited method • test/mocks/add_nodes_notifier_mocks.dart:202:27 • override_on_non_overriding_member 480: warning • The method doesn't override an inherited method • test/mocks/add_wired_nodes_notifier_mocks.dart:152:23 • override_on_non_overriding_member 481: warning • The method doesn't override an inherited method • test/mocks/auto_parent_first_login_notifier_mocks.dart:129:20 • override_on_non_overriding_member 482: warning • The method doesn't override an inherited method • test/mocks/auto_parent_first_login_notifier_mocks.dart:140:20 • override_on_non_overriding_member 483: warning • The method doesn't override an inherited method • test/mocks/auto_parent_first_login_notifier_mocks.dart:150:20 • override_on_non_overriding_member 484: error • 'MockPnpNotifier.autoConfigurationCheck' ('Future<AutoConfigurationSettings?> Function()') isn't a valid override of 'PnpNotifier.autoConfigurationCheck' ('Future<AutoConfigurationUIModel?> Function()') • test/mocks/pnp_notifier_mocks.dart:364:46 • invalid_override 485: error • 'MockPnpService.autoConfigurationCheck' ('Future<AutoConfigurationSettings?> Function()') isn't a valid override of 'PnpService.autoConfigurationCheck' ('Future<AutoConfigurationUIModel?> Function()') • test/mocks/pnp_service_mocks.dart:273:46 • invalid_override 486: error • 'FakePnpNotifier.autoConfigurationCheck' ('Future<AutoConfigurationSettings?> Function()') isn't a valid override of 'BasePnpNotifier.autoConfigurationCheck' ('Future<AutoConfigurationUIModel?> Function()') • test/page/instant_setup/localizations/pnp_setup_view_test.dart:108:38 • invalid_override 487: warning • Unused import: 'package:flutter_riverpod/flutter_riverpod.dart' • test/page/nodes/services/add_nodes_service_test.dart:3:8 • unused_import 488: info • Use 'const' literals as arguments to constructors of '@immutable' classes • test/page/nodes/services/add_nodes_service_test.dart:191:72 • prefer_const_literals_to_create_immutables 489: info • Use 'const' literals as arguments to constructors of '@immutable' classes • test/page/nodes/services/add_wired_nodes_service_test.dart:110:74 • prefer_const_literals_to_create_immutables 490: 26 issues found. (ran in 37.2s) 491: ##[error]Process completed with exit code 1. 492: Post job cleanup. 493: Stopping SSH agent 494: The "file" argument must be of type string. Received undefined 495: Error stopping the SSH agent, proceeding anyway 496: Post job cleanup.

[AustinChangLinksys]

LGTM!

I confirmed that:

1. Decoupling is achieved: RouterProvider now depends on AutoConfigurationUIModel instead of the raw JNAP model, which is a solid improvement.
2. Architecture Alignment: While PnpService returning a UI model might look like a layering violation in strict Clean Architecture, it aligns with our project's design decision where the Service layer handles Domain-to-Presentation mapping.
3. Tests: Mocks and tests have been correctly updated to match the modified Service signature.

Approved.