diff --git a/app/lib/screens/settings_screen.dart b/app/lib/screens/settings_screen.dart index 9e32dc2..80f170e 100644 --- a/app/lib/screens/settings_screen.dart +++ b/app/lib/screens/settings_screen.dart @@ -1,9 +1,10 @@ -/// GitHub sync configuration. Primary path: "Connect GitHub" runs the OAuth -/// **device flow** (authorize in a browser, no token pasting). A manually -/// pasted PAT remains as a fallback under "Advanced". Auto-sync (app launch -/// + lifecycle pause/resume) lives in [LogMealScreen] and is silent on -/// failure -- this screen is where errors get surfaced, as inline status -/// text. +/// Sync configuration. Primary path: "Connect Firebase" — sign in with the +/// shared sync account, whose password is kept in the OS keystore. GitHub is +/// only the cutover mirror, so its whole setup (device-flow OAuth, owner/repo, +/// client id, PAT fallback) sits under "Advanced (GitHub mirror)" rather than +/// competing with Firebase as a visible choice. Auto-sync (app launch + +/// lifecycle pause/resume) lives in [LogMealScreen] and is silent on failure +/// -- this screen is where errors get surfaced, as inline status text. library; import 'dart:async'; @@ -232,9 +233,11 @@ class _SettingsScreenState extends State { final client = createGitHubClient(settings, httpClient: widget.httpClient); try { final ok = await client.canAccessRepo(); - _showMessage(ok ? 'Connection OK.' : 'Connection failed.'); + _showMessage( + ok ? 'GitHub connection OK.' : 'GitHub connection failed.', + ); } on Exception catch (e) { - _showMessage('Connection failed: $e'); + _showMessage('GitHub connection failed: $e'); } finally { client.close(); if (mounted) setState(() => _busy = false); @@ -395,70 +398,94 @@ class _SettingsScreenState extends State { style: Theme.of(context).textTheme.bodySmall, ), const SizedBox(height: 12), - TextField( - controller: _firebaseEmailController, - keyboardType: TextInputType.emailAddress, - autocorrect: false, - decoration: const InputDecoration( - labelText: 'Sync account email', - ), - ), - const SizedBox(height: 8), - TextField( - controller: _firebasePasswordController, - obscureText: true, - autocorrect: false, - decoration: const InputDecoration( - labelText: 'Sync account password', - ), - ), - const SizedBox(height: 12), - Row( - children: [ - FilledButton.icon( - onPressed: _busy ? null : _saveFirebaseAccount, - icon: const Icon(Icons.cloud_done), - label: const Text('Connect Firebase'), - ), - if (_firebaseConfigured) ...[ + // Once connected, the account is shown as read-only text rather + // than as prefilled inputs: an editable email box next to an + // empty password box reads as "you still have to enter this", + // which made a connected device look unconfigured. + if (_firebaseConfigured) + Row( + children: [ + const Icon(Icons.cloud_done, size: 20), const SizedBox(width: 8), + Expanded( + child: Text( + _firebaseEmailController.text, + style: Theme.of(context).textTheme.bodyMedium, + ), + ), TextButton( onPressed: _busy ? null : _disconnectFirebase, child: const Text('Disconnect'), ), ], - ], - ), + ) + else ...[ + TextField( + controller: _firebaseEmailController, + keyboardType: TextInputType.emailAddress, + autocorrect: false, + decoration: const InputDecoration( + labelText: 'Sync account email', + ), + ), + const SizedBox(height: 8), + TextField( + controller: _firebasePasswordController, + obscureText: true, + autocorrect: false, + decoration: const InputDecoration( + labelText: 'Sync account password', + ), + ), + const SizedBox(height: 12), + FilledButton.icon( + onPressed: _busy ? null : _saveFirebaseAccount, + icon: const Icon(Icons.cloud_done), + label: const Text('Connect Firebase'), + ), + ], const SizedBox(height: 24), const Divider(), const SizedBox(height: 8), - Text( - 'Authorize in your browser — no token to paste. Syncs to ' - 'kuhyx/syncs by default.', - style: Theme.of(context).textTheme.bodySmall, - ), - const SizedBox(height: 12), - FilledButton.icon( - onPressed: _connectGitHub, - icon: const Icon(Icons.login), - label: const Text('Connect GitHub'), - ), - const SizedBox(height: 16), - TextField( - controller: _ownerController, - decoration: const InputDecoration(labelText: 'GitHub owner'), - ), - const SizedBox(height: 8), - TextField( - controller: _repoController, - decoration: const InputDecoration(labelText: 'Repo'), - ), - const SizedBox(height: 8), + // GitHub is a mirror during the cutover, not something to pick: + // its whole configuration lives under "Advanced" so the primary + // path stays a single choice. See CLAUDE.md "Cross-device sync". ExpansionTile( - title: const Text('Advanced'), + title: const Text('Advanced (GitHub mirror)'), tilePadding: EdgeInsets.zero, childrenPadding: const EdgeInsets.only(bottom: 8), children: [ + Align( + alignment: Alignment.centerLeft, + child: Text( + 'Syncs still mirror to GitHub until every device has ' + 'moved to Firebase. Authorize in your browser — no ' + 'token to paste.', + style: Theme.of(context).textTheme.bodySmall, + ), + ), + const SizedBox(height: 12), + Align( + alignment: Alignment.centerLeft, + child: FilledButton.icon( + onPressed: _connectGitHub, + icon: const Icon(Icons.login), + label: const Text('Connect GitHub'), + ), + ), + const SizedBox(height: 16), + TextField( + controller: _ownerController, + decoration: const InputDecoration( + labelText: 'GitHub owner', + ), + ), + const SizedBox(height: 8), + TextField( + controller: _repoController, + decoration: const InputDecoration(labelText: 'Repo'), + ), + const SizedBox(height: 8), TextField( controller: _clientIdController, decoration: const InputDecoration( @@ -498,7 +525,10 @@ class _SettingsScreenState extends State { ), OutlinedButton( onPressed: _busy ? null : _testConnection, - child: const Text('Test connection'), + // Named for what it actually does: _testConnection builds a + // GitHub client and calls canAccessRepo(). It says nothing + // about Firebase, and the bare label implied otherwise. + child: const Text('Test GitHub connection'), ), ElevatedButton( onPressed: _busy ? null : _syncNow, diff --git a/app/test/screens/settings_screen_test.dart b/app/test/screens/settings_screen_test.dart index 2b2da2f..23a99e6 100644 --- a/app/test/screens/settings_screen_test.dart +++ b/app/test/screens/settings_screen_test.dart @@ -1,10 +1,12 @@ import 'dart:convert'; import 'dart:io'; +import 'package:crdt_sync/crdt_sync.dart'; import 'package:diet_guard_app/services/document_store_io.dart'; import 'package:diet_guard_app/screens/settings_screen.dart'; import 'package:diet_guard_app/services/app_settings_service.dart'; import 'package:diet_guard_app/services/budget_history_service.dart'; +import 'package:diet_guard_app/services/firebase_backend.dart'; import 'package:diet_guard_app/services/foodbank_service.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; import 'package:flutter/material.dart'; @@ -77,6 +79,16 @@ void main() { await tester.pumpAndSettle(); } + /// Expands the "Advanced (GitHub mirror)" section. + /// + /// GitHub is the cutover mirror rather than a choice the user makes, so its + /// whole configuration (owner, repo, client id, PAT, "Connect GitHub") is + /// collapsed by default. Every GitHub-facing test has to open it first. + Future openAdvanced(WidgetTester tester) async { + await tester.tap(find.text('Advanced (GitHub mirror)')); + await tester.pumpAndSettle(); + } + /// Drains the device flow's real `Future.delayed` poll (GitHubDeviceAuth /// injects no test delay, so under `runAsync` it is a genuine Timer, not /// the fake-clock one `tester.pump(duration)` advances) by interleaving @@ -99,6 +111,9 @@ void main() { await tester.pumpWidget(const MaterialApp(home: SettingsScreen())); await settle(tester); + // owner/repo moved under the collapsed GitHub-mirror section. + await openAdvanced(tester); + expect(find.widgetWithText(TextField, 'kuhyx'), findsOneWidget); expect(find.widgetWithText(TextField, 'syncs'), findsOneWidget); }); @@ -109,8 +124,7 @@ void main() { await tester.pumpWidget(const MaterialApp(home: SettingsScreen())); await settle(tester); - await tester.tap(find.text('Advanced')); - await tester.pumpAndSettle(); + await openAdvanced(tester); await tester.enterText( find.widgetWithText(TextField, 'Personal access token (fallback)'), 'my-pat', @@ -132,10 +146,12 @@ void main() { ); await settle(tester); - await tester.tap(find.widgetWithText(OutlinedButton, 'Test connection')); + await tester.tap( + find.widgetWithText(OutlinedButton, 'Test GitHub connection'), + ); await settle(tester); - expect(find.text('Connection OK.'), findsOneWidget); + expect(find.text('GitHub connection OK.'), findsOneWidget); }); }); @@ -147,10 +163,12 @@ void main() { ); await settle(tester); - await tester.tap(find.widgetWithText(OutlinedButton, 'Test connection')); + await tester.tap( + find.widgetWithText(OutlinedButton, 'Test GitHub connection'), + ); await settle(tester); - expect(find.text('Connection failed.'), findsOneWidget); + expect(find.text('GitHub connection failed.'), findsOneWidget); }); }); @@ -189,10 +207,12 @@ void main() { ); await settle(tester); - await tester.tap(find.widgetWithText(OutlinedButton, 'Test connection')); + await tester.tap( + find.widgetWithText(OutlinedButton, 'Test GitHub connection'), + ); await settle(tester); - expect(find.textContaining('Connection failed:'), findsOneWidget); + expect(find.textContaining('GitHub connection failed:'), findsOneWidget); }); }); @@ -211,18 +231,67 @@ void main() { }); }); - testWidgets('shows the Connect GitHub button', (tester) async { + testWidgets('offers the account form when Firebase is not connected', ( + tester, + ) async { await tester.runAsync(() async { await tester.pumpWidget(const MaterialApp(home: SettingsScreen())); await settle(tester); + expect( + find.widgetWithText(TextField, 'Sync account email'), + findsOneWidget, + ); + expect( + find.widgetWithText(TextField, 'Sync account password'), + findsOneWidget, + ); + expect(find.text('Disconnect'), findsNothing); + }); + }); + + testWidgets('shows the connected account as text, not as an empty form', ( + tester, + ) async { + // An editable email box beside a blank password box reads as "you still + // have to enter this", so a connected device looked unconfigured. + await saveAccount( + const FirebaseAccount(email: 'sync@example.com', password: 'pw'), + ); + await tester.runAsync(() async { + await tester.pumpWidget(const MaterialApp(home: SettingsScreen())); + await settle(tester); + + expect(find.text('sync@example.com'), findsOneWidget); + expect(find.text('Disconnect'), findsOneWidget); + expect( + find.widgetWithText(TextField, 'Sync account password'), + findsNothing, + ); + expect(find.text('Connect Firebase'), findsNothing); + }); + }); + + testWidgets('hides the Connect GitHub button until Advanced is expanded', ( + tester, + ) async { + await tester.runAsync(() async { + await tester.pumpWidget(const MaterialApp(home: SettingsScreen())); + await settle(tester); + + // GitHub is the cutover mirror, not a choice competing with Firebase, + // so nothing GitHub-facing is visible on the primary path. + expect(find.text('Connect GitHub'), findsNothing); + + await openAdvanced(tester); + expect(find.text('Connect GitHub'), findsOneWidget); }); }); /// Expands "Advanced" and types [clientId] into the client-id field. Future enterClientId(WidgetTester tester, String clientId) async { - await tester.tap(find.text('Advanced')); + await tester.tap(find.text('Advanced (GitHub mirror)')); await tester.pumpAndSettle(); await tester.enterText( find.widgetWithText(TextField, 'OAuth App client id'), diff --git a/pyproject.toml b/pyproject.toml index b44701a..3b41ec9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ description = "Tkinter/systemd log-to-unlock meal gate with calorie/macro budget requires-python = ">=3.10" dependencies = [ "gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock", - "crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.3.0#subdirectory=crdt-sync", + "crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.5.1#subdirectory=crdt-sync", "requests>=2.31.0", ] diff --git a/requirements.txt b/requirements.txt index 924a2ef..30324bf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ bandit>=1.7.0 codespell>=2.2.0 coverage>=7.4.0 -crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.3.0#subdirectory=crdt-sync +crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.5.1#subdirectory=crdt-sync gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock mcp>=2,<3 mypy>=1.8.0