From 05d1714912589737636735baac8e5edce2c26ee1 Mon Sep 17 00:00:00 2001 From: klopez4212 Date: Sun, 4 Oct 2026 11:39:13 +0100 Subject: [PATCH] fix(mobile): manage channel members from profile sheets (#8078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep member administration reachable when small channels omit “See all.” Tapping a member now opens profile-sheet rows to make a channel admin, change an admin to member, or remove the person with confirmation, using the existing grouped list style. Actions follow the live roster, respect channel permissions, prevent duplicate submissions, and surface failures. Existing guests can become members or admins; owners, self, archived channels, and DMs do not expose these controls. Validation: mobile analysis, formatting, file-size checks, and the full mobile suite passed. Release iOS build installed and launched on a physical iPhone. Full repository `just ci` was previously stopped for low disk space during desktop compilation. --------- Signed-off-by: kenny lopez --- .../channels/channel_detail_page.dart | 5 + .../channels/channel_details_page.dart | 1 + .../channel_member_profile_actions.dart | 192 +++++++++++ .../lib/features/channels/members_sheet.dart | 5 + .../features/profile/user_profile_sheet.dart | 19 +- mobile/lib/shared/widgets/app_list_card.dart | 8 +- .../channels/channel_detail_page_test.dart | 9 + .../channel_member_profile_actions_test.dart | 298 ++++++++++++++++++ 8 files changed, 533 insertions(+), 4 deletions(-) create mode 100644 mobile/lib/features/channels/channel_member_profile_actions.dart create mode 100644 mobile/test/features/channels/channel_member_profile_actions_test.dart diff --git a/mobile/lib/features/channels/channel_detail_page.dart b/mobile/lib/features/channels/channel_detail_page.dart index 8be0e96139..55b6c48c2f 100644 --- a/mobile/lib/features/channels/channel_detail_page.dart +++ b/mobile/lib/features/channels/channel_detail_page.dart @@ -42,6 +42,7 @@ import 'android_ime_lift.dart'; import 'channel.dart'; import 'channel_actions_sheet.dart'; import 'channel_identity_names_provider.dart'; +import 'channel_member_profile_actions.dart'; import 'channel_link_navigation.dart'; import 'agent_activity/working_bots_provider.dart'; import 'channel_management_provider.dart'; @@ -591,6 +592,10 @@ class ChannelDetailPage extends HookConsumerWidget { context, pubkey, names: channelIdentityNamesProvider(resolvedChannel.id), + contextualActions: (_) => ChannelMemberProfileActions( + channel: resolvedChannel, + pubkey: pubkey, + ), ), sectionId: ref .read(channelSectionsProvider) diff --git a/mobile/lib/features/channels/channel_details_page.dart b/mobile/lib/features/channels/channel_details_page.dart index 863163432e..194f114b9f 100644 --- a/mobile/lib/features/channels/channel_details_page.dart +++ b/mobile/lib/features/channels/channel_details_page.dart @@ -207,6 +207,7 @@ class ChannelDetailsPage extends HookConsumerWidget { builder: (_) => MembersSheet( channel: resolvedChannel, currentPubkey: resolvedCurrentPubkey, + onMemberTap: onMemberTap, ), ); diff --git a/mobile/lib/features/channels/channel_member_profile_actions.dart b/mobile/lib/features/channels/channel_member_profile_actions.dart new file mode 100644 index 0000000000..b7d170a2e0 --- /dev/null +++ b/mobile/lib/features/channels/channel_member_profile_actions.dart @@ -0,0 +1,192 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_hooks/flutter_hooks.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:lucide_icons_flutter/lucide_icons.dart'; + +import '../../shared/theme/theme.dart'; +import '../../shared/widgets/app_list.dart'; +import '../../shared/widgets/app_list_card.dart'; +import '../../shared/widgets/modal_presentation.dart'; +import 'channel.dart'; +import 'channel_identity_names_provider.dart'; +import 'channel_management_provider.dart'; +import 'channels_provider.dart'; + +/// Channel-scoped management rows embedded below a member's profile actions. +class ChannelMemberProfileActions extends HookConsumerWidget { + const ChannelMemberProfileActions({ + super.key, + required this.channel, + required this.pubkey, + }); + + final Channel channel; + final String pubkey; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final busy = useState(false); + final error = useState(null); + final actor = ref.watch(currentPubkeyProvider)?.toLowerCase(); + final roster = ref.watch(channelMembersProvider(channel.id)); + final channels = ref.watch(channelsProvider).asData?.value; + final currentChannel = + channels + ?.where((candidate) => candidate.id == channel.id) + .firstOrNull ?? + channel; + final target = pubkey.toLowerCase(); + + ChannelMember? manageableMember( + AsyncValue> snapshot, + String? currentActor, + Channel current, + ) { + if (current.isDm || + current.isArchived || + snapshot.isLoading || + snapshot.hasError || + currentActor == null || + target == currentActor) { + return null; + } + final members = snapshot.asData?.value ?? const []; + final viewer = members + .where((m) => m.pubkey.toLowerCase() == currentActor) + .firstOrNull; + final member = members + .where((m) => m.pubkey.toLowerCase() == target) + .firstOrNull; + if (viewer?.isElevated != true || member == null || member.isOwner) { + return null; + } + return member; + } + + final member = manageableMember(roster, actor, currentChannel); + if (member == null) return const SizedBox.shrink(); + + bool stillAllowed() { + final latest = + ref + .read(channelsProvider) + .asData + ?.value + .where((candidate) => candidate.id == channel.id) + .firstOrNull ?? + currentChannel; + return ref.read(currentPubkeyProvider)?.toLowerCase() == actor && + manageableMember( + ref.read(channelMembersProvider(channel.id)), + actor, + latest, + )?.role == + member.role; + } + + Future perform({String? role}) async { + if (busy.value || !stillAllowed()) return; + busy.value = true; + error.value = null; + try { + if (role == null) { + final label = ref + .read(channelIdentityNamesProvider(channel.id)) + .labelFor(target); + final confirmed = await showBuzzDialog( + context: context, + builder: (dialogContext) => AlertDialog( + title: const Text('Remove from channel?'), + content: Text('Remove $label from ${currentChannel.name}?'), + actions: [ + TextButton( + onPressed: () => Navigator.of(dialogContext).pop(false), + child: const Text('Cancel'), + ), + TextButton( + onPressed: () => Navigator.of(dialogContext).pop(true), + child: const Text('Remove'), + ), + ], + ), + ); + if (!context.mounted || confirmed != true || !stillAllowed()) return; + await ref + .read(channelActionsProvider) + .removeMember(channelId: channel.id, pubkey: target); + if (context.mounted) Navigator.of(context).pop(); + } else { + await ref + .read(channelActionsProvider) + .changeMemberRole( + channelId: channel.id, + pubkey: target, + role: role, + ); + } + } catch (_) { + if (context.mounted) { + error.value = role == null + ? 'Could not remove this person. Please try again.' + : 'Could not change this role. Please try again.'; + } + } finally { + if (context.mounted) busy.value = false; + } + } + + return Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + AppListCard( + horizontalPadding: 0, + children: [ + if (!member.isBot) ...[ + if (member.role != 'admin') + AppListRow( + icon: LucideIcons.shieldCheck, + title: 'Make channel admin', + onTap: busy.value ? null : () => perform(role: 'admin'), + ), + if (member.role == 'admin' || member.role == 'guest') + AppListRow( + icon: LucideIcons.user, + title: member.role == 'guest' + ? 'Make member' + : 'Change to member', + onTap: busy.value ? null : () => perform(role: 'member'), + ), + ], + AppListRow( + icon: LucideIcons.userMinus, + title: 'Remove from channel', + titleColor: context.colors.error, + onTap: busy.value ? null : () => perform(), + ), + ], + ), + if (busy.value) + Semantics( + liveRegion: true, + label: 'Updating channel member', + child: const ExcludeSemantics( + child: Center(child: Text('Updating…')), + ), + ), + if (error.value != null) + Semantics( + liveRegion: true, + label: error.value!, + child: ExcludeSemantics( + child: Text( + error.value!, + style: context.textTheme.bodyMedium?.copyWith( + color: context.colors.error, + ), + ), + ), + ), + ], + ); + } +} diff --git a/mobile/lib/features/channels/members_sheet.dart b/mobile/lib/features/channels/members_sheet.dart index 8b14f411b4..27b32383a8 100644 --- a/mobile/lib/features/channels/members_sheet.dart +++ b/mobile/lib/features/channels/members_sheet.dart @@ -20,11 +20,13 @@ import 'channel_management_provider.dart'; class MembersSheet extends HookConsumerWidget { final Channel channel; final String? currentPubkey; + final void Function(BuildContext context, String pubkey)? onMemberTap; const MembersSheet({ super.key, required this.channel, required this.currentPubkey, + this.onMemberTap, }); @override @@ -108,6 +110,9 @@ class MembersSheet extends HookConsumerWidget { currentPubkey?.toLowerCase(), channelId: channel.id, userStatus: statusCache[member.pubkey.toLowerCase()], + onActivityTap: onMemberTap == null + ? null + : () => onMemberTap!(context, member.pubkey), ), ], if (bots.isNotEmpty) ...[ diff --git a/mobile/lib/features/profile/user_profile_sheet.dart b/mobile/lib/features/profile/user_profile_sheet.dart index 192b7448a7..28e2fddd83 100644 --- a/mobile/lib/features/profile/user_profile_sheet.dart +++ b/mobile/lib/features/profile/user_profile_sheet.dart @@ -34,12 +34,17 @@ void showUserProfileSheet( BuildContext context, String pubkey, { ProviderListenable? names, + WidgetBuilder? contextualActions, }) { showBuzzModalBottomSheet( context: context, isScrollControlled: true, showDragHandle: false, - builder: (_) => UserProfileSheet(pubkey: pubkey, names: names), + builder: (_) => UserProfileSheet( + pubkey: pubkey, + names: names, + contextualActions: contextualActions, + ), ).then((channel) { if (channel == null || !context.mounted) return; Navigator.of(context).push( @@ -58,7 +63,15 @@ class UserProfileSheet extends HookConsumerWidget { /// owns the context, so this sheet depends on no other feature's state. final ProviderListenable? names; - const UserProfileSheet({super.key, required this.pubkey, this.names}); + /// Optional actions supplied by the opening surface, below the profile tiles. + final WidgetBuilder? contextualActions; + + const UserProfileSheet({ + super.key, + required this.pubkey, + this.names, + this.contextualActions, + }); @override Widget build(BuildContext context, WidgetRef ref) { @@ -285,6 +298,8 @@ class UserProfileSheet extends HookConsumerWidget { ], ), const SizedBox(height: Grid.xs), + if (contextualActions != null) + Builder(builder: contextualActions!), ], ), ), diff --git a/mobile/lib/shared/widgets/app_list_card.dart b/mobile/lib/shared/widgets/app_list_card.dart index 229212192d..f51e03e1a5 100644 --- a/mobile/lib/shared/widgets/app_list_card.dart +++ b/mobile/lib/shared/widgets/app_list_card.dart @@ -12,6 +12,7 @@ class AppListCard extends StatelessWidget { this.label, this.dividerIndent, this.verticalPadding = Grid.xxs, + this.horizontalPadding = Grid.gutter, required this.children, }); @@ -29,6 +30,9 @@ class AppListCard extends StatelessWidget { /// passing [Grid.twelve] creates a 24dp rhythm between grouped surfaces. final double verticalPadding; + /// Outer horizontal inset; use zero inside an already padded sheet. + final double horizontalPadding; + final List children; static const _inset = Grid.xs; @@ -61,9 +65,9 @@ class AppListCard extends StatelessWidget { return Padding( padding: EdgeInsets.fromLTRB( - Grid.gutter, + horizontalPadding, verticalPadding, - Grid.gutter, + horizontalPadding, verticalPadding, ), child: Column( diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index b9eb616cad..a16ac4de94 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -2449,6 +2449,15 @@ void main() { expect(addRow, findsOneWidget); expect(memberRow, findsOneWidget); expect(seeAllRow, findsNothing); + await tester.tap( + find.byKey(const ValueKey('channel-details-member-alice')), + ); + await tester.pumpAndSettle(); + expect(find.byType(UserProfileSheet), findsOneWidget); + expect(find.text('Make channel admin'), findsOneWidget); + expect(find.text('Remove from channel'), findsOneWidget); + Navigator.of(tester.element(find.byType(UserProfileSheet))).pop(); + await tester.pumpAndSettle(); expect( tester.widget(find.text('Add members')).style, Theme.of(tester.element(addRow)).textTheme.bodyLarge, diff --git a/mobile/test/features/channels/channel_member_profile_actions_test.dart b/mobile/test/features/channels/channel_member_profile_actions_test.dart new file mode 100644 index 0000000000..883ae9adfb --- /dev/null +++ b/mobile/test/features/channels/channel_member_profile_actions_test.dart @@ -0,0 +1,298 @@ +import 'dart:async'; +import 'dart:ui' show SemanticsAction; + +import 'package:buzz/features/channels/channel.dart'; +import 'package:buzz/features/channels/channel_management_provider.dart'; +import 'package:buzz/features/channels/channel_member_profile_actions.dart'; +import 'package:buzz/features/channels/channels_provider.dart'; +import 'package:buzz/shared/theme/theme.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; + +class _Channels extends ChannelsNotifier { + _Channels(this.channel); + final Channel channel; + @override + Future> build() async => [channel]; +} + +class _Actions extends Fake implements ChannelActions { + final calls = []; + Completer? pending; + bool fail = false; + + @override + Future changeMemberRole({ + required String channelId, + required String pubkey, + required String role, + }) async { + calls.add('$channelId:$pubkey:$role'); + if (pending != null) await pending!.future; + if (fail) throw Exception('rejected'); + } + + @override + Future removeMember({ + required String channelId, + required String pubkey, + }) async { + calls.add('$channelId:$pubkey:remove'); + if (fail) throw Exception('rejected'); + } +} + +Channel _channel({bool dm = false, bool archived = false}) => Channel( + id: 'test', + name: 'General', + channelType: dm ? 'dm' : 'stream', + visibility: 'private', + description: '', + createdBy: 'self', + createdAt: DateTime(2025), + memberCount: 3, + archivedAt: archived ? DateTime(2025) : null, +); +ChannelMember _member(String pubkey, String role) => + ChannelMember(pubkey: pubkey, role: role, joinedAt: DateTime(2025)); + +Future _pump( + WidgetTester tester, { + String actorRole = 'owner', + String targetRole = 'member', + String target = 'alice', + bool dm = false, + bool archived = false, + bool inSheet = false, + _Actions? actions, + Future> Function()? load, +}) async { + final channel = _channel(dm: dm, archived: archived); + await tester.pumpWidget( + ProviderScope( + overrides: [ + currentPubkeyProvider.overrideWithValue('self'), + channelsProvider.overrideWith(() => _Channels(channel)), + channelMembersProvider('test').overrideWith( + (ref) async => load == null + ? [_member('self', actorRole), _member('alice', targetRole)] + : await load(), + ), + channelActionsProvider.overrideWithValue(actions ?? _Actions()), + ], + child: MaterialApp( + theme: AppTheme.light(), + home: Scaffold( + body: inSheet + ? Builder( + builder: (context) => TextButton( + onPressed: () => showModalBottomSheet( + context: context, + builder: (_) => ChannelMemberProfileActions( + channel: channel, + pubkey: target, + ), + ), + child: const Text('Open profile'), + ), + ) + : ChannelMemberProfileActions(channel: channel, pubkey: target), + ), + ), + ), + ); + await tester.pump(); + if (inSheet) { + await tester.tap(find.text('Open profile')); + await tester.pumpAndSettle(); + } + return ProviderScope.containerOf( + tester.element(find.byType(ChannelMemberProfileActions)), + ); +} + +void main() { + testWidgets('successful removal closes the member sheet', (tester) async { + final actions = _Actions(); + await _pump(tester, actions: actions, inSheet: true); + await tester.tap(find.text('Remove from channel')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Remove')); + await tester.pumpAndSettle(); + expect(actions.calls, ['test:alice:remove']); + expect(find.byType(ChannelMemberProfileActions), findsNothing); + expect(find.text('Open profile'), findsOneWidget); + }); + testWidgets('role action follows the refreshed roster', (tester) async { + var role = 'member'; + final actions = _Actions(); + final container = await _pump( + tester, + actions: actions, + load: () async => [_member('self', 'owner'), _member('alice', role)], + ); + await tester.tap(find.text('Make channel admin')); + await tester.pumpAndSettle(); + role = 'admin'; + container.invalidate(channelMembersProvider('test')); + await tester.pumpAndSettle(); + expect(find.text('Make channel admin'), findsNothing); + await tester.tap(find.text('Change to member')); + await tester.pumpAndSettle(); + expect(actions.calls, ['test:alice:admin', 'test:alice:member']); + }); + for (final role in ['member', 'admin', 'guest', 'bot']) { + testWidgets('management rows for $role', (tester) async { + await _pump(tester, targetRole: role); + expect( + find.text('Make channel admin'), + role == 'member' || role == 'guest' ? findsOneWidget : findsNothing, + ); + expect( + find.text('Change to member'), + role == 'admin' ? findsOneWidget : findsNothing, + ); + expect( + find.text('Make member'), + role == 'guest' ? findsOneWidget : findsNothing, + ); + expect(find.text('Remove from channel'), findsOneWidget); + }); + } + for (final scenario in [ + 'member viewer', + 'self', + 'owner', + 'dm', + 'archived', + 'missing', + 'loading', + 'error', + ]) { + testWidgets('hides management for $scenario', (tester) async { + await _pump( + tester, + actorRole: scenario == 'member viewer' ? 'member' : 'admin', + targetRole: scenario == 'owner' ? 'owner' : 'member', + target: scenario == 'self' + ? 'self' + : scenario == 'missing' + ? 'absent' + : 'alice', + dm: scenario == 'dm', + archived: scenario == 'archived', + load: scenario == 'loading' + ? () => Completer>().future + : scenario == 'error' + ? () => Future.error(Exception('offline')) + : null, + ); + expect(find.text('Make channel admin'), findsNothing); + expect(find.text('Remove from channel'), findsNothing); + }); + } + testWidgets('role updates block duplicates and show rejection', ( + tester, + ) async { + final semantics = tester.ensureSemantics(); + try { + final actions = _Actions() + ..pending = Completer() + ..fail = true; + await _pump(tester, actions: actions); + await tester.tap(find.text('Make channel admin')); + await tester.pump(); + final pending = find.bySemanticsLabel('Updating channel member'); + expect(pending, findsOneWidget); + expect( + tester + .getSemantics(pending) + .getSemanticsData() + .flagsCollection + .isLiveRegion, + isTrue, + ); + expect(find.bySemanticsLabel('Updating…'), findsNothing); + for (final label in ['Make channel admin', 'Remove from channel']) { + expect( + tester + .getSemantics(find.text(label)) + .getSemanticsData() + .hasAction(SemanticsAction.tap), + isFalse, + ); + } + await tester.tap(find.text('Make channel admin')); + expect(actions.calls, ['test:alice:admin']); + actions.pending!.complete(); + await tester.pumpAndSettle(); + expect( + find.text('Could not change this role. Please try again.'), + findsOneWidget, + ); + expect(pending, findsNothing); + final error = find.bySemanticsLabel( + 'Could not change this role. Please try again.', + ); + expect(error, findsOneWidget); + expect( + tester + .getSemantics(error) + .getSemanticsData() + .flagsCollection + .isLiveRegion, + isTrue, + ); + expect( + tester + .getSemantics(find.text('Make channel admin')) + .getSemanticsData() + .hasAction(SemanticsAction.tap), + isTrue, + ); + } finally { + semantics.dispose(); + } + }); + testWidgets('removal requires confirmation and reports rejection', ( + tester, + ) async { + final actions = _Actions()..fail = true; + await _pump(tester, actions: actions); + await tester.tap(find.text('Remove from channel')); + await tester.pumpAndSettle(); + expect(actions.calls, isEmpty); + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + expect(actions.calls, isEmpty); + await tester.tap(find.text('Remove from channel')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Remove')); + await tester.pumpAndSettle(); + expect(actions.calls, ['test:alice:remove']); + expect( + find.text('Could not remove this person. Please try again.'), + findsOneWidget, + ); + }); + testWidgets('revoked authority while confirming never submits removal', ( + tester, + ) async { + var role = 'admin'; + final actions = _Actions(); + final container = await _pump( + tester, + actions: actions, + load: () async => [_member('self', role), _member('alice', 'member')], + ); + await tester.tap(find.text('Remove from channel')); + await tester.pumpAndSettle(); + role = 'member'; + container.invalidate(channelMembersProvider('test')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Remove')); + await tester.pumpAndSettle(); + expect(actions.calls, isEmpty); + }); +}