Skip to content

Fix DM handling, improve message persistence, and stabilize tests#37

Merged
Refraggerator merged 4 commits intomainfrom
fix/visual-tests-and-bug-verification
Mar 10, 2026
Merged

Fix DM handling, improve message persistence, and stabilize tests#37
Refraggerator merged 4 commits intomainfrom
fix/visual-tests-and-bug-verification

Conversation

@Refraggerator
Copy link
Copy Markdown
Owner

This pull request introduces several important improvements to the app's architecture, integration tests, and documentation, focusing on enhanced reliability for messaging and voice features, improved presence handling, and expanded test coverage. The most significant changes include new integration tests for multi-user voice scenarios, fixes for presence event initialization, and updates to guild child room power levels to ensure all members have appropriate permissions.

Voice and Presence Reliability:

  • Added eager initialization of the presenceProvider in both app code and integration tests to ensure presence events from the initial Matrix sync are not missed due to lazy provider setup. [1] [2] [3]
  • Expanded integration tests for voice channels, including multi-user scenarios where both users' m.call.member state events are visible and verified via participant grid tiles. [1] [2] [3]

Messaging and Guild Room Permissions:

  • Updated guild child room creation to use power_level_content_override with users_default: 100 and state_default: 0, allowing all members to send messages, set voice state, and create channels without admin rights; prevents accidental E2EE enablement. [1] [2]
  • Documented and implemented automatic message history restoration (requestHistory) after re-login when the local cache is empty, ensuring general channel messages are never lost.

Test Coverage and Infrastructure:

  • Increased unit/widget test count from 290 to 296 (including new chat-provider and voice integration tests), and API integration tests from 28 to 29, with messaging tests now using the CS API directly for reliability. [1] [2] [3]
  • Refactored test setup to use FFI-based SQLite databases for integration tests, improving isolation and reliability. [1] [2]

Documentation Updates:

  • Updated FEATURES.md and ARCHITECTURE.md to reflect new test counts, improved guild channel permissions, presence initialization, and messaging reliability. [1] [2] [3]

Other Improvements:

  • Refined integration test logic to verify participant visibility via specific widget keys, and improved channel selection logic for voice tests. [1] [2] [3]

These changes collectively strengthen the app's reliability, user experience, and test coverage, especially for real-time messaging and voice features.

- Replace raw createRoom(isDirect:true) with client.startDirectChat()
  in DmListNotifier.createDm and integration tests  ensures m.direct
  account data is set so Room.isDirectChat returns true immediately.
- Add m.direct fallback in _buildDmList for partner resolution when
  room members haven't synced yet.
- Add ValueKeys for DM integration testing (dm_item_, dm_home_button,
  dm_user_id_input, dm_start_button).
- Add 6 DM integration tests (Group 9) covering list display, dialog,
  create, select, send message, and room reuse.
- Fix extra closing bracket in dm_list.dart build method.

All 246 unit tests and 61 integration tests pass.
Bug fixes:
- selectedDmIdProvider changed from autoDispose to non-autoDispose so selection
  persists across sync-triggered rebuilds (fixes User B disappearing from DM list)
- Auto-join pending DM invites in DmList.build() so User B auto-accepts incoming
  DMs and sees messages sent by User A (fixes empty chat history for invitees)
- Hide guild header in channel_list.dart when no guild selected to prevent
  duplicate 'Direct Messages' header when DmList is shown

New features:
- deleteDm() method in DmListNotifier: calls room.removeFromDirectChat() then
  room.leave(), clears selection if the deleted DM was selected
- Delete DM via right-click / long-press context menu in DmList with confirm dialog
  (ValueKeys: dm_context_delete, confirm_delete_dm)
- Copy-to-clipboard IconButton for read-only Username and Matrix ID fields in
  profile edit dialog (uses Clipboard.setData + SnackBar feedback)

