mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(mobile): preserve exact mention recipients in saved drafts (#7387)
<!-- Draft PR body for #7387 · fix(mobile): preserve exact mention recipients in saved drafts · https://github.com/block/buzz/pull/7387 --> 🤖 ## Summary Saved drafts remembered their text but not exactly who each mention referred to: reopening a draft (or restarting the app) could re-bind a mention to the wrong same-name teammate, and a draft restored in one thread could capture text arriving for another. This PR saves the exact selected identities alongside the draft text and restores them together, so a reopened draft names the same recipients — with their agent markings — that you picked, including after a failed send. - Persists selected keys and agent presentation in the app's existing per-account/channel/thread draft storage (device-local; no cross-device portability claim). - Restores recipient bindings before text listeners run, including recovery from a rejected send; malformed legacy entries are ignored rather than guessed. - Retires the previous draft's listeners before restoring another, so switching threads can no longer write incoming text into your outgoing draft. ### Related issue - Fixes: N/A. No separate issue; the related work is the stack below. - Depends on #7385 (declared base) — same-name selections and qualified labels come from that PR. - Landing note: branches in this series overlap in the composer — when rebasing, keep exact/durable mention bindings, the explicit invite/reference-only choice, the account/visit/revision fences, and authorization before membership preparation and publication; don't resolve conflicts by taking either side wholesale. - Draft — not requesting merge yet; the security advisory doesn't run on this stacked base (it reviews main-based ranges). ### Testing - Regressions cover two namesakes, restart, rename, rejected send, a second restart, and changing draft scope. - At `e544b247c32bdccf1b316d3329be247faf412f5f`: `just mobile-check` and full `just mobile-test` pass (2,085 tests). The earlier full `just ci` receipt linked below is reused only for unchanged non-mobile code/tooling, not claimed as a rerun at this head. - Previous-head evidence: `just mobile-check`, the full mobile test suite, and full local `just ci` all pass — receipts in the [exact-head evidence comment](https://github.com/block/buzz/pull/7387#issuecomment-5574635970). - Verification is widget-test level; no native device or simulator run is claimed. To see it: mention an agent or a namesake, leave the channel, restart the app, and return — the draft still names the exact recipients you picked. ### Screenshots Flutter production-widget test renders — not native-device screenshots or acceptance captures. | Scenario | Before | After | |---|---|---| | Draft reopened after leaving the channel and restarting the app |  |  | <details> <summary>Capture provenance</summary> Rendered by the Flutter widget engine in a `flutter test` run (production widgets, production theme; no device or simulator). Before: this PR's declared base `39afd73b0adfde14164f4b10dbd089cb498312b6`. After: its head `8a781108507dc4501192b0101185decc1ed6f118`. </details> Signed-off-by: Logan Johnson <loganj@squareup.com>
This commit is contained in:
Logan Johnson
@@ -81,6 +81,10 @@ void _useComposeDraftLifecycle({
|
||||
required _IOSAttachmentPopoverController iosAttachmentPopover,
|
||||
required VoidCallback onDraftIdentityChanged,
|
||||
}) {
|
||||
// Retire the old listener before restoring another scope's text: replacement
|
||||
// effects can run before the previous effect's cleanup.
|
||||
final owner = useMemoized(Object.new, [draftKey, draftIdentity]);
|
||||
final currentOwner = useRef(owner)..value = owner;
|
||||
final lastDraftIdentity = useRef<String?>(null);
|
||||
useEffect(() {
|
||||
final identity = '$draftIdentity\u0000$draftKey';
|
||||
@@ -126,6 +130,7 @@ void _useComposeDraftLifecycle({
|
||||
for (final e in mentionMap.value.entries) e.key: e.value.pubkey,
|
||||
};
|
||||
void persistDraft() {
|
||||
if (!identical(currentOwner.value, owner)) return;
|
||||
final text = controller.text;
|
||||
if (text != lastPersistedText) {
|
||||
// Prune before the atomic snapshot, not in a later editor listener.
|
||||
|
||||
@@ -36,6 +36,7 @@ import 'package:buzz/shared/widgets/mobile_tab_footer_backdrop.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
|
||||
part 'compose_bar_test/exact_mention_tests.dart';
|
||||
part 'compose_bar_test/durable_mention_tests.dart';
|
||||
|
||||
final _pngBytes = Uint8List.fromList([
|
||||
0x89,
|
||||
@@ -197,6 +198,7 @@ Widget _buildComposeBar({
|
||||
ValueChanged<VoidCallback>? onFocusRestorerChanged,
|
||||
AppLifecycleNotifier Function()? appLifecycle,
|
||||
String composeBarKey = 'compose-bar',
|
||||
String? threadHeadId,
|
||||
VoiceNoteRecorder Function()? voiceNoteRecorderFactory,
|
||||
VoiceNotePlayerController Function()? voiceNotePlayerFactory,
|
||||
}) {
|
||||
@@ -260,6 +262,7 @@ Widget _buildComposeBar({
|
||||
final composeBar = ComposeBar(
|
||||
key: ValueKey(composeBarKey),
|
||||
channelId: 'channel-1',
|
||||
threadHeadId: threadHeadId,
|
||||
focusNode: focusNode,
|
||||
onFocusRestorerChanged: onFocusRestorerChanged,
|
||||
onFocusRequested: onFocusRequested,
|
||||
@@ -649,6 +652,7 @@ class _FakeChannelsNotifier extends ChannelsNotifier {
|
||||
|
||||
void main() {
|
||||
exactMentionTests();
|
||||
durableMentionTests();
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
setUp(() async {
|
||||
|
||||
@@ -0,0 +1,226 @@
|
||||
part of '../compose_bar_test.dart';
|
||||
|
||||
void durableMentionTests() {
|
||||
testWidgets(
|
||||
'exact draft recipients survive restart and failed-send recovery',
|
||||
(tester) async {
|
||||
final keys = nostr.Keys.generate();
|
||||
final first = 'a' * 64;
|
||||
final second = 'b' * 64;
|
||||
var renamed = false;
|
||||
var fail = true;
|
||||
List<String>? sent;
|
||||
Widget build({String? thread}) => _buildComposeBar(
|
||||
threadHeadId: thread,
|
||||
uploadService: _testUploadService(keys.nsec),
|
||||
relayConfig: () => _SwitchableRelayConfigNotifier(
|
||||
RelayConfig(baseUrl: 'http://localhost:3000', nsec: keys.nsec),
|
||||
),
|
||||
channels: [_makeCurrentChannel()],
|
||||
members: [
|
||||
for (final key in [first, second])
|
||||
ChannelMember(
|
||||
pubkey: key,
|
||||
displayName: renamed ? 'Renamed' : 'Scout',
|
||||
role: 'member',
|
||||
joinedAt: DateTime(2025),
|
||||
),
|
||||
],
|
||||
onSend: (_, mentions, {mediaTags = const []}) async {
|
||||
sent = mentions;
|
||||
if (fail) throw Exception('relay rejected');
|
||||
},
|
||||
);
|
||||
// Same-name members get contextual picker labels (`Scout · <npub
|
||||
// suffix>`); the inserted mention text still uses the plain name.
|
||||
Finder scoutRows() => find.descendant(
|
||||
of: find.byKey(const ValueKey('mention-suggestions-popover')),
|
||||
matching: find.textContaining('Scout'),
|
||||
);
|
||||
await tester.pumpWidget(build());
|
||||
await _expandComposer(tester);
|
||||
await tester.enterText(find.byType(TextField), '@');
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(scoutRows().first);
|
||||
await tester.pumpAndSettle();
|
||||
await tester.enterText(find.byType(TextField), '@Scout @');
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(scoutRows().last);
|
||||
await tester.pumpAndSettle();
|
||||
final draft = tester
|
||||
.widget<TextField>(find.byType(TextField))
|
||||
.controller!
|
||||
.text;
|
||||
expect(draft, '@Scout @Scout ($second) ');
|
||||
|
||||
// Same mounted composer, different thread and back: old listeners may
|
||||
// not erase the original persisted bindings while restoring another key.
|
||||
await tester.pumpWidget(build(thread: 'other'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.pumpWidget(build());
|
||||
await tester.pumpAndSettle();
|
||||
expect(
|
||||
tester.widget<TextField>(find.byType(TextField)).controller!.text,
|
||||
draft,
|
||||
);
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
renamed = true;
|
||||
await tester.pumpWidget(build());
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text(draft.trim()));
|
||||
await tester.pumpAndSettle();
|
||||
expect(
|
||||
tester.widget<TextField>(find.byType(TextField)).controller!.text,
|
||||
draft,
|
||||
);
|
||||
await tester.tap(find.byIcon(LucideIcons.arrowUp));
|
||||
await tester.pumpAndSettle();
|
||||
expect(sent, [first, second]);
|
||||
expect(
|
||||
tester.widget<TextField>(find.byType(TextField)).controller!.text,
|
||||
draft,
|
||||
);
|
||||
|
||||
// Recovery must persist the bindings before notifying text listeners.
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
fail = false;
|
||||
sent = null;
|
||||
await tester.pumpWidget(build());
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text(draft.trim()));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.byIcon(LucideIcons.arrowUp));
|
||||
await tester.pumpAndSettle();
|
||||
expect(sent, [first, second]);
|
||||
},
|
||||
);
|
||||
|
||||
for (final fallback in ['unavailable', 'human']) {
|
||||
testWidgets('saved agent loses provenance across restart: $fallback', (
|
||||
tester,
|
||||
) async {
|
||||
final signer = nostr.Keys.generate();
|
||||
final agent = 'a' * 64;
|
||||
final events = <Map<String, dynamic>>[];
|
||||
var eligible = true;
|
||||
Future<void> mount() async {
|
||||
await tester.pumpWidget(
|
||||
_buildComposeBar(
|
||||
uploadService: _testUploadService(signer.nsec),
|
||||
currentPubkey: signer.public,
|
||||
relayConfig: () => _SwitchableRelayConfigNotifier(
|
||||
RelayConfig(baseUrl: 'https://relay.example', nsec: signer.nsec),
|
||||
),
|
||||
channels: [_makeCurrentChannel(), _makeSharedMemberChannel()],
|
||||
relayAgents: [
|
||||
if (eligible)
|
||||
AgentDirectoryEntry(
|
||||
pubkey: agent,
|
||||
displayName: 'Helper Bot',
|
||||
respondTo: 'anyone',
|
||||
channelIds: const ['shared-channel'],
|
||||
),
|
||||
],
|
||||
onSend: (text, keys, {mediaTags = const []}) async {
|
||||
final container = ProviderScope.containerOf(
|
||||
tester.element(find.byType(ComposeBar)),
|
||||
);
|
||||
final session = container.read(relaySessionProvider.notifier);
|
||||
await SendMessage(
|
||||
signedEventRelay: SignedEventRelay(
|
||||
session: session,
|
||||
nsec: signer.nsec,
|
||||
),
|
||||
fetchMembers: (_) async => const [],
|
||||
readUserCache: () => const {},
|
||||
addLocalMessage: (_, _) {},
|
||||
completeLocalMessage: (_, _) {},
|
||||
removeLocalMessage: (_, _) {},
|
||||
)(
|
||||
channelId: 'channel-1',
|
||||
content: text,
|
||||
mentionPubkeys: keys,
|
||||
mediaTags: mediaTags,
|
||||
);
|
||||
},
|
||||
),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
final container = ProviderScope.containerOf(
|
||||
tester.element(find.byType(ComposeBar)),
|
||||
);
|
||||
final session = container.read(relaySessionProvider.notifier);
|
||||
session.debugAttachSocketForTest(
|
||||
_RecordingRelaySocket(
|
||||
events,
|
||||
session.debugHandleSocketMessageForTest,
|
||||
),
|
||||
);
|
||||
if (!eligible && fallback == 'human') {
|
||||
container
|
||||
.read(userCacheProvider.notifier)
|
||||
.put(UserProfile(pubkey: agent, displayName: 'Former Helper'));
|
||||
await tester.pumpAndSettle();
|
||||
}
|
||||
}
|
||||
|
||||
await mount();
|
||||
await _expandComposer(tester);
|
||||
await tester.enterText(find.byType(TextField), '@');
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text('Helper Bot'));
|
||||
await tester.pumpAndSettle();
|
||||
final prefsKey =
|
||||
'compose_drafts_v1:https://relay.example:${signer.public}';
|
||||
final saved = jsonDecode(_testPrefs.getString(prefsKey)!) as List;
|
||||
expect(saved.single['mention_keys'], {'Helper Bot': agent});
|
||||
expect(saved.single['text'], '@Helper Bot ');
|
||||
// The parent deliberately persists no agent bit to trust after restart.
|
||||
expect(saved.single.containsKey('is_agent'), isFalse);
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
await tester.pumpAndSettle();
|
||||
eligible = false;
|
||||
await mount();
|
||||
await tester.tap(find.text('@Helper Bot'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.byIcon(LucideIcons.arrowUp));
|
||||
await tester.pumpAndSettle();
|
||||
expect(events.where((e) => e['kind'] == 9000), isEmpty);
|
||||
if (fallback == 'human') {
|
||||
expect(
|
||||
find.textContaining('Former Helper is not in this channel'),
|
||||
findsOneWidget,
|
||||
);
|
||||
await tester.tap(find.text('Do nothing'));
|
||||
await tester.pumpAndSettle();
|
||||
final message = events.singleWhere(
|
||||
(e) => e['kind'] == EventKind.streamMessage,
|
||||
);
|
||||
expect((message['tags'] as List).where((t) => t[0] == 'p'), isEmpty);
|
||||
expect(
|
||||
(message['tags'] as List).where((t) => t[0] == 'mention').toList(),
|
||||
[
|
||||
['mention', agent],
|
||||
],
|
||||
);
|
||||
expect(message['sig'], matches(RegExp(r'^[0-9a-f]{128}$')));
|
||||
} else {
|
||||
expect(
|
||||
events.where((e) => e['kind'] == EventKind.streamMessage),
|
||||
isEmpty,
|
||||
);
|
||||
expect(
|
||||
find.textContaining('Saved mention is no longer available'),
|
||||
findsOneWidget,
|
||||
);
|
||||
expect(
|
||||
tester.widget<TextField>(find.byType(TextField)).controller!.text,
|
||||
'@Helper Bot ',
|
||||
);
|
||||
}
|
||||
expect(events.where((e) => e['kind'] == 9000), isEmpty);
|
||||
await tester.pumpWidget(const SizedBox.shrink());
|
||||
await tester.pump(const Duration(milliseconds: 250));
|
||||
});
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user