Add the Firebase connect UI, without which the cutover could never happen
Some checks failed
release-apk / release (APK) (push) Has been cancelled

The Firebase backend has been wired up since the cutover commit, but nothing
ever called saveAccount(). openFirebase() therefore read an account that was
never written, returned null every time, and the app quietly synced over
GitHub forever -- on real hardware, regardless of what the tests said. The
same hole was found and fixed in diet-guard; it is still open in
home_inventory, workout_app, build_your_x and leetcode-guard.

Settings now has a Firebase section that stores the account and signs in
immediately, so a wrong password shows up on screen instead of as a silent
no-op later. A rejected sign-in clears the account rather than leaving it
half-stored.

The three UI problems kuhy hit in diet-guard are avoided rather than
repeated: GitHub moves into "Advanced (GitHub mirror)" instead of sitting
beside Firebase as a peer choice, and a connected account renders as
read-only text with a Disconnect button rather than an empty-looking form
that implies you still have to type something.

The keystore accessors are injectable, like firebaseFactory already was, so
the connect/reject/disconnect branches are testable without a platform
channel. 252 tests, back to 100% line coverage.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017FH32BgGQbQLDstBhR26Xa
This commit is contained in:
Krzysztof Rudnicki 2026-08-05 09:29:41 +02:00
parent f933b4baa4
commit ea76a04388
3 changed files with 342 additions and 36 deletions

View File