l10n additions (de + en):
- dmDeleteTitle, dmDeleteConfirmation(name), copiedToClipboard, copyToClipboard

Tests:
- 3 new unit tests for deleteDm in dm_provider_test.dart
- 1 new unit test for SelectedDmIdNotifier.clear()
- Integration test 9.7: User B receives DM from User A and sees message (auto-join)
- Integration test 9.8: Two-way DM - A sends, B replies, A sees reply via sync
- Integration test 9.9: Delete DM via context menu with confirmation
All 250 unit tests + 64 integration tests pass
…lti-user voice tests

Bug 2+3 (channels not visible / no rights after join):
- guilds.go: Add power_level_content_override with users_default:100 and
  state_default:0 to all child room creation (general + voice). This ensures
  joining members can send messages, set m.call.member state, and create new
  channels without needing owner/admin power level.
- m.room.encrypted:100 prevents accidental E2EE enablement in guild rooms.

Bug 1 (messages lost after re-login):
- chat_provider.dart: Call requestHistory(historyCount:50) when the local
  SQLite timeline cache is empty after initial getTimeline(). This ensures
  message history is fetched from Synapse on fresh login or after cache wipe.
- New tests: app/test/features/chat/providers/chat_provider_test.dart (4 tests)

Bug 4 (presence not syncing in real-time):
- main.dart: Eagerly watch presenceProvider in ConcordApp.build() alongside
  autoStatusProvider so the client.onPresenceChanged stream subscription is
  active before the first Matrix sync delivers presence events (Riverpod
  providers are lazy by default  late init caused early events to be missed).

Feature 5 (multi-user voice integration tests):
- voice_channel_integration_test.dart: Added 2 new tests verifying that both
  User A and User B are visible via m.call.member state events  both
  simultaneous and sequential join scenarios.

Test fixes (pre-existing flaky tests in matrix_api_test.dart):
- Test 3.1: Rewritten to use CS API PUT /rooms/{id}/send directly instead of
  Matrix SDK sendTextEvent + oneShotSync (was timing out after 30s on Windows).
- Test 3.2: Added polling retry loop (10 x 300ms) for Synapse eventual
  consistency  message written by 3.1 may not appear in /messages immediately.

All 331 tests pass (296 unit/widget + 29 API integration + 6 voice integration).
Summary of changes:
- Patched Matrix SDK event handling to include m.call.member in importantStateEvents.
- Fixed message persistence bug by triggering requestHistory on sync.
- Optimized sqflite initialization to reduce test console noise.
- Fixed broken unit tests in chat_provider_test.dart.
- Refactored integration tests 10.1 and 13.1 for better reliability.
Copilot AI review requested due to automatic review settings March 10, 2026 22:53
@Refraggerator Refraggerator merged commit 836ec23 into main Mar 10, 2026
2 checks passed
@Refraggerator Refraggerator deleted the fix/visual-tests-and-bug-verification branch March 10, 2026 22:54
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves reliability of Matrix-based messaging, DMs, presence, and voice participation by adjusting client/server behavior and adding/strengthening automated tests and documentation to cover the related regressions.

Changes:

  • Adjust Matrix client initialization and app bootstrap to eagerly handle presence + persist voice state (m.call.member) in partial/lazy-loaded rooms.
  • Improve DM UX (create/delete, invite auto-join) and chat history restoration behavior, with new unit/widget/integration tests.
  • Stabilize integration tests via FFI SQLite isolation and more deterministic UI selectors (widget keys), and update project docs/test counts.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
server/handlers/guilds.go Adds child-room power level overrides during guild creation.
app/lib/main.dart Configures Matrix client importantStateEvents and eagerly watches presenceProvider.
app/lib/features/chat/providers/chat_provider.dart Adds first-sync requestHistory logic to restore messages after re-login.
app/lib/features/dm/providers/dm_provider.dart Switches DM creation to startDirectChat, adds DM deletion, adjusts selection provider lifetime.
app/lib/features/dm/widgets/dm_list.dart Adds DM delete UI + context menu and attempts to auto-join pending DM invites.
app/lib/features/voice/widgets/voice_channel_view.dart Adds stable widget keys for voice participant tiles.
app/lib/features/home/widgets/member_list.dart Adds stable widget keys for member list rows and presence indicators.
app/lib/features/guilds/widgets/guild_rail.dart Adds stable key for the DM/Home button.
app/lib/features/channels/widgets/channel_list.dart Hides guild header in DM mode to avoid duplicated headers.
app/lib/features/auth/widgets/profile_edit_dialog.dart Adds copy-to-clipboard UI for read-only fields and truncation handling.
app/test/features/chat/providers/chat_provider_test.dart New unit tests for chat history restoration behavior.
app/test/features/dm/providers/dm_provider_test.dart Expands DM provider tests (m.direct fallback + deletion + selection clearing).
app/test/features/voice/integration/voice_channel_integration_test.dart Adds multi-user voice state visibility tests.
app/test/integration/matrix_api_test.dart Switches messaging test send path to CS API + adds retry polling.
app/integration_test/voice_and_profile_test.dart Stabilizes tests with FFI SQLite, eager presence subscription, and stricter widget-key checks.
app/integration_test/visual_test.dart Stabilizes tests with FFI SQLite, eager presence subscription, and adds new regression tests.
app/lib/core/l10n/strings_en.dart Adds DM deletion and clipboard strings.
app/lib/core/l10n/strings_de.dart Adds DM deletion and clipboard strings (German).
app/lib/core/l10n/app_localizations.dart Exposes new localization getters/helpers.
FEATURES.md Updates feature list and test counts to reflect new behavior/tests.
ARCHITECTURE.md Documents child room power levels, presence initialization, and updated test counts.
Comments suppressed due to low confidence (1)

app/lib/features/auth/widgets/profile_edit_dialog.dart:5

  • This file still uses Uint8List for avatar bytes, but dart:typed_data is no longer imported (it was removed while adding flutter/services.dart). Re-add import 'dart:typed_data'; so the code compiles.
import 'dart:io';