@ -7,18 +7,23 @@ import 'package:flutter/services.dart';
import 'package:http/http.dart' as http;
import 'package:todo/data/note_repository.dart';
import 'package:todo/sync/backlog_export.dart';
import 'package:todo/sync/firebase_backend.dart';
import 'package:todo/sync/github_device_auth.dart';
import 'package:todo/sync/notes_markdown.dart';
import 'package:todo/sync/run_sync.dart';
import 'package:todo/sync/sync_settings.dart';
import 'package:url_launcher/url_launcher.dart';
/// Settings screen for GitHub sync configuration and note backup.
/// Settings screen for sync configuration and note backup.
///
/// Primary sync path: the "Connect GitHub" button runs the OAuth **device
/// flow** (authorize in a browser, no token pasting). The manual token field
/// remains as a fallback. The Backup section exports/imports all notes as a
/// single Markdown file (see [NotesMarkdown]).
/// Primary sync path: "Connect Firebase" signs in with the shared sync
/// account, whose password is kept in the OS keystore. GitHub is only the
/// cutover mirror, so its device-flow connection, owner/repo and token
/// fallback all sit under "Advanced (GitHub mirror)" rather than competing
/// with Firebase as a visible choice.
///
/// The Backup section exports/imports all notes as a single Markdown file
/// (see [NotesMarkdown]).
class SettingsScreen extends StatefulWidget {
/// Creates a [SettingsScreen] pre-filled with [initial] settings.
const SettingsScreen({
@ -27,6 +32,9 @@ class SettingsScreen extends StatefulWidget {
this.httpClient,
this.firebaseFactory,
this.stateStore,
this.accountLoader,
this.accountSaver,
this.accountClearer,
super.key,
});
@ -43,6 +51,18 @@ class SettingsScreen extends StatefulWidget {
/// Revision cache. Injected so tests need no application-support directory.
final SyncStateStore? stateStore;
/// Keystore accessors for the Firebase account. Injected as a group so the
/// connect/disconnect flows are testable without a platform channel —
/// `flutter test` has no binding for one, and the real keystore is the only
/// thing standing between these branches and full coverage.
final Future<FirebaseAccount?> Function()? accountLoader;
/// Persists the account. See [accountLoader].
final Future<void> Function(FirebaseAccount)? accountSaver;
/// Forgets the account and any cached session. See [accountLoader].
final Future<void> Function()? accountClearer;
/// Optional HTTP client for the GitHub calls (test-connection and device
/// flow). Injected by tests; production uses each client's default.
final http.Client? httpClient;
@ -65,15 +85,86 @@ class _SettingsScreenState extends State<SettingsScreen> {
text: widget.initial.clientId,
);
final TextEditingController _email = TextEditingController();
final TextEditingController _password = TextEditingController();
bool _testing = false;
bool _busy = false;
bool _firebaseConnected = false;
String? _status;
@override
void initState() {
super.initState();
unawaited(_loadFirebaseAccount());
}
/// Reflects a previously-stored account, so a returning user sees the real
/// state instead of an empty form that looks unconfigured.
Future<void> _loadFirebaseAccount() async {
final account = await (widget.accountLoader ?? loadAccount)();
if (!mounted) return;
if (account != null) _email.text = account.email;
setState(() => _firebaseConnected = account != null);
}
/// Stores the typed account and signs in immediately, so a typo surfaces
/// here rather than as a silent background failure on the next sync.
///
/// Without this the app could never reach Firebase at all: `openFirebase()`
/// reads an account nothing had ever written.
Future<void> _connectFirebase() async {
final email = _email.text.trim();
final password = _password.text;
if (email.isEmpty || password.isEmpty) {
setState(() => _status = 'Enter the sync account email and password.');
return;
}
setState(() {
_busy = true;
_status = 'Signing in…';
});
await (widget.accountSaver ?? saveAccount)(
FirebaseAccount(email: email, password: password),
);
final client = await (widget.firebaseFactory ?? openFirebase)();
if (!mounted) return;
if (client == null) {
await (widget.accountClearer ?? clearAccount)();
setState(() {
_busy = false;
_firebaseConnected = false;
_status = 'Firebase rejected that account.';
});
return;
}
_password.clear();
setState(() {
_busy = false;
_firebaseConnected = true;
_status = 'Connected to Firebase.';
});
}
Future<void> _disconnectFirebase() async {
await (widget.accountClearer ?? clearAccount)();
if (!mounted) return;
_email.clear();
_password.clear();
setState(() {
_firebaseConnected = false;
_status = 'Firebase disconnected.';
});
}
@override
void dispose() {
_owner.dispose();
_repo.dispose();
_token.dispose();
_clientId.dispose();
_email.dispose();
_password.dispose();
super.dispose();
}
@ -228,44 +319,108 @@ class _SettingsScreenState extends State<SettingsScreen> {
padding: const EdgeInsets.all(16),
children: [
Text(
'Connect to GitHub',
'Firebase sync',
style: Theme.of(context).textTheme.titleMedium,
),
const SizedBox(height: 4),
Text(
'Authorize in your browser — no token to paste. Syncs to '
'kuhyx/syncs by default.',
_firebaseConnected
? 'Connected. Notes go to Firebase first, and still mirror '
'to GitHub until every device has moved.'
: 'Not connected — syncing over GitHub only. Enter the '
'shared sync account to move this device over. The '
'password is kept in the device keystore, never in the '
'app or the repo.',
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: _owner,
decoration: const InputDecoration(
labelText: 'GitHub owner',
border: OutlineInputBorder(),
// Once connected the account is read-only text: an editable email
// box beside an empty password box reads as "you still have to
// enter this", making a connected device look unconfigured.
if (_firebaseConnected)
Row(
children: [
const Icon(Icons.cloud_done, size: 20),
const SizedBox(width: 8),
Expanded(child: Text(_email.text)),
TextButton(
onPressed: _busy ? null : _disconnectFirebase,
child: const Text('Disconnect'),
),
],
)
else ...[
TextField(
controller: _email,
keyboardType: TextInputType.emailAddress,
autocorrect: false,
decoration: const InputDecoration(
labelText: 'Sync account email',
border: OutlineInputBorder(),
),
),
),
const SizedBox(height: 12),
TextField(
controller: _repo,
decoration: const InputDecoration(
labelText: 'Repository name',
border: OutlineInputBorder(),
const SizedBox(height: 12),
TextField(
controller: _password,
obscureText: true,
autocorrect: false,
decoration: const InputDecoration(
labelText: 'Sync account password',
border: OutlineInputBorder(),
),
),
),
const SizedBox(height: 12),
Align(
alignment: Alignment.centerLeft,
child: FilledButton.icon(
onPressed: _busy ? null : _connectFirebase,
icon: const Icon(Icons.cloud_done),
label: const Text('Connect Firebase'),
),
),
],
const SizedBox(height: 8),
// Manual escape hatches — most users never open this.
// GitHub is the cutover mirror, not a choice competing with
// Firebase, so its whole setup lives behind one disclosure.
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(
'Authorize in your browser — no token to paste. Syncs to '
'kuhyx/syncs by default.',
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: _owner,
decoration: const InputDecoration(
labelText: 'GitHub owner',
border: OutlineInputBorder(),
),
),
const SizedBox(height: 12),
TextField(
controller: _repo,
decoration: const InputDecoration(
labelText: 'Repository name',
border: OutlineInputBorder(),
),
),
const SizedBox(height: 12),
TextField(
controller: _clientId,
decoration: const InputDecoration(

View File

@ -286,7 +286,7 @@ void main() {
await tester.tap(find.byTooltip('Sync settings'));
await tester.pumpAndSettle(); // route transition
expect(find.text('Connect GitHub'), findsOneWidget); // settings is up
expect(find.text('Firebase sync'), findsOneWidget); // settings is up
// Saving pops a SyncSettings back to the capture screen (covers the
// result-adoption branch in _openSettings). Scope to the settings route —
@ -299,7 +299,7 @@ void main() {
);
await tester.pumpAndSettle(); // save + pop transition
expect(find.text('Connect GitHub'), findsNothing); // back on capture
expect(find.text('Firebase sync'), findsNothing); // back on capture
expect(find.byTooltip('Sync settings'), findsOneWidget);
});

View File

@ -87,6 +87,10 @@ void main() {
http.Client? httpClient,
List<Note> seed = const [],
FakeNoteRepository? repository,
Future<FirebaseRestClient?> Function()? firebaseFactory,
Future<FirebaseAccount?> Function()? accountLoader,
Future<void> Function(FirebaseAccount)? accountSaver,
Future<void> Function()? accountClearer,
}) async {
SharedPreferences.setMockInitialValues({});
installFakeSecureStorage();
@ -108,7 +112,10 @@ void main() {
// Both injected so the widget never reaches for the platform: the
// real factories want the OS keystore and an application-support
// directory, neither of which exists under `flutter test`.
firebaseFactory: () async => null,
firebaseFactory: firebaseFactory ?? () async => null,
accountLoader: accountLoader ?? () async => null,
accountSaver: accountSaver,
accountClearer: accountClearer,
stateStore: InMemorySyncStateStore(),
),
),
@ -117,17 +124,145 @@ void main() {
return repo;
}
/// Expands the "Advanced (GitHub mirror)" section.
///
/// GitHub is the cutover mirror rather than a choice the user makes, so
/// everything GitHub-facing is collapsed by default; a test that touches
/// those widgets has to open it first.
Future<void> openAdvanced(WidgetTester tester) async {
await tester.tap(find.text('Advanced (GitHub mirror)'));
await tester.pumpAndSettle();
}
testWidgets('renders sync fields and the backup actions', (tester) async {
await pumpSettings(tester);
// GitHub is the mirror now: hidden until the disclosure is opened.
expect(find.text('Connect GitHub'), findsNothing);
expect(find.text('Connect Firebase'), findsOneWidget);
await openAdvanced(tester);
expect(find.text('Connect GitHub'), findsOneWidget);
expect(find.text('Export notes'), findsOneWidget);
expect(find.text('Import notes'), findsOneWidget);
});
testWidgets('offers the Firebase account form when not connected', (
tester,
) async {
await pumpSettings(tester);
expect(
find.widgetWithText(TextField, 'Sync account email'),
findsOneWidget,
);
expect(
find.widgetWithText(TextField, 'Sync account password'),
findsOneWidget,
);
expect(find.text('Disconnect'), findsNothing);
});
testWidgets('Connect Firebase with empty fields asks for credentials', (
tester,
) async {
await pumpSettings(tester);
await tester.tap(find.text('Connect Firebase'));
await tester.pump();
expect(
find.textContaining('Enter the sync account email and password'),
findsOneWidget,
);
});
testWidgets('Connect Firebase stores the account and reports success', (
tester,
) async {
FirebaseAccount? saved;
await pumpSettings(
tester,
accountSaver: (a) async => saved = a,
firebaseFactory: () async => _stubClient(),
);
await tester.enterText(
find.widgetWithText(TextField, 'Sync account email'),
'sync@example.com',
);
await tester.enterText(
find.widgetWithText(TextField, 'Sync account password'),
'pw',
);
await tester.tap(find.text('Connect Firebase'));
await tester.pumpAndSettle();
// saveAccount must actually be called: without it openFirebase() reads an
// account nothing ever wrote, and Firebase is a silent no-op forever.
expect(saved?.email, 'sync@example.com');
expect(saved?.password, 'pw');
expect(find.text('Connected to Firebase.'), findsOneWidget);
// Connected state is read-only text, not a form implying re-entry.
expect(find.text('sync@example.com'), findsOneWidget);
expect(find.text('Disconnect'), findsOneWidget);
expect(
find.widgetWithText(TextField, 'Sync account password'),
findsNothing,
);
});
testWidgets('a rejected account is cleared rather than left half-stored', (
tester,
) async {
var cleared = false;
await pumpSettings(
tester,
accountSaver: (_) async {},
accountClearer: () async => cleared = true,
firebaseFactory: () async => null,
);
await tester.enterText(
find.widgetWithText(TextField, 'Sync account email'),
'wrong@example.com',
);
await tester.enterText(
find.widgetWithText(TextField, 'Sync account password'),
'bad',
);
await tester.tap(find.text('Connect Firebase'));
await tester.pumpAndSettle();
expect(cleared, isTrue);
expect(find.text('Firebase rejected that account.'), findsOneWidget);
});
testWidgets('a stored account shows as connected on open, and disconnects', (
tester,
) async {
var cleared = false;
await pumpSettings(
tester,
accountLoader: () async =>
const FirebaseAccount(email: 'stored@example.com', password: 'pw'),
accountClearer: () async => cleared = true,
);
await tester.pumpAndSettle();
expect(find.text('stored@example.com'), findsOneWidget);
await tester.tap(find.text('Disconnect'));
await tester.pumpAndSettle();
expect(cleared, isTrue);
expect(find.text('Firebase disconnected.'), findsOneWidget);
expect(
find.widgetWithText(TextField, 'Sync account email'),
findsOneWidget,
);
});
testWidgets('Connect GitHub without a client id shows guidance', (
tester,
) async {
await pumpSettings(tester);
await openAdvanced(tester);
await tester.tap(find.text('Connect GitHub'));
await tester.pump();
expect(
@ -140,7 +275,9 @@ void main() {
final mock = MockClient((_) async => http.Response('{}', 200));
await pumpSettings(tester, httpClient: mock);
await tester.tap(find.text('Advanced')); // Test connection lives here now
await tester.tap(
find.text('Advanced (GitHub mirror)'),
); // Test connection lives here now
await tester.pumpAndSettle();
await tester.tap(find.text('Test connection'));
await tester.pump(); // start
@ -153,7 +290,7 @@ void main() {
final mock = MockClient((_) async => http.Response('', 404));
await pumpSettings(tester, httpClient: mock);
await tester.tap(find.text('Advanced'));
await tester.tap(find.text('Advanced (GitHub mirror)'));
await tester.pumpAndSettle();
await tester.tap(find.text('Test connection'));
await tester.pump();
@ -166,7 +303,7 @@ void main() {
final mock = MockClient((_) async => throw Exception('offline'));
await pumpSettings(tester, httpClient: mock);
await tester.tap(find.text('Advanced'));
await tester.tap(find.text('Advanced (GitHub mirror)'));
await tester.pumpAndSettle();
await tester.tap(find.text('Test connection'));
await tester.pump();
@ -188,6 +325,7 @@ void main() {
httpClient: mock,
);
await openAdvanced(tester);
await tester.tap(find.text('Connect GitHub'));
await tester.pump();
await tester.pump();
@ -224,6 +362,7 @@ void main() {
httpClient: mock,
);
await openAdvanced(tester);
await tester.tap(find.text('Connect GitHub'));
await tester.pump(); // requestDeviceCode
await tester.pump(); // dialog builds, shows the user code
@ -272,6 +411,7 @@ void main() {
httpClient: mock,
);
await openAdvanced(tester);
await tester.tap(find.text('Connect GitHub'));
await tester.pump();
await tester.pump();
@ -387,7 +527,7 @@ void main() {
await tester.tap(find.text('open'));
await tester.pump();
await tester.pump(const Duration(milliseconds: 400)); // route transition
expect(find.text('Connect GitHub'), findsOneWidget); // settings is up
expect(find.text('Firebase sync'), findsOneWidget); // settings is up
await tester.tap(find.text('Save'));
await tester.pump(); // run _save (persist + pop)
@ -502,6 +642,7 @@ void main() {
httpClient: mock,
);
await openAdvanced(tester);
await tester.tap(find.text('Connect GitHub'));
await tester.pump(); // requestDeviceCode
await tester.pump(); // dialog builds, poll starts (interval 0)
@ -524,3 +665,13 @@ void main() {
expect(find.text('WXYZ-1234'), findsNothing); // dialog dismissed
});
}
/// A real [FirebaseRestClient] that is never called: the settings screen only
/// checks it for null, so no network or keystore is touched.
FirebaseRestClient _stubClient() => FirebaseRestClient(
databaseUrl: 'https://example.invalid',
auth: FirebaseTokenProvider(
apiKey: 'k',
store: InMemoryCredentialStore(),
),
);