import 'package:concord_app/core/matrix/matrix_provider.dart';
import 'package:concord_app/core/services/image_processing_service.dart';
import 'package:concord_app/core/utils/mxc_utils.dart';

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread server/handlers/guilds.go
Comment on lines +62 to +75
// childPowerLevels gives every guild member (users_default: 100)
// full rights in child rooms so they can:
// • send text messages (events_default: 0)
// • set m.call.member for voice (state_default: 0)
// • create new channels (m.space.child: 0 on the Space is separate)
// The AS bot gets 50 so it can still invite on behalf of users.
childPowerLevels := map[string]interface{}{
"users": map[string]interface{}{
userID: 100,
botUserID: 50,
},
"users_default": 100,
"events_default": 0,
"state_default": 0,
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

users_default: 100 + state_default: 0 in childPowerLevels effectively gives every member admin-level power and allows any state event (including m.room.power_levels) at level 0. This is a significant permissions/security regression. Prefer a low users_default (e.g. 0) and a safer state_default (e.g. 50), then explicitly allow only required events (e.g. m.call.member: 0) while keeping m.room.encrypted high.

Suggested change
// childPowerLevels gives every guild member (users_default: 100)
// full rights in child rooms so they can:
// • send text messages (events_default: 0)
// • set m.call.member for voice (state_default: 0)
// • create new channels (m.space.child: 0 on the Space is separate)
// The AS bot gets 50 so it can still invite on behalf of users.
childPowerLevels := map[string]interface{}{
"users": map[string]interface{}{
userID: 100,
botUserID: 50,
},
"users_default": 100,
"events_default": 0,
"state_default": 0,
// childPowerLevels defines power levels in child rooms:
// • regular members: users_default = 0 (can send messages but not change state)
// • state_default = 50 so only moderators/admins can modify most state events
// • allow m.call.member at level 0 for voice functionality
// • keep m.room.encrypted at 100 to prevent accidental encryption changes
// The AS bot gets 50 so it can still invite on behalf of users.
childPowerLevels := map[string]interface{}{
"users": map[string]interface{}{
userID: 100,
botUserID: 50,
},
"users_default": 0,
"events_default": 0,
"state_default": 50,

Copilot uses AI. Check for mistakes.
// re-login / fresh-client scenario (Bug #1).
_syncSub = client.onSync.stream.listen((_) {
_emitMessages();
if (!_historyRequested) {
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestHistory() is invoked on the first sync unconditionally. The intended behavior (per PR description) is to restore history only when the timeline cache is empty; otherwise this adds an unnecessary network call on every channel open. Gate this behind a check like _timeline?.events.isEmpty == true (or another explicit cache-empty signal) before calling requestHistory().

Suggested change
if (!_historyRequested) {
// Only request history on the first sync *and* when the timeline cache
// is effectively empty, to avoid an unnecessary network call on every
// channel open.
if (!_historyRequested && (_timeline?.events.isEmpty ?? true)) {

Copilot uses AI. Check for mistakes.
final client = ref.watch(matrixClientProvider);

// Auto-join pending DM invitations so the invited user sees the chat.
_autoJoinPendingDmInvites(client);
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling _autoJoinPendingDmInvites(client) from build() introduces network side-effects during widget build and can re-trigger join() on every rebuild (sync/presence/theme). Move this into a provider/notifier or a ref.listen/post-frame callback with a guard so each invite is joined at most once.

Suggested change
_autoJoinPendingDmInvites(client);
// Run this as a side-effect when the DM list changes, instead of directly in build.
ref.listen(dmListProvider, (previous, next) {
next.whenData((_) {
_autoJoinPendingDmInvites(client);
});
});

Copilot uses AI. Check for mistakes.
Comment on lines +358 to +361
showMenu<String>(
context: context,
color: const Color(0xFF111214),
position: RelativeRect.fill,
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The context menu is anchored using position: RelativeRect.fill, which makes the menu placement independent of the clicked DM item (often appearing in an unexpected location). Capture the pointer position (e.g. via onTapDown/onSecondaryTapDown) or compute an anchor rect from the item's RenderBox, and pass that to showMenu so the menu opens next to the item/cursor.

Suggested change
showMenu<String>(
context: context,
color: const Color(0xFF111214),
position: RelativeRect.fill,
// Compute the position of this DM item within the overlay so the menu
// appears anchored to the clicked item instead of filling the screen.
final RenderBox button = context.findRenderObject() as RenderBox;
final RenderBox overlay =
Overlay.of(context).context.findRenderObject() as RenderBox;
final RelativeRect position = RelativeRect.fromRect(
Rect.fromPoints(
button.localToGlobal(Offset.zero, ancestor: overlay),
button.localToGlobal(
button.size.bottomRight(Offset.zero),
ancestor: overlay,
),
),
Offset.zero & overlay.size,
);
showMenu<String>(
context: context,
color: const Color(0xFF111214),
position: position,

Copilot uses AI. Check for mistakes.
Comment on lines +388 to +389
// The message was sent in 3.1 via the Matrix SDK; the CS API /messages
// endpoint may not reflect it immediately on the very next HTTP call.
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the message in 3.1 was sent "via the Matrix SDK", but this test now sends it via the CS API PUT /rooms/{id}/send/.... Update the comment to match the current mechanism (CS API) to avoid confusion when debugging eventual consistency behavior.

Suggested change
// The message was sent in 3.1 via the Matrix SDK; the CS API /messages
// endpoint may not reflect it immediately on the very next HTTP call.
// The message in 3.1 was sent via the CS API /rooms/{id}/send/...; the
// /messages endpoint may not reflect it immediately on the very next HTTP call.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants